| Message ID | 20220309063456.102895-1-hpa@redhat.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | [v2] staging: media: ipu3: Fix AF x_start position when rightmost stripe is used | expand |
Hi Kate, Thanks for the update. On Wed, Mar 09, 2022 at 02:34:56PM +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. > > Changes in v2: > 1. Remove the setting of the first stripe. > > Signed-off-by: Kate Hsuan <hpa@redhat.com> > --- > drivers/staging/media/ipu3/ipu3-css-params.c | 4 ++++ > 1 file changed, 4 insertions(+) > > diff --git a/drivers/staging/media/ipu3/ipu3-css-params.c b/drivers/staging/media/ipu3/ipu3-css-params.c > index d9e3c3785075..5a8c07f34756 100644 > --- a/drivers/staging/media/ipu3/ipu3-css-params.c > +++ b/drivers/staging/media/ipu3/ipu3-css-params.c > @@ -2556,6 +2556,10 @@ 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[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; Could you calculate these the same way as in the case both stripes are enabled? Some bits in x_start is masked and then x_end is calculated from width. > } 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 */
Hi Sakari, On Thu, Mar 10, 2022 at 1:03 AM Sakari Ailus <sakari.ailus@linux.intel.com> wrote: > > Hi Kate, > > Thanks for the update. > > On Wed, Mar 09, 2022 at 02:34:56PM +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. > > > > Changes in v2: > > 1. Remove the setting of the first stripe. > > > > Signed-off-by: Kate Hsuan <hpa@redhat.com> > > --- > > drivers/staging/media/ipu3/ipu3-css-params.c | 4 ++++ > > 1 file changed, 4 insertions(+) > > > > diff --git a/drivers/staging/media/ipu3/ipu3-css-params.c b/drivers/staging/media/ipu3/ipu3-css-params.c > > index d9e3c3785075..5a8c07f34756 100644 > > --- a/drivers/staging/media/ipu3/ipu3-css-params.c > > +++ b/drivers/staging/media/ipu3/ipu3-css-params.c > > @@ -2556,6 +2556,10 @@ 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[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; > > Could you calculate these the same way as in the case both stripes are > enabled? Some bits in x_start is masked and then x_end is calculated from > width. Okay, I will do this in my v3 patch. > > > } 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 */ > > -- > Regards, > > Sakari Ailus >
diff --git a/drivers/staging/media/ipu3/ipu3-css-params.c b/drivers/staging/media/ipu3/ipu3-css-params.c index d9e3c3785075..5a8c07f34756 100644 --- a/drivers/staging/media/ipu3/ipu3-css-params.c +++ b/drivers/staging/media/ipu3/ipu3-css-params.c @@ -2556,6 +2556,10 @@ 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[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; } 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 */
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. Changes in v2: 1. Remove the setting of the first stripe. Signed-off-by: Kate Hsuan <hpa@redhat.com> --- drivers/staging/media/ipu3/ipu3-css-params.c | 4 ++++ 1 file changed, 4 insertions(+)