Re: [PATCH 1/2] drm: Add aux plane verification in addFB2

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

On Fri, Mar 18, 2016 at 10:20:52PM +0530, Vandana Kannan wrote:
> From: Daniel Vetter <daniel.vetter@ffwll.ch>
> 
> For render compression, userspace passes aux stride and offset values as an
> additional entry in the fb structure. This should not be treated as garbage
> and discarded as data belonging to no plane.
> This patch introduces a check related to AUX plane to support the
> scenario of render compression.
> 
> v2: Based on a discussion with Siva
> Move num_planes check below the increment.
> 
> Signed-off-by: Daniel Vetter <daniel@ffwll.ch>
> Signed-off-by: Vandana Kannan <vandana.kannan@intel.com>
> Cc: Sivakumar Thulasimani <sivakumar.thulasimani@intel.com>
> ---
>  drivers/gpu/drm/drm_crtc.c  | 19 ++++++++++++++++++-
>  drivers/gpu/drm/drm_ioctl.c |  3 +++
>  include/drm/drm_crtc.h      |  3 +++
>  include/uapi/drm/drm.h      |  1 +
>  include/uapi/drm/drm_mode.h |  1 +
>  5 files changed, 26 insertions(+), 1 deletion(-)
> 
> diff --git a/drivers/gpu/drm/drm_crtc.c b/drivers/gpu/drm/drm_crtc.c
> index a4e90c7..8659748 100644
> --- a/drivers/gpu/drm/drm_crtc.c
> +++ b/drivers/gpu/drm/drm_crtc.c
> @@ -3287,6 +3287,16 @@ static int framebuffer_check(const struct drm_mode_fb_cmd2 *r)
>  		}
>  	}
>  
> +	if (r->flags & DRM_MODE_FB_AUX_PLANE) {
> +		num_planes++;
> +
> +		if (num_planes == 4) {
> +			DRM_DEBUG_KMS("Number of planes cannot exceed 3"
> +					"(including aux plane)\n");
> +			return -EINVAL;
> +		}
> +	}

That's not going to work. For one drm_format_plane_cpp() doesn't know
anything about your AUX plane. None of this code can really understand
anything about the AUX plane since that's hw specific pretty much by
definition.

> +
>  	for (i = num_planes; i < 4; i++) {
>  		if (r->modifier[i]) {
>  			DRM_DEBUG_KMS("non-zero modifier for unused plane %d\n", i);
> @@ -3325,7 +3335,8 @@ internal_framebuffer_create(struct drm_device *dev,
>  	struct drm_framebuffer *fb;
>  	int ret;
>  
> -	if (r->flags & ~(DRM_MODE_FB_INTERLACED | DRM_MODE_FB_MODIFIERS)) {
> +	if (r->flags & ~(DRM_MODE_FB_INTERLACED | DRM_MODE_FB_MODIFIERS |
> +		DRM_MODE_FB_AUX_PLANE)) {
>  		DRM_DEBUG_KMS("bad framebuffer flags 0x%08x\n", r->flags);
>  		return ERR_PTR(-EINVAL);
>  	}
> @@ -3347,6 +3358,12 @@ internal_framebuffer_create(struct drm_device *dev,
>  		return ERR_PTR(-EINVAL);
>  	}
>  
> +	if (r->flags & DRM_MODE_FB_AUX_PLANE &&
> +	    !dev->mode_config.allow_aux_plane) {
> +		DRM_DEBUG_KMS("driver does not support render compression\n");
> +		return ERR_PTR(-EINVAL);
> +	}
> +
>  	ret = framebuffer_check(r);
>  	if (ret)
>  		return ERR_PTR(ret);
> diff --git a/drivers/gpu/drm/drm_ioctl.c b/drivers/gpu/drm/drm_ioctl.c
> index 8ce2a0c..ee00782 100644
> --- a/drivers/gpu/drm/drm_ioctl.c
> +++ b/drivers/gpu/drm/drm_ioctl.c
> @@ -312,6 +312,9 @@ static int drm_getcap(struct drm_device *dev, void *data, struct drm_file *file_
>  	case DRM_CAP_ADDFB2_MODIFIERS:
>  		req->value = dev->mode_config.allow_fb_modifiers;
>  		break;
> +	case DRM_CAP_RENDER_COMPRESSION:
> +		req->value = dev->mode_config.allow_aux_plane;
> +		break;
>  	default:
>  		return -EINVAL;
>  	}
> diff --git a/include/drm/drm_crtc.h b/include/drm/drm_crtc.h
> index 7fad193..00b1f59 100644
> --- a/include/drm/drm_crtc.h
> +++ b/include/drm/drm_crtc.h
> @@ -2141,6 +2141,9 @@ struct drm_mode_config {
>  	/* whether the driver supports fb modifiers */
>  	bool allow_fb_modifiers;
>  
> +	/* whether the driver supports render compression */
> +	bool allow_aux_plane;
> +
>  	/* cursor size */
>  	uint32_t cursor_width, cursor_height;
>  };
> diff --git a/include/uapi/drm/drm.h b/include/uapi/drm/drm.h
> index a0ebfe7..01561834 100644
> --- a/include/uapi/drm/drm.h
> +++ b/include/uapi/drm/drm.h
> @@ -632,6 +632,7 @@ struct drm_gem_open {
>  #define DRM_CAP_CURSOR_WIDTH		0x8
>  #define DRM_CAP_CURSOR_HEIGHT		0x9
>  #define DRM_CAP_ADDFB2_MODIFIERS	0x10
> +#define DRM_CAP_RENDER_COMPRESSION	0x11
>  
>  /** DRM_IOCTL_GET_CAP ioctl argument type */
>  struct drm_get_cap {
> diff --git a/include/uapi/drm/drm_mode.h b/include/uapi/drm/drm_mode.h
> index 50adb46..f900dc95 100644
> --- a/include/uapi/drm/drm_mode.h
> +++ b/include/uapi/drm/drm_mode.h
> @@ -354,6 +354,7 @@ struct drm_mode_fb_cmd {
>  
>  #define DRM_MODE_FB_INTERLACED	(1<<0) /* for interlaced framebuffers */
>  #define DRM_MODE_FB_MODIFIERS	(1<<1) /* enables ->modifer[] */
> +#define DRM_MODE_FB_AUX_PLANE   (1<<2) /* for compressed buffer */
>  
>  struct drm_mode_fb_cmd2 {
>  	__u32 fb_id;
> -- 
> 1.9.1
> 
> _______________________________________________
> Intel-gfx mailing list
> Intel-gfx@lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/intel-gfx

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