On 10/17/18 4:10 AM, Philipp Zabel wrote:
On Fri, 2018-10-12 at 17:33 -0700, Steve Longerbeam wrote:
On 09/18/2018 02:34 AM, Philipp Zabel wrote:
<snip>
+/*
+ * Tile left edges are required to be aligned to multiples of 8 bytes
+ * by the IDMAC.
+ */
+static inline u32 tile_left_align(const struct ipu_image_pixfmt *fmt)
+{
+ return fmt->planar ? 8 * fmt->uv_width_dec : 64 / fmt->bpp;
+}
<snip>
As I indicated, shouldn't this be
return fmt->planar ? 8 * fmt->uv_width_dec : 8;
?
Just from a unit analysis perspective, "64 / fmt->bp" has
units of pixels / 8-bytes, it should have units of bytes.
The tile alignment is in pixels, not in bytes.
Ah, yes of course you are right, I used to know this :) I am
loosing track of this code.
For 16-bit and 32-bit
packed formats, we only need to align to 4 or 2 pixels, respectively,
as the LCM of 8-byte alignment and 2-byte or 4-byte pixel size is
always 8 bytes.
Yes I agree, the LCM of 8-byte alignment and bytes-per-pixel should
be the tile left edge alignment in pixels.
But now that you pointed it out, it is quite obvious that this can't
work for 24-bit packed formats. Here the LCM of 8-byte alignment and 3-
byte pixels is 24 bytes, or 8 pixels.
How about:
if (fmt->planar)
return fmt->uv_packed ? 8 : 8 * fmt->uv_width_dec;
else
return fmt->bpp == 32 ? 2 : fmt->bpp == 16 ? 4 : 8;
Yep, that looks better. I tested this and it works fine.
Steve
