Donate to e Foundation | Murena handsets with /e/OS | Own a part of Murena! Learn more

Commit e3eb3250 authored by Rob Clark's avatar Rob Clark Committed by Daniel Vetter
Browse files

drm: add support for tiled/compressed/etc modifier in addfb2 



In DRM/KMS we are lacking a good way to deal with tiled/compressed
formats.  Especially in the case of dmabuf/prime buffer sharing, where
we cannot always rely on under-the-hood flags passed to driver specific
gem-create ioctl to pass around these extra flags.

The proposal is to add a per-plane format modifier.  This allows to, if
necessary, use different tiling patters for sub-sampled planes, etc.
The format modifiers are added at the end of the ioctl struct, so for
legacy userspace it will be zero padded.

v1: original
v1.5: increase modifier to 64b

v2: Incorporate review comments from the big thread, plus a few more.

- Add a getcap so that userspace doesn't have to jump through hoops.
- Allow modifiers only when a flag is set. That way drivers know when
  they're dealing with old userspace and need to fish out e.g. tiling
  from other information.
- After rolling out checks for ->modifier to all drivers I've decided
  that this is way too fragile and needs an explicit opt-in flag. So
  do that instead.
- Add a define (just for documentation really) for the "NONE"
  modifier. Imo we don't need to add mask #defines since drivers
  really should only do exact matches against values defined with
  fourcc_mod_code.
- Drop the Samsung tiling modifier on Rob's request since he's not yet
  sure whether that one is accurate.

v3:
- Also add a new ->modifier[] array to struct drm_framebuffer and fill
  it in drm_helper_mode_fill_fb_struct. Requested by Tvrkto Uruslin.
- Remove TODO in comment and add code comment that modifiers should be
  properly documented, requested by Rob.

Cc: Rob Clark <robdclark@gmail.com>
Cc: Tvrtko Ursulin <tvrtko.ursulin@linux.intel.com>
Cc: Laurent Pinchart <laurent.pinchart@ideasonboard.com>
Cc: Daniel Stone <daniel@fooishbar.org>
Cc: Ville Syrjälä <ville.syrjala@linux.intel.com>
Cc: Michel Dänzer <michel@daenzer.net>
Signed-off-by: Rob Clark <robdclark@gmail.com> (v1.5)
Reviewed-by: default avatarRob Clark <robdclark@gmail.com>
Reviewed-by: default avatarDaniel Stone <daniels@collabora.com>
Acked-by: default avatarDave Airlie <airlied@redhat.com>
Signed-off-by: default avatarDaniel Vetter <daniel.vetter@intel.com>
parent 17d5538d

drivers/gpu/drm/drm_crtc.c

+13 −1
Original line number Diff line number Diff line
@@ -3314,6 +3314,12 @@ static int framebuffer_check(const struct drm_mode_fb_cmd2 *r) 
			DRM_DEBUG_KMS("bad pitch %u for plane %d\n", r->pitches[i], i);

			return -EINVAL;

		}



		if (r->modifier[i] && !(r->flags & DRM_MODE_FB_MODIFIERS)) {

			DRM_DEBUG_KMS("bad fb modifier %llu for plane %d\n",

				      r->modifier[i], i);

			return -EINVAL;

		}

	}



	return 0;
@@ -3327,7 +3333,7 @@ static struct drm_framebuffer *add_framebuffer_internal(struct drm_device *dev, 
	struct drm_framebuffer *fb;

	int ret;



	if (r->flags & ~DRM_MODE_FB_INTERLACED) {

	if (r->flags & ~(DRM_MODE_FB_INTERLACED | DRM_MODE_FB_MODIFIERS)) {

		DRM_DEBUG_KMS("bad framebuffer flags 0x%08x\n", r->flags);

		return ERR_PTR(-EINVAL);

	}
@@ -3343,6 +3349,12 @@ static struct drm_framebuffer *add_framebuffer_internal(struct drm_device *dev, 
		return ERR_PTR(-EINVAL);

	}



	if (r->flags & DRM_MODE_FB_MODIFIERS &&

	    !dev->mode_config.allow_fb_modifiers) {

		DRM_DEBUG_KMS("driver does not support fb modifiers\n");

		return ERR_PTR(-EINVAL);

	}



	ret = framebuffer_check(r);

	if (ret)

		return ERR_PTR(ret);

drivers/gpu/drm/drm_crtc_helper.c

+1 −0
Original line number Diff line number Diff line
@@ -837,6 +837,7 @@ void drm_helper_mode_fill_fb_struct(struct drm_framebuffer *fb, 
	for (i = 0; i < 4; i++) {

		fb->pitches[i] = mode_cmd->pitches[i];

		fb->offsets[i] = mode_cmd->offsets[i];

		fb->modifier[i] = mode_cmd->modifier[i];

	}

	drm_fb_get_bpp_depth(mode_cmd->pixel_format, &fb->depth,

				    &fb->bits_per_pixel);

drivers/gpu/drm/drm_ioctl.c

+3 −0
Original line number Diff line number Diff line
@@ -321,6 +321,9 @@ static int drm_getcap(struct drm_device *dev, void *data, struct drm_file *file_ 
		else

			req->value = 64;

		break;

	case DRM_CAP_ADDFB2_MODIFIERS:

		req->value = dev->mode_config.allow_fb_modifiers;

		break;

	default:

		return -EINVAL;

	}

include/drm/drm_crtc.h

+4 −0
Original line number Diff line number Diff line
@@ -202,6 +202,7 @@ struct drm_framebuffer { 
	const struct drm_framebuffer_funcs *funcs;

	unsigned int pitches[4];

	unsigned int offsets[4];

	uint64_t modifier[4];

	unsigned int width;

	unsigned int height;

	/* depth can be 15 or 16 */
@@ -1152,6 +1153,9 @@ struct drm_mode_config { 
	/* whether async page flip is supported or not */

	bool async_page_flip;



	/* whether the driver supports fb modifiers */

	bool allow_fb_modifiers;



	/* cursor size */

	uint32_t cursor_width, cursor_height;

};

include/uapi/drm/drm.h

+1 −0
Original line number Diff line number Diff line
@@ -630,6 +630,7 @@ struct drm_gem_open { 
 */

#define DRM_CAP_CURSOR_WIDTH		0x8

#define DRM_CAP_CURSOR_HEIGHT		0x9

#define DRM_CAP_ADDFB2_MODIFIERS	0x10



/** DRM_IOCTL_GET_CAP ioctl argument type */

struct drm_get_cap {