[igt-dev] [PATCH i-g-t] [RFC] tests/kms_prime: Aligned pitch to 64 byte for Intel platforms

[Mark Yacoub]

On Mon, May 31, 2021 at 10:47 AM Srinivas, Vidya
<vidya.srinivas at intel.com> wrote:
>
> Hello Ville,
>
> Thank you very much.
> Before reaching our i915's i915_gem_dumb_create, it goes to vgem_gem_dumb_create for kms_prime.
>
> The pitch gets calculated there and it is not 64 byte aligned. Due to this, intel_framebuffer_init reports "pitch must be 64 byte aligned"
> and framebuffer creation fails. I tried submitting vgem patch where 64 byte alignment can be done in vgem_gem_dumb_create and that also
> passes. But we did not get approval yet as few of them felt, vgem is generic and other platforms might fail if we do 64 byte alignment there.
>
> Kindly suggest. Thanks a lot.
>
> Regards
> Vidya
>
> -----Original Message-----
> From: Ville Syrjälä <ville.syrjala at linux.intel.com>
> Sent: Monday, May 31, 2021 7:48 PM
> To: Srinivas, Vidya <vidya.srinivas at intel.com>
> Cc: intel-gfx at lists.freedesktop.org; igt-dev at lists.freedesktop.org; Lin, Charlton <charlton.lin at intel.com>
> Subject: Re: [igt-dev] [PATCH i-g-t] [RFC] tests/kms_prime: Aligned pitch to 64 byte for Intel platforms
>
> On Fri, May 28, 2021 at 10:04:03AM +0530, Vidya Srinivas wrote:
> > For Intel platforms, pitch needs to be 64 byte aligned.
> > Kernel code vgem_gem_dumb_create which is platform generic code doesnt
> > do the alignment. This causes frame buffer creation to fail on Intel
> > platforms where the pitch is not 64 byte aligned.
> >
> > tests: test run on Intel platforms with panel resolution 1366x768
> >
Tested on ChromeOS on JSL (Drawlat).
Tested-by: Mark Yacoub <markyacoub at chromium.org>
> > Signed-off-by: Vidya Srinivas <vidya.srinivas at intel.com>
> > ---
> >  tests/kms_prime.c | 5 ++++-
> >  1 file changed, 4 insertions(+), 1 deletion(-)
> >
> > diff --git a/tests/kms_prime.c b/tests/kms_prime.c index
> > 8cb2ca2a9dc3..fdc941fe8100 100644
> > --- a/tests/kms_prime.c
> > +++ b/tests/kms_prime.c
> > @@ -51,6 +51,8 @@ static struct {
> >       { .r = 1.0, .g = 0.0, .b = 0.0, .color = 0xffff0000 },  };
> >
> > +bool check_platform;
I think we can do a more precise name to indicate which platform.
Something like is_intel_device or is_i915 would be more appropriate.
alternatively, we can drop the boolean and just do the check when needed.
So it would look something like
+ is_i915_device(fd) ? ALIGN(scratch->width, 64) : scratch->width,

> > +
> >  IGT_TEST_DESCRIPTION("Prime tests, focusing on KMS side");
> >
> >  static bool has_prime_import(int fd)
> > @@ -101,7 +103,7 @@ static void prepare_scratch(int exporter_fd, struct dumb_bo *scratch,
> >       scratch->bpp = 32;
> >
> >       scratch->handle = kmstest_dumb_create(exporter_fd,
> > -                     scratch->width,
> > +                     check_platform? ALIGN(scratch->width, 64): scratch->width,
>
> The dumb_create ioctl already does this for us.
>
> >                       scratch->height,
> >                       scratch->bpp,
> >                       &scratch->pitch,
> > @@ -262,6 +264,7 @@ igt_main
> >
> >               /* ANY = anything that is not VGEM */
> >               first_fd = __drm_open_driver_another(0, DRIVER_ANY | DRIVER_VGEM);
> > +             check_platform = is_i915_device(first_fd);
> >               igt_require(first_fd >= 0);
> >
> >               second_fd = __drm_open_driver_another(1, DRIVER_ANY | DRIVER_VGEM);
> > --
> > 2.7.4
> >
> > _______________________________________________
> > igt-dev mailing list
> > igt-dev at lists.freedesktop.org
> > https://lists.freedesktop.org/mailman/listinfo/igt-dev
>
> --
> Ville Syrjälä
> Intel
> _______________________________________________
> igt-dev mailing list
> igt-dev at lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/igt-dev