| Message ID | 20201116155008.118124-1-robert.foss@linaro.org (mailing list archive) |
|---|---|
| State | Rejected |
| Headers | show |
| Series | media: ov8856: Remove 3280x2464 mode | expand |
Hi Robert, Thanks for the patch. On Mon, 2020-11-16 at 16:50 +0100, Robert Foss wrote: > Remove the 3280x2464 mode as it can't be reproduced and yields > an output resolution of 3264x2448 instead of the desired one. > > Furthermore the 3264x2448 resolution is the highest resolution > that the product brief lists. > > Since 3280x2464 neither works correctly nor seems to be supported > by the sensor, let's remove it. > In fact, I was also confused about 3280x2464 setting at the beginning. From datasheet, the OV8856 shall support an active array of 3264x2448 pixels (8-megapixel, the maximum) operating at 30fps. > Signed-off-by: Robert Foss <robert.foss@linaro.org> > --- > drivers/media/i2c/ov8856.c | 202 ------------------------------------- > 1 file changed, 202 deletions(-) > > diff --git a/drivers/media/i2c/ov8856.c b/drivers/media/i2c/ov8856.c > index 2f4ceaa80593..3365d19a303d 100644 > --- a/drivers/media/i2c/ov8856.c > +++ b/drivers/media/i2c/ov8856.c > @@ -148,196 +148,6 @@ static const struct ov8856_reg mipi_data_rate_360mbps[] = { > {0x031e, 0x0c}, > }; > > -static const struct ov8856_reg mode_3280x2464_regs[] = { > - {0x3000, 0x20}, > - {0x3003, 0x08}, > - {0x300e, 0x20}, > - {0x3010, 0x00}, > - {0x3015, 0x84}, > - {0x3018, 0x72}, > - {0x3021, 0x23}, > - {0x3033, 0x24}, > - {0x3500, 0x00}, > - {0x3501, 0x9a}, > - {0x3502, 0x20}, > - {0x3503, 0x08}, > - {0x3505, 0x83}, > - {0x3508, 0x01}, > - {0x3509, 0x80}, > - {0x350c, 0x00}, > - {0x350d, 0x80}, > - {0x350e, 0x04}, > - {0x350f, 0x00}, > - {0x3510, 0x00}, > - {0x3511, 0x02}, > - {0x3512, 0x00}, > - {0x3600, 0x72}, > - {0x3601, 0x40}, > - {0x3602, 0x30}, > - {0x3610, 0xc5}, > - {0x3611, 0x58}, > - {0x3612, 0x5c}, > - {0x3613, 0xca}, > - {0x3614, 0x20}, > - {0x3628, 0xff}, > - {0x3629, 0xff}, > - {0x362a, 0xff}, > - {0x3633, 0x10}, > - {0x3634, 0x10}, > - {0x3635, 0x10}, > - {0x3636, 0x10}, > - {0x3663, 0x08}, > - {0x3669, 0x34}, > - {0x366e, 0x10}, > - {0x3706, 0x86}, > - {0x370b, 0x7e}, > - {0x3714, 0x23}, > - {0x3730, 0x12}, > - {0x3733, 0x10}, > - {0x3764, 0x00}, > - {0x3765, 0x00}, > - {0x3769, 0x62}, > - {0x376a, 0x2a}, > - {0x376b, 0x30}, > - {0x3780, 0x00}, > - {0x3781, 0x24}, > - {0x3782, 0x00}, > - {0x3783, 0x23}, > - {0x3798, 0x2f}, > - {0x37a1, 0x60}, > - {0x37a8, 0x6a}, > - {0x37ab, 0x3f}, > - {0x37c2, 0x04}, > - {0x37c3, 0xf1}, > - {0x37c9, 0x80}, > - {0x37cb, 0x16}, > - {0x37cc, 0x16}, > - {0x37cd, 0x16}, > - {0x37ce, 0x16}, > - {0x3800, 0x00}, > - {0x3801, 0x00}, > - {0x3802, 0x00}, > - {0x3803, 0x06}, > - {0x3804, 0x0c}, > - {0x3805, 0xdf}, > - {0x3806, 0x09}, > - {0x3807, 0xa7}, > - {0x3808, 0x0c}, > - {0x3809, 0xd0}, > - {0x380a, 0x09}, > - {0x380b, 0xa0}, > - {0x380c, 0x07}, > - {0x380d, 0x88}, > - {0x380e, 0x09}, > - {0x380f, 0xb8}, > - {0x3810, 0x00}, > - {0x3811, 0x00}, > - {0x3812, 0x00}, > - {0x3813, 0x01}, > - {0x3814, 0x01}, > - {0x3815, 0x01}, > - {0x3816, 0x00}, > - {0x3817, 0x00}, > - {0x3818, 0x00}, > - {0x3819, 0x10}, > - {0x3820, 0x80}, > - {0x3821, 0x46}, > - {0x382a, 0x01}, > - {0x382b, 0x01}, > - {0x3830, 0x06}, > - {0x3836, 0x02}, > - {0x3862, 0x04}, > - {0x3863, 0x08}, > - {0x3cc0, 0x33}, > - {0x3d85, 0x17}, > - {0x3d8c, 0x73}, > - {0x3d8d, 0xde}, > - {0x4001, 0xe0}, > - {0x4003, 0x40}, > - {0x4008, 0x00}, > - {0x4009, 0x0b}, > - {0x400a, 0x00}, > - {0x400b, 0x84}, > - {0x400f, 0x80}, > - {0x4010, 0xf0}, > - {0x4011, 0xff}, > - {0x4012, 0x02}, > - {0x4013, 0x01}, > - {0x4014, 0x01}, > - {0x4015, 0x01}, > - {0x4042, 0x00}, > - {0x4043, 0x80}, > - {0x4044, 0x00}, > - {0x4045, 0x80}, > - {0x4046, 0x00}, > - {0x4047, 0x80}, > - {0x4048, 0x00}, > - {0x4049, 0x80}, > - {0x4041, 0x03}, > - {0x404c, 0x20}, > - {0x404d, 0x00}, > - {0x404e, 0x20}, > - {0x4203, 0x80}, > - {0x4307, 0x30}, > - {0x4317, 0x00}, > - {0x4503, 0x08}, > - {0x4601, 0x80}, > - {0x4800, 0x44}, > - {0x4816, 0x53}, > - {0x481b, 0x58}, > - {0x481f, 0x27}, > - {0x4837, 0x16}, > - {0x483c, 0x0f}, > - {0x484b, 0x05}, > - {0x5000, 0x57}, > - {0x5001, 0x0a}, > - {0x5004, 0x04}, > - {0x502e, 0x03}, > - {0x5030, 0x41}, > - {0x5780, 0x14}, > - {0x5781, 0x0f}, > - {0x5782, 0x44}, > - {0x5783, 0x02}, > - {0x5784, 0x01}, > - {0x5785, 0x01}, > - {0x5786, 0x00}, > - {0x5787, 0x04}, > - {0x5788, 0x02}, > - {0x5789, 0x0f}, > - {0x578a, 0xfd}, > - {0x578b, 0xf5}, > - {0x578c, 0xf5}, > - {0x578d, 0x03}, > - {0x578e, 0x08}, > - {0x578f, 0x0c}, > - {0x5790, 0x08}, > - {0x5791, 0x04}, > - {0x5792, 0x00}, > - {0x5793, 0x52}, > - {0x5794, 0xa3}, > - {0x5795, 0x02}, > - {0x5796, 0x20}, > - {0x5797, 0x20}, > - {0x5798, 0xd5}, > - {0x5799, 0xd5}, > - {0x579a, 0x00}, > - {0x579b, 0x50}, > - {0x579c, 0x00}, > - {0x579d, 0x2c}, > - {0x579e, 0x0c}, > - {0x579f, 0x40}, > - {0x57a0, 0x09}, > - {0x57a1, 0x40}, > - {0x59f8, 0x3d}, > - {0x5a08, 0x02}, > - {0x5b00, 0x02}, > - {0x5b01, 0x10}, > - {0x5b02, 0x03}, > - {0x5b03, 0xcf}, > - {0x5b05, 0x6c}, > - {0x5e00, 0x00} > -}; > - > static const struct ov8856_reg mode_3264x2448_regs[] = { > {0x0103, 0x01}, > {0x0302, 0x3c}, > @@ -963,18 +773,6 @@ static const struct ov8856_link_freq_config link_freq_configs[] = { > }; > > static const struct ov8856_mode supported_modes[] = { > - { > - .width = 3280, > - .height = 2464, > - .hts = 1928, > - .vts_def = 2488, > - .vts_min = 2488, > - .reg_list = { > - .num_of_regs = ARRAY_SIZE(mode_3280x2464_regs), > - .regs = mode_3280x2464_regs, > - }, > - .link_freq_index = OV8856_LINK_FREQ_720MBPS, > - }, If 3280x2464 resolution is removed, bayer order needs to be updated in the meantime. From OV8856's datasheet, bayer order turns to be BGGR if sensor adopts full mode (3264x2448) or binning mode (1632x1224). > { > .width = 3264, > .height = 2448,
Hi Dongchun, On Tue, Nov 24, 2020 at 03:40:51PM +0800, Dongchun Zhu wrote: > > static const struct ov8856_mode supported_modes[] = { > > - { > > - .width = 3280, > > - .height = 2464, > > - .hts = 1928, > > - .vts_def = 2488, > > - .vts_min = 2488, > > - .reg_list = { > > - .num_of_regs = ARRAY_SIZE(mode_3280x2464_regs), > > - .regs = mode_3280x2464_regs, > > - }, > > - .link_freq_index = OV8856_LINK_FREQ_720MBPS, > > - }, > > If 3280x2464 resolution is removed, bayer order needs to be updated in > the meantime. From OV8856's datasheet, bayer order turns to be BGGR if > sensor adopts full mode (3264x2448) or binning mode (1632x1224). How is this related to the patch? The next largest size is 16 by 16 less, so the Bayer order is the same. If it's wrong currently (as it would appear to), it should be a separate patch.
Hi, Robert I remember that the full size of ov8856 image sensor is 3296x2480 and we can get the 3280x2464 frames based on current settings. Do you have any issues with this mode? On 11/16/20 11:50 PM, Robert Foss wrote: > 0x3812, 0x00}, > 236 {0x3813, 0x01},
Hi Sakari, On Tue, 2020-11-24 at 10:43 +0200, Sakari Ailus wrote: > Hi Dongchun, > > On Tue, Nov 24, 2020 at 03:40:51PM +0800, Dongchun Zhu wrote: > > > static const struct ov8856_mode supported_modes[] = { > > > - { > > > - .width = 3280, > > > - .height = 2464, > > > - .hts = 1928, > > > - .vts_def = 2488, > > > - .vts_min = 2488, > > > - .reg_list = { > > > - .num_of_regs = ARRAY_SIZE(mode_3280x2464_regs), > > > - .regs = mode_3280x2464_regs, > > > - }, > > > - .link_freq_index = OV8856_LINK_FREQ_720MBPS, > > > - }, > > > > If 3280x2464 resolution is removed, bayer order needs to be updated in > > the meantime. From OV8856's datasheet, bayer order turns to be BGGR if > > sensor adopts full mode (3264x2448) or binning mode (1632x1224). > > How is this related to the patch? > Yes, it seems to be another issue. But it is very often that bayer order is strongly related to the image window size and mirror/flip setting. > The next largest size is 16 by 16 less, so the Bayer order is the same. If > it's wrong currently (as it would appear to), it should be a separate > patch. > OV8856 sensor array region consists of 3 main window settings. The inner window is controlled by [H_win_off, V_win_off]. From the old unusual 3280x2464 and 1640x1232 setting, H_win_off(R3810-R3811) is 0, V_win_off(R3812-R3813) is 1. Considering that the register TEST_PATTERN_CTRL(R4320) controlling pixel order is not set (default: 0x80, meaning BG/GR) and mirror/flip are both OFF, the absolute coordinate of crop_start is expressed as: [H_crop_start+H_win_off, V_crop_start+V_win_off] = [0, 7] Thus the first pixel shall start with G channel(according to datasheet). This is different with current resolutions (3264x2448 and 1632x1224).
On Tue, 24 Nov 2020 at 11:17, Dongchun Zhu (朱东春) <Dongchun.Zhu@mediatek.com> wrote: > > Hi Robert, > > > > Just updated the identification method of first pixel on list. > > The bayer order for the new setting (either FULL or BINNING mode) shall be BG/GR. > > This has been proved both theoretically and experimentally. Thank you for verifying that BGGR mode actually what is produced.
On Tue, 24 Nov 2020 at 10:42, Bingbu Cao <bingbu.cao@linux.intel.com> wrote: > > Hi, Robert > > I remember that the full size of ov8856 image sensor is 3296x2480 and we can get the 3280x2464 > frames based on current settings. > > Do you have any issues with this mode? As far as I can tell using the 3280x2464 mode actually yields an output resolution that is 3264x2448. What does your hardware setup look like? And which revision of the sensor are you using?
> > OV8856 sensor array region consists of 3 main window settings. > The inner window is controlled by [H_win_off, V_win_off]. > From the old unusual 3280x2464 and 1640x1232 setting, > H_win_off(R3810-R3811) is 0, V_win_off(R3812-R3813) is 1. > > Considering that the register TEST_PATTERN_CTRL(R4320) controlling pixel > order is not set (default: 0x80, meaning BG/GR) and mirror/flip are both > OFF, the absolute coordinate of crop_start is expressed as: > [H_crop_start+H_win_off, V_crop_start+V_win_off] = [0, 7] > > Thus the first pixel shall start with G channel(according to datasheet). > This is different with current resolutions (3264x2448 and 1632x1224). > Sakari: So this means that the patches introducing 3264x2448 and 1632x1224 modes really should have included code for configuring BGGR format for those two specific modes only. Let me whip up another patch for that, and put a pin in the bayer mode part of this conversation.
Hi Robert, On Tue, Nov 17, 2020 at 12:52 AM Robert Foss <robert.foss@linaro.org> wrote: > > Remove the 3280x2464 mode as it can't be reproduced and yields > an output resolution of 3264x2448 instead of the desired one. > > Furthermore the 3264x2448 resolution is the highest resolution > that the product brief lists. > > Since 3280x2464 neither works correctly nor seems to be supported > by the sensor, let's remove it. > Let me check which modes are used by our projects. For one I'm sure it's the 3264, but not sure about the other. To be fair, 3280 sounds like a valid setup, with black pixels on the edges. It's sometimes needed to add the black pixels either due to ISP requirements or to obtain the black pixel values. Best regards, Tomasz > Signed-off-by: Robert Foss <robert.foss@linaro.org> > --- > drivers/media/i2c/ov8856.c | 202 ------------------------------------- > 1 file changed, 202 deletions(-) > > diff --git a/drivers/media/i2c/ov8856.c b/drivers/media/i2c/ov8856.c > index 2f4ceaa80593..3365d19a303d 100644 > --- a/drivers/media/i2c/ov8856.c > +++ b/drivers/media/i2c/ov8856.c > @@ -148,196 +148,6 @@ static const struct ov8856_reg mipi_data_rate_360mbps[] = { > {0x031e, 0x0c}, > }; > > -static const struct ov8856_reg mode_3280x2464_regs[] = { > - {0x3000, 0x20}, > - {0x3003, 0x08}, > - {0x300e, 0x20}, > - {0x3010, 0x00}, > - {0x3015, 0x84}, > - {0x3018, 0x72}, > - {0x3021, 0x23}, > - {0x3033, 0x24}, > - {0x3500, 0x00}, > - {0x3501, 0x9a}, > - {0x3502, 0x20}, > - {0x3503, 0x08}, > - {0x3505, 0x83}, > - {0x3508, 0x01}, > - {0x3509, 0x80}, > - {0x350c, 0x00}, > - {0x350d, 0x80}, > - {0x350e, 0x04}, > - {0x350f, 0x00}, > - {0x3510, 0x00}, > - {0x3511, 0x02}, > - {0x3512, 0x00}, > - {0x3600, 0x72}, > - {0x3601, 0x40}, > - {0x3602, 0x30}, > - {0x3610, 0xc5}, > - {0x3611, 0x58}, > - {0x3612, 0x5c}, > - {0x3613, 0xca}, > - {0x3614, 0x20}, > - {0x3628, 0xff}, > - {0x3629, 0xff}, > - {0x362a, 0xff}, > - {0x3633, 0x10}, > - {0x3634, 0x10}, > - {0x3635, 0x10}, > - {0x3636, 0x10}, > - {0x3663, 0x08}, > - {0x3669, 0x34}, > - {0x366e, 0x10}, > - {0x3706, 0x86}, > - {0x370b, 0x7e}, > - {0x3714, 0x23}, > - {0x3730, 0x12}, > - {0x3733, 0x10}, > - {0x3764, 0x00}, > - {0x3765, 0x00}, > - {0x3769, 0x62}, > - {0x376a, 0x2a}, > - {0x376b, 0x30}, > - {0x3780, 0x00}, > - {0x3781, 0x24}, > - {0x3782, 0x00}, > - {0x3783, 0x23}, > - {0x3798, 0x2f}, > - {0x37a1, 0x60}, > - {0x37a8, 0x6a}, > - {0x37ab, 0x3f}, > - {0x37c2, 0x04}, > - {0x37c3, 0xf1}, > - {0x37c9, 0x80}, > - {0x37cb, 0x16}, > - {0x37cc, 0x16}, > - {0x37cd, 0x16}, > - {0x37ce, 0x16}, > - {0x3800, 0x00}, > - {0x3801, 0x00}, > - {0x3802, 0x00}, > - {0x3803, 0x06}, > - {0x3804, 0x0c}, > - {0x3805, 0xdf}, > - {0x3806, 0x09}, > - {0x3807, 0xa7}, > - {0x3808, 0x0c}, > - {0x3809, 0xd0}, > - {0x380a, 0x09}, > - {0x380b, 0xa0}, > - {0x380c, 0x07}, > - {0x380d, 0x88}, > - {0x380e, 0x09}, > - {0x380f, 0xb8}, > - {0x3810, 0x00}, > - {0x3811, 0x00}, > - {0x3812, 0x00}, > - {0x3813, 0x01}, > - {0x3814, 0x01}, > - {0x3815, 0x01}, > - {0x3816, 0x00}, > - {0x3817, 0x00}, > - {0x3818, 0x00}, > - {0x3819, 0x10}, > - {0x3820, 0x80}, > - {0x3821, 0x46}, > - {0x382a, 0x01}, > - {0x382b, 0x01}, > - {0x3830, 0x06}, > - {0x3836, 0x02}, > - {0x3862, 0x04}, > - {0x3863, 0x08}, > - {0x3cc0, 0x33}, > - {0x3d85, 0x17}, > - {0x3d8c, 0x73}, > - {0x3d8d, 0xde}, > - {0x4001, 0xe0}, > - {0x4003, 0x40}, > - {0x4008, 0x00}, > - {0x4009, 0x0b}, > - {0x400a, 0x00}, > - {0x400b, 0x84}, > - {0x400f, 0x80}, > - {0x4010, 0xf0}, > - {0x4011, 0xff}, > - {0x4012, 0x02}, > - {0x4013, 0x01}, > - {0x4014, 0x01}, > - {0x4015, 0x01}, > - {0x4042, 0x00}, > - {0x4043, 0x80}, > - {0x4044, 0x00}, > - {0x4045, 0x80}, > - {0x4046, 0x00}, > - {0x4047, 0x80}, > - {0x4048, 0x00}, > - {0x4049, 0x80}, > - {0x4041, 0x03}, > - {0x404c, 0x20}, > - {0x404d, 0x00}, > - {0x404e, 0x20}, > - {0x4203, 0x80}, > - {0x4307, 0x30}, > - {0x4317, 0x00}, > - {0x4503, 0x08}, > - {0x4601, 0x80}, > - {0x4800, 0x44}, > - {0x4816, 0x53}, > - {0x481b, 0x58}, > - {0x481f, 0x27}, > - {0x4837, 0x16}, > - {0x483c, 0x0f}, > - {0x484b, 0x05}, > - {0x5000, 0x57}, > - {0x5001, 0x0a}, > - {0x5004, 0x04}, > - {0x502e, 0x03}, > - {0x5030, 0x41}, > - {0x5780, 0x14}, > - {0x5781, 0x0f}, > - {0x5782, 0x44}, > - {0x5783, 0x02}, > - {0x5784, 0x01}, > - {0x5785, 0x01}, > - {0x5786, 0x00}, > - {0x5787, 0x04}, > - {0x5788, 0x02}, > - {0x5789, 0x0f}, > - {0x578a, 0xfd}, > - {0x578b, 0xf5}, > - {0x578c, 0xf5}, > - {0x578d, 0x03}, > - {0x578e, 0x08}, > - {0x578f, 0x0c}, > - {0x5790, 0x08}, > - {0x5791, 0x04}, > - {0x5792, 0x00}, > - {0x5793, 0x52}, > - {0x5794, 0xa3}, > - {0x5795, 0x02}, > - {0x5796, 0x20}, > - {0x5797, 0x20}, > - {0x5798, 0xd5}, > - {0x5799, 0xd5}, > - {0x579a, 0x00}, > - {0x579b, 0x50}, > - {0x579c, 0x00}, > - {0x579d, 0x2c}, > - {0x579e, 0x0c}, > - {0x579f, 0x40}, > - {0x57a0, 0x09}, > - {0x57a1, 0x40}, > - {0x59f8, 0x3d}, > - {0x5a08, 0x02}, > - {0x5b00, 0x02}, > - {0x5b01, 0x10}, > - {0x5b02, 0x03}, > - {0x5b03, 0xcf}, > - {0x5b05, 0x6c}, > - {0x5e00, 0x00} > -}; > - > static const struct ov8856_reg mode_3264x2448_regs[] = { > {0x0103, 0x01}, > {0x0302, 0x3c}, > @@ -963,18 +773,6 @@ static const struct ov8856_link_freq_config link_freq_configs[] = { > }; > > static const struct ov8856_mode supported_modes[] = { > - { > - .width = 3280, > - .height = 2464, > - .hts = 1928, > - .vts_def = 2488, > - .vts_min = 2488, > - .reg_list = { > - .num_of_regs = ARRAY_SIZE(mode_3280x2464_regs), > - .regs = mode_3280x2464_regs, > - }, > - .link_freq_index = OV8856_LINK_FREQ_720MBPS, > - }, > { > .width = 3264, > .height = 2448, > -- > 2.27.0 >
On 11/24/20 6:20 PM, Robert Foss wrote: > On Tue, 24 Nov 2020 at 10:42, Bingbu Cao <bingbu.cao@linux.intel.com> wrote: >> >> Hi, Robert >> >> I remember that the full size of ov8856 image sensor is 3296x2480 and we can get the 3280x2464 >> frames based on current settings. >> >> Do you have any issues with this mode? > > As far as I can tell using the 3280x2464 mode actually yields an > output resolution that is 3264x2448. > > What does your hardware setup look like? And which revision of the > sensor are you using? > Robert, the sensor revision I am using is v1.1. I just checked the actual output pixels on our hardware, the output resolution with 2464 mode is 3280x2464, no black pixels. As Tomasz said, some ISP has the requirement of extra pixel padding, From the ov8856 datasheet, the central 3264x2448 pixels are *suggested* to be output from the pixel array and the boundary pixels can be used for additional processing. In my understanding, the 32 dummy lines are not black lines.
Hi Bingbu, On Wed, Nov 25, 2020 at 1:15 PM Bingbu Cao <bingbu.cao@linux.intel.com> wrote: > > > > On 11/24/20 6:20 PM, Robert Foss wrote: > > On Tue, 24 Nov 2020 at 10:42, Bingbu Cao <bingbu.cao@linux.intel.com> wrote: > >> > >> Hi, Robert > >> > >> I remember that the full size of ov8856 image sensor is 3296x2480 and we can get the 3280x2464 > >> frames based on current settings. > >> > >> Do you have any issues with this mode? > > > > As far as I can tell using the 3280x2464 mode actually yields an > > output resolution that is 3264x2448. > > > > What does your hardware setup look like? And which revision of the > > sensor are you using? > > > > Robert, the sensor revision I am using is v1.1. I just checked the actual output pixels on our > hardware, the output resolution with 2464 mode is 3280x2464, no black pixels. > > As Tomasz said, some ISP has the requirement of extra pixel padding, From the ov8856 datasheet, > the central 3264x2448 pixels are *suggested* to be output from the pixel array and the boundary > pixels can be used for additional processing. In my understanding, the 32 dummy lines are not > black lines. The datasheet says that only 3264x2448 are active pixels. What pixel values are you seeing outside of that central area? In the datasheet, those look like "optically black" pixels, which are not 100% black, but rather as if the sensor cells didn't receive any light - noise can be still there. Best regards, Tomasz
On Wed, 25 Nov 2020 at 08:32, Tomasz Figa <tfiga@chromium.org> wrote: > > Hi Bingbu, > > On Wed, Nov 25, 2020 at 1:15 PM Bingbu Cao <bingbu.cao@linux.intel.com> wrote: > > > > > > > > On 11/24/20 6:20 PM, Robert Foss wrote: > > > On Tue, 24 Nov 2020 at 10:42, Bingbu Cao <bingbu.cao@linux.intel.com> wrote: > > >> > > >> Hi, Robert > > >> > > >> I remember that the full size of ov8856 image sensor is 3296x2480 and we can get the 3280x2464 > > >> frames based on current settings. > > >> > > >> Do you have any issues with this mode? > > > > > > As far as I can tell using the 3280x2464 mode actually yields an > > > output resolution that is 3264x2448. > > > > > > What does your hardware setup look like? And which revision of the > > > sensor are you using? > > > > > > > Robert, the sensor revision I am using is v1.1. I just checked the actual output pixels on our > > hardware, the output resolution with 2464 mode is 3280x2464, no black pixels. > > > > As Tomasz said, some ISP has the requirement of extra pixel padding, From the ov8856 datasheet, > > the central 3264x2448 pixels are *suggested* to be output from the pixel array and the boundary > > pixels can be used for additional processing. In my understanding, the 32 dummy lines are not > > black lines. > > The datasheet says that only 3264x2448 are active pixels. What pixel > values are you seeing outside of that central area? In the datasheet, > those look like "optically black" pixels, which are not 100% black, > but rather as if the sensor cells didn't receive any light - noise can > be still there. > I've been developing support for some Qcom ISP functionality, and during the course of this I ran into the issue I was describing, where the 3280x2464 mode actually outputs 3264x2448. I can think of two reasons for this, either ISP driver bugs on my end or the fact that the sensor is being run outside of the specification and which could be resulting in differences between how the ov8856 sensors behave.
On Thu, Nov 26, 2020 at 7:00 PM Robert Foss <robert.foss@linaro.org> wrote: > > On Wed, 25 Nov 2020 at 08:32, Tomasz Figa <tfiga@chromium.org> wrote: > > > > Hi Bingbu, > > > > On Wed, Nov 25, 2020 at 1:15 PM Bingbu Cao <bingbu.cao@linux.intel.com> wrote: > > > > > > > > > > > > On 11/24/20 6:20 PM, Robert Foss wrote: > > > > On Tue, 24 Nov 2020 at 10:42, Bingbu Cao <bingbu.cao@linux.intel.com> wrote: > > > >> > > > >> Hi, Robert > > > >> > > > >> I remember that the full size of ov8856 image sensor is 3296x2480 and we can get the 3280x2464 > > > >> frames based on current settings. > > > >> > > > >> Do you have any issues with this mode? > > > > > > > > As far as I can tell using the 3280x2464 mode actually yields an > > > > output resolution that is 3264x2448. > > > > > > > > What does your hardware setup look like? And which revision of the > > > > sensor are you using? > > > > > > > > > > Robert, the sensor revision I am using is v1.1. I just checked the actual output pixels on our > > > hardware, the output resolution with 2464 mode is 3280x2464, no black pixels. > > > > > > As Tomasz said, some ISP has the requirement of extra pixel padding, From the ov8856 datasheet, > > > the central 3264x2448 pixels are *suggested* to be output from the pixel array and the boundary > > > pixels can be used for additional processing. In my understanding, the 32 dummy lines are not > > > black lines. > > > > The datasheet says that only 3264x2448 are active pixels. What pixel > > values are you seeing outside of that central area? In the datasheet, > > those look like "optically black" pixels, which are not 100% black, > > but rather as if the sensor cells didn't receive any light - noise can > > be still there. > > > > I've been developing support for some Qcom ISP functionality, and > during the course of this I ran into the issue I was describing, where > the 3280x2464 mode actually outputs 3264x2448. > > I can think of two reasons for this, either ISP driver bugs on my end > or the fact that the sensor is being run outside of the specification > and which could be resulting in differences between how the ov8856 > sensors behave. I just confirmed and we're indeed using this mode in a number of our projects based on the Intel ISP and it seems to be producing a proper image with all pixels of the 3280x2464 matrix having proper values. I'm now double checking whether this isn't some processing done by the ISP, but I suspect the quality would be bad if it stretched the central 3264x2448 part into the 3280x2464 frame. Best regards, Tomasz
Thanks for digging into this everyone! Assuming Tomasz doesn't find any stretching, I think we can conclude that this mode works, and should be kept. Thanks Dongchun for parsing the datasheet and finding the Bayer mode issue for the two other recently added resolutions. On Fri, 27 Nov 2020 at 11:26, Tomasz Figa <tfiga@chromium.org> wrote: > > On Thu, Nov 26, 2020 at 7:00 PM Robert Foss <robert.foss@linaro.org> wrote: > > > > On Wed, 25 Nov 2020 at 08:32, Tomasz Figa <tfiga@chromium.org> wrote: > > > > > > Hi Bingbu, > > > > > > On Wed, Nov 25, 2020 at 1:15 PM Bingbu Cao <bingbu.cao@linux.intel.com> wrote: > > > > > > > > > > > > > > > > On 11/24/20 6:20 PM, Robert Foss wrote: > > > > > On Tue, 24 Nov 2020 at 10:42, Bingbu Cao <bingbu.cao@linux.intel.com> wrote: > > > > >> > > > > >> Hi, Robert > > > > >> > > > > >> I remember that the full size of ov8856 image sensor is 3296x2480 and we can get the 3280x2464 > > > > >> frames based on current settings. > > > > >> > > > > >> Do you have any issues with this mode? > > > > > > > > > > As far as I can tell using the 3280x2464 mode actually yields an > > > > > output resolution that is 3264x2448. > > > > > > > > > > What does your hardware setup look like? And which revision of the > > > > > sensor are you using? > > > > > > > > > > > > > Robert, the sensor revision I am using is v1.1. I just checked the actual output pixels on our > > > > hardware, the output resolution with 2464 mode is 3280x2464, no black pixels. > > > > > > > > As Tomasz said, some ISP has the requirement of extra pixel padding, From the ov8856 datasheet, > > > > the central 3264x2448 pixels are *suggested* to be output from the pixel array and the boundary > > > > pixels can be used for additional processing. In my understanding, the 32 dummy lines are not > > > > black lines. > > > > > > The datasheet says that only 3264x2448 are active pixels. What pixel > > > values are you seeing outside of that central area? In the datasheet, > > > those look like "optically black" pixels, which are not 100% black, > > > but rather as if the sensor cells didn't receive any light - noise can > > > be still there. > > > > > > > I've been developing support for some Qcom ISP functionality, and > > during the course of this I ran into the issue I was describing, where > > the 3280x2464 mode actually outputs 3264x2448. > > > > I can think of two reasons for this, either ISP driver bugs on my end > > or the fact that the sensor is being run outside of the specification > > and which could be resulting in differences between how the ov8856 > > sensors behave. > > I just confirmed and we're indeed using this mode in a number of our > projects based on the Intel ISP and it seems to be producing a proper > image with all pixels of the 3280x2464 matrix having proper values. > I'm now double checking whether this isn't some processing done by the > ISP, but I suspect the quality would be bad if it stretched the > central 3264x2448 part into the 3280x2464 frame. > > Best regards, > Tomasz
On Fri, Nov 27, 2020 at 10:38 PM Robert Foss <robert.foss@linaro.org> wrote: > > Thanks for digging into this everyone! > > Assuming Tomasz doesn't find any stretching, I think we can conclude > that this mode works, and should be kept. Thanks Dongchun for parsing > the datasheet and finding the Bayer mode issue for the two other > recently added resolutions. I checked the raw output and it actually seems to have 3296x2464 non-black pixels. The rightmost 16 ones seem a copy of the ones from 3248. That might be just some padding from the output DMA, though. Generally all the datasheets I've seen still suggest that only the middle 3264x2448 are active pixels to be output, so this warrants double checking this with Omnivision. Let me see what we can do about this. Best regards, Tomasz > > On Fri, 27 Nov 2020 at 11:26, Tomasz Figa <tfiga@chromium.org> wrote: > > > > On Thu, Nov 26, 2020 at 7:00 PM Robert Foss <robert.foss@linaro.org> wrote: > > > > > > On Wed, 25 Nov 2020 at 08:32, Tomasz Figa <tfiga@chromium.org> wrote: > > > > > > > > Hi Bingbu, > > > > > > > > On Wed, Nov 25, 2020 at 1:15 PM Bingbu Cao <bingbu.cao@linux.intel.com> wrote: > > > > > > > > > > > > > > > > > > > > On 11/24/20 6:20 PM, Robert Foss wrote: > > > > > > On Tue, 24 Nov 2020 at 10:42, Bingbu Cao <bingbu.cao@linux.intel.com> wrote: > > > > > >> > > > > > >> Hi, Robert > > > > > >> > > > > > >> I remember that the full size of ov8856 image sensor is 3296x2480 and we can get the 3280x2464 > > > > > >> frames based on current settings. > > > > > >> > > > > > >> Do you have any issues with this mode? > > > > > > > > > > > > As far as I can tell using the 3280x2464 mode actually yields an > > > > > > output resolution that is 3264x2448. > > > > > > > > > > > > What does your hardware setup look like? And which revision of the > > > > > > sensor are you using? > > > > > > > > > > > > > > > > Robert, the sensor revision I am using is v1.1. I just checked the actual output pixels on our > > > > > hardware, the output resolution with 2464 mode is 3280x2464, no black pixels. > > > > > > > > > > As Tomasz said, some ISP has the requirement of extra pixel padding, From the ov8856 datasheet, > > > > > the central 3264x2448 pixels are *suggested* to be output from the pixel array and the boundary > > > > > pixels can be used for additional processing. In my understanding, the 32 dummy lines are not > > > > > black lines. > > > > > > > > The datasheet says that only 3264x2448 are active pixels. What pixel > > > > values are you seeing outside of that central area? In the datasheet, > > > > those look like "optically black" pixels, which are not 100% black, > > > > but rather as if the sensor cells didn't receive any light - noise can > > > > be still there. > > > > > > > > > > I've been developing support for some Qcom ISP functionality, and > > > during the course of this I ran into the issue I was describing, where > > > the 3280x2464 mode actually outputs 3264x2448. > > > > > > I can think of two reasons for this, either ISP driver bugs on my end > > > or the fact that the sensor is being run outside of the specification > > > and which could be resulting in differences between how the ov8856 > > > sensors behave. > > > > I just confirmed and we're indeed using this mode in a number of our > > projects based on the Intel ISP and it seems to be producing a proper > > image with all pixels of the 3280x2464 matrix having proper values. > > I'm now double checking whether this isn't some processing done by the > > ISP, but I suspect the quality would be bad if it stretched the > > central 3264x2448 part into the 3280x2464 frame. > > > > Best regards, > > Tomasz
diff --git a/drivers/media/i2c/ov8856.c b/drivers/media/i2c/ov8856.c index 2f4ceaa80593..3365d19a303d 100644 --- a/drivers/media/i2c/ov8856.c +++ b/drivers/media/i2c/ov8856.c @@ -148,196 +148,6 @@ static const struct ov8856_reg mipi_data_rate_360mbps[] = { {0x031e, 0x0c}, }; -static const struct ov8856_reg mode_3280x2464_regs[] = { - {0x3000, 0x20}, - {0x3003, 0x08}, - {0x300e, 0x20}, - {0x3010, 0x00}, - {0x3015, 0x84}, - {0x3018, 0x72}, - {0x3021, 0x23}, - {0x3033, 0x24}, - {0x3500, 0x00}, - {0x3501, 0x9a}, - {0x3502, 0x20}, - {0x3503, 0x08}, - {0x3505, 0x83}, - {0x3508, 0x01}, - {0x3509, 0x80}, - {0x350c, 0x00}, - {0x350d, 0x80}, - {0x350e, 0x04}, - {0x350f, 0x00}, - {0x3510, 0x00}, - {0x3511, 0x02}, - {0x3512, 0x00}, - {0x3600, 0x72}, - {0x3601, 0x40}, - {0x3602, 0x30}, - {0x3610, 0xc5}, - {0x3611, 0x58}, - {0x3612, 0x5c}, - {0x3613, 0xca}, - {0x3614, 0x20}, - {0x3628, 0xff}, - {0x3629, 0xff}, - {0x362a, 0xff}, - {0x3633, 0x10}, - {0x3634, 0x10}, - {0x3635, 0x10}, - {0x3636, 0x10}, - {0x3663, 0x08}, - {0x3669, 0x34}, - {0x366e, 0x10}, - {0x3706, 0x86}, - {0x370b, 0x7e}, - {0x3714, 0x23}, - {0x3730, 0x12}, - {0x3733, 0x10}, - {0x3764, 0x00}, - {0x3765, 0x00}, - {0x3769, 0x62}, - {0x376a, 0x2a}, - {0x376b, 0x30}, - {0x3780, 0x00}, - {0x3781, 0x24}, - {0x3782, 0x00}, - {0x3783, 0x23}, - {0x3798, 0x2f}, - {0x37a1, 0x60}, - {0x37a8, 0x6a}, - {0x37ab, 0x3f}, - {0x37c2, 0x04}, - {0x37c3, 0xf1}, - {0x37c9, 0x80}, - {0x37cb, 0x16}, - {0x37cc, 0x16}, - {0x37cd, 0x16}, - {0x37ce, 0x16}, - {0x3800, 0x00}, - {0x3801, 0x00}, - {0x3802, 0x00}, - {0x3803, 0x06}, - {0x3804, 0x0c}, - {0x3805, 0xdf}, - {0x3806, 0x09}, - {0x3807, 0xa7}, - {0x3808, 0x0c}, - {0x3809, 0xd0}, - {0x380a, 0x09}, - {0x380b, 0xa0}, - {0x380c, 0x07}, - {0x380d, 0x88}, - {0x380e, 0x09}, - {0x380f, 0xb8}, - {0x3810, 0x00}, - {0x3811, 0x00}, - {0x3812, 0x00}, - {0x3813, 0x01}, - {0x3814, 0x01}, - {0x3815, 0x01}, - {0x3816, 0x00}, - {0x3817, 0x00}, - {0x3818, 0x00}, - {0x3819, 0x10}, - {0x3820, 0x80}, - {0x3821, 0x46}, - {0x382a, 0x01}, - {0x382b, 0x01}, - {0x3830, 0x06}, - {0x3836, 0x02}, - {0x3862, 0x04}, - {0x3863, 0x08}, - {0x3cc0, 0x33}, - {0x3d85, 0x17}, - {0x3d8c, 0x73}, - {0x3d8d, 0xde}, - {0x4001, 0xe0}, - {0x4003, 0x40}, - {0x4008, 0x00}, - {0x4009, 0x0b}, - {0x400a, 0x00}, - {0x400b, 0x84}, - {0x400f, 0x80}, - {0x4010, 0xf0}, - {0x4011, 0xff}, - {0x4012, 0x02}, - {0x4013, 0x01}, - {0x4014, 0x01}, - {0x4015, 0x01}, - {0x4042, 0x00}, - {0x4043, 0x80}, - {0x4044, 0x00}, - {0x4045, 0x80}, - {0x4046, 0x00}, - {0x4047, 0x80}, - {0x4048, 0x00}, - {0x4049, 0x80}, - {0x4041, 0x03}, - {0x404c, 0x20}, - {0x404d, 0x00}, - {0x404e, 0x20}, - {0x4203, 0x80}, - {0x4307, 0x30}, - {0x4317, 0x00}, - {0x4503, 0x08}, - {0x4601, 0x80}, - {0x4800, 0x44}, - {0x4816, 0x53}, - {0x481b, 0x58}, - {0x481f, 0x27}, - {0x4837, 0x16}, - {0x483c, 0x0f}, - {0x484b, 0x05}, - {0x5000, 0x57}, - {0x5001, 0x0a}, - {0x5004, 0x04}, - {0x502e, 0x03}, - {0x5030, 0x41}, - {0x5780, 0x14}, - {0x5781, 0x0f}, - {0x5782, 0x44}, - {0x5783, 0x02}, - {0x5784, 0x01}, - {0x5785, 0x01}, - {0x5786, 0x00}, - {0x5787, 0x04}, - {0x5788, 0x02}, - {0x5789, 0x0f}, - {0x578a, 0xfd}, - {0x578b, 0xf5}, - {0x578c, 0xf5}, - {0x578d, 0x03}, - {0x578e, 0x08}, - {0x578f, 0x0c}, - {0x5790, 0x08}, - {0x5791, 0x04}, - {0x5792, 0x00}, - {0x5793, 0x52}, - {0x5794, 0xa3}, - {0x5795, 0x02}, - {0x5796, 0x20}, - {0x5797, 0x20}, - {0x5798, 0xd5}, - {0x5799, 0xd5}, - {0x579a, 0x00}, - {0x579b, 0x50}, - {0x579c, 0x00}, - {0x579d, 0x2c}, - {0x579e, 0x0c}, - {0x579f, 0x40}, - {0x57a0, 0x09}, - {0x57a1, 0x40}, - {0x59f8, 0x3d}, - {0x5a08, 0x02}, - {0x5b00, 0x02}, - {0x5b01, 0x10}, - {0x5b02, 0x03}, - {0x5b03, 0xcf}, - {0x5b05, 0x6c}, - {0x5e00, 0x00} -}; - static const struct ov8856_reg mode_3264x2448_regs[] = { {0x0103, 0x01}, {0x0302, 0x3c}, @@ -963,18 +773,6 @@ static const struct ov8856_link_freq_config link_freq_configs[] = { }; static const struct ov8856_mode supported_modes[] = { - { - .width = 3280, - .height = 2464, - .hts = 1928, - .vts_def = 2488, - .vts_min = 2488, - .reg_list = { - .num_of_regs = ARRAY_SIZE(mode_3280x2464_regs), - .regs = mode_3280x2464_regs, - }, - .link_freq_index = OV8856_LINK_FREQ_720MBPS, - }, { .width = 3264, .height = 2448,
Remove the 3280x2464 mode as it can't be reproduced and yields an output resolution of 3264x2448 instead of the desired one. Furthermore the 3264x2448 resolution is the highest resolution that the product brief lists. Since 3280x2464 neither works correctly nor seems to be supported by the sensor, let's remove it. Signed-off-by: Robert Foss <robert.foss@linaro.org> --- drivers/media/i2c/ov8856.c | 202 ------------------------------------- 1 file changed, 202 deletions(-)