* [PATCH 0/2] fix and improve for Hi846 @ 2026-04-29 7:03 Pengyu Luo 2026-04-29 7:03 ` [PATCH 1/2] media: hi846: fix hi846_write_reg_16 handling Pengyu Luo 2026-04-29 7:03 ` [PATCH 2/2] media: hi846: Add 6MP and 8MP modes support Pengyu Luo 0 siblings, 2 replies; 7+ messages in thread From: Pengyu Luo @ 2026-04-29 7:03 UTC (permalink / raw) To: Sakari Ailus, Martin Kepplinger-Novakovic, Mauro Carvalho Chehab, Hans Verkuil Cc: linux-media, linux-kernel, Pengyu Luo This series fixes a error blocking Hi846 driver function, and supports 6MP and 8MP modes on Hi846. Signed-off-by: Pengyu Luo <mitltlatltl@gmail.com> --- Pengyu Luo (2): media: hi846: fix hi846_write_reg_16 handling media: hi846: Add 6MP and 8MP mode support drivers/media/i2c/hi846.c | 156 ++++++++++- 1 files changed, 155 insertions(+), 1 deletions(-) -- 2.54.0 ^ permalink raw reply [flat|nested] 7+ messages in thread
* [PATCH 1/2] media: hi846: fix hi846_write_reg_16 handling 2026-04-29 7:03 [PATCH 0/2] fix and improve for Hi846 Pengyu Luo @ 2026-04-29 7:03 ` Pengyu Luo 2026-04-29 7:44 ` Sakari Ailus 2026-04-29 7:03 ` [PATCH 2/2] media: hi846: Add 6MP and 8MP modes support Pengyu Luo 1 sibling, 1 reply; 7+ messages in thread From: Pengyu Luo @ 2026-04-29 7:03 UTC (permalink / raw) To: Sakari Ailus, Martin Kepplinger-Novakovic, Mauro Carvalho Chehab, Hans Verkuil Cc: linux-media, linux-kernel, Pengyu Luo hi846_write_reg_16() does not clear a positive *err value on success. pm_runtime_get_if_in_use() returns a positive value when the device is already in use. When hi846_set_ctrl() passes &ret holding this positive value) to hi846_write_reg_16(), the function returns with ret as is, the positive value propagates back as a return code, which callers interpret as an error. Fix this by resetting *err to 0 only when it is positive. Fixes: 04fc06f6dc15 ("media: hi846: fix usage of pm_runtime_get_if_in_use()") Signed-off-by: Pengyu Luo <mitltlatltl@gmail.com> --- drivers/media/i2c/hi846.c | 2 ++ 1 file changed, 2 insertions(+) diff --git a/drivers/media/i2c/hi846.c b/drivers/media/i2c/hi846.c index a3f77b8434ca..09c109f3fba9 100644 --- a/drivers/media/i2c/hi846.c +++ b/drivers/media/i2c/hi846.c @@ -1270,6 +1270,8 @@ static void hi846_write_reg_16(struct hi846 *hi846, u16 reg, u16 val, int *err) if (*err < 0) return; + else + *err = 0; put_unaligned_be16(reg, buf); put_unaligned_be16(val, buf + 2); -- 2.54.0 ^ permalink raw reply related [flat|nested] 7+ messages in thread
* Re: [PATCH 1/2] media: hi846: fix hi846_write_reg_16 handling 2026-04-29 7:03 ` [PATCH 1/2] media: hi846: fix hi846_write_reg_16 handling Pengyu Luo @ 2026-04-29 7:44 ` Sakari Ailus 0 siblings, 0 replies; 7+ messages in thread From: Sakari Ailus @ 2026-04-29 7:44 UTC (permalink / raw) To: Pengyu Luo Cc: Martin Kepplinger-Novakovic, Mauro Carvalho Chehab, Hans Verkuil, linux-media, linux-kernel Hi Pengyu, Thanks for the set. On Wed, Apr 29, 2026 at 03:03:50PM +0800, Pengyu Luo wrote: > hi846_write_reg_16() does not clear a positive *err value on success. > pm_runtime_get_if_in_use() returns a positive value when the device > is already in use. When hi846_set_ctrl() passes &ret holding this > positive value) to hi846_write_reg_16(), the function returns with ret > as is, the positive value propagates back as a return code, which > callers interpret as an error. > > Fix this by resetting *err to 0 only when it is positive. > > Fixes: 04fc06f6dc15 ("media: hi846: fix usage of pm_runtime_get_if_in_use()") > Signed-off-by: Pengyu Luo <mitltlatltl@gmail.com> > --- > drivers/media/i2c/hi846.c | 2 ++ > 1 file changed, 2 insertions(+) > > diff --git a/drivers/media/i2c/hi846.c b/drivers/media/i2c/hi846.c > index a3f77b8434ca..09c109f3fba9 100644 > --- a/drivers/media/i2c/hi846.c > +++ b/drivers/media/i2c/hi846.c > @@ -1270,6 +1270,8 @@ static void hi846_write_reg_16(struct hi846 *hi846, u16 reg, u16 val, int *err) > > if (*err < 0) > return; > + else Else is useless here. > + *err = 0; > > put_unaligned_be16(reg, buf); > put_unaligned_be16(val, buf + 2); -- Regards, Sakari Ailus ^ permalink raw reply [flat|nested] 7+ messages in thread
* [PATCH 2/2] media: hi846: Add 6MP and 8MP modes support 2026-04-29 7:03 [PATCH 0/2] fix and improve for Hi846 Pengyu Luo 2026-04-29 7:03 ` [PATCH 1/2] media: hi846: fix hi846_write_reg_16 handling Pengyu Luo @ 2026-04-29 7:03 ` Pengyu Luo 2026-04-29 9:01 ` Sakari Ailus 1 sibling, 1 reply; 7+ messages in thread From: Pengyu Luo @ 2026-04-29 7:03 UTC (permalink / raw) To: Sakari Ailus, Martin Kepplinger-Novakovic, Mauro Carvalho Chehab, Hans Verkuil Cc: linux-media, linux-kernel, Pengyu Luo Hi846 is an 8MP sensor, but the upstream driver has only supported 2MP mode for years. This patch adds 6MP and 8MP modes to maximize sensor utilization. Note that these modes require 4-lane MIPI CSI-2, as the downstream driver only exposes 2MP, 6MP, and 8MP configurations in 4-lane operation on the target device. The register sequences are extracted from the downstream Windows driver. Signed-off-by: Pengyu Luo <mitltlatltl@gmail.com> --- drivers/media/i2c/hi846.c | 154 +++++++++++++++++++++++++++++++++++++- 1 file changed, 153 insertions(+), 1 deletion(-) diff --git a/drivers/media/i2c/hi846.c b/drivers/media/i2c/hi846.c index 09c109f3fba9..b8ae7344f1a0 100644 --- a/drivers/media/i2c/hi846.c +++ b/drivers/media/i2c/hi846.c @@ -1027,6 +1027,106 @@ static const struct hi846_reg mode_1632x1224_mipi_4lane[] = { {HI846_REG_TG_ENABLE, 0x0100}, }; +static const struct hi846_reg mode_3264x1836_config[] = { + {HI846_REG_MODE_SELECT, 0x0000}, + {HI846_REG_Y_ODD_INC_FOBP, 0x1111}, + {HI846_REG_Y_ODD_INC_VACT, 0x1111}, + {HI846_REG_Y_ADDR_START_VACT_H, 0x0172}, + {HI846_REG_Y_ADDR_END_VACT_H, 0x089d}, + {HI846_REG_UNKNOWN_005C, 0x2101}, + {HI846_REG_FLL, 0x09de}, + {HI846_REG_LLP, 0x0ed8}, + {HI846_REG_BINNING_MODE, 0x0022}, + {HI846_REG_HBIN_MODE, 0x0000}, + {HI846_REG_UNKNOWN_0A24, 0x0000}, + {HI846_REG_X_START_H, 0x0000}, + {HI846_REG_X_OUTPUT_SIZE_H, 0x0cc0}, + {HI846_REG_Y_OUTPUT_SIZE_H, 0x072c}, + {HI846_REG_EXPOSURE, 0x09d8}, + + /* For OTP */ + {HI846_REG_UNKNOWN_021C, 0x0001}, + {HI846_REG_UNKNOWN_021E, 0x0235}, + + {HI846_REG_ISP_EN_H, 0x014a}, + {HI846_REG_UNKNOWN_0418, 0x023e}, + {HI846_REG_UNKNOWN_0B02, 0xe04d}, + {HI846_REG_UNKNOWN_0B10, 0x6821}, + {HI846_REG_UNKNOWN_0B12, 0x0120}, + {HI846_REG_UNKNOWN_0B14, 0x0001}, + {HI846_REG_UNKNOWN_2008, 0x38fd}, + {HI846_REG_UNKNOWN_326E, 0x0000}, +}; + +static const struct hi846_reg mode_3264x1836_mipi_4lane[] = { + {HI846_REG_UNKNOWN_0900, 0x0300}, + {HI846_REG_MIPI_TX_OP_MODE, 0xc319}, + {HI846_REG_UNKNOWN_0914, 0xc109}, + {HI846_REG_TCLK_PREPARE, 0x061a}, + {HI846_REG_UNKNOWN_0918, 0x0407}, + {HI846_REG_THS_ZERO, 0x0a0b}, + {HI846_REG_TCLK_POST, 0x0e08}, + {HI846_REG_UNKNOWN_091E, 0x0a00}, + {HI846_REG_UNKNOWN_090C, 0x0427}, + {HI846_REG_UNKNOWN_090E, 0x0059}, + {HI846_REG_UNKNOWN_0954, 0x0089}, + {HI846_REG_UNKNOWN_0956, 0x0000}, + {HI846_REG_UNKNOWN_0958, 0xca80}, + {HI846_REG_UNKNOWN_095A, 0x9240}, + {HI846_REG_PLL_CFG_MIPI2_H, 0x4124}, + {HI846_REG_TG_ENABLE, 0x0100}, +}; + +static const struct hi846_reg mode_3264x2448_config[] = { + {HI846_REG_MODE_SELECT, 0x0000}, + {HI846_REG_Y_ODD_INC_FOBP, 0x1111}, + {HI846_REG_Y_ODD_INC_VACT, 0x1111}, + {HI846_REG_Y_ADDR_START_VACT_H, 0x0040}, + {HI846_REG_Y_ADDR_END_VACT_H, 0x09cf}, + {HI846_REG_UNKNOWN_005C, 0x2101}, + {HI846_REG_FLL, 0x09de}, + {HI846_REG_LLP, 0x0ed8}, + {HI846_REG_BINNING_MODE, 0x0022}, + {HI846_REG_HBIN_MODE, 0x0000}, + {HI846_REG_UNKNOWN_0A24, 0x0000}, + {HI846_REG_X_START_H, 0x0000}, + {HI846_REG_X_OUTPUT_SIZE_H, 0x0cc0}, + {HI846_REG_Y_OUTPUT_SIZE_H, 0x0990}, + {HI846_REG_EXPOSURE, 0x09d8}, + + /* For OTP */ + {HI846_REG_UNKNOWN_021C, 0x0001}, + {HI846_REG_UNKNOWN_021E, 0x0235}, + + {HI846_REG_ISP_EN_H, 0x014a}, + {HI846_REG_UNKNOWN_0418, 0x0000}, + {HI846_REG_UNKNOWN_0B02, 0xe04d}, + {HI846_REG_UNKNOWN_0B10, 0x6821}, + {HI846_REG_UNKNOWN_0B12, 0x0120}, + {HI846_REG_UNKNOWN_0B14, 0x0001}, + {HI846_REG_UNKNOWN_2008, 0x38fd}, + {HI846_REG_UNKNOWN_326E, 0x0000}, +}; + +static const struct hi846_reg mode_3264x2448_mipi_4lane[] = { + {HI846_REG_UNKNOWN_0900, 0x0300}, + {HI846_REG_MIPI_TX_OP_MODE, 0xc319}, + {HI846_REG_UNKNOWN_0914, 0xc109}, + {HI846_REG_TCLK_PREPARE, 0x061a}, + {HI846_REG_UNKNOWN_0918, 0x0407}, + {HI846_REG_THS_ZERO, 0x0a0b}, + {HI846_REG_TCLK_POST, 0x0e08}, + {HI846_REG_UNKNOWN_091E, 0x0a00}, + {HI846_REG_UNKNOWN_090C, 0x0427}, + {HI846_REG_UNKNOWN_090E, 0x0059}, + {HI846_REG_UNKNOWN_0954, 0x0089}, + {HI846_REG_UNKNOWN_0956, 0x0000}, + {HI846_REG_UNKNOWN_0958, 0xca80}, + {HI846_REG_UNKNOWN_095A, 0x9240}, + {HI846_REG_PLL_CFG_MIPI2_H, 0x4124}, + {HI846_REG_TG_ENABLE, 0x0100}, +}; + static const char * const hi846_test_pattern_menu[] = { "Disabled", "Solid Colour", @@ -1042,9 +1142,11 @@ static const char * const hi846_test_pattern_menu[] = { #define FREQ_INDEX_640 0 #define FREQ_INDEX_1280 1 +#define FREQ_INDEX_3264 2 static const s64 hi846_link_freqs[] = { [FREQ_INDEX_640] = 80000000, [FREQ_INDEX_1280] = 200000000, + [FREQ_INDEX_3264] = 288000000, }; static const struct hi846_reg_list hi846_init_regs_list_2lane = { @@ -1134,7 +1236,57 @@ static const struct hi846_mode supported_modes[] = { .width = 1632 * 2, .height = 1224 * 2, }, - } + }, + { + .width = 3264, + .height = 1836, + .link_freq_index = FREQ_INDEX_3264, + .fps = 30, + .frame_len = 2526, + .llp = HI846_LINE_LENGTH, + .reg_list_config = { + .num_of_regs = ARRAY_SIZE(mode_3264x1836_config), + .regs = mode_3264x1836_config, + }, + .reg_list_2lane = { + .num_of_regs = 0, + }, + .reg_list_4lane = { + .num_of_regs = ARRAY_SIZE(mode_3264x1836_mipi_4lane), + .regs = mode_3264x1836_mipi_4lane, + }, + .crop = { + .left = 0x46, + .top = 0x172, + .width = 3264, + .height = 1836, + }, + }, + { + .width = 3264, + .height = 2448, + .link_freq_index = FREQ_INDEX_3264, + .fps = 30, + .frame_len = 2526, + .llp = HI846_LINE_LENGTH, + .reg_list_config = { + .num_of_regs = ARRAY_SIZE(mode_3264x2448_config), + .regs = mode_3264x2448_config, + }, + .reg_list_2lane = { + .num_of_regs = 0, + }, + .reg_list_4lane = { + .num_of_regs = ARRAY_SIZE(mode_3264x2448_mipi_4lane), + .regs = mode_3264x2448_mipi_4lane, + }, + .crop = { + .left = 0x46, + .top = 0x40, + .width = 3264, + .height = 2448, + }, + }, }; struct hi846_datafmt { -- 2.54.0 ^ permalink raw reply related [flat|nested] 7+ messages in thread
* Re: [PATCH 2/2] media: hi846: Add 6MP and 8MP modes support 2026-04-29 7:03 ` [PATCH 2/2] media: hi846: Add 6MP and 8MP modes support Pengyu Luo @ 2026-04-29 9:01 ` Sakari Ailus 2026-04-30 16:18 ` Pengyu Luo 0 siblings, 1 reply; 7+ messages in thread From: Sakari Ailus @ 2026-04-29 9:01 UTC (permalink / raw) To: Pengyu Luo Cc: Martin Kepplinger-Novakovic, Mauro Carvalho Chehab, Hans Verkuil, linux-media, linux-kernel Hi Pengyu, On Wed, Apr 29, 2026 at 03:03:51PM +0800, Pengyu Luo wrote: > Hi846 is an 8MP sensor, but the upstream driver has only supported 2MP > mode for years. This patch adds 6MP and 8MP modes to maximize sensor > utilization. > > Note that these modes require 4-lane MIPI CSI-2, as the downstream > driver only exposes 2MP, 6MP, and 8MP configurations in 4-lane > operation on the target device. The register sequences are extracted > from the downstream Windows driver. > > Signed-off-by: Pengyu Luo <mitltlatltl@gmail.com> > --- > drivers/media/i2c/hi846.c | 154 +++++++++++++++++++++++++++++++++++++- > 1 file changed, 153 insertions(+), 1 deletion(-) > > diff --git a/drivers/media/i2c/hi846.c b/drivers/media/i2c/hi846.c > index 09c109f3fba9..b8ae7344f1a0 100644 > --- a/drivers/media/i2c/hi846.c > +++ b/drivers/media/i2c/hi846.c > @@ -1027,6 +1027,106 @@ static const struct hi846_reg mode_1632x1224_mipi_4lane[] = { > {HI846_REG_TG_ENABLE, 0x0100}, > }; > > +static const struct hi846_reg mode_3264x1836_config[] = { > + {HI846_REG_MODE_SELECT, 0x0000}, > + {HI846_REG_Y_ODD_INC_FOBP, 0x1111}, > + {HI846_REG_Y_ODD_INC_VACT, 0x1111}, > + {HI846_REG_Y_ADDR_START_VACT_H, 0x0172}, > + {HI846_REG_Y_ADDR_END_VACT_H, 0x089d}, > + {HI846_REG_UNKNOWN_005C, 0x2101}, > + {HI846_REG_FLL, 0x09de}, > + {HI846_REG_LLP, 0x0ed8}, > + {HI846_REG_BINNING_MODE, 0x0022}, > + {HI846_REG_HBIN_MODE, 0x0000}, > + {HI846_REG_UNKNOWN_0A24, 0x0000}, > + {HI846_REG_X_START_H, 0x0000}, > + {HI846_REG_X_OUTPUT_SIZE_H, 0x0cc0}, > + {HI846_REG_Y_OUTPUT_SIZE_H, 0x072c}, > + {HI846_REG_EXPOSURE, 0x09d8}, > + > + /* For OTP */ > + {HI846_REG_UNKNOWN_021C, 0x0001}, > + {HI846_REG_UNKNOWN_021E, 0x0235}, > + > + {HI846_REG_ISP_EN_H, 0x014a}, > + {HI846_REG_UNKNOWN_0418, 0x023e}, > + {HI846_REG_UNKNOWN_0B02, 0xe04d}, > + {HI846_REG_UNKNOWN_0B10, 0x6821}, > + {HI846_REG_UNKNOWN_0B12, 0x0120}, > + {HI846_REG_UNKNOWN_0B14, 0x0001}, > + {HI846_REG_UNKNOWN_2008, 0x38fd}, > + {HI846_REG_UNKNOWN_326E, 0x0000}, > +}; > + > +static const struct hi846_reg mode_3264x1836_mipi_4lane[] = { > + {HI846_REG_UNKNOWN_0900, 0x0300}, > + {HI846_REG_MIPI_TX_OP_MODE, 0xc319}, > + {HI846_REG_UNKNOWN_0914, 0xc109}, > + {HI846_REG_TCLK_PREPARE, 0x061a}, > + {HI846_REG_UNKNOWN_0918, 0x0407}, > + {HI846_REG_THS_ZERO, 0x0a0b}, > + {HI846_REG_TCLK_POST, 0x0e08}, > + {HI846_REG_UNKNOWN_091E, 0x0a00}, > + {HI846_REG_UNKNOWN_090C, 0x0427}, > + {HI846_REG_UNKNOWN_090E, 0x0059}, > + {HI846_REG_UNKNOWN_0954, 0x0089}, > + {HI846_REG_UNKNOWN_0956, 0x0000}, > + {HI846_REG_UNKNOWN_0958, 0xca80}, > + {HI846_REG_UNKNOWN_095A, 0x9240}, > + {HI846_REG_PLL_CFG_MIPI2_H, 0x4124}, > + {HI846_REG_TG_ENABLE, 0x0100}, > +}; > + > +static const struct hi846_reg mode_3264x2448_config[] = { > + {HI846_REG_MODE_SELECT, 0x0000}, > + {HI846_REG_Y_ODD_INC_FOBP, 0x1111}, > + {HI846_REG_Y_ODD_INC_VACT, 0x1111}, > + {HI846_REG_Y_ADDR_START_VACT_H, 0x0040}, > + {HI846_REG_Y_ADDR_END_VACT_H, 0x09cf}, > + {HI846_REG_UNKNOWN_005C, 0x2101}, > + {HI846_REG_FLL, 0x09de}, > + {HI846_REG_LLP, 0x0ed8}, > + {HI846_REG_BINNING_MODE, 0x0022}, > + {HI846_REG_HBIN_MODE, 0x0000}, > + {HI846_REG_UNKNOWN_0A24, 0x0000}, > + {HI846_REG_X_START_H, 0x0000}, > + {HI846_REG_X_OUTPUT_SIZE_H, 0x0cc0}, > + {HI846_REG_Y_OUTPUT_SIZE_H, 0x0990}, > + {HI846_REG_EXPOSURE, 0x09d8}, > + > + /* For OTP */ > + {HI846_REG_UNKNOWN_021C, 0x0001}, > + {HI846_REG_UNKNOWN_021E, 0x0235}, > + > + {HI846_REG_ISP_EN_H, 0x014a}, > + {HI846_REG_UNKNOWN_0418, 0x0000}, > + {HI846_REG_UNKNOWN_0B02, 0xe04d}, > + {HI846_REG_UNKNOWN_0B10, 0x6821}, > + {HI846_REG_UNKNOWN_0B12, 0x0120}, > + {HI846_REG_UNKNOWN_0B14, 0x0001}, > + {HI846_REG_UNKNOWN_2008, 0x38fd}, > + {HI846_REG_UNKNOWN_326E, 0x0000}, > +}; > + > +static const struct hi846_reg mode_3264x2448_mipi_4lane[] = { > + {HI846_REG_UNKNOWN_0900, 0x0300}, > + {HI846_REG_MIPI_TX_OP_MODE, 0xc319}, > + {HI846_REG_UNKNOWN_0914, 0xc109}, > + {HI846_REG_TCLK_PREPARE, 0x061a}, > + {HI846_REG_UNKNOWN_0918, 0x0407}, > + {HI846_REG_THS_ZERO, 0x0a0b}, > + {HI846_REG_TCLK_POST, 0x0e08}, > + {HI846_REG_UNKNOWN_091E, 0x0a00}, > + {HI846_REG_UNKNOWN_090C, 0x0427}, > + {HI846_REG_UNKNOWN_090E, 0x0059}, > + {HI846_REG_UNKNOWN_0954, 0x0089}, > + {HI846_REG_UNKNOWN_0956, 0x0000}, > + {HI846_REG_UNKNOWN_0958, 0xca80}, > + {HI846_REG_UNKNOWN_095A, 0x9240}, > + {HI846_REG_PLL_CFG_MIPI2_H, 0x4124}, > + {HI846_REG_TG_ENABLE, 0x0100}, > +}; > + > static const char * const hi846_test_pattern_menu[] = { > "Disabled", > "Solid Colour", > @@ -1042,9 +1142,11 @@ static const char * const hi846_test_pattern_menu[] = { > > #define FREQ_INDEX_640 0 > #define FREQ_INDEX_1280 1 > +#define FREQ_INDEX_3264 2 > static const s64 hi846_link_freqs[] = { > [FREQ_INDEX_640] = 80000000, > [FREQ_INDEX_1280] = 200000000, > + [FREQ_INDEX_3264] = 288000000, Looking at the driver, the PLL configuration is present in the lane number specific register list so the link frequency is in fact the same for all modes. This problem isn't introduced by this patch but I think this needs to be fixed before adding further modes to the driver. The pixel rate is likely incorrect as well. > }; > > static const struct hi846_reg_list hi846_init_regs_list_2lane = { > @@ -1134,7 +1236,57 @@ static const struct hi846_mode supported_modes[] = { > .width = 1632 * 2, > .height = 1224 * 2, > }, > - } > + }, > + { > + .width = 3264, > + .height = 1836, > + .link_freq_index = FREQ_INDEX_3264, > + .fps = 30, > + .frame_len = 2526, > + .llp = HI846_LINE_LENGTH, > + .reg_list_config = { > + .num_of_regs = ARRAY_SIZE(mode_3264x1836_config), > + .regs = mode_3264x1836_config, > + }, > + .reg_list_2lane = { > + .num_of_regs = 0, > + }, > + .reg_list_4lane = { > + .num_of_regs = ARRAY_SIZE(mode_3264x1836_mipi_4lane), > + .regs = mode_3264x1836_mipi_4lane, > + }, > + .crop = { > + .left = 0x46, > + .top = 0x172, > + .width = 3264, > + .height = 1836, > + }, > + }, > + { > + .width = 3264, > + .height = 2448, > + .link_freq_index = FREQ_INDEX_3264, > + .fps = 30, > + .frame_len = 2526, > + .llp = HI846_LINE_LENGTH, > + .reg_list_config = { > + .num_of_regs = ARRAY_SIZE(mode_3264x2448_config), > + .regs = mode_3264x2448_config, > + }, > + .reg_list_2lane = { > + .num_of_regs = 0, > + }, > + .reg_list_4lane = { > + .num_of_regs = ARRAY_SIZE(mode_3264x2448_mipi_4lane), > + .regs = mode_3264x2448_mipi_4lane, > + }, > + .crop = { > + .left = 0x46, > + .top = 0x40, > + .width = 3264, > + .height = 2448, > + }, > + }, > }; > > struct hi846_datafmt { -- Regards, Sakari Ailus ^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [PATCH 2/2] media: hi846: Add 6MP and 8MP modes support 2026-04-29 9:01 ` Sakari Ailus @ 2026-04-30 16:18 ` Pengyu Luo 2026-04-30 20:39 ` Sakari Ailus 0 siblings, 1 reply; 7+ messages in thread From: Pengyu Luo @ 2026-04-30 16:18 UTC (permalink / raw) To: Sakari Ailus Cc: Martin Kepplinger-Novakovic, Mauro Carvalho Chehab, Hans Verkuil, linux-media, linux-kernel On Wed, Apr 29, 2026 at 5:01 PM Sakari Ailus <sakari.ailus@linux.intel.com> wrote: > > Hi Pengyu, > > On Wed, Apr 29, 2026 at 03:03:51PM +0800, Pengyu Luo wrote: > > Hi846 is an 8MP sensor, but the upstream driver has only supported 2MP > > mode for years. This patch adds 6MP and 8MP modes to maximize sensor > > utilization. > > > > Note that these modes require 4-lane MIPI CSI-2, as the downstream > > driver only exposes 2MP, 6MP, and 8MP configurations in 4-lane > > operation on the target device. The register sequences are extracted > > from the downstream Windows driver. > > > > Signed-off-by: Pengyu Luo <mitltlatltl@gmail.com> > > --- > > drivers/media/i2c/hi846.c | 154 +++++++++++++++++++++++++++++++++++++- > > 1 file changed, 153 insertions(+), 1 deletion(-) > > > > diff --git a/drivers/media/i2c/hi846.c b/drivers/media/i2c/hi846.c > > index 09c109f3fba9..b8ae7344f1a0 100644 > > --- a/drivers/media/i2c/hi846.c > > +++ b/drivers/media/i2c/hi846.c > > @@ -1027,6 +1027,106 @@ static const struct hi846_reg mode_1632x1224_mipi_4lane[] = { > > {HI846_REG_TG_ENABLE, 0x0100}, > > }; > > > > +static const struct hi846_reg mode_3264x1836_config[] = { > > + {HI846_REG_MODE_SELECT, 0x0000}, > > + {HI846_REG_Y_ODD_INC_FOBP, 0x1111}, > > + {HI846_REG_Y_ODD_INC_VACT, 0x1111}, > > + {HI846_REG_Y_ADDR_START_VACT_H, 0x0172}, > > + {HI846_REG_Y_ADDR_END_VACT_H, 0x089d}, > > + {HI846_REG_UNKNOWN_005C, 0x2101}, > > + {HI846_REG_FLL, 0x09de}, > > + {HI846_REG_LLP, 0x0ed8}, > > + {HI846_REG_BINNING_MODE, 0x0022}, > > + {HI846_REG_HBIN_MODE, 0x0000}, > > + {HI846_REG_UNKNOWN_0A24, 0x0000}, > > + {HI846_REG_X_START_H, 0x0000}, > > + {HI846_REG_X_OUTPUT_SIZE_H, 0x0cc0}, > > + {HI846_REG_Y_OUTPUT_SIZE_H, 0x072c}, > > + {HI846_REG_EXPOSURE, 0x09d8}, > > + > > + /* For OTP */ > > + {HI846_REG_UNKNOWN_021C, 0x0001}, > > + {HI846_REG_UNKNOWN_021E, 0x0235}, > > + > > + {HI846_REG_ISP_EN_H, 0x014a}, > > + {HI846_REG_UNKNOWN_0418, 0x023e}, > > + {HI846_REG_UNKNOWN_0B02, 0xe04d}, > > + {HI846_REG_UNKNOWN_0B10, 0x6821}, > > + {HI846_REG_UNKNOWN_0B12, 0x0120}, > > + {HI846_REG_UNKNOWN_0B14, 0x0001}, > > + {HI846_REG_UNKNOWN_2008, 0x38fd}, > > + {HI846_REG_UNKNOWN_326E, 0x0000}, > > +}; > > + > > +static const struct hi846_reg mode_3264x1836_mipi_4lane[] = { > > + {HI846_REG_UNKNOWN_0900, 0x0300}, > > + {HI846_REG_MIPI_TX_OP_MODE, 0xc319}, > > + {HI846_REG_UNKNOWN_0914, 0xc109}, > > + {HI846_REG_TCLK_PREPARE, 0x061a}, > > + {HI846_REG_UNKNOWN_0918, 0x0407}, > > + {HI846_REG_THS_ZERO, 0x0a0b}, > > + {HI846_REG_TCLK_POST, 0x0e08}, > > + {HI846_REG_UNKNOWN_091E, 0x0a00}, > > + {HI846_REG_UNKNOWN_090C, 0x0427}, > > + {HI846_REG_UNKNOWN_090E, 0x0059}, > > + {HI846_REG_UNKNOWN_0954, 0x0089}, > > + {HI846_REG_UNKNOWN_0956, 0x0000}, > > + {HI846_REG_UNKNOWN_0958, 0xca80}, > > + {HI846_REG_UNKNOWN_095A, 0x9240}, > > + {HI846_REG_PLL_CFG_MIPI2_H, 0x4124}, > > + {HI846_REG_TG_ENABLE, 0x0100}, > > +}; > > + > > +static const struct hi846_reg mode_3264x2448_config[] = { > > + {HI846_REG_MODE_SELECT, 0x0000}, > > + {HI846_REG_Y_ODD_INC_FOBP, 0x1111}, > > + {HI846_REG_Y_ODD_INC_VACT, 0x1111}, > > + {HI846_REG_Y_ADDR_START_VACT_H, 0x0040}, > > + {HI846_REG_Y_ADDR_END_VACT_H, 0x09cf}, > > + {HI846_REG_UNKNOWN_005C, 0x2101}, > > + {HI846_REG_FLL, 0x09de}, > > + {HI846_REG_LLP, 0x0ed8}, > > + {HI846_REG_BINNING_MODE, 0x0022}, > > + {HI846_REG_HBIN_MODE, 0x0000}, > > + {HI846_REG_UNKNOWN_0A24, 0x0000}, > > + {HI846_REG_X_START_H, 0x0000}, > > + {HI846_REG_X_OUTPUT_SIZE_H, 0x0cc0}, > > + {HI846_REG_Y_OUTPUT_SIZE_H, 0x0990}, > > + {HI846_REG_EXPOSURE, 0x09d8}, > > + > > + /* For OTP */ > > + {HI846_REG_UNKNOWN_021C, 0x0001}, > > + {HI846_REG_UNKNOWN_021E, 0x0235}, > > + > > + {HI846_REG_ISP_EN_H, 0x014a}, > > + {HI846_REG_UNKNOWN_0418, 0x0000}, > > + {HI846_REG_UNKNOWN_0B02, 0xe04d}, > > + {HI846_REG_UNKNOWN_0B10, 0x6821}, > > + {HI846_REG_UNKNOWN_0B12, 0x0120}, > > + {HI846_REG_UNKNOWN_0B14, 0x0001}, > > + {HI846_REG_UNKNOWN_2008, 0x38fd}, > > + {HI846_REG_UNKNOWN_326E, 0x0000}, > > +}; > > + > > +static const struct hi846_reg mode_3264x2448_mipi_4lane[] = { > > + {HI846_REG_UNKNOWN_0900, 0x0300}, > > + {HI846_REG_MIPI_TX_OP_MODE, 0xc319}, > > + {HI846_REG_UNKNOWN_0914, 0xc109}, > > + {HI846_REG_TCLK_PREPARE, 0x061a}, > > + {HI846_REG_UNKNOWN_0918, 0x0407}, > > + {HI846_REG_THS_ZERO, 0x0a0b}, > > + {HI846_REG_TCLK_POST, 0x0e08}, > > + {HI846_REG_UNKNOWN_091E, 0x0a00}, > > + {HI846_REG_UNKNOWN_090C, 0x0427}, > > + {HI846_REG_UNKNOWN_090E, 0x0059}, > > + {HI846_REG_UNKNOWN_0954, 0x0089}, > > + {HI846_REG_UNKNOWN_0956, 0x0000}, > > + {HI846_REG_UNKNOWN_0958, 0xca80}, > > + {HI846_REG_UNKNOWN_095A, 0x9240}, > > + {HI846_REG_PLL_CFG_MIPI2_H, 0x4124}, > > + {HI846_REG_TG_ENABLE, 0x0100}, > > +}; > > + > > static const char * const hi846_test_pattern_menu[] = { > > "Disabled", > > "Solid Colour", > > @@ -1042,9 +1142,11 @@ static const char * const hi846_test_pattern_menu[] = { > > > > #define FREQ_INDEX_640 0 > > #define FREQ_INDEX_1280 1 > > +#define FREQ_INDEX_3264 2 > > static const s64 hi846_link_freqs[] = { > > [FREQ_INDEX_640] = 80000000, > > [FREQ_INDEX_1280] = 200000000, > > + [FREQ_INDEX_3264] = 288000000, > > Looking at the driver, the PLL configuration is present in the lane number > specific register list so the link frequency is in fact the same for all > modes. This problem isn't introduced by this patch but I think this needs > to be fixed before adding further modes to the driver. > TBH, I didn't investigate the calculations. I just searched the datasheet for it. They are exactly wrong. Thans for pointing out this. I can fix it, but the question is this driver was writing against 25Mhz mclk, but the typical value from datasheet is 24Mhz, and one device is using 25Mhz as the clock rate, my device is using 24Mhz, 25Mhz is unsupported on my platform. It seems that making the list not be runtime is not allowed as nobody did like this. Best wishes, Pengyu > The pixel rate is likely incorrect as well. > > > }; > > > > static const struct hi846_reg_list hi846_init_regs_list_2lane = { > > @@ -1134,7 +1236,57 @@ static const struct hi846_mode supported_modes[] = { > > .width = 1632 * 2, > > .height = 1224 * 2, > > }, > > - } > > + }, > > + { > > + .width = 3264, > > + .height = 1836, > > + .link_freq_index = FREQ_INDEX_3264, > > + .fps = 30, > > + .frame_len = 2526, > > + .llp = HI846_LINE_LENGTH, > > + .reg_list_config = { > > + .num_of_regs = ARRAY_SIZE(mode_3264x1836_config), > > + .regs = mode_3264x1836_config, > > + }, > > + .reg_list_2lane = { > > + .num_of_regs = 0, > > + }, > > + .reg_list_4lane = { > > + .num_of_regs = ARRAY_SIZE(mode_3264x1836_mipi_4lane), > > + .regs = mode_3264x1836_mipi_4lane, > > + }, > > + .crop = { > > + .left = 0x46, > > + .top = 0x172, > > + .width = 3264, > > + .height = 1836, > > + }, > > + }, > > + { > > + .width = 3264, > > + .height = 2448, > > + .link_freq_index = FREQ_INDEX_3264, > > + .fps = 30, > > + .frame_len = 2526, > > + .llp = HI846_LINE_LENGTH, > > + .reg_list_config = { > > + .num_of_regs = ARRAY_SIZE(mode_3264x2448_config), > > + .regs = mode_3264x2448_config, > > + }, > > + .reg_list_2lane = { > > + .num_of_regs = 0, > > + }, > > + .reg_list_4lane = { > > + .num_of_regs = ARRAY_SIZE(mode_3264x2448_mipi_4lane), > > + .regs = mode_3264x2448_mipi_4lane, > > + }, > > + .crop = { > > + .left = 0x46, > > + .top = 0x40, > > + .width = 3264, > > + .height = 2448, > > + }, > > + }, > > }; > > > > struct hi846_datafmt { > > -- > Regards, > > Sakari Ailus ^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [PATCH 2/2] media: hi846: Add 6MP and 8MP modes support 2026-04-30 16:18 ` Pengyu Luo @ 2026-04-30 20:39 ` Sakari Ailus 0 siblings, 0 replies; 7+ messages in thread From: Sakari Ailus @ 2026-04-30 20:39 UTC (permalink / raw) To: Pengyu Luo Cc: Martin Kepplinger-Novakovic, Mauro Carvalho Chehab, Hans Verkuil, linux-media, linux-kernel On Fri, May 01, 2026 at 12:18:09AM +0800, Pengyu Luo wrote: > > > @@ -1042,9 +1142,11 @@ static const char * const hi846_test_pattern_menu[] = { > > > > > > #define FREQ_INDEX_640 0 > > > #define FREQ_INDEX_1280 1 > > > +#define FREQ_INDEX_3264 2 > > > static const s64 hi846_link_freqs[] = { > > > [FREQ_INDEX_640] = 80000000, > > > [FREQ_INDEX_1280] = 200000000, > > > + [FREQ_INDEX_3264] = 288000000, > > > > Looking at the driver, the PLL configuration is present in the lane number > > specific register list so the link frequency is in fact the same for all > > modes. This problem isn't introduced by this patch but I think this needs > > to be fixed before adding further modes to the driver. > > > > TBH, I didn't investigate the calculations. I just searched the > datasheet for it. They are exactly wrong. Thans for pointing out this. > > I can fix it, but the question is this driver was writing against > 25Mhz mclk, but the typical value from datasheet is 24Mhz, and one > device is using 25Mhz as the clock rate, my device is using 24Mhz, > 25Mhz is unsupported on my platform. It seems that making the list not > be runtime is not allowed as nobody did like this. What do you mean? PLL configuration calculated from platform and runtime configuration is definitely preferred over hard-coded configuration, albeit much harder to implement. It also helps to prevent problems such as the one above. -- Sakari Ailus ^ permalink raw reply [flat|nested] 7+ messages in thread
end of thread, other threads:[~2026-04-30 20:39 UTC | newest] Thread overview: 7+ messages (download: mbox.gz follow: Atom feed -- links below jump to the message on this page -- 2026-04-29 7:03 [PATCH 0/2] fix and improve for Hi846 Pengyu Luo 2026-04-29 7:03 ` [PATCH 1/2] media: hi846: fix hi846_write_reg_16 handling Pengyu Luo 2026-04-29 7:44 ` Sakari Ailus 2026-04-29 7:03 ` [PATCH 2/2] media: hi846: Add 6MP and 8MP modes support Pengyu Luo 2026-04-29 9:01 ` Sakari Ailus 2026-04-30 16:18 ` Pengyu Luo 2026-04-30 20:39 ` Sakari Ailus
This is a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox