Re: [PATCH 4/4] drm: Add getfb2 ioctl

["Ville Syrjälä" <ville.syrjala@linux.intel.com>]

On Fri, Mar 23, 2018 at 01:45:52PM +0000, Daniel Stone wrote:
> getfb2 allows us to pass multiple planes and modifiers, just like addfb2
> over addfb.
> 
> Signed-off-by: Daniel Stone <daniels@collabora.com>
> ---
>  drivers/gpu/drm/drm_crtc_internal.h |   2 +
>  drivers/gpu/drm/drm_framebuffer.c   | 109 ++++++++++++++++++++++++++++++++++++
>  drivers/gpu/drm/drm_ioctl.c         |   2 +
>  include/uapi/drm/drm.h              |   1 +
>  4 files changed, 114 insertions(+)
> 
> diff --git a/drivers/gpu/drm/drm_crtc_internal.h b/drivers/gpu/drm/drm_crtc_internal.h
> index 3c2b82865ad2..0cd02f3d203d 100644
> --- a/drivers/gpu/drm/drm_crtc_internal.h
> +++ b/drivers/gpu/drm/drm_crtc_internal.h
> @@ -173,6 +173,8 @@ int drm_mode_rmfb(struct drm_device *dev,
>  		  void *data, struct drm_file *file_priv);
>  int drm_mode_getfb(struct drm_device *dev,
>  		   void *data, struct drm_file *file_priv);
> +int drm_mode_getfb2_ioctl(struct drm_device *dev,
> +			  void *data, struct drm_file *file_priv);
>  int drm_mode_dirtyfb_ioctl(struct drm_device *dev,
>  			   void *data, struct drm_file *file_priv);
>  
> diff --git a/drivers/gpu/drm/drm_framebuffer.c b/drivers/gpu/drm/drm_framebuffer.c
> index 6d5ff541225a..f1cfb6ddc776 100644
> --- a/drivers/gpu/drm/drm_framebuffer.c
> +++ b/drivers/gpu/drm/drm_framebuffer.c
> @@ -24,6 +24,7 @@
>  #include <drm/drmP.h>
>  #include <drm/drm_auth.h>
>  #include <drm/drm_framebuffer.h>
> +#include <drm/drm_gem.h>
>  #include <drm/drm_atomic.h>
>  #include <drm/drm_print.h>
>  
> @@ -494,7 +495,115 @@ int drm_mode_getfb(struct drm_device *dev,
>  
>  out:
>  	drm_framebuffer_put(fb);
> +	return ret;
> +}
> +
> +/**
> + * drm_mode_getfb2 - get extended FB info
> + * @dev: drm device for the ioctl
> + * @data: data pointer for the ioctl
> + * @file_priv: drm file for the ioctl call
> + *
> + * Lookup the FB given its ID and return info about it.
> + *
> + * Called by the user via ioctl.
> + *
> + * Returns:
> + * Zero on success, negative errno on failure.
> + */
> +int drm_mode_getfb2_ioctl(struct drm_device *dev,
> +			  void *data, struct drm_file *file_priv)
> +{
> +	struct drm_mode_fb_cmd2 *r = data;
> +	struct drm_framebuffer *fb;
> +	unsigned int i;
> +	int ret;
> +
> +	if (!drm_core_check_feature(dev, DRIVER_MODESET))
> +		return -EINVAL;
>  
> +	fb = drm_framebuffer_lookup(dev, file_priv, r->fb_id);
> +	if (!fb)
> +		return -ENOENT;
> +
> +	/* For multi-plane framebuffers, we require the driver to place the
> +	 * GEM objects directly in the drm_framebuffer. For single-plane
> +	 * framebuffers, we can fall back to create_handle.
> +	 */
> +	if (!fb->obj[0] &&
> +	    (fb->format->num_planes > 1 || !fb->funcs->create_handle)) {
> +		ret = -ENODEV;
> +		goto out;
> +	}
> +
> +	r->height = fb->height;
> +	r->width = fb->width;
> +	r->pixel_format = fb->format->format;
> +
> +	r->flags = 0;
> +	if (dev->mode_config.allow_fb_modifiers)
> +		r->flags |= DRM_MODE_FB_MODIFIERS;
> +
> +	for (i = 0; i < ARRAY_SIZE(r->handles); i++) {
> +		r->handles[i] = 0;
> +		r->pitches[i] = 0;
> +		r->offsets[i] = 0;
> +		r->modifier[i] = DRM_FORMAT_MOD_INVALID;

Don't we want to leave this zeroed too? For addfb2 we require any unused
modifier to be 0, so if someone does 'getfb2(&cmd); addfb2(&cmd);' they
would get an error from the addfb2().

> +	}
> +
> +	for (i = 0; i < fb->format->num_planes; i++) {
> +		int j;
> +
> +		r->pitches[i] = fb->pitches[i];
> +		r->offsets[i] = fb->offsets[i];
> +		if (dev->mode_config.allow_fb_modifiers)
> +			r->modifier[i] = fb->modifier;
> +
> +		/* If we reuse the same object for multiple planes, also
> +		 * return the same handle.
> +		 */
> +		for (j = 0; j < i; j++) {
> +			if (fb->obj[i] == fb->obj[j]) {
> +				r->handles[i] = r->handles[j];
> +				break;
> +			}
> +		}
> +
> +		if (r->handles[i])
> +			continue;
> +
> +		if (fb->obj[i]) {
> +			ret = drm_gem_handle_create(file_priv, fb->obj[i],
> +						    &r->handles[i]);
> +		} else {
> +			WARN_ON(i > 0);
> +			ret = fb->funcs->create_handle(fb, file_priv,
> +						       &r->handles[i]);
> +		}
> +
> +		if (ret != 0)
> +			goto out;
> +	}
> +
> +out:
> +	if (ret != 0) {
> +		/* Delete any previously-created handles on failure. */
> +		for (i = 0; i < ARRAY_SIZE(r->handles); i++) {
> +			int j;
> +
> +			if (r->handles[i])
> +				drm_gem_handle_delete(file_priv, r->handles[i]);
> +
> +			/* Zero out any handles identical to the one we just
> +			 * deleted. */
> +			for (j = i + 1; j < ARRAY_SIZE(r->handles); j++) {
> +				if (r->handles[j] == r->handles[i])
> +					r->handles[j] = 0;
> +			}
> +		}
> +	}
> +
> +	drm_framebuffer_put(fb);
>  	return ret;
>  }
>  
> diff --git a/drivers/gpu/drm/drm_ioctl.c b/drivers/gpu/drm/drm_ioctl.c
> index af782911c505..b5896e3615e5 100644
> --- a/drivers/gpu/drm/drm_ioctl.c
> +++ b/drivers/gpu/drm/drm_ioctl.c
> @@ -669,6 +669,8 @@ static const struct drm_ioctl_desc drm_ioctls[] = {
>  	DRM_IOCTL_DEF(DRM_IOCTL_MODE_LIST_LESSEES, drm_mode_list_lessees_ioctl, DRM_MASTER|DRM_CONTROL_ALLOW|DRM_UNLOCKED),
>  	DRM_IOCTL_DEF(DRM_IOCTL_MODE_GET_LEASE, drm_mode_get_lease_ioctl, DRM_MASTER|DRM_CONTROL_ALLOW|DRM_UNLOCKED),
>  	DRM_IOCTL_DEF(DRM_IOCTL_MODE_REVOKE_LEASE, drm_mode_revoke_lease_ioctl, DRM_MASTER|DRM_CONTROL_ALLOW|DRM_UNLOCKED),
> +
> +	DRM_IOCTL_DEF(DRM_IOCTL_MODE_GETFB2, drm_mode_getfb2_ioctl, DRM_MASTER|DRM_CONTROL_ALLOW|DRM_UNLOCKED),
>  };
>  
>  #define DRM_CORE_IOCTL_COUNT	ARRAY_SIZE( drm_ioctls )
> diff --git a/include/uapi/drm/drm.h b/include/uapi/drm/drm.h
> index 6fdff5945c8a..9a33613394a9 100644
> --- a/include/uapi/drm/drm.h
> +++ b/include/uapi/drm/drm.h
> @@ -892,6 +892,7 @@ extern "C" {
>  #define DRM_IOCTL_MODE_LIST_LESSEES	DRM_IOWR(0xC7, struct drm_mode_list_lessees)
>  #define DRM_IOCTL_MODE_GET_LEASE	DRM_IOWR(0xC8, struct drm_mode_get_lease)
>  #define DRM_IOCTL_MODE_REVOKE_LEASE	DRM_IOWR(0xC9, struct drm_mode_revoke_lease)
> +#define DRM_IOCTL_MODE_GETFB2		DRM_IOWR(0xCA, struct drm_mode_fb_cmd2)
>  
>  /**
>   * Device specific ioctls should only be in their respective headers
> -- 
> 2.16.2
> 
> _______________________________________________
> dri-devel mailing list
> dri-devel@lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/dri-devel

-- 
Ville Syrjälä
Intel OTC
_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel