Hi Tomi,
Thank you for the patch.
On Wednesday 17 May 2017 10:56:43 Tomi Valkeinen wrote:
> TILER rotation with YUV422 pixelformats does not work at the moment. All
> other pixel formats work, because the pixelformat's pixel size is equal
> to tiler unit size (e.g. XR24's pixel size is 32 bits, and the TILER
> unit size that has to be used is 32 bits).
>
> For YUV422 formats this is not the case, as the TILER unit size has to
> be 32 bits, but the pixel size is 16 bits. The end result is OCP errors
> and sync losts.
>
> This patch adds the code to adjust the variables for YUV422 formats.
>
> We could make the code more generic by passing around the pixel format,
> rotation type, angle and the tiler unit size, which would allow us to do
> calculations without special case for YUV422. However, this would make
> the code more complex, and at least for now this is much more easier to
> handle with these two special cases for YUV422.
>
> Signed-off-by: Tomi Valkeinen <tomi.valkei...@ti.com>
> ---
> drivers/gpu/drm/omapdrm/dss/dispc.c | 20 ++++++++++++++++++--
> drivers/gpu/drm/omapdrm/omap_fb.c | 14 ++++++++++++++
> 2 files changed, 32 insertions(+), 2 deletions(-)
>
> diff --git a/drivers/gpu/drm/omapdrm/dss/dispc.c
> b/drivers/gpu/drm/omapdrm/dss/dispc.c index a25db6e25165..80c75e5913cb
> 100644
> --- a/drivers/gpu/drm/omapdrm/dss/dispc.c
> +++ b/drivers/gpu/drm/omapdrm/dss/dispc.c
> @@ -1917,7 +1917,8 @@ static s32 pixinc(int pixels, u8 ps)
> static void calc_offset(u16 screen_width, u16 width,
> u32 fourcc, bool fieldmode,
> unsigned int field_offset, unsigned *offset0, unsigned
*offset1,
> - s32 *row_inc, s32 *pix_inc, int x_predecim, int y_predecim)
> + s32 *row_inc, s32 *pix_inc, int x_predecim, int y_predecim,
> + enum omap_dss_rotation_type rotation_type, u8 rotation)
> {
> u8 ps;
>
> @@ -1925,6 +1926,20 @@ static void calc_offset(u16 screen_width, u16 width,
>
> DSSDBG("scrw %d, width %d\n", screen_width, width);
>
> + if (rotation_type == OMAP_DSS_ROT_TILER &&
> + (fourcc == DRM_FORMAT_UYVY || fourcc == DRM_FORMAT_YUYV) &&
> + drm_rotation_90_or_270(rotation)) {
> + /*
> + * HACK: ROW_INC needs to be calculated with TILER units.
> + * We get such 'screen_width' that multiplying it with the
> + * YUV422 pixel size gives the correct TILER container width.
> + * However, 'width' is in pixels and multiplying it with
YUV422
> + * pixel size gives incorrect result. We thus multiply it here
> + * with 2 to match the 32 bit TILER unit size.
> + */
> + width *= 2;
> + }
> +
> /*
> * field 0 = even field = bottom field
> * field 1 = odd field = top field
> @@ -2473,7 +2488,8 @@ static int dispc_ovl_setup_common(enum omap_plane_id
> plane, calc_offset(screen_width, frame_width,
> fourcc, fieldmode, field_offset,
> &offset0, &offset1, &row_inc, &pix_inc,
> - x_predecim, y_predecim);
> + x_predecim, y_predecim,
> + rotation_type, rotation);
>
> DSSDBG("offset0 %u, offset1 %u, row_inc %d, pix_inc %d\n",
> offset0, offset1, row_inc, pix_inc);
> diff --git a/drivers/gpu/drm/omapdrm/omap_fb.c
> b/drivers/gpu/drm/omapdrm/omap_fb.c index bd05976fc20b..e5cc13799e73 100644
> --- a/drivers/gpu/drm/omapdrm/omap_fb.c
> +++ b/drivers/gpu/drm/omapdrm/omap_fb.c
> @@ -184,16 +184,30 @@ void omap_framebuffer_update_scanout(struct
> drm_framebuffer *fb,
>
> orient = drm_rotation_to_tiler(state->rotation);
>
> + /*
> + * omap_gem_rotated_paddr() wants the x & y in tiler units.
> + * Usually tiler unit size is the same as the pixel size,
except
> + * for YUV422 formats, for which the tiler unit size is 32
bits
> + * and pixel size is 16 bits.
> + */
> + if (fb->format->format == DRM_FORMAT_UYVY ||
> + fb->format->format == DRM_FORMAT_YUYV) {
That's a very peculiar indentation.
> + x /= 2;
> + w /= 2;
> + }
> +
> /* adjust x,y offset for flip/invert: */
> if (orient & MASK_Y_INVERT)
> y += h - 1;
> if (orient & MASK_X_INVERT)
> x += w - 1;
>
> + /* Note: x and y are in TILER units, not pixels */
> omap_gem_rotated_dma_addr(plane->bo, orient, x, y,
> &info->paddr);
> info->rotation_type = OMAP_DSS_ROT_TILER;
> info->rotation = state->rotation ?: DRM_ROTATE_0;
> + /* Note: stride in TILER units, not pixels */
Nitpicking, I would have combined the two comments.
Reviewed-by: Laurent Pinchart <laurent.pinch...@ideasonboard.com>
> info->screen_width = omap_gem_tiled_stride(plane->bo,
orient);
> } else {
> switch (state->rotation & DRM_ROTATE_MASK) {
--
Regards,
Laurent Pinchart
_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel
