[igt-dev] [i-g-t V2] tests/kms_prime: Use 256B aligned width

[Daniel Vetter]

On Fri, Jul 23, 2021 at 9:54 AM Surendrakumar Upadhyay, TejaskumarX
<tejaskumarx.surendrakumar.upadhyay at intel.com> wrote:
>
>
>
> > -----Original Message-----
> > From: Kempczynski, Zbigniew <zbigniew.kempczynski at intel.com>
> > Sent: 23 July 2021 12:56
> > To: Daniel Vetter <daniel.vetter at ffwll.ch>
> > Cc: Surendrakumar Upadhyay, TejaskumarX
> > <tejaskumarx.surendrakumar.upadhyay at intel.com>; IGT development <igt-
> > dev at lists.freedesktop.org>
> > Subject: Re: [igt-dev] [i-g-t V2] tests/kms_prime: Use 256B aligned width
> >
> > On Thu, Jul 22, 2021 at 02:55:47PM +0200, Daniel Vetter wrote:
> > > On Tue, Jul 20, 2021 at 1:05 PM Tejas Upadhyay
> > > <tejaskumarx.surendrakumar.upadhyay at intel.com> wrote:
> > > >
> > > > Having different alignment requirement by different drivers, 256B
> > > > aligned should work for all drm drivers.
> > > >
> > > > amdgpu and radeon, amdgpu_align_pitch: 256B armada, armada_pitch:
> > > > 128B
> > > > exynos_drm_gem_dumb_create: No alignment required
> > > > drm_gem_shmem_dumb_create: 8B
> > > > drm_gem_vram_fill_create_dumb: 8B
> > > >
> > > > Thus 256B covers everything we see in the kernel drm drivers.
> > > >
> > > > Changes since V1:
> > > >         - Edited commit message with driver compatible with 256B
> > > > align-Daniel
> > > >
> > > > Signed-off-by: Tejas Upadhyay
> > > > <tejaskumarx.surendrakumar.upadhyay at intel.com>
> > >
> > > Acked-by: Daniel Vetter <daniel.vetter at ffwll.ch>
> >
> > Why we should create bigger dumb buffer on specific driver than required?
>
> In order to have generic minimum align requirement across vendor drivers, we need to put 256B.

This doesn't explain anything about why _this_ location is the right
spot to put this alignment. And I agree with Zbyscek that the commit
message really doesn't explain that. I guess that needs to be added
before merging ...

Also I think Zbyscek volunteered to push this, please don't shop
around for committers until someone just pushes without asking
questions or trying to understand the patch beforehand. That's not how
it's supposed to work.
-Daniel

> Thanks,
> Tejas
> >
> > --
> > Zbigniew
> >
> >
> > >
> > > > ---
> > > >  tests/kms_prime.c | 2 +-
> > > >  1 file changed, 1 insertion(+), 1 deletion(-)
> > > >
> > > > diff --git a/tests/kms_prime.c b/tests/kms_prime.c index
> > > > 35f4b5b7..2e20c58b 100644
> > > > --- a/tests/kms_prime.c
> > > > +++ b/tests/kms_prime.c
> > > > @@ -101,7 +101,7 @@ static void prepare_scratch(int exporter_fd, struct
> > dumb_bo *scratch,
> > > >         scratch->bpp = 32;
> > > >
> > > >         scratch->handle = kmstest_dumb_create(exporter_fd,
> > > > -                       scratch->width,
> > > > +                       ALIGN(scratch->width, 256),
> > > >                         scratch->height,
> > > >                         scratch->bpp,
> > > >                         &scratch->pitch,
> > > > --
> > > > 2.31.1
> > > >
> > > > _______________________________________________
> > > > igt-dev mailing list
> > > > igt-dev at lists.freedesktop.org
> > > > https://lists.freedesktop.org/mailman/listinfo/igt-dev
> > >
> > >
> > >
> > > --
> > > Daniel Vetter
> > > Software Engineer, Intel Corporation
> > > http://blog.ffwll.ch
> > > _______________________________________________
> > > igt-dev mailing list
> > > igt-dev at lists.freedesktop.org
> > > https://lists.freedesktop.org/mailman/listinfo/igt-dev



-- 
Daniel Vetter
Software Engineer, Intel Corporation
http://blog.ffwll.ch