[PATCH] drm: Allow CAP_DUMB_BUFFERS on !MODESET

[PATCH] drm: Allow CAP_DUMB_BUFFERS on !MODESET

[Ezequiel Garcia]

It's perfectly possible to get dumb buffers out of drivers
that don't support modeset. This is the case of vgem,
which can be used to export dmabuf to run various tests.
Therefore, move the dumb buffer capability check before
the mode set check.

Inspired by commit f3f4c4d68a28 ("drm: Allow CAP_PRIME on !MODESET").

Fixes: d5264ed3823a ("drm: Return -ENOTSUPP when called for KMS cap with a non-KMS driver")
Signed-off-by: Ezequiel Garcia <ezequiel@collabora.com>
---
 drivers/gpu/drm/drm_ioctl.c | 7 +++----
 1 file changed, 3 insertions(+), 4 deletions(-)

diff --git a/drivers/gpu/drm/drm_ioctl.c b/drivers/gpu/drm/drm_ioctl.c
index af782911c505..deac011a0c25 100644
--- a/drivers/gpu/drm/drm_ioctl.c
+++ b/drivers/gpu/drm/drm_ioctl.c
@@ -244,6 +244,9 @@ static int drm_getcap(struct drm_device *dev, void *data, struct drm_file *file_
 	case DRM_CAP_SYNCOBJ:
 		req->value = drm_core_check_feature(dev, DRIVER_SYNCOBJ);
 		return 0;
+	case DRM_CAP_DUMB_BUFFER:
+		req->value |= dev->driver->dumb_create ? 1 : 0;
+		return 0;
 	}
 
 	/* Other caps only work with KMS drivers */
@@ -251,10 +254,6 @@ static int drm_getcap(struct drm_device *dev, void *data, struct drm_file *file_
 		return -ENOTSUPP;
 
 	switch (req->capability) {
-	case DRM_CAP_DUMB_BUFFER:
-		if (dev->driver->dumb_create)
-			req->value = 1;
-		break;
 	case DRM_CAP_VBLANK_HIGH_CRTC:
 		req->value = 1;
 		break;
-- 
2.16.3

_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

Re: [PATCH] drm: Allow CAP_DUMB_BUFFERS on !MODESET

[Maarten Lankhorst]

Op 04-05-18 om 16:25 schreef Ezequiel Garcia:
> It's perfectly possible to get dumb buffers out of drivers
> that don't support modeset. This is the case of vgem,
> which can be used to export dmabuf to run various tests.
> Therefore, move the dumb buffer capability check before
> the mode set check.
>
> Inspired by commit f3f4c4d68a28 ("drm: Allow CAP_PRIME on !MODESET").
>
> Fixes: d5264ed3823a ("drm: Return -ENOTSUPP when called for KMS cap with a non-KMS driver")
> Signed-off-by: Ezequiel Garcia <ezequiel@collabora.com>
> ---
>  drivers/gpu/drm/drm_ioctl.c | 7 +++----
>  1 file changed, 3 insertions(+), 4 deletions(-)
>
> diff --git a/drivers/gpu/drm/drm_ioctl.c b/drivers/gpu/drm/drm_ioctl.c
> index af782911c505..deac011a0c25 100644
> --- a/drivers/gpu/drm/drm_ioctl.c
> +++ b/drivers/gpu/drm/drm_ioctl.c
> @@ -244,6 +244,9 @@ static int drm_getcap(struct drm_device *dev, void *data, struct drm_file *file_
>  	case DRM_CAP_SYNCOBJ:
>  		req->value = drm_core_check_feature(dev, DRIVER_SYNCOBJ);
>  		return 0;
> +	case DRM_CAP_DUMB_BUFFER:
> +		req->value |= dev->driver->dumb_create ? 1 : 0;
Why the |= ?
> +		return 0;
>  	}
>  
>  	/* Other caps only work with KMS drivers */
> @@ -251,10 +254,6 @@ static int drm_getcap(struct drm_device *dev, void *data, struct drm_file *file_
>  		return -ENOTSUPP;
>  
>  	switch (req->capability) {
> -	case DRM_CAP_DUMB_BUFFER:
> -		if (dev->driver->dumb_create)
> -			req->value = 1;
> -		break;
>  	case DRM_CAP_VBLANK_HIGH_CRTC:
>  		req->value = 1;
>  		break;


_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

Re: [PATCH] drm: Allow CAP_DUMB_BUFFERS on !MODESET

[Ezequiel Garcia]

On Mon, 2018-05-07 at 11:36 +0200, Maarten Lankhorst wrote:
> Op 04-05-18 om 16:25 schreef Ezequiel Garcia:
> > It's perfectly possible to get dumb buffers out of drivers
> > that don't support modeset. This is the case of vgem,
> > which can be used to export dmabuf to run various tests.
> > Therefore, move the dumb buffer capability check before
> > the mode set check.
> > 
> > Inspired by commit f3f4c4d68a28 ("drm: Allow CAP_PRIME on !MODESET").
> > 
> > Fixes: d5264ed3823a ("drm: Return -ENOTSUPP when called for KMS cap with a non-KMS driver")
> > Signed-off-by: Ezequiel Garcia <ezequiel@collabora.com>
> > ---
> >  drivers/gpu/drm/drm_ioctl.c | 7 +++----
> >  1 file changed, 3 insertions(+), 4 deletions(-)
> > 
> > diff --git a/drivers/gpu/drm/drm_ioctl.c b/drivers/gpu/drm/drm_ioctl.c
> > index af782911c505..deac011a0c25 100644
> > --- a/drivers/gpu/drm/drm_ioctl.c
> > +++ b/drivers/gpu/drm/drm_ioctl.c
> > @@ -244,6 +244,9 @@ static int drm_getcap(struct drm_device *dev, void *data, struct drm_file *file_
> >  	case DRM_CAP_SYNCOBJ:
> >  		req->value = drm_core_check_feature(dev, DRIVER_SYNCOBJ);
> >  		return 0;
> > +	case DRM_CAP_DUMB_BUFFER:
> > +		req->value |= dev->driver->dumb_create ? 1 : 0;
> 
> Why the |= ?

I'd say that's a silly copy-paste glitch. Will send a v2.

> > +		return 0;
> >  	}
> >  
> >  	/* Other caps only work with KMS drivers */
> > @@ -251,10 +254,6 @@ static int drm_getcap(struct drm_device *dev, void *data, struct drm_file *file_
> >  		return -ENOTSUPP;
> >  
> >  	switch (req->capability) {
> > -	case DRM_CAP_DUMB_BUFFER:
> > -		if (dev->driver->dumb_create)
> > -			req->value = 1;
> > -		break;
> >  	case DRM_CAP_VBLANK_HIGH_CRTC:
> >  		req->value = 1;
> >  		break;
> 
> 
_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

[PATCH v2] drm: Allow CAP_DUMB_BUFFERS on !MODESET

[Ezequiel Garcia]

It's perfectly possible to get dumb buffers out of drivers
that don't support modeset. This is the case of vgem,
which can be used to export dmabuf to run various tests.

Inspired by commit f3f4c4d68a28 ("drm: Allow CAP_PRIME on !MODESET").

Fixes: d5264ed3823a ("drm: Return -ENOTSUPP when called for KMS cap with a non-KMS driver")
Signed-off-by: Ezequiel Garcia <ezequiel@collabora.com>
---
Changes from v1:
 * Drop the bitwise-OR assignment op

 drivers/gpu/drm/drm_ioctl.c | 7 +++----
 1 file changed, 3 insertions(+), 4 deletions(-)

diff --git a/drivers/gpu/drm/drm_ioctl.c b/drivers/gpu/drm/drm_ioctl.c
index af782911c505..a5b879ce8f2c 100644
--- a/drivers/gpu/drm/drm_ioctl.c
+++ b/drivers/gpu/drm/drm_ioctl.c
@@ -244,6 +244,9 @@ static int drm_getcap(struct drm_device *dev, void *data, struct drm_file *file_
 	case DRM_CAP_SYNCOBJ:
 		req->value = drm_core_check_feature(dev, DRIVER_SYNCOBJ);
 		return 0;
+	case DRM_CAP_DUMB_BUFFER:
+		req->value = dev->driver->dumb_create ? 1 : 0;
+		return 0;
 	}
 
 	/* Other caps only work with KMS drivers */
@@ -251,10 +254,6 @@ static int drm_getcap(struct drm_device *dev, void *data, struct drm_file *file_
 		return -ENOTSUPP;
 
 	switch (req->capability) {
-	case DRM_CAP_DUMB_BUFFER:
-		if (dev->driver->dumb_create)
-			req->value = 1;
-		break;
 	case DRM_CAP_VBLANK_HIGH_CRTC:
 		req->value = 1;
 		break;
-- 
2.16.3

_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

Re: [PATCH v2] drm: Allow CAP_DUMB_BUFFERS on !MODESET

[Daniel Vetter]

On Tue, May 08, 2018 at 01:52:01AM -0300, Ezequiel Garcia wrote:
> It's perfectly possible to get dumb buffers out of drivers
> that don't support modeset. This is the case of vgem,
> which can be used to export dmabuf to run various tests.
> 
> Inspired by commit f3f4c4d68a28 ("drm: Allow CAP_PRIME on !MODESET").

Prime makes sense, because render-only drivers _really_ want to be able to
share buffers.

But dumb buffers are really meant for dumb userspace running on kms
drivers only, there's no need ever to tell userspace that dumb buffers are
supported on non-kms drivers. It's kinda abuse of ioctls, but oh well,
uapi is fixed forever. And you can still call the ioctls if you know
they're there (which is always the case for the render-side of drm,
there's no generic alloc ioctl for those).
-Daniel

> Fixes: d5264ed3823a ("drm: Return -ENOTSUPP when called for KMS cap with a non-KMS driver")
> Signed-off-by: Ezequiel Garcia <ezequiel@collabora.com>
> ---
> Changes from v1:
>  * Drop the bitwise-OR assignment op
> 
>  drivers/gpu/drm/drm_ioctl.c | 7 +++----
>  1 file changed, 3 insertions(+), 4 deletions(-)
> 
> diff --git a/drivers/gpu/drm/drm_ioctl.c b/drivers/gpu/drm/drm_ioctl.c
> index af782911c505..a5b879ce8f2c 100644
> --- a/drivers/gpu/drm/drm_ioctl.c
> +++ b/drivers/gpu/drm/drm_ioctl.c
> @@ -244,6 +244,9 @@ static int drm_getcap(struct drm_device *dev, void *data, struct drm_file *file_
>  	case DRM_CAP_SYNCOBJ:
>  		req->value = drm_core_check_feature(dev, DRIVER_SYNCOBJ);
>  		return 0;
> +	case DRM_CAP_DUMB_BUFFER:
> +		req->value = dev->driver->dumb_create ? 1 : 0;
> +		return 0;
>  	}
>  
>  	/* Other caps only work with KMS drivers */
> @@ -251,10 +254,6 @@ static int drm_getcap(struct drm_device *dev, void *data, struct drm_file *file_
>  		return -ENOTSUPP;
>  
>  	switch (req->capability) {
> -	case DRM_CAP_DUMB_BUFFER:
> -		if (dev->driver->dumb_create)
> -			req->value = 1;
> -		break;
>  	case DRM_CAP_VBLANK_HIGH_CRTC:
>  		req->value = 1;
>  		break;
> -- 
> 2.16.3
> 
> _______________________________________________
> dri-devel mailing list
> dri-devel@lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/dri-devel

-- 
Daniel Vetter
Software Engineer, Intel Corporation
http://blog.ffwll.ch
_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

Re: [PATCH v2] drm: Allow CAP_DUMB_BUFFERS on !MODESET

[Ezequiel Garcia]

Hey Daniel,

On Tue, 2018-05-08 at 09:55 +0200, Daniel Vetter wrote:
> On Tue, May 08, 2018 at 01:52:01AM -0300, Ezequiel Garcia wrote:
> > It's perfectly possible to get dumb buffers out of drivers
> > that don't support modeset. This is the case of vgem,
> > which can be used to export dmabuf to run various tests.
> > 
> > Inspired by commit f3f4c4d68a28 ("drm: Allow CAP_PRIME on !MODESET").
> 
> Prime makes sense, because render-only drivers _really_ want to be able to
> share buffers.
> 
> But dumb buffers are really meant for dumb userspace running on kms
> drivers only, there's no need ever to tell userspace that dumb buffers are
> supported on non-kms drivers. It's kinda abuse of ioctls, but oh well,
> uapi is fixed forever. And you can still call the ioctls if you know
> they're there (which is always the case for the render-side of drm,
> there's no generic alloc ioctl for those).
> 

Right, I see.

Well, I was mostly interested in dry testing, and vgem seemed a good
candidate for a dma-buf exporter. For instance, this would be useful
to test dmabuf video4linux pipelines.

Now, is this is a nack, then the other path is to support proper prime
operations on virtio-gpu (which I'm also working on).

Thanks for reviewing!
Eze

_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

Re: [PATCH v2] drm: Allow CAP_DUMB_BUFFERS on !MODESET

[Daniel Vetter]

On Tue, May 08, 2018 at 08:42:03PM -0300, Ezequiel Garcia wrote:
> Hey Daniel,
> 
> On Tue, 2018-05-08 at 09:55 +0200, Daniel Vetter wrote:
> > On Tue, May 08, 2018 at 01:52:01AM -0300, Ezequiel Garcia wrote:
> > > It's perfectly possible to get dumb buffers out of drivers
> > > that don't support modeset. This is the case of vgem,
> > > which can be used to export dmabuf to run various tests.
> > > 
> > > Inspired by commit f3f4c4d68a28 ("drm: Allow CAP_PRIME on !MODESET").
> > 
> > Prime makes sense, because render-only drivers _really_ want to be able to
> > share buffers.
> > 
> > But dumb buffers are really meant for dumb userspace running on kms
> > drivers only, there's no need ever to tell userspace that dumb buffers are
> > supported on non-kms drivers. It's kinda abuse of ioctls, but oh well,
> > uapi is fixed forever. And you can still call the ioctls if you know
> > they're there (which is always the case for the render-side of drm,
> > there's no generic alloc ioctl for those).
> > 
> 
> Right, I see.
> 
> Well, I was mostly interested in dry testing, and vgem seemed a good
> candidate for a dma-buf exporter. For instance, this would be useful
> to test dmabuf video4linux pipelines.

You can still use the ioctl ... we have tons of tests using vgem for
buffer import/export in our igt gpu tests. Testing prime is exactly what
vgem is for!

What you don't need is confuse generic userspace by enabling the getparam
for dumb buffers. But such vgem-specific tests aren't generic userspace.

https://cgit.freedesktop.org/drm/igt-gpu-tools/

https://cgit.freedesktop.org/drm/igt-gpu-tools/tree/tests/prime_vgem.c

Cheers, Daniel

> Now, is this is a nack, then the other path is to support proper prime
> operations on virtio-gpu (which I'm also working on).
> 
> Thanks for reviewing!
> Eze
> 

-- 
Daniel Vetter
Software Engineer, Intel Corporation
http://blog.ffwll.ch
_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel