On Sun, 2018-07-22 at 11:02 -0700, Steve Longerbeam wrote:
> On 07/16/2018 07:12 AM, Philipp Zabel wrote:
[...]
> > > > + /*
> > > > + * The IC burst reads 8 pixels at a time. Reading beyond the
> > > > + * end of the line is usually acceptable. Those pixels are
> > > > + * ignored, unless the IC has to write the scaled line in
> > > > + * reverse.
> > > > + */
> > > > + if (!ipu_rot_mode_is_irt(ctx->rot_mode) &&
> > > > + ctx->rot_mode && IPU_ROT_BIT_HFLIP)
> > > > + walign = 3;
> > >
> > > This looks wrong. Do you mean:
> > >
> > > if (ipu_rot_mode_is_irt(ctx->rot_mode) || (ctx->rot_mode & IPU_ROT_BIT_HFLIP))
> > > walign = 3;
> > > else
> > > walign = 1;
[...]
> > No, I specifically meant (!IRT && HFLIP).
>
> Right, but there is still a typo:
>
> if (!ipu_rot_mode_is_irt(ctx->rot_mode) && ctx->rot_mode && IPU_ROT_BIT_HFLIP)
>
> should be:
>
> if (!ipu_rot_mode_is_irt(ctx->rot_mode) && (ctx->rot_mode & IPU_ROT_BIT_HFLIP))
Ow, yes, thank you.
> > The rotator itself doesn't cause any input alignment restrictions, we
> > just have to make sure that the intermediate tiles after scaling are 8x8
> > aligned.
> >
> > > Also, why not simply call ipu_image_convert_adjust() in
> > > mem2mem_try_fmt()? If there is something missing in the former
> > > function, then it should be added there, instead of adding the
> > > missing checks in mem2mem_try_fmt().
> >
> > ipu_image_convert_adjust tries to adjust both input and output image at
> > the same time, here we just have the format of either input or output
> > image. Do you suggest to split this function into an input and an output
> > version?
>
> See b4362162c0 ("media: imx: mem2mem: Use ipu_image_convert_adjust
> in try format")
Alright, this looks fine to me. I was worried about inter-format
limitations, but the only one seems to be the output size lower bound to
1/4 of the input size. Should S_FMT(OUT) also update the capture format
if adjustments were made to keep a consistent state?
regards
Philipp
