On 07/16/2018 07:12 AM, Philipp Zabel wrote:
Hi Steve,
On Thu, 2018-07-05 at 15:09 -0700, Steve Longerbeam wrote:
[...]
[...]
+ halign = 0;
+ break;
+ }
+ if (f->type == V4L2_BUF_TYPE_VIDEO_OUTPUT) {
+ /*
+ * 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;
The input DMA burst width alignment is only necessary if the lines are
scanned from right to left (that is, if HF is enabled) in the scaling
step.
Ok, thanks for the explanation, that makes sense.
If the rotator is used, the flipping is done in the rotation step
instead,
Ah, I missed or forgot about that detail in the ref manual,
I reviewed it again and you are right...
so the alignment restriction would be on the width of the
intermediate tile (and thus on the output height). This is already
covered by the rotator 8x8 pixel block alignment.
so this makes sense too.
That is, require 8 byte width alignment for IRT or if HFLIP is enabled.
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))
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")
in my mediatree fork at git@xxxxxxxxxx:slongerbeam/mediatree.git.
Let's discuss this further in the v2 patches.
Steve
