Re: [igt-dev] [PATCH i-g-t 7/8] igt: fb: Fallback on KMS dumb buffer allocation for YUV buffers

Maxime Ripard
2018-12-07T15:30:39Z

[-- Attachment #1.1: Type: text/plain, Size: 2555 bytes --]

On Tue, Dec 04, 2018 at 09:32:05PM +0200, Ville Syrjälä wrote:
> On Tue, Dec 04, 2018 at 11:08:22AM +0100, Maxime Ripard wrote:
> > The current YUV buffer allocation only works on the i915 driver, since
> > it uses some private ioctl. However, we can to use that code on other
> > drivers that implement only KMS, so if the driver is something else
> > than the i915 driver, let's allocate a dumb buffer.
> >
> > Signed-off-by: Maxime Ripard <maxime.ripard@bootlin.com>
> > ---
> > lib/igt_fb.c | 35 +++++++++++++++++++++++++++++++++--
> > 1 file changed, 33 insertions(+), 2 deletions(-)
> >
> > diff --git a/lib/igt_fb.c b/lib/igt_fb.c
> > index d6242a6652f1..f2e6c89f3884 100644
> > --- a/lib/igt_fb.c
> > +++ b/lib/igt_fb.c
> > @@ -501,6 +501,8 @@ static int i915_create_gem_for_fb(struct igt_fb *fb)
> >
> > static int create_yuv_bo_for_fb(struct igt_fb *fb)
> > {
> > + unsigned int virtual_height;
> > + unsigned int bpp;
> > uint64_t size = calc_fb_size(fb);
> > int fd = fb->fd;
> >
> > @@ -511,8 +513,37 @@ static int create_yuv_bo_for_fb(struct igt_fb *fb)
> > if (is_i915_device(fd))
> > return i915_create_gem_for_fb(fb);
> >
> > - /* We cannot allocate any other buffer type */
> > - igt_assert(true);
> > + switch (fb->drm_format) {
> > + case DRM_FORMAT_NV12:
> > + bpp = 8;
> > + break;
> > +
> > + case DRM_FORMAT_UYVY:
> > + case DRM_FORMAT_VYUY:
> > + case DRM_FORMAT_YUYV:
> > + case DRM_FORMAT_YVYU:
> > + bpp = 16;
> > + break;
> > +
> > + default:
> > + igt_assert_f(false, "Unsupported YUV format\n");
> > + }
> > +
> > + switch (fb->drm_format) {
> > + case DRM_FORMAT_NV12:
> > + virtual_height = fb->height * 3 / 2;
> > + break;
>
> A bit of a hack. The main problem with this is that the kernel won't
> know anything about any stride alignment limitations etc. If we do use
> this hack I guess it would be more technically correct to write it as
> DIV_ROUND_UP(fb->size, fb->stride[0]), or something along those lines.

It's the same algorithm that modetest is using, which is why I went
for that. It must have been tested on a wide variety of platforms.

> Which also brings me to the other issue we have. calc_plane_stride()
> and calc_plane_size() are somewhat i915 specific. We should probably
> do something about that.

Ah, right, I missed that.

When will we start to put some intel specific code in a generic code
path?

Maxime

--
Maxime Ripard, Bootlin
Embedded Linux and Kernel engineering
https://bootlin.com

[-- Attachment #1.2: signature.asc --]
[-- Type: application/pgp-signature, Size: 228 bytes --]

[-- Attachment #2: Type: text/plain, Size: 154 bytes --]