lkml.org 
[lkml]   [2022]   [Mar]   [9]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
    /
    From
    Date
    SubjectRe: [PATCH 1/1] staging: media: ipu3: Fix AF x_start position when rightmost stripe is used
    Hi Sakari,

    On Thu, Mar 3, 2022 at 5:09 PM Sakari Ailus
    <sakari.ailus@linux.intel.com> wrote:
    >
    > Hi Kate,
    >
    > Thank you for the patch.
    >
    > On Wed, Feb 23, 2022 at 05:06:48PM +0800, Kate Hsuan wrote:
    > > For the AF configuration, if the rightmost stripe is used, the AF scene
    > > will be at the incorrect location of the sensor.
    > >
    > > The AF coordinate may be set to the right part of the sensor. This
    > > configuration would lead to x_start being greater than the
    > > down_scaled_stripes offset and the leftmost stripe would be disabled
    > > and only the rightmost stripe is used to control the AF coordinate. If
    > > the x_start doesn't perform any adjustments, the AF coordinate will be
    > > at the wrong place of the sensor since down_scaled_stripes offset
    > > would be the new zero of the coordinate system.
    > >
    > > In this patch, if only the rightmost stripe is used, x_start should
    > > minus down_scaled_stripes offset to maintain its correctness of AF
    > > scene coordinate.
    > >
    > > Signed-off-by: Kate Hsuan <hpa@redhat.com>
    > > ---
    > > drivers/staging/media/ipu3/ipu3-css-params.c | 9 ++++++++-
    > > 1 file changed, 8 insertions(+), 1 deletion(-)
    > >
    > > diff --git a/drivers/staging/media/ipu3/ipu3-css-params.c b/drivers/staging/media/ipu3/ipu3-css-params.c
    > > index d9e3c3785075..efe4de8ef205 100644
    > > --- a/drivers/staging/media/ipu3/ipu3-css-params.c
    > > +++ b/drivers/staging/media/ipu3/ipu3-css-params.c
    > > @@ -2556,6 +2556,14 @@ int imgu_css_cfg_acc(struct imgu_css *css, unsigned int pipe,
    > > /* Enable only for rightmost stripe, disable left */
    > > acc->af.stripes[0].grid_cfg.y_start &=
    > > ~IPU3_UAPI_GRID_Y_START_EN;
    > > + acc->af.stripes[0].grid_cfg.x_start -=
    > > + acc->stripe.down_scaled_stripes[1].offset;
    > > + acc->af.stripes[0].grid_cfg.x_end -=
    > > + acc->stripe.down_scaled_stripes[1].offset;
    > > + acc->af.stripes[1].grid_cfg.x_start -=
    > > + acc->stripe.down_scaled_stripes[1].offset;
    > > + acc->af.stripes[1].grid_cfg.x_end -=
    > > + acc->stripe.down_scaled_stripes[1].offset;
    >
    > The grid x/y ends are calculated from the width when both grids are enabled
    > and I think it should work the same way here. There's also masking of the
    > bits that aren't in use.

    Thanks for clarifying this :)

    >
    > The first stripe isn't enabled so changing values there has no effect as
    > far as I can tell.
    >

    Okay. I would remove the setting of the first stripe in the v2 patch.

    Thank you

    > Looking at the code a little, it seems all awb_fr, ae and af seem to suffer
    > from the same issue. Let's still iron out the fix for af first before
    > considering those.
    >
    > > } else if (acc->af.config.grid_cfg.x_end <=
    > > acc->stripe.bds_out_stripes[0].width - min_overlap) {
    > > /* Enable only for leftmost stripe, disable right */
    > > @@ -2563,7 +2571,6 @@ int imgu_css_cfg_acc(struct imgu_css *css, unsigned int pipe,
    > > ~IPU3_UAPI_GRID_Y_START_EN;
    > > } else {
    > > /* Enable for both stripes */
    > > -
    > > acc->af.stripes[0].grid_cfg.width =
    > > (acc->stripe.bds_out_stripes[0].width - min_overlap -
    > > acc->af.config.grid_cfg.x_start + 1) >>
    >
    > --
    > Kind regards,
    >
    > Sakari Ailus
    >


    --
    BR,
    Kate

    \
     
     \ /
      Last update: 2022-03-09 06:32    [W:6.140 / U:0.416 seconds]
    ©2003-2020 Jasper Spaans|hosted at Digital Ocean and TransIP|Read the blog|Advertise on this site