From: "Ville Syrjälä" <ville.syrjala@linux.intel.com>
To: Maxime Ripard <maxime.ripard@bootlin.com>
Cc: Paul Kocialkowski <paul.kocialkowski@bootlin.com>,
eben@raspberrypi.org, igt-dev@lists.freedesktop.org,
Thomas Petazzoni <thomas.petazzoni@bootlin.com>
Subject: Re: [igt-dev] [PATCH v7 04/14] fb: Create common function to convert frame formats
Date: Wed, 19 Sep 2018 16:03:25 +0300 [thread overview]
Message-ID: <20180919130325.GE5565@intel.com> (raw)
In-Reply-To: <dd62ceae2c027b591b00606f5ae8cf4d5148dfe7.1536655647.git-series.maxime.ripard@bootlin.com>
On Tue, Sep 11, 2018 at 10:47:31AM +0200, Maxime Ripard wrote:
> The current code to convert between two buffer formats is quite tied to the
> cairo surface infrastructure. Since we'll want to reuse it, make that
> function more generic by introducing a common structure that passes all the
> arguments and a common function that will call the right functions we
> needed.
>
> Reviewed-by: Eric Anholt <eric@anholt.net>
> Signed-off-by: Maxime Ripard <maxime.ripard@bootlin.com>
> ---
> lib/igt_fb.c | 220 ++++++++++++++++++++++++++++++----------------------
> 1 file changed, 130 insertions(+), 90 deletions(-)
>
> diff --git a/lib/igt_fb.c b/lib/igt_fb.c
> index 66bdc02a0ce7..f829df59e9fa 100644
> --- a/lib/igt_fb.c
> +++ b/lib/igt_fb.c
> @@ -1391,6 +1391,19 @@ struct fb_convert_blit_upload {
> struct fb_blit_linear linear;
> };
>
> +struct fb_convert_buf {
> + void *ptr;
> + struct igt_fb *fb;
> +};
> +
> +struct fb_convert {
> + unsigned int width;
> + unsigned int height;
Are we expecting to convert between different sized fbs, or why do we
need these?
> +
> + struct fb_convert_buf dst;
> + struct fb_convert_buf src;
> +};
> +
> static uint8_t clamprgb(float val)
> {
> return clamp((int)(val + 0.5f), 0, 255);
> @@ -1411,27 +1424,28 @@ static void write_rgb(uint8_t *rgb24, const struct igt_vec4 *rgb)
> rgb24[0] = clamprgb(rgb->d[2]);
> }
>
> -static void convert_nv12_to_rgb24(struct igt_fb *fb, struct fb_convert_blit_upload *blit)
> +static void convert_nv12_to_rgb24(struct fb_convert *cvt)
> {
> int i, j;
> const uint8_t *y, *uv;
> - uint8_t *rgb24 = blit->rgb24.map;
> - unsigned rgb24_stride = blit->rgb24.stride, planar_stride = blit->linear.stride;
> - uint8_t *buf = malloc(blit->linear.size);
> - struct igt_mat4 m = igt_ycbcr_to_rgb_matrix(fb->color_encoding,
> - fb->color_range);
> + uint8_t *rgb24 = cvt->dst.ptr;
> + unsigned int rgb24_stride = cvt->dst.fb->stride;
> + unsigned int planar_stride = cvt->src.fb->stride;
> + uint8_t *buf = malloc(cvt->src.fb->size);
> + struct igt_mat4 m = igt_ycbcr_to_rgb_matrix(cvt->src.fb->color_encoding,
> + cvt->src.fb->color_range);
>
> /*
> * Reading from the BO is awfully slow because of lack of read caching,
> * it's faster to copy the whole BO to a temporary buffer and convert
> * from there.
> */
> - igt_memcpy_from_wc(buf, blit->linear.map, blit->linear.size);
> - y = &buf[blit->linear.offsets[0]];
> - uv = &buf[blit->linear.offsets[1]];
> + igt_memcpy_from_wc(buf, cvt->src.ptr, cvt->src.fb->size);
> + y = cvt->src.ptr + cvt->src.fb->offsets[0];
> + uv = cvt->src.ptr + cvt->src.fb->offsets[1];
>
> - for (i = 0; i < fb->height / 2; i++) {
> - for (j = 0; j < fb->width / 2; j++) {
> + for (i = 0; i < cvt->height / 2; i++) {
> + for (j = 0; j < cvt->width / 2; j++) {
> /* Convert 2x2 pixel blocks */
> struct igt_vec4 yuv[4];
> struct igt_vec4 rgb[4];
> @@ -1456,7 +1470,7 @@ static void convert_nv12_to_rgb24(struct igt_fb *fb, struct fb_convert_blit_uplo
> write_rgb(&rgb24[j * 8 + 4 + rgb24_stride], &rgb[3]);
> }
>
> - if (fb->width & 1) {
> + if (cvt->width & 1) {
> /* Convert 1x2 pixel block */
> struct igt_vec4 yuv[2];
> struct igt_vec4 rgb[2];
> @@ -1480,9 +1494,9 @@ static void convert_nv12_to_rgb24(struct igt_fb *fb, struct fb_convert_blit_uplo
> uv += planar_stride;
> }
>
> - if (fb->height & 1) {
> + if (cvt->height & 1) {
> /* Convert last row */
> - for (j = 0; j < fb->width / 2; j++) {
> + for (j = 0; j < cvt->width / 2; j++) {
> /* Convert 2x1 pixel blocks */
> struct igt_vec4 yuv[2];
> struct igt_vec4 rgb[2];
> @@ -1500,7 +1514,7 @@ static void convert_nv12_to_rgb24(struct igt_fb *fb, struct fb_convert_blit_uplo
> write_rgb(&rgb24[j * 8 + 4], &rgb[0]);
> }
>
> - if (fb->width & 1) {
> + if (cvt->width & 1) {
> /* Convert single pixel */
> struct igt_vec4 yuv;
> struct igt_vec4 rgb;
> @@ -1519,22 +1533,22 @@ static void convert_nv12_to_rgb24(struct igt_fb *fb, struct fb_convert_blit_uplo
> free(buf);
> }
>
> -static void convert_rgb24_to_nv12(struct igt_fb *fb, struct fb_convert_blit_upload *blit)
> +static void convert_rgb24_to_nv12(struct fb_convert *cvt)
> {
> int i, j;
> - uint8_t *y = &blit->linear.map[blit->linear.offsets[0]];
> - uint8_t *uv = &blit->linear.map[blit->linear.offsets[1]];
> - const uint8_t *rgb24 = blit->rgb24.map;
> - unsigned rgb24_stride = blit->rgb24.stride;
> - unsigned planar_stride = blit->linear.stride;
> - struct igt_mat4 m = igt_rgb_to_ycbcr_matrix(fb->color_encoding,
> - fb->color_range);
> -
> - igt_assert_f(fb->drm_format == DRM_FORMAT_NV12,
> + uint8_t *y = cvt->dst.ptr + cvt->dst.fb->offsets[0];
> + uint8_t *uv = cvt->dst.ptr + cvt->dst.fb->offsets[1];
> + const uint8_t *rgb24 = cvt->src.ptr;
> + unsigned rgb24_stride = cvt->src.fb->stride;
> + unsigned planar_stride = cvt->dst.fb->stride;
> + struct igt_mat4 m = igt_rgb_to_ycbcr_matrix(cvt->dst.fb->color_encoding,
> + cvt->dst.fb->color_range);
> +
> + igt_assert_f(cvt->dst.fb->drm_format == DRM_FORMAT_NV12,
> "Conversion not implemented for !NV12 planar formats\n");
>
> - for (i = 0; i < fb->height / 2; i++) {
> - for (j = 0; j < fb->width / 2; j++) {
> + for (i = 0; i < cvt->height / 2; i++) {
> + for (j = 0; j < cvt->width / 2; j++) {
> /* Convert 2x2 pixel blocks */
> struct igt_vec4 rgb[4];
> struct igt_vec4 yuv[4];
> @@ -1563,7 +1577,7 @@ static void convert_rgb24_to_nv12(struct igt_fb *fb, struct fb_convert_blit_uplo
> uv[j * 2 + 1] = (yuv[0].d[2] + yuv[2].d[2]) / 2.0f;
> }
>
> - if (fb->width & 1) {
> + if (cvt->width & 1) {
> /* Convert 1x2 pixel block */
> struct igt_vec4 rgb[2];
> struct igt_vec4 yuv[2];
> @@ -1592,8 +1606,8 @@ static void convert_rgb24_to_nv12(struct igt_fb *fb, struct fb_convert_blit_uplo
> }
>
> /* Last row cannot be interpolated between 2 pixels, take the single value */
> - if (fb->height & 1) {
> - for (j = 0; j < fb->width / 2; j++) {
> + if (cvt->height & 1) {
> + for (j = 0; j < cvt->width / 2; j++) {
> /* Convert 2x1 pixel blocks */
> struct igt_vec4 rgb[2];
> struct igt_vec4 yuv[2];
> @@ -1610,7 +1624,7 @@ static void convert_rgb24_to_nv12(struct igt_fb *fb, struct fb_convert_blit_uplo
> uv[j * 2 + 1] = yuv[0].d[2];
> }
>
> - if (fb->width & 1) {
> + if (cvt->width & 1) {
> /* Convert single pixel */
> struct igt_vec4 rgb;
> struct igt_vec4 yuv;
> @@ -1647,27 +1661,28 @@ static const unsigned char *yuyv_swizzle(uint32_t format)
> }
> }
>
> -static void convert_yuyv_to_rgb24(struct igt_fb *fb, struct fb_convert_blit_upload *blit)
> +static void convert_yuyv_to_rgb24(struct fb_convert *cvt)
> {
> int i, j;
> const uint8_t *yuyv;
> - uint8_t *rgb24 = blit->rgb24.map;
> - unsigned rgb24_stride = blit->rgb24.stride, yuyv_stride = blit->linear.stride;
> - uint8_t *buf = malloc(blit->linear.size);
> - struct igt_mat4 m = igt_ycbcr_to_rgb_matrix(fb->color_encoding,
> - fb->color_range);
> - const unsigned char *swz = yuyv_swizzle(fb->drm_format);
> + uint8_t *rgb24 = cvt->dst.ptr;
> + unsigned int rgb24_stride = cvt->dst.fb->stride;
> + unsigned int yuyv_stride = cvt->src.fb->stride;
> + uint8_t *buf = malloc(cvt->src.fb->size);
> + struct igt_mat4 m = igt_ycbcr_to_rgb_matrix(cvt->src.fb->color_encoding,
> + cvt->src.fb->color_range);
> + const unsigned char *swz = yuyv_swizzle(cvt->src.fb->drm_format);
>
> /*
> * Reading from the BO is awfully slow because of lack of read caching,
> * it's faster to copy the whole BO to a temporary buffer and convert
> * from there.
> */
> - igt_memcpy_from_wc(buf, blit->linear.map, blit->linear.size);
> + igt_memcpy_from_wc(buf, cvt->src.ptr, cvt->src.fb->size);
> yuyv = buf;
>
> - for (i = 0; i < fb->height; i++) {
> - for (j = 0; j < fb->width / 2; j++) {
> + for (i = 0; i < cvt->height; i++) {
> + for (j = 0; j < cvt->width / 2; j++) {
> /* Convert 2x1 pixel blocks */
> struct igt_vec4 yuv[2];
> struct igt_vec4 rgb[2];
> @@ -1685,7 +1700,7 @@ static void convert_yuyv_to_rgb24(struct igt_fb *fb, struct fb_convert_blit_uplo
> write_rgb(&rgb24[j * 8 + 4], &rgb[1]);
> }
>
> - if (fb->width & 1) {
> + if (cvt->width & 1) {
> struct igt_vec4 yuv;
> struct igt_vec4 rgb;
>
> @@ -1706,25 +1721,25 @@ static void convert_yuyv_to_rgb24(struct igt_fb *fb, struct fb_convert_blit_uplo
> free(buf);
> }
>
> -static void convert_rgb24_to_yuyv(struct igt_fb *fb, struct fb_convert_blit_upload *blit)
> +static void convert_rgb24_to_yuyv(struct fb_convert *cvt)
> {
> int i, j;
> - uint8_t *yuyv = blit->linear.map;
> - const uint8_t *rgb24 = blit->rgb24.map;
> - unsigned rgb24_stride = blit->rgb24.stride;
> - unsigned yuyv_stride = blit->linear.stride;
> - struct igt_mat4 m = igt_rgb_to_ycbcr_matrix(fb->color_encoding,
> - fb->color_range);
> - const unsigned char *swz = yuyv_swizzle(fb->drm_format);
> -
> - igt_assert_f(fb->drm_format == DRM_FORMAT_YUYV ||
> - fb->drm_format == DRM_FORMAT_YVYU ||
> - fb->drm_format == DRM_FORMAT_UYVY ||
> - fb->drm_format == DRM_FORMAT_VYUY,
> + uint8_t *yuyv = cvt->dst.ptr;
> + const uint8_t *rgb24 = cvt->src.ptr;
> + unsigned rgb24_stride = cvt->src.fb->stride;
> + unsigned yuyv_stride = cvt->dst.fb->stride;
> + struct igt_mat4 m = igt_rgb_to_ycbcr_matrix(cvt->dst.fb->color_encoding,
> + cvt->dst.fb->color_range);
> + const unsigned char *swz = yuyv_swizzle(cvt->dst.fb->drm_format);
> +
> + igt_assert_f(cvt->dst.fb->drm_format == DRM_FORMAT_YUYV ||
> + cvt->dst.fb->drm_format == DRM_FORMAT_YVYU ||
> + cvt->dst.fb->drm_format == DRM_FORMAT_UYVY ||
> + cvt->dst.fb->drm_format == DRM_FORMAT_VYUY,
> "Conversion not implemented for !YUYV planar formats\n");
>
> - for (i = 0; i < fb->height; i++) {
> - for (j = 0; j < fb->width / 2; j++) {
> + for (i = 0; i < cvt->height; i++) {
> + for (j = 0; j < cvt->width / 2; j++) {
> /* Convert 2x1 pixel blocks */
> struct igt_vec4 rgb[2];
> struct igt_vec4 yuv[2];
> @@ -1741,7 +1756,7 @@ static void convert_rgb24_to_yuyv(struct igt_fb *fb, struct fb_convert_blit_uplo
> yuyv[j * 4 + swz[3]] = (yuv[0].d[2] + yuv[1].d[2]) / 2.0f;
> }
>
> - if (fb->width & 1) {
> + if (cvt->width & 1) {
> struct igt_vec4 rgb;
> struct igt_vec4 yuv;
>
> @@ -1759,27 +1774,59 @@ static void convert_rgb24_to_yuyv(struct igt_fb *fb, struct fb_convert_blit_uplo
> }
> }
>
> +static void fb_convert(struct fb_convert *cvt)
> +{
> + if (cvt->dst.fb->drm_format == DRM_FORMAT_RGB888) {
> + switch (cvt->src.fb->drm_format) {
> + case DRM_FORMAT_NV12:
> + convert_nv12_to_rgb24(cvt);
> + return;
> + case DRM_FORMAT_YUYV:
> + case DRM_FORMAT_YVYU:
> + case DRM_FORMAT_UYVY:
> + case DRM_FORMAT_VYUY:
> + convert_yuyv_to_rgb24(cvt);
> + return;
> + }
> + } else if (cvt->src.fb->drm_format == DRM_FORMAT_RGB888) {
> + switch (cvt->dst.fb->drm_format) {
> + case DRM_FORMAT_NV12:
> + convert_rgb24_to_nv12(cvt);
> + return;
> + case DRM_FORMAT_YUYV:
> + case DRM_FORMAT_YVYU:
> + case DRM_FORMAT_UYVY:
> + case DRM_FORMAT_VYUY:
> + convert_rgb24_to_yuyv(cvt);
> + return;
> + }
> + }
> +
> + igt_assert_f(false,
> + "Conversion not implemented (from format 0x%x to 0x%x)\n",
> + cvt->src.fb->drm_format, cvt->dst.fb->drm_format);
> +}
> +
> static void destroy_cairo_surface__convert(void *arg)
> {
> struct fb_convert_blit_upload *blit = arg;
> struct igt_fb *fb = blit->fb;
> -
> - /* Convert linear rgb back! */
> - switch(fb->drm_format) {
> - case DRM_FORMAT_NV12:
> - convert_rgb24_to_nv12(fb, blit);
> - break;
> - case DRM_FORMAT_YUYV:
> - case DRM_FORMAT_YVYU:
> - case DRM_FORMAT_UYVY:
> - case DRM_FORMAT_VYUY:
> - convert_rgb24_to_yuyv(fb, blit);
> - break;
> - default:
> - igt_assert_f(false, "Conversion not implemented for formats 0x%x\n",
> - fb->drm_format);
> - }
> -
> + struct fb_convert cvt = {
> + .width = fb->width,
> + .height = fb->height,
> +
> + .dst = {
> + .ptr = blit->linear.map,
> + .fb = blit->fb,
> + },
> +
> + .src = {
> + .ptr = blit->shadow_ptr,
> + .fb = &blit->shadow_fb,
> + },
> + };
> +
> + fb_convert(&cvt);
> igt_fb_unmap_buffer(&blit->shadow_fb, blit->shadow_ptr);
> igt_remove_fb(blit->fd, &blit->shadow_fb);
>
> @@ -1796,6 +1843,7 @@ static void destroy_cairo_surface__convert(void *arg)
> static void create_cairo_surface__convert(int fd, struct igt_fb *fb)
> {
> struct fb_convert_blit_upload *blit = malloc(sizeof(*blit));
> + struct fb_convert cvt = { 0 };
> int shadow_id;
>
> igt_assert(blit);
> @@ -1823,21 +1871,13 @@ static void create_cairo_surface__convert(int fd, struct igt_fb *fb)
> memcpy(blit->linear.offsets, fb->offsets, sizeof(fb->offsets));
> }
>
> - /* Convert to linear rgb! */
> - switch(fb->drm_format) {
> - case DRM_FORMAT_NV12:
> - convert_nv12_to_rgb24(fb, blit);
> - break;
> - case DRM_FORMAT_YUYV:
> - case DRM_FORMAT_YVYU:
> - case DRM_FORMAT_UYVY:
> - case DRM_FORMAT_VYUY:
> - convert_yuyv_to_rgb24(fb, blit);
> - break;
> - default:
> - igt_assert_f(false, "Conversion not implemented for formats 0x%x\n",
> - fb->drm_format);
> - }
> + cvt.width = fb->width;
> + cvt.height = fb->height;
> + cvt.dst.ptr = blit->shadow_ptr;
> + cvt.dst.fb = &blit->shadow_fb;
> + cvt.src.ptr = blit->linear.map;
> + cvt.src.fb = blit->fb;
> + fb_convert(&cvt);
>
> fb->cairo_surface =
> cairo_image_surface_create_for_data(blit->shadow_ptr,
> --
> git-series 0.9.1
--
Ville Syrjälä
Intel
_______________________________________________
igt-dev mailing list
igt-dev@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/igt-dev
next prev parent reply other threads:[~2018-09-19 13:03 UTC|newest]
Thread overview: 31+ messages / expand[flat|nested] mbox.gz Atom feed top
2018-09-11 8:47 [igt-dev] [PATCH v7 00/14] chamelium: Test the plane formats Maxime Ripard
2018-09-11 8:47 ` [igt-dev] [PATCH v7 01/14] fb: Add buffer map/unmap functions Maxime Ripard
2018-09-11 8:47 ` [igt-dev] [PATCH v7 02/14] fb: Use an igt_fb for the cairo shadow buffer Maxime Ripard
2018-09-19 13:21 ` Ville Syrjälä
2018-09-25 8:51 ` Maxime Ripard
2018-10-01 10:47 ` Arkadiusz Hiler
2018-10-01 13:55 ` Ville Syrjälä
2018-10-01 14:10 ` Arkadiusz Hiler
2018-10-02 14:56 ` Maxime Ripard
2018-09-11 8:47 ` [igt-dev] [PATCH v7 03/14] fb: convert: Remove swizzle from the arguments Maxime Ripard
2018-09-11 8:47 ` [igt-dev] [PATCH v7 04/14] fb: Create common function to convert frame formats Maxime Ripard
2018-09-19 13:03 ` Ville Syrjälä [this message]
2018-09-25 7:51 ` Maxime Ripard
2018-09-25 12:20 ` Ville Syrjälä
2018-09-11 8:47 ` [igt-dev] [PATCH v7 05/14] fb: Add format conversion routine Maxime Ripard
2018-09-11 8:47 ` [igt-dev] [PATCH v7 06/14] fb: Fix ARGB8888 color depth Maxime Ripard
2018-09-11 8:47 ` [igt-dev] [PATCH v7 07/14] fb: Add support for conversions through pixman Maxime Ripard
2018-10-01 9:26 ` Petri Latvala
2018-10-02 14:57 ` Maxime Ripard
2018-09-11 8:47 ` [igt-dev] [PATCH v7 08/14] fb: Add depth lookup function Maxime Ripard
2018-09-11 8:47 ` [igt-dev] [PATCH v7 09/14] fb: Add more formats Maxime Ripard
2018-09-19 13:14 ` Ville Syrjälä
2018-09-25 7:49 ` Maxime Ripard
2018-09-25 12:18 ` Ville Syrjälä
2018-09-11 8:47 ` [igt-dev] [PATCH v7 10/14] chamelium: Split CRC test function in two Maxime Ripard
2018-09-11 8:47 ` [igt-dev] [PATCH v7 11/14] chamelium: Change our pattern for a custom one if needed Maxime Ripard
2018-09-11 8:47 ` [igt-dev] [PATCH v7 12/14] chamelium: Add format support Maxime Ripard
2018-09-11 8:47 ` [igt-dev] [PATCH v7 13/14] chamelium: Add format subtests Maxime Ripard
2018-09-11 8:47 ` [igt-dev] [PATCH v7 14/14] tests: Add chamelium formats subtests to vc4 test lists Maxime Ripard
2018-09-12 8:12 ` [igt-dev] ✗ Fi.CI.BAT: failure for chamelium: Test the plane formats (rev6) Patchwork
2018-09-19 12:45 ` [igt-dev] [PATCH v7 00/14] chamelium: Test the plane formats Maxime Ripard
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=20180919130325.GE5565@intel.com \
--to=ville.syrjala@linux.intel.com \
--cc=eben@raspberrypi.org \
--cc=igt-dev@lists.freedesktop.org \
--cc=maxime.ripard@bootlin.com \
--cc=paul.kocialkowski@bootlin.com \
--cc=thomas.petazzoni@bootlin.com \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.