diff mbox series

drm: Reject dumb buffers when driver/device doesn't support modesetting

Message ID 20200318154959.9017-1-ville.syrjala@linux.intel.com (mailing list archive)
State New, archived
Headers show
Series drm: Reject dumb buffers when driver/device doesn't support modesetting | expand

Commit Message

Ville Syrjala March 18, 2020, 3:49 p.m. UTC 
From: Ville Syrjälä <ville.syrjala@linux.intel.com>

Currently a driver must not provide a .dumb_create() hook in the
drm_driver structure if it wants to declare dumb buffers as not
supported. So if the same driver wants to support both modeset
and non-modeset devices it would require two distinct drm_driver
structures in order to reject the dumb buffer operations on the
non-modeset devices. That's rather tedious, so let's make life
easier for such drivers by also checking for the DRIVER_MODESET
flag before we declare dumb buffers as supported. Now all the
driver has to do is clear the flag for any device that can't
do modesetting.

Signed-off-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
---
 drivers/gpu/drm/drm_client.c        |  2 +-
 drivers/gpu/drm/drm_crtc_internal.h |  1 +
 drivers/gpu/drm/drm_dumb_buffers.c  | 12 +++++++++---
 drivers/gpu/drm/drm_ioctl.c         |  2 +-
 4 files changed, 12 insertions(+), 5 deletions(-)

Comments

Matt Roper March 18, 2020, 8:31 p.m. UTC | #1 
On Wed, Mar 18, 2020 at 05:49:59PM +0200, Ville Syrjala wrote:
> From: Ville Syrjälä <ville.syrjala@linux.intel.com>
> 
> Currently a driver must not provide a .dumb_create() hook in the
> drm_driver structure if it wants to declare dumb buffers as not
> supported. So if the same driver wants to support both modeset
> and non-modeset devices it would require two distinct drm_driver
> structures in order to reject the dumb buffer operations on the
> non-modeset devices. That's rather tedious, so let's make life
> easier for such drivers by also checking for the DRIVER_MODESET
> flag before we declare dumb buffers as supported. Now all the
> driver has to do is clear the flag for any device that can't
> do modesetting.

Will this be a problem for vgem?  I thought it exposed dumb buffers
without modesetting support?


Matt

> 
> Signed-off-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
> ---
>  drivers/gpu/drm/drm_client.c        |  2 +-
>  drivers/gpu/drm/drm_crtc_internal.h |  1 +
>  drivers/gpu/drm/drm_dumb_buffers.c  | 12 +++++++++---
>  drivers/gpu/drm/drm_ioctl.c         |  2 +-
>  4 files changed, 12 insertions(+), 5 deletions(-)
> 
> diff --git a/drivers/gpu/drm/drm_client.c b/drivers/gpu/drm/drm_client.c
> index 6b0c6ef8b9b3..cf61d87b434d 100644
> --- a/drivers/gpu/drm/drm_client.c
> +++ b/drivers/gpu/drm/drm_client.c
> @@ -80,7 +80,7 @@ int drm_client_init(struct drm_device *dev, struct drm_client_dev *client,
>  {
>  	int ret;
>  
> -	if (!drm_core_check_feature(dev, DRIVER_MODESET) || !dev->driver->dumb_create)
> +	if (!drm_has_dumb_buffers(dev))
>  		return -EOPNOTSUPP;
>  
>  	if (funcs && !try_module_get(funcs->owner))
> diff --git a/drivers/gpu/drm/drm_crtc_internal.h b/drivers/gpu/drm/drm_crtc_internal.h
> index 16f2413403aa..c08ff0b7a509 100644
> --- a/drivers/gpu/drm/drm_crtc_internal.h
> +++ b/drivers/gpu/drm/drm_crtc_internal.h
> @@ -92,6 +92,7 @@ int drm_mode_getresources(struct drm_device *dev,
>  
>  
>  /* drm_dumb_buffers.c */
> +bool drm_has_dumb_buffers(struct drm_device *dev);
>  int drm_mode_create_dumb(struct drm_device *dev,
>  			 struct drm_mode_create_dumb *args,
>  			 struct drm_file *file_priv);
> diff --git a/drivers/gpu/drm/drm_dumb_buffers.c b/drivers/gpu/drm/drm_dumb_buffers.c
> index d18a740fe0f1..9859530362e2 100644
> --- a/drivers/gpu/drm/drm_dumb_buffers.c
> +++ b/drivers/gpu/drm/drm_dumb_buffers.c
> @@ -55,13 +55,19 @@
>   * a hardware-specific ioctl to allocate suitable buffer objects.
>   */
>  
> +bool drm_has_dumb_buffers(struct drm_device *dev)
> +{
> +	return dev->driver->dumb_create &&
> +		drm_core_check_feature(dev, DRIVER_MODESET);
> +}
> +
>  int drm_mode_create_dumb(struct drm_device *dev,
>  			 struct drm_mode_create_dumb *args,
>  			 struct drm_file *file_priv)
>  {
>  	u32 cpp, stride, size;
>  
> -	if (!dev->driver->dumb_create)
> +	if (!drm_has_dumb_buffers(dev))
>  		return -ENOSYS;
>  	if (!args->width || !args->height || !args->bpp)
>  		return -EINVAL;
> @@ -119,7 +125,7 @@ int drm_mode_mmap_dumb_ioctl(struct drm_device *dev,
>  {
>  	struct drm_mode_map_dumb *args = data;
>  
> -	if (!dev->driver->dumb_create)
> +	if (!drm_has_dumb_buffers(dev))
>  		return -ENOSYS;
>  
>  	if (dev->driver->dumb_map_offset)
> @@ -134,7 +140,7 @@ int drm_mode_mmap_dumb_ioctl(struct drm_device *dev,
>  int drm_mode_destroy_dumb(struct drm_device *dev, u32 handle,
>  			  struct drm_file *file_priv)
>  {
> -	if (!dev->driver->dumb_create)
> +	if (!drm_has_dumb_buffers(dev))
>  		return -ENOSYS;
>  
>  	if (dev->driver->dumb_destroy)
> diff --git a/drivers/gpu/drm/drm_ioctl.c b/drivers/gpu/drm/drm_ioctl.c
> index 9e41972c4bbc..437f1bee6869 100644
> --- a/drivers/gpu/drm/drm_ioctl.c
> +++ b/drivers/gpu/drm/drm_ioctl.c
> @@ -262,7 +262,7 @@ static int drm_getcap(struct drm_device *dev, void *data, struct drm_file *file_
>  
>  	switch (req->capability) {
>  	case DRM_CAP_DUMB_BUFFER:
> -		if (dev->driver->dumb_create)
> +		if (drm_has_dumb_buffers(dev))
>  			req->value = 1;
>  		break;
>  	case DRM_CAP_VBLANK_HIGH_CRTC:
> -- 
> 2.24.1
> 
> _______________________________________________
> dri-devel mailing list
> dri-devel@lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/dri-devel
Ville Syrjala March 18, 2020, 8:44 p.m. UTC | #2 
On Wed, Mar 18, 2020 at 01:31:07PM -0700, Matt Roper wrote:
> On Wed, Mar 18, 2020 at 05:49:59PM +0200, Ville Syrjala wrote:
> > From: Ville Syrjälä <ville.syrjala@linux.intel.com>
> > 
> > Currently a driver must not provide a .dumb_create() hook in the
> > drm_driver structure if it wants to declare dumb buffers as not
> > supported. So if the same driver wants to support both modeset
> > and non-modeset devices it would require two distinct drm_driver
> > structures in order to reject the dumb buffer operations on the
> > non-modeset devices. That's rather tedious, so let's make life
> > easier for such drivers by also checking for the DRIVER_MODESET
> > flag before we declare dumb buffers as supported. Now all the
> > driver has to do is clear the flag for any device that can't
> > do modesetting.
> 
> Will this be a problem for vgem?  I thought it exposed dumb buffers
> without modesetting support?

Well that's disappointing.
diff mbox series

Patch

diff --git a/drivers/gpu/drm/drm_client.c b/drivers/gpu/drm/drm_client.c
index 6b0c6ef8b9b3..cf61d87b434d 100644
--- a/drivers/gpu/drm/drm_client.c
+++ b/drivers/gpu/drm/drm_client.c
@@ -80,7 +80,7 @@  int drm_client_init(struct drm_device *dev, struct drm_client_dev *client,
 {
 	int ret;
 
-	if (!drm_core_check_feature(dev, DRIVER_MODESET) || !dev->driver->dumb_create)
+	if (!drm_has_dumb_buffers(dev))
 		return -EOPNOTSUPP;
 
 	if (funcs && !try_module_get(funcs->owner))
diff --git a/drivers/gpu/drm/drm_crtc_internal.h b/drivers/gpu/drm/drm_crtc_internal.h
index 16f2413403aa..c08ff0b7a509 100644
--- a/drivers/gpu/drm/drm_crtc_internal.h
+++ b/drivers/gpu/drm/drm_crtc_internal.h
@@ -92,6 +92,7 @@  int drm_mode_getresources(struct drm_device *dev,
 
 
 /* drm_dumb_buffers.c */
+bool drm_has_dumb_buffers(struct drm_device *dev);
 int drm_mode_create_dumb(struct drm_device *dev,
 			 struct drm_mode_create_dumb *args,
 			 struct drm_file *file_priv);
diff --git a/drivers/gpu/drm/drm_dumb_buffers.c b/drivers/gpu/drm/drm_dumb_buffers.c
index d18a740fe0f1..9859530362e2 100644
--- a/drivers/gpu/drm/drm_dumb_buffers.c
+++ b/drivers/gpu/drm/drm_dumb_buffers.c
@@ -55,13 +55,19 @@ 
  * a hardware-specific ioctl to allocate suitable buffer objects.
  */
 
+bool drm_has_dumb_buffers(struct drm_device *dev)
+{
+	return dev->driver->dumb_create &&
+		drm_core_check_feature(dev, DRIVER_MODESET);
+}
+
 int drm_mode_create_dumb(struct drm_device *dev,
 			 struct drm_mode_create_dumb *args,
 			 struct drm_file *file_priv)
 {
 	u32 cpp, stride, size;
 
-	if (!dev->driver->dumb_create)
+	if (!drm_has_dumb_buffers(dev))
 		return -ENOSYS;
 	if (!args->width || !args->height || !args->bpp)
 		return -EINVAL;
@@ -119,7 +125,7 @@  int drm_mode_mmap_dumb_ioctl(struct drm_device *dev,
 {
 	struct drm_mode_map_dumb *args = data;
 
-	if (!dev->driver->dumb_create)
+	if (!drm_has_dumb_buffers(dev))
 		return -ENOSYS;
 
 	if (dev->driver->dumb_map_offset)
@@ -134,7 +140,7 @@  int drm_mode_mmap_dumb_ioctl(struct drm_device *dev,
 int drm_mode_destroy_dumb(struct drm_device *dev, u32 handle,
 			  struct drm_file *file_priv)
 {
-	if (!dev->driver->dumb_create)
+	if (!drm_has_dumb_buffers(dev))
 		return -ENOSYS;
 
 	if (dev->driver->dumb_destroy)
diff --git a/drivers/gpu/drm/drm_ioctl.c b/drivers/gpu/drm/drm_ioctl.c
index 9e41972c4bbc..437f1bee6869 100644
--- a/drivers/gpu/drm/drm_ioctl.c
+++ b/drivers/gpu/drm/drm_ioctl.c
@@ -262,7 +262,7 @@  static int drm_getcap(struct drm_device *dev, void *data, struct drm_file *file_
 
 	switch (req->capability) {
 	case DRM_CAP_DUMB_BUFFER:
-		if (dev->driver->dumb_create)
+		if (drm_has_dumb_buffers(dev))
 			req->value = 1;
 		break;
 	case DRM_CAP_VBLANK_HIGH_CRTC: