[libcamera-devel] [PATCH] ipa: ipu3: af: Set default grid block width to the minimum value
Laurent Pinchart
laurent.pinchart at ideasonboard.com
Tue Mar 22 20:56:00 CET 2022
On Tue, Mar 22, 2022 at 09:48:21AM +0100, Jean-Michel Hautbois via libcamera-devel wrote:
> On 22/03/2022 09:29, Kate Hsuan wrote:
> > On Tue, Mar 22, 2022 at 2:17 PM Jean-Michel Hautbois wrote:
> >> On 22/03/2022 02:31, Kate Hsuan via libcamera-devel wrote:
> >>> Since x coordinate is incorrectly computed by a kernel issue, the block width
> >>> should be set to 4 to prevent using the second stripe when setting the AF scene
> >>> to the centre of the image. A kernel patch had fixed this issue. Therefore, this
> >>> value can be set to the default minimum value.
> >>
> >> If this is working fine with a width of 256 (2**4 * 16), why do you want
> >> to change it to have a smaller focus region ?
> >>
> >>> Signed-off-by: Kate Hsuan <hpa at redhat.com>
> >>> ---
> >>> The kernel patch is shown as following URL.
> >>> https://lore.kernel.org/linux-media/CAEth8oES8abPO4p7eFv43PwDXuxeOmg1661YtVvykBPrkagzKg@mail.gmail.com/T/#mb02fa73ce9e3089a4619c318badb2047a3ac39e2
This patch will only appear in the kernel in v5.19, so at best we'll
have to wait for that kernel to be released. We should however support
older kernel versions, so a runtime kernel version check is likely
needed.
Does the Chrome OS kernel integrate that kernel fix too ?
> >>> ---
> >>> src/ipa/ipu3/algorithms/af.h | 2 +-
> >>> 1 file changed, 1 insertion(+), 1 deletion(-)
> >>>
> >>> diff --git a/src/ipa/ipu3/algorithms/af.h b/src/ipa/ipu3/algorithms/af.h
> >>> index 13c7e0e8..108fcd18 100644
> >>> --- a/src/ipa/ipu3/algorithms/af.h
> >>> +++ b/src/ipa/ipu3/algorithms/af.h
> >>> @@ -20,7 +20,7 @@ static constexpr uint8_t kAfMinGridWidth = 16;
> >>> static constexpr uint8_t kAfMinGridHeight = 16;
> >>> static constexpr uint8_t kAfMaxGridWidth = 32;
> >>> static constexpr uint8_t kAfMaxGridHeight = 24;
> >>> -static constexpr uint16_t kAfMinGridBlockWidth = 4;
> >>> +static constexpr uint16_t kAfMinGridBlockWidth = 3;
> >>> static constexpr uint16_t kAfMinGridBlockHeight = 3;
> >>> static constexpr uint16_t kAfMaxGridBlockWidth = 6;
> >>> static constexpr uint16_t kAfMaxGridBlockHeight = 6;
> >
> > The kernel patch fixed the issue on the second (rightmost) stripe configuration.
> > This means we can set x_start more than 640.
> > Also, according to the chomiumOS implementation, it set
> > kAfMinGridBlockWidth to 3. So, it may align with chromium OS
> > implementation after the patch is merged to kernel release.
> > https://chromium.googlesource.com/chromiumos/platform/arc-camera/+/refs/heads/master/hal/intel/include/ia_imaging/af_public.h#36
>
> I get this, but it there a strong reason to do this ? Are we
> experimenting bad behaviour when the grid is bigger in full resolution
> maybe ? I would like to be sure it is not just "because CrOS does it"
> :-) at least we need to know why.
--
Regards,
Laurent Pinchart
More information about the libcamera-devel
mailing list