[igt-dev] [PATCH i-g-t 2/2] lib/igt_fb: Add support for testing of 16 bpc fixed point formats.
Mario Kleiner
mario.kleiner.de at gmail.com
Mon May 3 19:32:19 UTC 2021
On Mon, May 3, 2021 at 8:48 PM Ville Syrjälä
<ville.syrjala at linux.intel.com> wrote:
>
> On Mon, May 03, 2021 at 08:25:55PM +0200, Mario Kleiner wrote:
> > This is used to support testing the 16 bpc formats, e.g., via:
> >
> > kms_plane --run-subtest pixel-format-pipe-A-planes
> >
> > So far this was successfully tested on AMD RavenRidge with DCN-1
> > display hw.
> >
> > The new conversion routines are slightly adapted copies of the
> > convert_float_to_fp16() and convert_fp16_to_float() functions,
> > with the conversion math modified for float <-> uint16 instead.
> >
> > Signed-off-by: Mario Kleiner <mario.kleiner.de at gmail.com>
> > Cc: Ville Syrjälä <ville.syrjala at linux.intel.com>
> > Cc: Alex Deucher <alexander.deucher at amd.com>
> > ---
> > lib/igt_fb.c | 123 +++++++++++++++++++++++++++++++++++++++++++++++++++
> > 1 file changed, 123 insertions(+)
> >
> > diff --git a/lib/igt_fb.c b/lib/igt_fb.c
> > index 954e1181..148e6c0f 100644
> > --- a/lib/igt_fb.c
> > +++ b/lib/igt_fb.c
> > @@ -220,6 +220,22 @@ static const struct format_desc_struct {
> > .cairo_id = CAIRO_FORMAT_RGBA128F, .convert = true,
> > .num_planes = 1, .plane_bpp = { 64, },
> > },
> > + { .name = "XRGB16161616", .depth = -1, .drm_id = DRM_FORMAT_XRGB16161616,
> > + .cairo_id = CAIRO_FORMAT_RGBA128F, .convert = true,
> > + .num_planes = 1, .plane_bpp = { 64, },
> > + },
> > + { .name = "ARGB16161616", .depth = -1, .drm_id = DRM_FORMAT_ARGB16161616,
> > + .cairo_id = CAIRO_FORMAT_RGBA128F, .convert = true,
> > + .num_planes = 1, .plane_bpp = { 64, },
> > + },
> > + { .name = "XBGR16161616", .depth = -1, .drm_id = DRM_FORMAT_XBGR16161616,
> > + .cairo_id = CAIRO_FORMAT_RGBA128F, .convert = true,
> > + .num_planes = 1, .plane_bpp = { 64, },
> > + },
> > + { .name = "ABGR16161616", .depth = -1, .drm_id = DRM_FORMAT_ABGR16161616,
> > + .cairo_id = CAIRO_FORMAT_RGBA128F, .convert = true,
> > + .num_planes = 1, .plane_bpp = { 64, },
> > + },
> > { .name = "NV12", .depth = -1, .drm_id = DRM_FORMAT_NV12,
> > .cairo_id = CAIRO_FORMAT_RGB24, .convert = true,
> > .num_planes = 2, .plane_bpp = { 8, 16, },
> > @@ -3365,9 +3381,13 @@ static const unsigned char *rgbx_swizzle(uint32_t format)
> > default:
> > case DRM_FORMAT_XRGB16161616F:
> > case DRM_FORMAT_ARGB16161616F:
> > + case DRM_FORMAT_XRGB16161616:
> > + case DRM_FORMAT_ARGB16161616:
> > return swizzle_bgrx;
> > case DRM_FORMAT_XBGR16161616F:
> > case DRM_FORMAT_ABGR16161616F:
> > + case DRM_FORMAT_XBGR16161616:
> > + case DRM_FORMAT_ABGR16161616:
> > return swizzle_rgbx;
> > }
> > }
> > @@ -3451,6 +3471,97 @@ static void convert_float_to_fp16(struct fb_convert *cvt)
> > }
> > }
> >
> > +static void float_to_uint16(const float *f, uint16_t *h, unsigned int num)
> > +{
> > + for (int i = 0; i < num; i++)
> > + h[i] = f[i] * 65535.0f + 0.5f;
> > +}
> > +
> > +static void uint16_to_float(const uint16_t *h, float *f, unsigned int num)
> > +{
> > + for (int i = 0; i < num; i++)
> > + f[i] = ((float) h[i]) / 65535.0f;
>
> nit: the cast shouldn't be necessary.
>
> Looks good otherwise.
> Reviewed-by: Ville Syrjälä <ville.syrjala at linux.intel.com>
>
Thanks for the quick review. Compiler and test results agree with you :).
However, all other uintsomething_to_float() conversion routines, e.g.,
for the yuv formats and for fp16_to_float() all use that apparently
redundant cast, which is why I left it for consistency with the rest
of the code. And in case those wiser people who wrote those bits may
know something that i don't see?
Should I change the patch to drop the cast, and resend?
-mario
> > +}
> > +
> > +static void convert_uint16_to_float(struct fb_convert *cvt)
> > +{
> > + int i, j;
> > + uint16_t *up16;
> > + float *ptr = cvt->dst.ptr;
> > + unsigned int float_stride = cvt->dst.fb->strides[0] / sizeof(*ptr);
> > + unsigned int up16_stride = cvt->src.fb->strides[0] / sizeof(*up16);
> > + const unsigned char *swz = rgbx_swizzle(cvt->src.fb->drm_format);
> > + bool needs_reswizzle = swz != swizzle_rgbx;
> > +
> > + uint16_t *buf = convert_src_get(cvt);
> > + up16 = buf + cvt->src.fb->offsets[0] / sizeof(*buf);
> > +
> > + for (i = 0; i < cvt->dst.fb->height; i++) {
> > + if (needs_reswizzle) {
> > + const uint16_t *u16_tmp = up16;
> > + float *rgb_tmp = ptr;
> > +
> > + for (j = 0; j < cvt->dst.fb->width; j++) {
> > + struct igt_vec4 rgb;
> > +
> > + uint16_to_float(u16_tmp, rgb.d, 4);
> > +
> > + rgb_tmp[0] = rgb.d[swz[0]];
> > + rgb_tmp[1] = rgb.d[swz[1]];
> > + rgb_tmp[2] = rgb.d[swz[2]];
> > + rgb_tmp[3] = rgb.d[swz[3]];
> > +
> > + rgb_tmp += 4;
> > + u16_tmp += 4;
> > + }
> > + } else {
> > + uint16_to_float(up16, ptr, cvt->dst.fb->width * 4);
> > + }
> > +
> > + ptr += float_stride;
> > + up16 += up16_stride;
> > + }
> > +
> > + convert_src_put(cvt, buf);
> > +}
> > +
> > +static void convert_float_to_uint16(struct fb_convert *cvt)
> > +{
> > + int i, j;
> > + uint16_t *up16 = cvt->dst.ptr + cvt->dst.fb->offsets[0];
> > + const float *ptr = cvt->src.ptr;
> > + unsigned float_stride = cvt->src.fb->strides[0] / sizeof(*ptr);
> > + unsigned up16_stride = cvt->dst.fb->strides[0] / sizeof(*up16);
> > + const unsigned char *swz = rgbx_swizzle(cvt->dst.fb->drm_format);
> > + bool needs_reswizzle = swz != swizzle_rgbx;
> > +
> > + for (i = 0; i < cvt->dst.fb->height; i++) {
> > + if (needs_reswizzle) {
> > + const float *rgb_tmp = ptr;
> > + uint16_t *u16_tmp = up16;
> > +
> > + for (j = 0; j < cvt->dst.fb->width; j++) {
> > + struct igt_vec4 rgb;
> > +
> > + rgb.d[0] = rgb_tmp[swz[0]];
> > + rgb.d[1] = rgb_tmp[swz[1]];
> > + rgb.d[2] = rgb_tmp[swz[2]];
> > + rgb.d[3] = rgb_tmp[swz[3]];
> > +
> > + float_to_uint16(rgb.d, u16_tmp, 4);
> > +
> > + rgb_tmp += 4;
> > + u16_tmp += 4;
> > + }
> > + } else {
> > + float_to_uint16(ptr, up16, cvt->dst.fb->width * 4);
> > + }
> > +
> > + ptr += float_stride;
> > + up16 += up16_stride;
> > + }
> > +}
> > +
> > static void convert_pixman(struct fb_convert *cvt)
> > {
> > pixman_format_code_t src_pixman = drm_format_to_pixman(cvt->src.fb->drm_format);
> > @@ -3560,6 +3671,12 @@ static void fb_convert(struct fb_convert *cvt)
> > case DRM_FORMAT_ABGR16161616F:
> > convert_fp16_to_float(cvt);
> > return;
> > + case DRM_FORMAT_XRGB16161616:
> > + case DRM_FORMAT_XBGR16161616:
> > + case DRM_FORMAT_ARGB16161616:
> > + case DRM_FORMAT_ABGR16161616:
> > + convert_uint16_to_float(cvt);
> > + return;
> > }
> > } else if (cvt->src.fb->drm_format == IGT_FORMAT_FLOAT) {
> > switch (cvt->dst.fb->drm_format) {
> > @@ -3589,6 +3706,12 @@ static void fb_convert(struct fb_convert *cvt)
> > case DRM_FORMAT_ABGR16161616F:
> > convert_float_to_fp16(cvt);
> > return;
> > + case DRM_FORMAT_XRGB16161616:
> > + case DRM_FORMAT_XBGR16161616:
> > + case DRM_FORMAT_ARGB16161616:
> > + case DRM_FORMAT_ABGR16161616:
> > + convert_float_to_uint16(cvt);
> > + return;
> > }
> > }
> >
> > --
> > 2.25.1
>
> --
> Ville Syrjälä
> Intel
More information about the igt-dev
mailing list