From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from mail-lf1-f54.google.com (mail-lf1-f54.google.com [209.85.167.54]) (using TLSv1.2 with cipher ECDHE-RSA-AES128-GCM-SHA256 (128/128 bits)) (No client certificate requested) by smtp.subspace.kernel.org (Postfix) with ESMTPS id BB704344DB9 for ; Fri, 24 Jul 2026 15:03:35 +0000 (UTC) Authentication-Results: smtp.subspace.kernel.org; arc=none smtp.client-ip=209.85.167.54 ARC-Seal:i=1; a=rsa-sha256; d=subspace.kernel.org; s=arc-20240116; t=1784905419; cv=none; b=o3+fKQ/8koYpEA/wQ7ujRhVvn73yLCq8dXTpOM90mplI1pgwYUqkPoU2XGrPb7VYlU4NdkJNj1IvsF8QGAJXPqeI3CwMwg77HlFydjX4cGoDE0oiG6wBJOPQkO9qp2IjC3Ld/h3tC01GQWX1tjOIQIKYGXrBg22a9cDSAc4KJPI= ARC-Message-Signature:i=1; a=rsa-sha256; d=subspace.kernel.org; s=arc-20240116; t=1784905419; c=relaxed/simple; bh=AR8ld8XD/38w5MrYURYat7EqnABuYvI8nC43ZIxDCiA=; h=Message-ID:Date:MIME-Version:Subject:To:Cc:References:From: In-Reply-To:Content-Type; b=QVJyQQnMSR2EyCHxK2Zwnwce6qtyswsRszRE/ZTOQo+U9ErxE9HIpT3xRkMtjgrGZa2UgLZdih2ffSsogjE8joDxZlEkO0dA5IY8pmmqm4J1ABX8V5oHQ6Pv9UCtJvlLCwMVvVdBNNNStcoCrkOHMzYIA9HhDvW+rsdVV/WNdFQ= ARC-Authentication-Results:i=1; smtp.subspace.kernel.org; dmarc=pass (p=none dis=none) header.from=linaro.org; spf=pass smtp.mailfrom=linaro.org; dkim=pass (2048-bit key) header.d=linaro.org header.i=@linaro.org header.b=LTqPCJIc; arc=none smtp.client-ip=209.85.167.54 Authentication-Results: smtp.subspace.kernel.org; dmarc=pass (p=none dis=none) header.from=linaro.org Authentication-Results: smtp.subspace.kernel.org; spf=pass smtp.mailfrom=linaro.org Authentication-Results: smtp.subspace.kernel.org; dkim=pass (2048-bit key) header.d=linaro.org header.i=@linaro.org header.b="LTqPCJIc" Received: by mail-lf1-f54.google.com with SMTP id 2adb3069b0e04-5aeafa51b5cso69018e87.1 for ; Fri, 24 Jul 2026 08:03:35 -0700 (PDT) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=linaro.org; s=google; t=1784905414; x=1785510214; darn=vger.kernel.org; h=content-transfer-encoding:content-type:in-reply-to:from:references :cc:to:content-language:subject:user-agent:mime-version:date :message-id:from:to:cc:subject:date:message-id:reply-to:content-type; bh=08Xcvy5ZHNOma1gRzeCr8NdculbpMQV6NvuXqJhWjx8=; b=LTqPCJIc7MF2vyh57RAZvMUxWtFTlEuMQEUNnTBAaV3gtd3kS/gfifdRFGR40JmJPB wAfdThJ0EYNZ8DP+7SNR3ipzR/Vqf9SI4zdJJh6WDwMdmtRc433rQ+OG0lx3vCVhPG8P iA0ensh3vvDTzz+r7epLZv3UlBoNS868i7E2jV71tRcC9+O3/jzJ0lx2Xm52vrfjtarT 1KlqwkmGi5z99XTJKCCF0PXDhCd4YZsdQg85PobHQIoq0EaxvXxjStvdE3sBuLL/iw5x CD7OvmUTC7S08pTzSDuaCM12y9mhDl0eAjcsKujkCoDbSxfCr0otdnaWeKMt6RG/ebaG jBrQ== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20251104; t=1784905414; x=1785510214; h=content-transfer-encoding:content-type:in-reply-to:from:references :cc:to:content-language:subject:user-agent:mime-version:date :message-id:x-gm-gg:x-gm-message-state:from:to:cc:subject:date :message-id:reply-to:content-type; bh=08Xcvy5ZHNOma1gRzeCr8NdculbpMQV6NvuXqJhWjx8=; b=ARFHSIm3sDEj2U+jQqy6c5UmzFfhbln8fmtKV0paiUD1vQO5LnrPlF/R3iZ73Bi+Uu //eI5dzH89O4OP+6hSlF3CdC4fURWbIers5uf8kXqFbs75fAKgnEEU3XD/AbkPUphmGy Xtq5zgFaFBL3HLfsWgAqQGiPJXFMShCWlwAEZ134KzTlNzj5+YwOThZoFu11FvD9xHPN YeFXMi5jcZpybpnwTsOWG41uqX2IvU3HjdDn56CAJHtpc+8WHAEzLfdZmXPCKW9hK/TT rDCdJH3+Au/F5fINYS/rJxD84Eczbro7tb4Dfl5P9meILh0+g9u8aWT+yJm5esdLUhx3 tZ9Q== X-Forwarded-Encrypted: i=1; AHgh+RqlRIr8n0mpIT9iZ9c1EqOqvfZxR4LjcqB3DFrc068ORRYpTD3LRXCJjY9jLbJ5/NjQ6i1BQvJ53GeHa3M=@vger.kernel.org X-Gm-Message-State: AOJu0YzyWde91IRQUxHbjNJC+HGc21uYIDqJpkz5svdvglhATEIqJkdT NWj2jhC+lB6laso1Gtb5AZCH0cK2WsqoE26BM/QuQWaaoSOEbUbwUso2F3MxFhdTOBIyYW0AwWn uIc1xQsg= X-Gm-Gg: AR+sD10xN+88k1gKw0rtb3eWzbJ7zOmuH/KueFY9+zgpqMnei5zG27Ve89L3nCS26+8 j+o94wCOVLOsL0BFbfV3so3SCu3huhfOglabrq0W6CPlxbtURRTHLKvqa4700QDYbC2UkCSMEi+ URxKkPylLtq+k5se9etiwW0hwVjlbG5rpTB3twODsTlDAplkDg6//F8Tywp1ZXJjrCEoPNk/WGY c5rbSuye+FvvwpdFVmauXjr5PXQr9fF1aVsroy3UKxczZxhQjMpgtHjyHFlmY9U8yFrpacF6xwk eSMHYB/GwWG7Pb1JgUNqWBNnclfjLL/1O7f4rMfcl8GPBqaLyPnU/3M0kjvBkly/cNTs6Vm9Ls2 m1oRWSUoYldAsWEbClZMQNF9WAeeSxeOtUEq3Ry135CG3tag4W7pIzWwOmpRRvrrwX/DpojMLTR bJ5LECqYf2wi1wYC9OFtWPeFc03Hjgq8/ewHH3J1VSZmjPe3ON4/ofHVgx X-Received: by 2002:a05:6512:15a1:b0:5ae:c5b2:9cbd with SMTP id 2adb3069b0e04-5b2b7b17ca7mr532480e87.4.1784905413426; Fri, 24 Jul 2026 08:03:33 -0700 (PDT) Received: from [192.168.1.100] (91-159-24-186.elisa-laajakaista.fi. [91.159.24.186]) by smtp.gmail.com with ESMTPSA id 38308e7fff4ca-39f221739c1sm756201fa.6.2026.07.24.08.03.32 (version=TLS1_3 cipher=TLS_AES_128_GCM_SHA256 bits=128/128); Fri, 24 Jul 2026 08:03:32 -0700 (PDT) Message-ID: <39fcfa46-79a3-4ea8-b314-d59ed768e553@linaro.org> Date: Fri, 24 Jul 2026 18:03:32 +0300 Precedence: bulk X-Mailing-List: linux-kernel@vger.kernel.org List-Id: List-Subscribe: List-Unsubscribe: MIME-Version: 1.0 User-Agent: Mozilla Thunderbird Subject: Re: [PATCH v3 08/17] media: i2c: os05b10: add 12-bit RAW mode support Content-Language: ru-RU To: Tarang Raval , "sakari.ailus@linux.intel.com" , "mehdi.djait@linux.intel.com" Cc: Himanshu Bhavani , Elgin Perumbilly , Mauro Carvalho Chehab , Hans Verkuil , "linux-media@vger.kernel.org" , "linux-kernel@vger.kernel.org" References: <20260718200912.16001-1-tarang.raval@siliconsignals.io> <20260718200912.16001-9-tarang.raval@siliconsignals.io> From: Vladimir Zapolskiy In-Reply-To: Content-Type: text/plain; charset=UTF-8; format=flowed Content-Transfer-Encoding: 7bit On 7/24/26 17:30, Tarang Raval wrote: > Hi Vladimir, > > Thanks for the review. > >> On 7/18/26 23:08, Tarang Raval wrote: >>> Expose a 12-bit Bayer output option in the OS05B10 V4L2 sub-device driver. >>> >>> Add a 12-bit mode table alongside the existing 10-bit mode, extend the >>> enumerated mbus codes to include RAW12, and select the correct mode table >>> based on the requested mbus format in enum_frame_size and stream enable. >>> >>> Also move OS05B10_REG_MIPI_SC_CTRL_1 programming out of the common register >>> list and program it at stream-on depending on the selected mode bpp (10/12). >>> >>> Signed-off-by: Tarang Raval >>> --- >>> drivers/media/i2c/os05b10.c | 112 ++++++++++++++++++++++++++++++------ >>> 1 file changed, 96 insertions(+), 16 deletions(-) >>> >>> diff --git a/drivers/media/i2c/os05b10.c b/drivers/media/i2c/os05b10.c >>> index 4e177eacc815..e11a3c308299 100644 >>> --- a/drivers/media/i2c/os05b10.c >>> +++ b/drivers/media/i2c/os05b10.c >>> @@ -146,7 +146,6 @@ static const struct cci_reg_sequence os05b10_common_regs[] = { >>> { CCI_REG8(0x301e), 0xb4 }, >>> { CCI_REG8(0x301f), 0xd0 }, >>> { CCI_REG8(0x3021), 0x03 }, >>> - { OS05B10_REG_MIPI_SC_CTRL_1, 0x01 }, >>> { CCI_REG8(0x3107), 0xa1 }, >>> { CCI_REG8(0x3108), 0x7d }, >>> { CCI_REG8(0x3109), 0xfc }, >>> @@ -500,6 +499,21 @@ struct os05b10_mode { >>> struct os05b10_reg_list reg_list; >>> }; >>> >>> +static const struct os05b10_mode supported_modes_12bit[] = { >>> + { >>> + .width = 2592, >>> + .height = 1944, >>> + .vts = 2007, >>> + .hts = 1744, >> >> It's unusual to see .hts < .width and .vts < .height. Will is cause >> errors in hblank/vblank computations in os05b10_set_framing_limits()? > > Negative hblank is accepted by the framework, so no, I'm not seeing any > errors from it. > It could be so, but in os05b10_set_framing_limits() hblank/vblank types are unsigned (and that's correct), so there is a likely unwanted assigment of a negative value to an unsigned variable. It's better to correct it. > But you're right that it doesn't make logical/physical sense. I got a > similar comment from Jai on patch 15/17 about this. > > I'll update it accordingly, the datasheet doesn't have any information > about the HTS register's unit/clock domain, so I'm doing some testing on > hardware first and will update this once I have concrete numbers. > >>> + .exp = 1900, >>> + .bpp = 12, >>> + .reg_list = { >>> + .num_of_regs = ARRAY_SIZE(mode_2592_1944_regs), >>> + .regs = mode_2592_1944_regs, >>> + }, >>> + }, >>> +}; >>> + >>> static const struct os05b10_mode supported_modes_10bit[] = { >>> { >>> .width = 2592, >>> @@ -521,6 +535,7 @@ static const s64 link_frequencies[] = { >>> >>> static const u32 os05b10_mbus_codes[] = { >>> MEDIA_BUS_FMT_SBGGR10_1X10, >>> + MEDIA_BUS_FMT_SBGGR12_1X12, >>> }; >>> >>> static const char * const os05b10_test_pattern_menu[] = { >>> @@ -552,14 +567,20 @@ static inline struct os05b10 *to_os05b10(struct v4l2_subdev *sd) >>> return container_of_const(sd, struct os05b10, sd); >>> }; >>> >>> -static u32 os05b10_get_format_code(struct os05b10 *os05b10) >>> +static u32 os05b10_get_format_code(struct os05b10 *os05b10, u8 bpp) >>> { >>> - static const u32 codes[2][2] = { >>> - { MEDIA_BUS_FMT_SBGGR10_1X10, MEDIA_BUS_FMT_SGBRG10_1X10, }, >>> - { MEDIA_BUS_FMT_SGRBG10_1X10, MEDIA_BUS_FMT_SRGGB10_1X10, }, >>> + static const u32 codes[2][2][2] = { >>> + { /* 10 bpp */ >>> + { MEDIA_BUS_FMT_SBGGR10_1X10, MEDIA_BUS_FMT_SGBRG10_1X10 }, >>> + { MEDIA_BUS_FMT_SGRBG10_1X10, MEDIA_BUS_FMT_SRGGB10_1X10 }, >>> + }, >>> + { /* 12 bpp */ >>> + { MEDIA_BUS_FMT_SBGGR12_1X12, MEDIA_BUS_FMT_SGBRG12_1X12 }, >>> + { MEDIA_BUS_FMT_SGRBG12_1X12, MEDIA_BUS_FMT_SRGGB12_1X12 }, >>> + }, >>> }; >>> >>> - return codes[os05b10->vflip->val][os05b10->hflip->val]; >>> + return codes[bpp == 12][os05b10->vflip->val][os05b10->hflip->val]; >>> } >>> >>> static int os05b10_update_test_pattern(struct os05b10 *os05b10, u32 pattern) >>> @@ -571,6 +592,34 @@ static int os05b10_update_test_pattern(struct os05b10 *os05b10, u32 pattern) >>> os05b10_tp_val[pattern], NULL); >>> } >>> >>> +static int get_mode_table(struct os05b10 *os05b10, unsigned int code, >>> + const struct os05b10_mode **mode_list, >>> + unsigned int *num_modes) >>> +{ >>> + switch (code) { >>> + case MEDIA_BUS_FMT_SBGGR12_1X12: >>> + case MEDIA_BUS_FMT_SGBRG12_1X12: >>> + case MEDIA_BUS_FMT_SGRBG12_1X12: >>> + case MEDIA_BUS_FMT_SRGGB12_1X12: >>> + *mode_list = supported_modes_12bit; >>> + *num_modes = ARRAY_SIZE(supported_modes_12bit); >>> + return 0; >>> + >>> + case MEDIA_BUS_FMT_SBGGR10_1X10: >>> + case MEDIA_BUS_FMT_SGBRG10_1X10: >>> + case MEDIA_BUS_FMT_SGRBG10_1X10: >>> + case MEDIA_BUS_FMT_SRGGB10_1X10: >>> + *mode_list = supported_modes_10bit; >>> + *num_modes = ARRAY_SIZE(supported_modes_10bit); >>> + return 0; >>> + >>> + default: >>> + dev_err(os05b10->dev, >>> + "Unsupported media bus format: %#x\n", code); >>> + return -EINVAL; >>> + } >>> +} >>> + >>> static int os05b10_set_ctrl(struct v4l2_ctrl *ctrl) >>> { >>> struct os05b10 *os05b10 = container_of_const(ctrl->handler, >>> @@ -650,8 +699,8 @@ static int os05b10_enum_mbus_code(struct v4l2_subdev *sd, >>> if (code->index >= ARRAY_SIZE(os05b10_mbus_codes)) >>> return -EINVAL; >>> >>> - code->code = os05b10_get_format_code(os05b10); >>> - >>> + code->code = os05b10_get_format_code(os05b10, >>> + (code->index == 1) ? 12 : 10); >> >> os05b10_mbus_codes[code->index] == MEDIA_BUS_FMT_SBGGR12_1X12 is more >> verbose, but seems to be a better and more reliable check. >> >> Probably a simple inline function to get bpp from code->index can be added. >> >> Another option is to change the second argument of os05b10_get_format_code() >> from bpp to just media bus format, so you can write >> >> os05b10_get_format_code(os05b10, os05b10_mbus_codes[code->index]); > > Okay, I will update the os05b10_get_format_code 2nd argument. > Hopefully it's not a big disadvantage, a bit of complexity will be moved to function body from the caller's side. Thank you! -- Best wishes, Vladimir