* [PATCH v5 1/7] media: iris: retrieve UBWC platform configuration
2026-05-07 1:10 [PATCH v5 0/7] media: iris: migrate to using global UBWC config Dmitry Baryshkov
@ 2026-05-07 1:10 ` Dmitry Baryshkov
2026-05-07 2:40 ` Claude review: " Claude Code Review Bot
2026-05-07 1:10 ` [PATCH v5 2/7] media: iris: don't specify min_acc_length in the source code Dmitry Baryshkov
` (6 subsequent siblings)
7 siblings, 1 reply; 16+ messages in thread
From: Dmitry Baryshkov @ 2026-05-07 1:10 UTC (permalink / raw)
To: Bjorn Andersson, Konrad Dybcio, Rob Clark, Dmitry Baryshkov,
Abhinav Kumar, Jessica Zhang, Sean Paul, Marijn Suijten,
David Airlie, Simona Vetter, Akhil P Oommen, Vikash Garodia,
Dikshita Agarwal, Bryan O'Donoghue, Mauro Carvalho Chehab
Cc: Konrad Dybcio, linux-arm-msm, linux-kernel, dri-devel, freedreno,
linux-media, Bryan O'Donoghue, Wangao Wang
Specifying UBWC data in each driver doesn't scale and is prone to
errors. Request UBWC data from the central database in preparation to
using it through the rest of the driver.
Reviewed-by: Bryan O'Donoghue <bryan.odonoghue@linaro.org>
Reviewed-by: Konrad Dybcio <konrad.dybcio@oss.qualcomm.com>
Reviewed-by: Dikshita Agarwal <dikshita.agarwal@oss.qualcomm.com>
Tested-by: Wangao Wang <wangao.wang@oss.qualcomm.com>
Signed-off-by: Dmitry Baryshkov <dmitry.baryshkov@oss.qualcomm.com>
---
drivers/media/platform/qcom/iris/Kconfig | 1 +
drivers/media/platform/qcom/iris/iris_core.h | 4 ++++
drivers/media/platform/qcom/iris/iris_probe.c | 5 +++++
3 files changed, 10 insertions(+)
diff --git a/drivers/media/platform/qcom/iris/Kconfig b/drivers/media/platform/qcom/iris/Kconfig
index 3c803a05305a..39b06de6c3e6 100644
--- a/drivers/media/platform/qcom/iris/Kconfig
+++ b/drivers/media/platform/qcom/iris/Kconfig
@@ -5,6 +5,7 @@ config VIDEO_QCOM_IRIS
select V4L2_MEM2MEM_DEV
select QCOM_MDT_LOADER if ARCH_QCOM
select QCOM_SCM
+ select QCOM_UBWC_CONFIG
select VIDEOBUF2_DMA_CONTIG
help
This is a V4L2 driver for Qualcomm iris video accelerator
diff --git a/drivers/media/platform/qcom/iris/iris_core.h b/drivers/media/platform/qcom/iris/iris_core.h
index fb194c967ad4..d10a03aa5685 100644
--- a/drivers/media/platform/qcom/iris/iris_core.h
+++ b/drivers/media/platform/qcom/iris/iris_core.h
@@ -30,6 +30,8 @@ enum domain_type {
DECODER = BIT(1),
};
+struct qcom_ubwc_cfg_data;
+
/**
* struct iris_core - holds core parameters valid for all instances
*
@@ -52,6 +54,7 @@ enum domain_type {
* @resets: table of iris reset clocks
* @controller_resets: table of controller reset clocks
* @iris_platform_data: a structure for platform data
+ * @ubwc_cfg: UBWC configuration for the platform
* @state: current state of core
* @iface_q_table_daddr: device address for interface queue table memory
* @sfr_daddr: device address for SFR (Sub System Failure Reason) register memory
@@ -95,6 +98,7 @@ struct iris_core {
struct reset_control_bulk_data *resets;
struct reset_control_bulk_data *controller_resets;
const struct iris_platform_data *iris_platform_data;
+ const struct qcom_ubwc_cfg_data *ubwc_cfg;
enum iris_core_state state;
dma_addr_t iface_q_table_daddr;
dma_addr_t sfr_daddr;
diff --git a/drivers/media/platform/qcom/iris/iris_probe.c b/drivers/media/platform/qcom/iris/iris_probe.c
index ddaacda523ec..492f85f518eb 100644
--- a/drivers/media/platform/qcom/iris/iris_probe.c
+++ b/drivers/media/platform/qcom/iris/iris_probe.c
@@ -10,6 +10,7 @@
#include <linux/pm_opp.h>
#include <linux/pm_runtime.h>
#include <linux/reset.h>
+#include <linux/soc/qcom/ubwc.h>
#include "iris_core.h"
#include "iris_ctrls.h"
@@ -244,6 +245,10 @@ static int iris_probe(struct platform_device *pdev)
core->iris_platform_data = of_device_get_match_data(core->dev);
+ core->ubwc_cfg = qcom_ubwc_config_get_data();
+ if (IS_ERR(core->ubwc_cfg))
+ return PTR_ERR(core->ubwc_cfg);
+
ret = devm_request_threaded_irq(core->dev, core->irq, iris_hfi_isr,
iris_hfi_isr_handler, IRQF_TRIGGER_HIGH, "iris", core);
if (ret)
--
2.47.3
^ permalink raw reply related [flat|nested] 16+ messages in thread* Claude review: media: iris: retrieve UBWC platform configuration
2026-05-07 1:10 ` [PATCH v5 1/7] media: iris: retrieve UBWC platform configuration Dmitry Baryshkov
@ 2026-05-07 2:40 ` Claude Code Review Bot
0 siblings, 0 replies; 16+ messages in thread
From: Claude Code Review Bot @ 2026-05-07 2:40 UTC (permalink / raw)
To: dri-devel-reviews
Patch Review
This patch adds the `QCOM_UBWC_CONFIG` Kconfig dependency, a forward declaration of `struct qcom_ubwc_cfg_data` in `iris_core.h`, and retrieves the config at probe time.
The implementation is straightforward and correct:
```c
core->ubwc_cfg = qcom_ubwc_config_get_data();
if (IS_ERR(core->ubwc_cfg))
return PTR_ERR(core->ubwc_cfg);
```
**Minor nit:** Modern kernel probe functions prefer `dev_err_probe()` for error handling, which silences `EPROBE_DEFER` noise and provides a consistent error message for genuine failures:
```c
if (IS_ERR(core->ubwc_cfg))
return dev_err_probe(core->dev, PTR_ERR(core->ubwc_cfg),
"failed to get UBWC config\n");
```
The `select QCOM_UBWC_CONFIG` in Kconfig ensures the real implementation is compiled in, avoiding the stub that always returns `-EOPNOTSUPP`.
No other issues.
---
Generated by Claude Code Patch Reviewer
^ permalink raw reply [flat|nested] 16+ messages in thread
* [PATCH v5 2/7] media: iris: don't specify min_acc_length in the source code
2026-05-07 1:10 [PATCH v5 0/7] media: iris: migrate to using global UBWC config Dmitry Baryshkov
2026-05-07 1:10 ` [PATCH v5 1/7] media: iris: retrieve UBWC platform configuration Dmitry Baryshkov
@ 2026-05-07 1:10 ` Dmitry Baryshkov
2026-05-07 2:40 ` Claude review: " Claude Code Review Bot
2026-05-07 1:10 ` [PATCH v5 3/7] media: iris: don't specify highest_bank_bit " Dmitry Baryshkov
` (5 subsequent siblings)
7 siblings, 1 reply; 16+ messages in thread
From: Dmitry Baryshkov @ 2026-05-07 1:10 UTC (permalink / raw)
To: Bjorn Andersson, Konrad Dybcio, Rob Clark, Dmitry Baryshkov,
Abhinav Kumar, Jessica Zhang, Sean Paul, Marijn Suijten,
David Airlie, Simona Vetter, Akhil P Oommen, Vikash Garodia,
Dikshita Agarwal, Bryan O'Donoghue, Mauro Carvalho Chehab
Cc: Konrad Dybcio, linux-arm-msm, linux-kernel, dri-devel, freedreno,
linux-media, Bryan O'Donoghue, Wangao Wang
The min_acc length can be calculated from the platform UBWC
configuration. Use the freshly introduced helper and calculate min_acc
length based on the platform UBWC configuration instead of specifying it
directly in the source.
Reviewed-by: Bryan O'Donoghue <bryan.odonoghue@linaro.org>
Reviewed-by: Konrad Dybcio <konrad.dybcio@oss.qualcomm.com>
Reviewed-by: Dikshita Agarwal <dikshita.agarwal@oss.qualcomm.com>
Tested-by: Wangao Wang <wangao.wang@oss.qualcomm.com>
Signed-off-by: Dmitry Baryshkov <dmitry.baryshkov@oss.qualcomm.com>
---
drivers/media/platform/qcom/iris/iris_hfi_gen2_packet.c | 6 +++++-
drivers/media/platform/qcom/iris/iris_platform_common.h | 1 -
drivers/media/platform/qcom/iris/iris_platform_gen2.c | 1 -
3 files changed, 5 insertions(+), 3 deletions(-)
diff --git a/drivers/media/platform/qcom/iris/iris_hfi_gen2_packet.c b/drivers/media/platform/qcom/iris/iris_hfi_gen2_packet.c
index d77fa29f44fc..aa4520b27739 100644
--- a/drivers/media/platform/qcom/iris/iris_hfi_gen2_packet.c
+++ b/drivers/media/platform/qcom/iris/iris_hfi_gen2_packet.c
@@ -3,6 +3,9 @@
* Copyright (c) 2022-2024 Qualcomm Innovation Center, Inc. All rights reserved.
*/
+#include <linux/printk.h>
+#include <linux/soc/qcom/ubwc.h>
+
#include "iris_hfi_common.h"
#include "iris_hfi_gen2.h"
#include "iris_hfi_gen2_packet.h"
@@ -120,6 +123,7 @@ static void iris_hfi_gen2_create_packet(struct iris_hfi_header *hdr, u32 pkt_typ
void iris_hfi_gen2_packet_sys_init(struct iris_core *core, struct iris_hfi_header *hdr)
{
+ const struct qcom_ubwc_cfg_data *ubwc = core->ubwc_cfg;
u32 payload = 0;
iris_hfi_gen2_create_header(hdr, 0, core->header_id++);
@@ -146,7 +150,7 @@ void iris_hfi_gen2_packet_sys_init(struct iris_core *core, struct iris_hfi_heade
&payload,
sizeof(u32));
- payload = core->iris_platform_data->ubwc_config->mal_length;
+ payload = qcom_ubwc_min_acc_length_64b(ubwc) ? 64 : 32;
iris_hfi_gen2_create_packet(hdr,
HFI_PROP_UBWC_MAL_LENGTH,
HFI_HOST_FLAGS_NONE,
diff --git a/drivers/media/platform/qcom/iris/iris_platform_common.h b/drivers/media/platform/qcom/iris/iris_platform_common.h
index 5a489917580e..08a9529e599b 100644
--- a/drivers/media/platform/qcom/iris/iris_platform_common.h
+++ b/drivers/media/platform/qcom/iris/iris_platform_common.h
@@ -77,7 +77,6 @@ struct tz_cp_config {
struct ubwc_config_data {
u32 max_channels;
- u32 mal_length;
u32 highest_bank_bit;
u32 bank_swzl_level;
u32 bank_swz2_level;
diff --git a/drivers/media/platform/qcom/iris/iris_platform_gen2.c b/drivers/media/platform/qcom/iris/iris_platform_gen2.c
index 5da90d47f9c6..01c6ffa7e084 100644
--- a/drivers/media/platform/qcom/iris/iris_platform_gen2.c
+++ b/drivers/media/platform/qcom/iris/iris_platform_gen2.c
@@ -792,7 +792,6 @@ static const char * const sm8550_opp_clk_table[] = {
static struct ubwc_config_data ubwc_config_sm8550 = {
.max_channels = 8,
- .mal_length = 32,
.highest_bank_bit = 16,
.bank_swzl_level = 0,
.bank_swz2_level = 1,
--
2.47.3
^ permalink raw reply related [flat|nested] 16+ messages in thread* Claude review: media: iris: don't specify min_acc_length in the source code
2026-05-07 1:10 ` [PATCH v5 2/7] media: iris: don't specify min_acc_length in the source code Dmitry Baryshkov
@ 2026-05-07 2:40 ` Claude Code Review Bot
0 siblings, 0 replies; 16+ messages in thread
From: Claude Code Review Bot @ 2026-05-07 2:40 UTC (permalink / raw)
To: dri-devel-reviews
Patch Review
Replaces `core->iris_platform_data->ubwc_config->mal_length` (which was hardcoded to `32` for SM8550) with:
```c
payload = qcom_ubwc_min_acc_length_64b(ubwc) ? 64 : 32;
```
The helper `qcom_ubwc_min_acc_length_64b()` returns true only when `ubwc_enc_version == UBWC_1_0` and `ubwc_dec_version` is `UBWC_2_0` or `UBWC_3_0`. SM8550 uses UBWC 4.x, so this returns false, yielding 32 -- matching the original hardcoded value.
The upstream header does note this is "the best guess, based on the MDSS driver," but that's the API's responsibility, not this patch's concern.
No issues.
---
Generated by Claude Code Patch Reviewer
^ permalink raw reply [flat|nested] 16+ messages in thread
* [PATCH v5 3/7] media: iris: don't specify highest_bank_bit in the source code
2026-05-07 1:10 [PATCH v5 0/7] media: iris: migrate to using global UBWC config Dmitry Baryshkov
2026-05-07 1:10 ` [PATCH v5 1/7] media: iris: retrieve UBWC platform configuration Dmitry Baryshkov
2026-05-07 1:10 ` [PATCH v5 2/7] media: iris: don't specify min_acc_length in the source code Dmitry Baryshkov
@ 2026-05-07 1:10 ` Dmitry Baryshkov
2026-05-07 2:40 ` Claude review: " Claude Code Review Bot
2026-05-07 1:10 ` [PATCH v5 4/7] media: iris: don't specify ubwc_swizzle " Dmitry Baryshkov
` (4 subsequent siblings)
7 siblings, 1 reply; 16+ messages in thread
From: Dmitry Baryshkov @ 2026-05-07 1:10 UTC (permalink / raw)
To: Bjorn Andersson, Konrad Dybcio, Rob Clark, Dmitry Baryshkov,
Abhinav Kumar, Jessica Zhang, Sean Paul, Marijn Suijten,
David Airlie, Simona Vetter, Akhil P Oommen, Vikash Garodia,
Dikshita Agarwal, Bryan O'Donoghue, Mauro Carvalho Chehab
Cc: Konrad Dybcio, linux-arm-msm, linux-kernel, dri-devel, freedreno,
linux-media, Bryan O'Donoghue, Wangao Wang
The highest_bank_bit param is specified both in the Iris driver and in
the platform UBWC config. Use the platform UBWC configuration instead of
specifying it directly in the source.
Reviewed-by: Konrad Dybcio <konrad.dybcio@oss.qualcomm.com>
Reviewed-by: Bryan O'Donoghue <bryan.odonoghue@linaro.org>
Reviewed-by: Dikshita Agarwal <dikshita.agarwal@oss.qualcomm.com>
Tested-by: Wangao Wang <wangao.wang@oss.qualcomm.com>
Signed-off-by: Dmitry Baryshkov <dmitry.baryshkov@oss.qualcomm.com>
---
drivers/media/platform/qcom/iris/iris_hfi_gen2_packet.c | 2 +-
drivers/media/platform/qcom/iris/iris_platform_common.h | 1 -
drivers/media/platform/qcom/iris/iris_platform_gen2.c | 1 -
3 files changed, 1 insertion(+), 3 deletions(-)
diff --git a/drivers/media/platform/qcom/iris/iris_hfi_gen2_packet.c b/drivers/media/platform/qcom/iris/iris_hfi_gen2_packet.c
index aa4520b27739..6dc0cbaa9c19 100644
--- a/drivers/media/platform/qcom/iris/iris_hfi_gen2_packet.c
+++ b/drivers/media/platform/qcom/iris/iris_hfi_gen2_packet.c
@@ -160,7 +160,7 @@ void iris_hfi_gen2_packet_sys_init(struct iris_core *core, struct iris_hfi_heade
&payload,
sizeof(u32));
- payload = core->iris_platform_data->ubwc_config->highest_bank_bit;
+ payload = ubwc->highest_bank_bit;
iris_hfi_gen2_create_packet(hdr,
HFI_PROP_UBWC_HBB,
HFI_HOST_FLAGS_NONE,
diff --git a/drivers/media/platform/qcom/iris/iris_platform_common.h b/drivers/media/platform/qcom/iris/iris_platform_common.h
index 08a9529e599b..5639eb5a75b6 100644
--- a/drivers/media/platform/qcom/iris/iris_platform_common.h
+++ b/drivers/media/platform/qcom/iris/iris_platform_common.h
@@ -77,7 +77,6 @@ struct tz_cp_config {
struct ubwc_config_data {
u32 max_channels;
- u32 highest_bank_bit;
u32 bank_swzl_level;
u32 bank_swz2_level;
u32 bank_swz3_level;
diff --git a/drivers/media/platform/qcom/iris/iris_platform_gen2.c b/drivers/media/platform/qcom/iris/iris_platform_gen2.c
index 01c6ffa7e084..bdeb92e0b7bc 100644
--- a/drivers/media/platform/qcom/iris/iris_platform_gen2.c
+++ b/drivers/media/platform/qcom/iris/iris_platform_gen2.c
@@ -792,7 +792,6 @@ static const char * const sm8550_opp_clk_table[] = {
static struct ubwc_config_data ubwc_config_sm8550 = {
.max_channels = 8,
- .highest_bank_bit = 16,
.bank_swzl_level = 0,
.bank_swz2_level = 1,
.bank_swz3_level = 1,
--
2.47.3
^ permalink raw reply related [flat|nested] 16+ messages in thread* Claude review: media: iris: don't specify highest_bank_bit in the source code
2026-05-07 1:10 ` [PATCH v5 3/7] media: iris: don't specify highest_bank_bit " Dmitry Baryshkov
@ 2026-05-07 2:40 ` Claude Code Review Bot
0 siblings, 0 replies; 16+ messages in thread
From: Claude Code Review Bot @ 2026-05-07 2:40 UTC (permalink / raw)
To: dri-devel-reviews
Patch Review
Replaces `core->iris_platform_data->ubwc_config->highest_bank_bit` with direct access to the UBWC config:
```c
payload = ubwc->highest_bank_bit;
```
The old field was `u32`, the new struct field is `int`. Since `highest_bank_bit` values are always positive (representing a DDR bank bit position, e.g. 16 for SM8550), the implicit conversion to `u32 payload` is safe.
The key benefit: the central registry can derive HBB from runtime DDR type detection rather than using a fixed value, which was called out in the cover letter as one of the motivations for this series.
No issues.
---
Generated by Claude Code Patch Reviewer
^ permalink raw reply [flat|nested] 16+ messages in thread
* [PATCH v5 4/7] media: iris: don't specify ubwc_swizzle in the source code
2026-05-07 1:10 [PATCH v5 0/7] media: iris: migrate to using global UBWC config Dmitry Baryshkov
` (2 preceding siblings ...)
2026-05-07 1:10 ` [PATCH v5 3/7] media: iris: don't specify highest_bank_bit " Dmitry Baryshkov
@ 2026-05-07 1:10 ` Dmitry Baryshkov
2026-05-07 2:40 ` Claude review: " Claude Code Review Bot
2026-05-07 1:10 ` [PATCH v5 5/7] media: iris: don't specify bank_spreading " Dmitry Baryshkov
` (3 subsequent siblings)
7 siblings, 1 reply; 16+ messages in thread
From: Dmitry Baryshkov @ 2026-05-07 1:10 UTC (permalink / raw)
To: Bjorn Andersson, Konrad Dybcio, Rob Clark, Dmitry Baryshkov,
Abhinav Kumar, Jessica Zhang, Sean Paul, Marijn Suijten,
David Airlie, Simona Vetter, Akhil P Oommen, Vikash Garodia,
Dikshita Agarwal, Bryan O'Donoghue, Mauro Carvalho Chehab
Cc: Konrad Dybcio, linux-arm-msm, linux-kernel, dri-devel, freedreno,
linux-media, Bryan O'Donoghue, Wangao Wang
The UBWC swizzle is specified both in the Iris driver and in the
platform UBWC config. Use the platform UBWC configuration instead of
specifying it directly in the source.
Reviewed-by: Konrad Dybcio <konrad.dybcio@oss.qualcomm.com>
Reviewed-by: Bryan O'Donoghue <bryan.odonoghue@linaro.org>
Reviewed-by: Dikshita Agarwal <dikshita.agarwal@oss.qualcomm.com>
Tested-by: Wangao Wang <wangao.wang@oss.qualcomm.com>
Signed-off-by: Dmitry Baryshkov <dmitry.baryshkov@oss.qualcomm.com>
---
drivers/media/platform/qcom/iris/iris_hfi_gen2_packet.c | 6 +++---
drivers/media/platform/qcom/iris/iris_platform_common.h | 3 ---
drivers/media/platform/qcom/iris/iris_platform_gen2.c | 3 ---
3 files changed, 3 insertions(+), 9 deletions(-)
diff --git a/drivers/media/platform/qcom/iris/iris_hfi_gen2_packet.c b/drivers/media/platform/qcom/iris/iris_hfi_gen2_packet.c
index 6dc0cbaa9c19..a4d9efdbb43b 100644
--- a/drivers/media/platform/qcom/iris/iris_hfi_gen2_packet.c
+++ b/drivers/media/platform/qcom/iris/iris_hfi_gen2_packet.c
@@ -170,7 +170,7 @@ void iris_hfi_gen2_packet_sys_init(struct iris_core *core, struct iris_hfi_heade
&payload,
sizeof(u32));
- payload = core->iris_platform_data->ubwc_config->bank_swzl_level;
+ payload = !!(qcom_ubwc_swizzle(ubwc) & UBWC_SWIZZLE_ENABLE_LVL1);
iris_hfi_gen2_create_packet(hdr,
HFI_PROP_UBWC_BANK_SWZL_LEVEL1,
HFI_HOST_FLAGS_NONE,
@@ -180,7 +180,7 @@ void iris_hfi_gen2_packet_sys_init(struct iris_core *core, struct iris_hfi_heade
&payload,
sizeof(u32));
- payload = core->iris_platform_data->ubwc_config->bank_swz2_level;
+ payload = !!(qcom_ubwc_swizzle(ubwc) & UBWC_SWIZZLE_ENABLE_LVL2);
iris_hfi_gen2_create_packet(hdr,
HFI_PROP_UBWC_BANK_SWZL_LEVEL2,
HFI_HOST_FLAGS_NONE,
@@ -190,7 +190,7 @@ void iris_hfi_gen2_packet_sys_init(struct iris_core *core, struct iris_hfi_heade
&payload,
sizeof(u32));
- payload = core->iris_platform_data->ubwc_config->bank_swz3_level;
+ payload = !!(qcom_ubwc_swizzle(ubwc) & UBWC_SWIZZLE_ENABLE_LVL3);
iris_hfi_gen2_create_packet(hdr,
HFI_PROP_UBWC_BANK_SWZL_LEVEL3,
HFI_HOST_FLAGS_NONE,
diff --git a/drivers/media/platform/qcom/iris/iris_platform_common.h b/drivers/media/platform/qcom/iris/iris_platform_common.h
index 5639eb5a75b6..e217f15ef028 100644
--- a/drivers/media/platform/qcom/iris/iris_platform_common.h
+++ b/drivers/media/platform/qcom/iris/iris_platform_common.h
@@ -77,9 +77,6 @@ struct tz_cp_config {
struct ubwc_config_data {
u32 max_channels;
- u32 bank_swzl_level;
- u32 bank_swz2_level;
- u32 bank_swz3_level;
u32 bank_spreading;
};
diff --git a/drivers/media/platform/qcom/iris/iris_platform_gen2.c b/drivers/media/platform/qcom/iris/iris_platform_gen2.c
index bdeb92e0b7bc..8072f430bd26 100644
--- a/drivers/media/platform/qcom/iris/iris_platform_gen2.c
+++ b/drivers/media/platform/qcom/iris/iris_platform_gen2.c
@@ -792,9 +792,6 @@ static const char * const sm8550_opp_clk_table[] = {
static struct ubwc_config_data ubwc_config_sm8550 = {
.max_channels = 8,
- .bank_swzl_level = 0,
- .bank_swz2_level = 1,
- .bank_swz3_level = 1,
.bank_spreading = 1,
};
--
2.47.3
^ permalink raw reply related [flat|nested] 16+ messages in thread* Claude review: media: iris: don't specify ubwc_swizzle in the source code
2026-05-07 1:10 ` [PATCH v5 4/7] media: iris: don't specify ubwc_swizzle " Dmitry Baryshkov
@ 2026-05-07 2:40 ` Claude Code Review Bot
0 siblings, 0 replies; 16+ messages in thread
From: Claude Code Review Bot @ 2026-05-07 2:40 UTC (permalink / raw)
To: dri-devel-reviews
Patch Review
Replaces three separate `bank_swzl_level` / `bank_swz2_level` / `bank_swz3_level` fields (each `u32`, values 0 or 1) with bitmask checks:
```c
payload = !!(qcom_ubwc_swizzle(ubwc) & UBWC_SWIZZLE_ENABLE_LVL1);
payload = !!(qcom_ubwc_swizzle(ubwc) & UBWC_SWIZZLE_ENABLE_LVL2);
payload = !!(qcom_ubwc_swizzle(ubwc) & UBWC_SWIZZLE_ENABLE_LVL3);
```
Verified against the SM8550 original values (`bank_swzl_level=0`, `bank_swz2_level=1`, `bank_swz3_level=1`):
- `UBWC_SWIZZLE_ENABLE_LVL1` = `BIT(0)` -- UBWC 2.0+ disables level 1 → 0. Correct.
- `UBWC_SWIZZLE_ENABLE_LVL2` = `BIT(1)` -- enabled → 1. Correct.
- `UBWC_SWIZZLE_ENABLE_LVL3` = `BIT(2)` -- enabled → 1. Correct.
The `!!` double-negation normalizes to 0/1, matching the original semantics.
No issues.
---
Generated by Claude Code Patch Reviewer
^ permalink raw reply [flat|nested] 16+ messages in thread
* [PATCH v5 5/7] media: iris: don't specify bank_spreading in the source code
2026-05-07 1:10 [PATCH v5 0/7] media: iris: migrate to using global UBWC config Dmitry Baryshkov
` (3 preceding siblings ...)
2026-05-07 1:10 ` [PATCH v5 4/7] media: iris: don't specify ubwc_swizzle " Dmitry Baryshkov
@ 2026-05-07 1:10 ` Dmitry Baryshkov
2026-05-07 2:40 ` Claude review: " Claude Code Review Bot
2026-05-07 1:10 ` [PATCH v5 6/7] media: iris: don't specify max_channels " Dmitry Baryshkov
` (2 subsequent siblings)
7 siblings, 1 reply; 16+ messages in thread
From: Dmitry Baryshkov @ 2026-05-07 1:10 UTC (permalink / raw)
To: Bjorn Andersson, Konrad Dybcio, Rob Clark, Dmitry Baryshkov,
Abhinav Kumar, Jessica Zhang, Sean Paul, Marijn Suijten,
David Airlie, Simona Vetter, Akhil P Oommen, Vikash Garodia,
Dikshita Agarwal, Bryan O'Donoghue, Mauro Carvalho Chehab
Cc: Konrad Dybcio, linux-arm-msm, linux-kernel, dri-devel, freedreno,
linux-media, Bryan O'Donoghue, Wangao Wang
The UBWC bank spreading is specified both in the Iris driver and in the
platform UBWC config. Use the platform UBWC configuration instead of
specifying it directly in the source.
Reviewed-by: Konrad Dybcio <konrad.dybcio@oss.qualcomm.com>
Reviewed-by: Bryan O'Donoghue <bryan.odonoghue@linaro.org>
Reviewed-by: Dikshita Agarwal <dikshita.agarwal@oss.qualcomm.com>
Tested-by: Wangao Wang <wangao.wang@oss.qualcomm.com>
Signed-off-by: Dmitry Baryshkov <dmitry.baryshkov@oss.qualcomm.com>
---
drivers/media/platform/qcom/iris/iris_hfi_gen2_packet.c | 2 +-
drivers/media/platform/qcom/iris/iris_platform_common.h | 1 -
drivers/media/platform/qcom/iris/iris_platform_gen2.c | 1 -
3 files changed, 1 insertion(+), 3 deletions(-)
diff --git a/drivers/media/platform/qcom/iris/iris_hfi_gen2_packet.c b/drivers/media/platform/qcom/iris/iris_hfi_gen2_packet.c
index a4d9efdbb43b..a49394b92768 100644
--- a/drivers/media/platform/qcom/iris/iris_hfi_gen2_packet.c
+++ b/drivers/media/platform/qcom/iris/iris_hfi_gen2_packet.c
@@ -200,7 +200,7 @@ void iris_hfi_gen2_packet_sys_init(struct iris_core *core, struct iris_hfi_heade
&payload,
sizeof(u32));
- payload = core->iris_platform_data->ubwc_config->bank_spreading;
+ payload = qcom_ubwc_bank_spread(ubwc);
iris_hfi_gen2_create_packet(hdr,
HFI_PROP_UBWC_BANK_SPREADING,
HFI_HOST_FLAGS_NONE,
diff --git a/drivers/media/platform/qcom/iris/iris_platform_common.h b/drivers/media/platform/qcom/iris/iris_platform_common.h
index e217f15ef028..07c58cf3a14a 100644
--- a/drivers/media/platform/qcom/iris/iris_platform_common.h
+++ b/drivers/media/platform/qcom/iris/iris_platform_common.h
@@ -77,7 +77,6 @@ struct tz_cp_config {
struct ubwc_config_data {
u32 max_channels;
- u32 bank_spreading;
};
struct platform_inst_caps {
diff --git a/drivers/media/platform/qcom/iris/iris_platform_gen2.c b/drivers/media/platform/qcom/iris/iris_platform_gen2.c
index 8072f430bd26..4e617176dee4 100644
--- a/drivers/media/platform/qcom/iris/iris_platform_gen2.c
+++ b/drivers/media/platform/qcom/iris/iris_platform_gen2.c
@@ -792,7 +792,6 @@ static const char * const sm8550_opp_clk_table[] = {
static struct ubwc_config_data ubwc_config_sm8550 = {
.max_channels = 8,
- .bank_spreading = 1,
};
static const struct tz_cp_config tz_cp_config_sm8550[] = {
--
2.47.3
^ permalink raw reply related [flat|nested] 16+ messages in thread* [PATCH v5 6/7] media: iris: don't specify max_channels in the source code
2026-05-07 1:10 [PATCH v5 0/7] media: iris: migrate to using global UBWC config Dmitry Baryshkov
` (4 preceding siblings ...)
2026-05-07 1:10 ` [PATCH v5 5/7] media: iris: don't specify bank_spreading " Dmitry Baryshkov
@ 2026-05-07 1:10 ` Dmitry Baryshkov
2026-05-07 2:40 ` Claude review: " Claude Code Review Bot
2026-05-07 1:10 ` [PATCH v5 7/7] media: iris: drop remnants of UBWC configuration Dmitry Baryshkov
2026-05-07 2:40 ` Claude review: media: iris: migrate to using global UBWC config Claude Code Review Bot
7 siblings, 1 reply; 16+ messages in thread
From: Dmitry Baryshkov @ 2026-05-07 1:10 UTC (permalink / raw)
To: Bjorn Andersson, Konrad Dybcio, Rob Clark, Dmitry Baryshkov,
Abhinav Kumar, Jessica Zhang, Sean Paul, Marijn Suijten,
David Airlie, Simona Vetter, Akhil P Oommen, Vikash Garodia,
Dikshita Agarwal, Bryan O'Donoghue, Mauro Carvalho Chehab
Cc: Konrad Dybcio, linux-arm-msm, linux-kernel, dri-devel, freedreno,
linux-media, Bryan O'Donoghue, Wangao Wang
The UBWC max_channels spreading is specified in the Iris driver, but it
also can be calculated from the platform UBWC config. Use the platform
UBWC configuration instead of specifying it directly in the source.
Reviewed-by: Konrad Dybcio <konrad.dybcio@oss.qualcomm.com>
Reviewed-by: Bryan O'Donoghue <bryan.odonoghue@linaro.org>
Reviewed-by: Dikshita Agarwal <dikshita.agarwal@oss.qualcomm.com>
Tested-by: Wangao Wang <wangao.wang@oss.qualcomm.com>
Signed-off-by: Dmitry Baryshkov <dmitry.baryshkov@oss.qualcomm.com>
---
drivers/media/platform/qcom/iris/iris_hfi_gen2_packet.c | 2 +-
drivers/media/platform/qcom/iris/iris_platform_common.h | 1 -
drivers/media/platform/qcom/iris/iris_platform_gen2.c | 1 -
3 files changed, 1 insertion(+), 3 deletions(-)
diff --git a/drivers/media/platform/qcom/iris/iris_hfi_gen2_packet.c b/drivers/media/platform/qcom/iris/iris_hfi_gen2_packet.c
index a49394b92768..0d05dd2afc07 100644
--- a/drivers/media/platform/qcom/iris/iris_hfi_gen2_packet.c
+++ b/drivers/media/platform/qcom/iris/iris_hfi_gen2_packet.c
@@ -140,7 +140,7 @@ void iris_hfi_gen2_packet_sys_init(struct iris_core *core, struct iris_hfi_heade
&payload,
sizeof(u32));
- payload = core->iris_platform_data->ubwc_config->max_channels;
+ payload = qcom_ubwc_macrotile_mode(ubwc) ? 8 : 4;
iris_hfi_gen2_create_packet(hdr,
HFI_PROP_UBWC_MAX_CHANNELS,
HFI_HOST_FLAGS_NONE,
diff --git a/drivers/media/platform/qcom/iris/iris_platform_common.h b/drivers/media/platform/qcom/iris/iris_platform_common.h
index 07c58cf3a14a..e8b5446dce76 100644
--- a/drivers/media/platform/qcom/iris/iris_platform_common.h
+++ b/drivers/media/platform/qcom/iris/iris_platform_common.h
@@ -76,7 +76,6 @@ struct tz_cp_config {
};
struct ubwc_config_data {
- u32 max_channels;
};
struct platform_inst_caps {
diff --git a/drivers/media/platform/qcom/iris/iris_platform_gen2.c b/drivers/media/platform/qcom/iris/iris_platform_gen2.c
index 4e617176dee4..05b1dd11abce 100644
--- a/drivers/media/platform/qcom/iris/iris_platform_gen2.c
+++ b/drivers/media/platform/qcom/iris/iris_platform_gen2.c
@@ -791,7 +791,6 @@ static const char * const sm8550_opp_clk_table[] = {
};
static struct ubwc_config_data ubwc_config_sm8550 = {
- .max_channels = 8,
};
static const struct tz_cp_config tz_cp_config_sm8550[] = {
--
2.47.3
^ permalink raw reply related [flat|nested] 16+ messages in thread* Claude review: media: iris: don't specify max_channels in the source code
2026-05-07 1:10 ` [PATCH v5 6/7] media: iris: don't specify max_channels " Dmitry Baryshkov
@ 2026-05-07 2:40 ` Claude Code Review Bot
0 siblings, 0 replies; 16+ messages in thread
From: Claude Code Review Bot @ 2026-05-07 2:40 UTC (permalink / raw)
To: dri-devel-reviews
Patch Review
Replaces `core->iris_platform_data->ubwc_config->max_channels` (hardcoded to 8 for SM8550) with:
```c
payload = qcom_ubwc_macrotile_mode(ubwc) ? 8 : 4;
```
Per the UBWC header documentation: macrotile_mode `0` means 4-channel, `1` means 8-channel (introduced in UBWC 3.1). SM8550 with its UBWC 4.x would use 8-channel macrotiling, so `macrotile_mode` is true → 8. Matches the original.
No issues.
---
Generated by Claude Code Patch Reviewer
^ permalink raw reply [flat|nested] 16+ messages in thread
* [PATCH v5 7/7] media: iris: drop remnants of UBWC configuration
2026-05-07 1:10 [PATCH v5 0/7] media: iris: migrate to using global UBWC config Dmitry Baryshkov
` (5 preceding siblings ...)
2026-05-07 1:10 ` [PATCH v5 6/7] media: iris: don't specify max_channels " Dmitry Baryshkov
@ 2026-05-07 1:10 ` Dmitry Baryshkov
2026-05-07 2:40 ` Claude review: " Claude Code Review Bot
2026-05-07 2:40 ` Claude review: media: iris: migrate to using global UBWC config Claude Code Review Bot
7 siblings, 1 reply; 16+ messages in thread
From: Dmitry Baryshkov @ 2026-05-07 1:10 UTC (permalink / raw)
To: Bjorn Andersson, Konrad Dybcio, Rob Clark, Dmitry Baryshkov,
Abhinav Kumar, Jessica Zhang, Sean Paul, Marijn Suijten,
David Airlie, Simona Vetter, Akhil P Oommen, Vikash Garodia,
Dikshita Agarwal, Bryan O'Donoghue, Mauro Carvalho Chehab
Cc: Konrad Dybcio, linux-arm-msm, linux-kernel, dri-devel, freedreno,
linux-media, Bryan O'Donoghue, Wangao Wang
Now as all UBWC configuration bits were migrated to be used or derived
from the global UBWC platform-specific data, drop the unused struct and
field definitions.
Reviewed-by: Konrad Dybcio <konrad.dybcio@oss.qualcomm.com>
Reviewed-by: Bryan O'Donoghue <bryan.odonoghue@linaro.org>
Reviewed-by: Dikshita Agarwal <dikshita.agarwal@oss.qualcomm.com>
Tested-by: Wangao Wang <wangao.wang@oss.qualcomm.com>
Signed-off-by: Dmitry Baryshkov <dmitry.baryshkov@oss.qualcomm.com>
---
drivers/media/platform/qcom/iris/iris_platform_common.h | 4 ----
drivers/media/platform/qcom/iris/iris_platform_gen2.c | 7 -------
2 files changed, 11 deletions(-)
diff --git a/drivers/media/platform/qcom/iris/iris_platform_common.h b/drivers/media/platform/qcom/iris/iris_platform_common.h
index e8b5446dce76..f42e1798747c 100644
--- a/drivers/media/platform/qcom/iris/iris_platform_common.h
+++ b/drivers/media/platform/qcom/iris/iris_platform_common.h
@@ -75,9 +75,6 @@ struct tz_cp_config {
u32 cp_nonpixel_size;
};
-struct ubwc_config_data {
-};
-
struct platform_inst_caps {
u32 min_frame_width;
u32 max_frame_width;
@@ -241,7 +238,6 @@ struct iris_platform_data {
u32 tz_cp_config_data_size;
u32 core_arch;
u32 hw_response_timeout;
- struct ubwc_config_data *ubwc_config;
u32 num_vpp_pipe;
bool no_aon;
u32 max_session_count;
diff --git a/drivers/media/platform/qcom/iris/iris_platform_gen2.c b/drivers/media/platform/qcom/iris/iris_platform_gen2.c
index 05b1dd11abce..a526b50a1cd3 100644
--- a/drivers/media/platform/qcom/iris/iris_platform_gen2.c
+++ b/drivers/media/platform/qcom/iris/iris_platform_gen2.c
@@ -790,9 +790,6 @@ static const char * const sm8550_opp_clk_table[] = {
NULL,
};
-static struct ubwc_config_data ubwc_config_sm8550 = {
-};
-
static const struct tz_cp_config tz_cp_config_sm8550[] = {
{
.cp_start = 0,
@@ -949,7 +946,6 @@ const struct iris_platform_data sm8550_data = {
.tz_cp_config_data_size = ARRAY_SIZE(tz_cp_config_sm8550),
.core_arch = VIDEO_ARCH_LX,
.hw_response_timeout = HW_RESPONSE_TIMEOUT_VALUE,
- .ubwc_config = &ubwc_config_sm8550,
.num_vpp_pipe = 4,
.max_session_count = 16,
.max_core_mbpf = NUM_MBS_8K * 2,
@@ -1054,7 +1050,6 @@ const struct iris_platform_data sm8650_data = {
.tz_cp_config_data_size = ARRAY_SIZE(tz_cp_config_sm8550),
.core_arch = VIDEO_ARCH_LX,
.hw_response_timeout = HW_RESPONSE_TIMEOUT_VALUE,
- .ubwc_config = &ubwc_config_sm8550,
.num_vpp_pipe = 4,
.max_session_count = 16,
.max_core_mbpf = NUM_MBS_8K * 2,
@@ -1150,7 +1145,6 @@ const struct iris_platform_data sm8750_data = {
.tz_cp_config_data_size = ARRAY_SIZE(tz_cp_config_sm8550),
.core_arch = VIDEO_ARCH_LX,
.hw_response_timeout = HW_RESPONSE_TIMEOUT_VALUE,
- .ubwc_config = &ubwc_config_sm8550,
.num_vpp_pipe = 4,
.max_session_count = 16,
.max_core_mbpf = NUM_MBS_8K * 2,
@@ -1250,7 +1244,6 @@ const struct iris_platform_data qcs8300_data = {
.tz_cp_config_data_size = ARRAY_SIZE(tz_cp_config_sm8550),
.core_arch = VIDEO_ARCH_LX,
.hw_response_timeout = HW_RESPONSE_TIMEOUT_VALUE,
- .ubwc_config = &ubwc_config_sm8550,
.num_vpp_pipe = 2,
.max_session_count = 16,
.max_core_mbpf = ((4096 * 2176) / 256) * 4,
--
2.47.3
^ permalink raw reply related [flat|nested] 16+ messages in thread* Claude review: media: iris: migrate to using global UBWC config
2026-05-07 1:10 [PATCH v5 0/7] media: iris: migrate to using global UBWC config Dmitry Baryshkov
` (6 preceding siblings ...)
2026-05-07 1:10 ` [PATCH v5 7/7] media: iris: drop remnants of UBWC configuration Dmitry Baryshkov
@ 2026-05-07 2:40 ` Claude Code Review Bot
7 siblings, 0 replies; 16+ messages in thread
From: Claude Code Review Bot @ 2026-05-07 2:40 UTC (permalink / raw)
To: dri-devel-reviews
Overall Series Review
Subject: media: iris: migrate to using global UBWC config
Author: Dmitry Baryshkov <dmitry.baryshkov@oss.qualcomm.com>
Patches: 8
Reviewed: 2026-05-07T12:40:16.438595
---
This is a clean, well-structured 7-patch series that migrates the Qualcomm Iris video decoder driver from maintaining its own UBWC (Universal Bandwidth Compression) configuration to using the centralized `QCOM_UBWC_CONFIG` registry. The motivation is sound: duplicating UBWC parameters (HBB, swizzle, bank spreading, etc.) per-driver is error-prone, especially since values like `highest_bank_bit` can depend on DDR type at runtime rather than being fixed constants.
The series is incrementally decomposed -- each patch replaces one parameter, which makes bisection and review straightforward. All patches carry Reviewed-by from Konrad Dybcio, Bryan O'Donoghue, and Dikshita Agarwal, plus a Tested-by from Wangao Wang. The cover letter notes this is compile-tested only by the author due to lack of Gen2 hardware, which is an honest disclosure.
The API mappings from the old per-driver values to the new central UBWC helpers are correct based on my verification of the `<linux/soc/qcom/ubwc.h>` header. One minor style concern noted below.
**Verdict: Series looks good. One minor nit on probe error handling.**
---
---
Generated by Claude Code Patch Reviewer
^ permalink raw reply [flat|nested] 16+ messages in thread