diff mbox series

drm/vgem: Use 256B aligned pitch

Message ID 20210630120215.930829-1-tejaskumarx.surendrakumar.upadhyay@intel.com (mailing list archive)
State New, archived
Headers show
Series drm/vgem: Use 256B aligned pitch | expand

Commit Message

Tejas Upadhyay June 30, 2021, 12:02 p.m. UTC
Having different alignment requirement by different drivers,
256B aligned should work for all drm drivers.

Signed-off-by: Tejas Upadhyay <tejaskumarx.surendrakumar.upadhyay@intel.com>
---
 drivers/gpu/drm/vgem/vgem_drv.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

Comments

Daniel Vetter June 30, 2021, 12:21 p.m. UTC | #1
On Wed, Jun 30, 2021 at 05:32:15PM +0530, Tejas Upadhyay wrote:
> Having different alignment requirement by different drivers,
> 256B aligned should work for all drm drivers.

What.

Like yes vgem abuses dumb_create, but it's not a kms driver. Pitch is
meaningless, and that's why we align it minimally to 1 byte (bpp = bits
per pixel here).

Maybe start with explaining what you're trying to do here.
-Daniel
> 
> Signed-off-by: Tejas Upadhyay <tejaskumarx.surendrakumar.upadhyay@intel.com>
> ---
>  drivers/gpu/drm/vgem/vgem_drv.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/drivers/gpu/drm/vgem/vgem_drv.c b/drivers/gpu/drm/vgem/vgem_drv.c
> index bf38a7e319d1..1da6df5e256a 100644
> --- a/drivers/gpu/drm/vgem/vgem_drv.c
> +++ b/drivers/gpu/drm/vgem/vgem_drv.c
> @@ -215,7 +215,7 @@ static int vgem_gem_dumb_create(struct drm_file *file, struct drm_device *dev,
>  	struct drm_gem_object *gem_object;
>  	u64 pitch, size;
>  
> -	pitch = args->width * DIV_ROUND_UP(args->bpp, 8);
> +	pitch = ALIGN(args->width * DIV_ROUND_UP(args->bpp, 8), 256);
>  	size = args->height * pitch;
>  	if (size == 0)
>  		return -EINVAL;
> -- 
> 2.31.1
> 
> _______________________________________________
> Intel-gfx mailing list
> Intel-gfx@lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/intel-gfx
Tejas Upadhyay June 30, 2021, 12:46 p.m. UTC | #2
> -----Original Message-----
> From: Daniel Vetter <daniel@ffwll.ch>
> Sent: 30 June 2021 17:52
> To: Surendrakumar Upadhyay, TejaskumarX
> <tejaskumarx.surendrakumar.upadhyay@intel.com>
> Cc: dri-devel@lists.freedesktop.org; intel-gfx@lists.freedesktop.org;
> chris@chris-wilson.co.uk
> Subject: Re: [Intel-gfx] [PATCH] drm/vgem: Use 256B aligned pitch
> 
> On Wed, Jun 30, 2021 at 05:32:15PM +0530, Tejas Upadhyay wrote:
> > Having different alignment requirement by different drivers, 256B
> > aligned should work for all drm drivers.
> 
> What.
> 
> Like yes vgem abuses dumb_create, but it's not a kms driver. Pitch is
> meaningless, and that's why we align it minimally to 1 byte (bpp = bits per
> pixel here).
> 
> Maybe start with explaining what you're trying to do here.
> -Daniel
> >

Igt tool tests which are trying to exercise tests through VGEM are getting failure (if not 64B aligned) on Intel platforms in creating framebuffer as they need them to be 64B aligned. Then 64B alignment is not 
A requirement for all drm drivers.

Thanks,
Tejas

> > Signed-off-by: Tejas Upadhyay
> > <tejaskumarx.surendrakumar.upadhyay@intel.com>
> > ---
> >  drivers/gpu/drm/vgem/vgem_drv.c | 2 +-
> >  1 file changed, 1 insertion(+), 1 deletion(-)
> >
> > diff --git a/drivers/gpu/drm/vgem/vgem_drv.c
> > b/drivers/gpu/drm/vgem/vgem_drv.c index bf38a7e319d1..1da6df5e256a
> > 100644
> > --- a/drivers/gpu/drm/vgem/vgem_drv.c
> > +++ b/drivers/gpu/drm/vgem/vgem_drv.c
> > @@ -215,7 +215,7 @@ static int vgem_gem_dumb_create(struct drm_file
> *file, struct drm_device *dev,
> >  	struct drm_gem_object *gem_object;
> >  	u64 pitch, size;
> >
> > -	pitch = args->width * DIV_ROUND_UP(args->bpp, 8);
> > +	pitch = ALIGN(args->width * DIV_ROUND_UP(args->bpp, 8), 256);
> >  	size = args->height * pitch;
> >  	if (size == 0)
> >  		return -EINVAL;
> > --
> > 2.31.1
> >
> > _______________________________________________
> > Intel-gfx mailing list
> > Intel-gfx@lists.freedesktop.org
> > https://lists.freedesktop.org/mailman/listinfo/intel-gfx
> 
> --
> Daniel Vetter
> Software Engineer, Intel Corporation
> http://blog.ffwll.ch
Daniel Vetter June 30, 2021, 2:03 p.m. UTC | #3
On Wed, Jun 30, 2021 at 12:46:27PM +0000, Surendrakumar Upadhyay, TejaskumarX wrote:
> 
> 
> > -----Original Message-----
> > From: Daniel Vetter <daniel@ffwll.ch>
> > Sent: 30 June 2021 17:52
> > To: Surendrakumar Upadhyay, TejaskumarX
> > <tejaskumarx.surendrakumar.upadhyay@intel.com>
> > Cc: dri-devel@lists.freedesktop.org; intel-gfx@lists.freedesktop.org;
> > chris@chris-wilson.co.uk
> > Subject: Re: [Intel-gfx] [PATCH] drm/vgem: Use 256B aligned pitch
> > 
> > On Wed, Jun 30, 2021 at 05:32:15PM +0530, Tejas Upadhyay wrote:
> > > Having different alignment requirement by different drivers, 256B
> > > aligned should work for all drm drivers.
> > 
> > What.
> > 
> > Like yes vgem abuses dumb_create, but it's not a kms driver. Pitch is
> > meaningless, and that's why we align it minimally to 1 byte (bpp = bits per
> > pixel here).
> > 
> > Maybe start with explaining what you're trying to do here.
> > -Daniel
> > >
> 
> Igt tool tests which are trying to exercise tests through VGEM are getting failure (if not 64B aligned) on Intel platforms in creating framebuffer as they need them to be 64B aligned. Then 64B alignment is not 
> A requirement for all drm drivers.

Fix the tests. We're not going to encode alignment constraints for all kms
drivers in vgem, that's not what vgem is for.

Really what we should have done is just give vgem an ioctl to allocate a
buffer, with the size specified in bytes. Not this "abuse dumb_create"
business we're doing. But that's a past mistake, can't really fix that.
-Daniel

> 
> Thanks,
> Tejas
> 
> > > Signed-off-by: Tejas Upadhyay
> > > <tejaskumarx.surendrakumar.upadhyay@intel.com>
> > > ---
> > >  drivers/gpu/drm/vgem/vgem_drv.c | 2 +-
> > >  1 file changed, 1 insertion(+), 1 deletion(-)
> > >
> > > diff --git a/drivers/gpu/drm/vgem/vgem_drv.c
> > > b/drivers/gpu/drm/vgem/vgem_drv.c index bf38a7e319d1..1da6df5e256a
> > > 100644
> > > --- a/drivers/gpu/drm/vgem/vgem_drv.c
> > > +++ b/drivers/gpu/drm/vgem/vgem_drv.c
> > > @@ -215,7 +215,7 @@ static int vgem_gem_dumb_create(struct drm_file
> > *file, struct drm_device *dev,
> > >  	struct drm_gem_object *gem_object;
> > >  	u64 pitch, size;
> > >
> > > -	pitch = args->width * DIV_ROUND_UP(args->bpp, 8);
> > > +	pitch = ALIGN(args->width * DIV_ROUND_UP(args->bpp, 8), 256);
> > >  	size = args->height * pitch;
> > >  	if (size == 0)
> > >  		return -EINVAL;
> > > --
> > > 2.31.1
> > >
> > > _______________________________________________
> > > Intel-gfx mailing list
> > > Intel-gfx@lists.freedesktop.org
> > > https://lists.freedesktop.org/mailman/listinfo/intel-gfx
> > 
> > --
> > Daniel Vetter
> > Software Engineer, Intel Corporation
> > http://blog.ffwll.ch
diff mbox series

Patch

diff --git a/drivers/gpu/drm/vgem/vgem_drv.c b/drivers/gpu/drm/vgem/vgem_drv.c
index bf38a7e319d1..1da6df5e256a 100644
--- a/drivers/gpu/drm/vgem/vgem_drv.c
+++ b/drivers/gpu/drm/vgem/vgem_drv.c
@@ -215,7 +215,7 @@  static int vgem_gem_dumb_create(struct drm_file *file, struct drm_device *dev,
 	struct drm_gem_object *gem_object;
 	u64 pitch, size;
 
-	pitch = args->width * DIV_ROUND_UP(args->bpp, 8);
+	pitch = ALIGN(args->width * DIV_ROUND_UP(args->bpp, 8), 256);
 	size = args->height * pitch;
 	if (size == 0)
 		return -EINVAL;