diff mbox

[1/3] drm: Add new DRM_IOCTL_MODE_GETPLANE2

Message ID 20170112005118.19146-2-ben@bwidawsk.net (mailing list archive)
State New, archived
Headers show

Commit Message

Ben Widawsky Jan. 12, 2017, 12:51 a.m. UTC
Originally based off of a patch by Kristian.

This new ioctl extends DRM_IOCTL_MODE_GETPLANE, by returning information
about the modifiers that will work with each format.

It's modified from Kristian's patch in that the modifiers and formats
are setup by the driver, and then a callback is used to create the
format list. The LOC was enough difference that I don't think it made
sense to leave his authorship, but the new UABI was primarily his idea.

Additionally, I hit a couple of drivers which Kristian missed updating.

It also contains a change requested by Daniel to make the modifiers
array a sentinel based structure instead of a sized one. Upon discussion
on IRC, it was determined that having an invalid modifier might make
sense in general as well.

References: https://patchwork.kernel.org/patch/9482393/
Signed-off-by: Ben Widawsky <ben@bwidawsk.net>
---
 drivers/gpu/drm/arc/arcpgu_crtc.c               |  1 +
 drivers/gpu/drm/arm/hdlcd_crtc.c                |  1 +
 drivers/gpu/drm/arm/malidp_planes.c             |  2 +-
 drivers/gpu/drm/armada/armada_crtc.c            |  1 +
 drivers/gpu/drm/armada/armada_overlay.c         |  1 +
 drivers/gpu/drm/atmel-hlcdc/atmel_hlcdc_plane.c |  4 +-
 drivers/gpu/drm/drm_ioctl.c                     |  2 +-
 drivers/gpu/drm/drm_modeset_helper.c            |  1 +
 drivers/gpu/drm/drm_plane.c                     | 67 ++++++++++++++++++++++++-
 drivers/gpu/drm/drm_simple_kms_helper.c         |  3 ++
 drivers/gpu/drm/exynos/exynos_drm_plane.c       |  2 +-
 drivers/gpu/drm/fsl-dcu/fsl_dcu_drm_plane.c     |  2 +-
 drivers/gpu/drm/hisilicon/hibmc/hibmc_drm_de.c  |  1 +
 drivers/gpu/drm/hisilicon/kirin/kirin_drm_ade.c |  2 +-
 drivers/gpu/drm/i915/intel_display.c            |  7 ++-
 drivers/gpu/drm/i915/intel_sprite.c             |  4 +-
 drivers/gpu/drm/imx/ipuv3-plane.c               |  4 +-
 drivers/gpu/drm/mediatek/mtk_drm_plane.c        |  2 +-
 drivers/gpu/drm/meson/meson_plane.c             |  1 +
 drivers/gpu/drm/msm/mdp/mdp4/mdp4_plane.c       |  2 +-
 drivers/gpu/drm/msm/mdp/mdp5/mdp5_plane.c       |  2 +-
 drivers/gpu/drm/mxsfb/mxsfb_drv.c               |  2 +-
 drivers/gpu/drm/nouveau/nv50_display.c          |  5 +-
 drivers/gpu/drm/omapdrm/omap_plane.c            |  3 +-
 drivers/gpu/drm/rcar-du/rcar_du_plane.c         |  4 +-
 drivers/gpu/drm/rcar-du/rcar_du_vsp.c           |  5 +-
 drivers/gpu/drm/rockchip/rockchip_drm_vop.c     |  4 +-
 drivers/gpu/drm/sti/sti_cursor.c                |  1 +
 drivers/gpu/drm/sti/sti_gdp.c                   |  2 +-
 drivers/gpu/drm/sti/sti_hqvdp.c                 |  2 +-
 drivers/gpu/drm/sun4i/sun4i_layer.c             |  1 +
 drivers/gpu/drm/tegra/dc.c                      | 12 ++---
 drivers/gpu/drm/vc4/vc4_plane.c                 |  2 +-
 drivers/gpu/drm/virtio/virtgpu_plane.c          |  2 +-
 drivers/gpu/drm/zte/zx_plane.c                  |  2 +-
 include/drm/drm_plane.h                         | 21 +++++++-
 include/drm/drm_simple_kms_helper.h             |  1 +
 include/uapi/drm/drm.h                          |  1 +
 include/uapi/drm/drm_fourcc.h                   | 11 ++++
 include/uapi/drm/drm_mode.h                     | 27 ++++++++++
 40 files changed, 182 insertions(+), 38 deletions(-)

Comments

Rob Clark Jan. 12, 2017, 1:43 a.m. UTC | #1
On Wed, Jan 11, 2017 at 7:51 PM, Ben Widawsky <ben@bwidawsk.net> wrote:
>
> +struct drm_format_modifier {
> +       /* Bitmask of formats in get_plane format list this info
> +        * applies to. */
> +       uint64_t formats;

re: the uabi, I'd suggest to at least make this 'u32 offset; u32
formats'.. we can keep the existing implementation in this patch and
always set 'offset' to zero, and let the first one to hit more than 32
formats deal with the implementation.  (Maybe a strategically placed
WARN_ON() if you go that route..)

Otherwise I guess it is just a couple years until getplane3 ;-)

BR,
-R

> +
> +       /* This modifier can be used with the format for this plane. */
> +       uint64_t modifier;
> +};
Ville Syrjala Jan. 12, 2017, 9:38 a.m. UTC | #2
On Wed, Jan 11, 2017 at 08:43:16PM -0500, Rob Clark wrote:
> On Wed, Jan 11, 2017 at 7:51 PM, Ben Widawsky <ben@bwidawsk.net> wrote:
> >
> > +struct drm_format_modifier {
> > +       /* Bitmask of formats in get_plane format list this info
> > +        * applies to. */
> > +       uint64_t formats;
> 
> re: the uabi, I'd suggest to at least make this 'u32 offset; u32
> formats'.. we can keep the existing implementation in this patch and
> always set 'offset' to zero, and let the first one to hit more than 32
> formats deal with the implementation.  (Maybe a strategically placed
> WARN_ON() if you go that route..)

Isn't an implicit offset enough? As in first mask for a specific
modifier is for format indexes 0-63, second mask for the same modifier
is for 64-127, and so on.

The bigger issue is the userspace side I think. If we don't add the
userspace side code to handle this case from the get go, it's going to
be hard to actually start doing it from the kernel side.

> 
> Otherwise I guess it is just a couple years until getplane3 ;-)
> 
> BR,
> -R
> 
> > +
> > +       /* This modifier can be used with the format for this plane. */
> > +       uint64_t modifier;
> > +};
> _______________________________________________
> dri-devel mailing list
> dri-devel@lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/dri-devel
Ville Syrjala Jan. 12, 2017, 10:23 a.m. UTC | #3
On Wed, Jan 11, 2017 at 04:51:16PM -0800, Ben Widawsky wrote:
> Originally based off of a patch by Kristian.
> 
> This new ioctl extends DRM_IOCTL_MODE_GETPLANE, by returning information
> about the modifiers that will work with each format.
> 
> It's modified from Kristian's patch in that the modifiers and formats
> are setup by the driver, and then a callback is used to create the
> format list. The LOC was enough difference that I don't think it made
> sense to leave his authorship, but the new UABI was primarily his idea.
> 
> Additionally, I hit a couple of drivers which Kristian missed updating.
> 
> It also contains a change requested by Daniel to make the modifiers
> array a sentinel based structure instead of a sized one. Upon discussion
> on IRC, it was determined that having an invalid modifier might make
> sense in general as well.
> 
> References: https://patchwork.kernel.org/patch/9482393/
> Signed-off-by: Ben Widawsky <ben@bwidawsk.net>
> ---
<snip>
> diff --git a/drivers/gpu/drm/drm_modeset_helper.c b/drivers/gpu/drm/drm_modeset_helper.c
> index 2b33825f2f93..9cb1eede0b4d 100644
> --- a/drivers/gpu/drm/drm_modeset_helper.c
> +++ b/drivers/gpu/drm/drm_modeset_helper.c
> @@ -124,6 +124,7 @@ static struct drm_plane *create_primary_plane(struct drm_device *dev)
>  				       &drm_primary_helper_funcs,
>  				       safe_modeset_formats,
>  				       ARRAY_SIZE(safe_modeset_formats),
> +				       NULL,
>  				       DRM_PLANE_TYPE_PRIMARY, NULL);
>  	if (ret) {
>  		kfree(primary);
> diff --git a/drivers/gpu/drm/drm_plane.c b/drivers/gpu/drm/drm_plane.c
> index 7b7275f0c2df..2d4fad5db8ed 100644
> --- a/drivers/gpu/drm/drm_plane.c
> +++ b/drivers/gpu/drm/drm_plane.c
> @@ -70,6 +70,7 @@ static unsigned int drm_num_planes(struct drm_device *dev)
>   * @funcs: callbacks for the new plane
>   * @formats: array of supported formats (DRM_FORMAT\_\*)
>   * @format_count: number of elements in @formats
> + * @format_modifiers: array of struct drm_format modifiers terminated by INVALID
>   * @type: type of plane (overlay, primary, cursor)
>   * @name: printf style format string for the plane name, or NULL for default name
>   *
> @@ -82,10 +83,12 @@ int drm_universal_plane_init(struct drm_device *dev, struct drm_plane *plane,
>  			     uint32_t possible_crtcs,
>  			     const struct drm_plane_funcs *funcs,
>  			     const uint32_t *formats, unsigned int format_count,
> +			     const uint64_t *format_modifiers,
>  			     enum drm_plane_type type,
>  			     const char *name, ...)
>  {
>  	struct drm_mode_config *config = &dev->mode_config;
> +	unsigned int format_modifier_count = 0;
>  	int ret;
>  
>  	ret = drm_mode_object_get(dev, &plane->base, DRM_MODE_OBJECT_PLANE);
> @@ -105,6 +108,28 @@ int drm_universal_plane_init(struct drm_device *dev, struct drm_plane *plane,
>  		return -ENOMEM;
>  	}
>  
> +	if (format_modifiers) {
> +		const uint64_t *temp_modifiers = format_modifiers;
> +		while (*temp_modifiers++ != DRM_FORMAT_MOD_INVALID)
> +			format_modifier_count++;
> +	}
> +
> +	if (format_modifier_count)
> +		DRM_DEBUG_KMS("%d format modifiers added to list\n",
> +			      format_modifier_count);

nit: Not sure this is printk worthy.

> +
> +	plane->modifier_count = format_modifier_count;
> +	plane->modifiers = kmalloc_array(format_modifier_count,
> +					 sizeof(format_modifiers[0]),
> +					 GFP_KERNEL);
> +
> +	if (format_modifier_count && !plane->modifiers) {
> +		DRM_DEBUG_KMS("out of memory when allocating plane\n");
> +		kfree(plane->format_types);
> +		drm_mode_object_unregister(dev, &plane->base);
> +		return -ENOMEM;
> +	}
> +
>  	if (name) {
>  		va_list ap;
>  
> @@ -117,12 +142,15 @@ int drm_universal_plane_init(struct drm_device *dev, struct drm_plane *plane,
>  	}
>  	if (!plane->name) {
>  		kfree(plane->format_types);
> +		kfree(plane->modifiers);
>  		drm_mode_object_unregister(dev, &plane->base);
>  		return -ENOMEM;
>  	}
>  
>  	memcpy(plane->format_types, formats, format_count * sizeof(uint32_t));
>  	plane->format_count = format_count;
> +	memcpy(plane->modifiers, format_modifiers,
> +	       format_modifier_count * sizeof(format_modifiers[0]));

Looks all right since we do the same for formats anyway. But it did
occur to me (twice at least) that a kmemdup_array() might a nice thing
to have for things like this. But that's a separate topic.

>  	plane->possible_crtcs = possible_crtcs;
>  	plane->type = type;
>  
> @@ -205,7 +233,8 @@ int drm_plane_init(struct drm_device *dev, struct drm_plane *plane,
>  
>  	type = is_primary ? DRM_PLANE_TYPE_PRIMARY : DRM_PLANE_TYPE_OVERLAY;
>  	return drm_universal_plane_init(dev, plane, possible_crtcs, funcs,
> -					formats, format_count, type, NULL);
> +					formats, format_count,
> +					NULL, type, NULL);
>  }
>  EXPORT_SYMBOL(drm_plane_init);
>  
> @@ -224,6 +253,7 @@ void drm_plane_cleanup(struct drm_plane *plane)
>  	drm_modeset_lock_fini(&plane->mutex);
>  
>  	kfree(plane->format_types);
> +	kfree(plane->modifiers);
>  	drm_mode_object_unregister(dev, &plane->base);
>  
>  	BUG_ON(list_empty(&plane->head));
> @@ -380,12 +410,15 @@ int drm_mode_getplane_res(struct drm_device *dev, void *data,
>  int drm_mode_getplane(struct drm_device *dev, void *data,
>  		      struct drm_file *file_priv)
>  {
> -	struct drm_mode_get_plane *plane_resp = data;
> +	struct drm_mode_get_plane2 *plane_resp = data;
>  	struct drm_plane *plane;
>  	uint32_t __user *format_ptr;
> +	struct drm_format_modifier __user *modifier_ptr;
>  
>  	if (!drm_core_check_feature(dev, DRIVER_MODESET))
>  		return -EINVAL;
> +	if (plane_resp->flags)
> +		return -EINVAL;
>  
>  	plane = drm_plane_find(dev, plane_resp->plane_id);
>  	if (!plane)
> @@ -426,6 +459,36 @@ int drm_mode_getplane(struct drm_device *dev, void *data,
>  	}
>  	plane_resp->count_format_types = plane->format_count;
>  
> +	if (plane->modifier_count &&
> +	    plane_resp->count_format_modifiers >= plane->modifier_count) {
> +		struct drm_format_modifier mod;
> +		int i;
> +
> +		modifier_ptr = (struct drm_format_modifier __user *)
> +			(unsigned long)plane_resp->format_modifier_ptr;

Didn't we have some something_user_ptr() thing? Hmm, I guess it's in i915 only.

> +
> +		/* Build the mask for each modifier */
> +		for (i = 0; i < plane->modifier_count; i++) {
> +			int j;
> +			mod.modifier = plane->modifiers[i];
> +			for (j = 0; j < plane->format_count; j++) {

If we don't want to try and deal with more than 64 formats, I think we
need to make drm_universal_plane_init() WARN+bail if the driver
passes in more than that.

> +				if (plane->funcs->format_mod_supported &&
> +				    plane->funcs->format_mod_supported(plane,
> +								       plane->format_types[j],
> +								       plane->modifiers[i])) {
> +					mod.formats |= 1 << j;
> +				}
> +			}
> +
> +			if (copy_to_user(modifier_ptr, &mod, sizeof(mod)))
> +				return -EFAULT;
> +
> +			modifier_ptr++;
> +		}
> +	}
> +
> +	plane_resp->count_format_modifiers = plane->modifier_count;
> +
>  	return 0;
>  }
>  
> diff --git a/drivers/gpu/drm/drm_simple_kms_helper.c b/drivers/gpu/drm/drm_simple_kms_helper.c
> index 35c5d99296b9..0406e71b38e8 100644
> --- a/drivers/gpu/drm/drm_simple_kms_helper.c
> +++ b/drivers/gpu/drm/drm_simple_kms_helper.c
> @@ -193,6 +193,7 @@ EXPORT_SYMBOL(drm_simple_display_pipe_attach_bridge);
>   * @funcs: callbacks for the display pipe (optional)
>   * @formats: array of supported formats (DRM_FORMAT\_\*)
>   * @format_count: number of elements in @formats
> + * @modifiers: array of formats modifiers

@format_modifiers

>   * @connector: connector to attach and register (optional)
>   *
>   * Sets up a display pipeline which consist of a really simple
> @@ -213,6 +214,7 @@ int drm_simple_display_pipe_init(struct drm_device *dev,
>  			struct drm_simple_display_pipe *pipe,
>  			const struct drm_simple_display_pipe_funcs *funcs,
>  			const uint32_t *formats, unsigned int format_count,
> +			const uint64_t *format_modifiers,
>  			struct drm_connector *connector)
>  {
>  	struct drm_encoder *encoder = &pipe->encoder;
> @@ -227,6 +229,7 @@ int drm_simple_display_pipe_init(struct drm_device *dev,
>  	ret = drm_universal_plane_init(dev, plane, 0,
>  				       &drm_simple_kms_plane_funcs,
>  				       formats, format_count,
> +				       format_modifiers,
>  				       DRM_PLANE_TYPE_PRIMARY, NULL);
>  	if (ret)
>  		return ret;
<snip>
> diff --git a/include/uapi/drm/drm_mode.h b/include/uapi/drm/drm_mode.h
> index ce7efe2e8a5e..cea3de3aa301 100644
> --- a/include/uapi/drm/drm_mode.h
> +++ b/include/uapi/drm/drm_mode.h
> @@ -209,6 +209,33 @@ struct drm_mode_get_plane {
>  	__u64 format_type_ptr;
>  };
>  
> +struct drm_format_modifier {
> +	/* Bitmask of formats in get_plane format list this info
> +	 * applies to. */
> +	uint64_t formats;
> +
> +	/* This modifier can be used with the format for this plane. */
> +	uint64_t modifier;
> +};
> +
> +struct drm_mode_get_plane2 {
> +	__u32 plane_id;
> +
> +	__u32 crtc_id;
> +	__u32 fb_id;
> +
> +	__u32 possible_crtcs;
> +	__u32 gamma_size;
> +
> +	__u32 count_format_types;
> +	__u64 format_type_ptr;
> +
> +	/* New in v2 */
> +	__u32 count_format_modifiers;
> +	__u32 flags;

ocd induced idea: Maybe put flags before the count?

> +	__u64 format_modifier_ptr;
> +};
> +
>  struct drm_mode_get_plane_res {
>  	__u64 plane_id_ptr;
>  	__u32 count_planes;
Rob Clark Jan. 12, 2017, 2:56 p.m. UTC | #4
On Thu, Jan 12, 2017 at 4:38 AM, Ville Syrjälä
<ville.syrjala@linux.intel.com> wrote:
> On Wed, Jan 11, 2017 at 08:43:16PM -0500, Rob Clark wrote:
>> On Wed, Jan 11, 2017 at 7:51 PM, Ben Widawsky <ben@bwidawsk.net> wrote:
>> >
>> > +struct drm_format_modifier {
>> > +       /* Bitmask of formats in get_plane format list this info
>> > +        * applies to. */
>> > +       uint64_t formats;
>>
>> re: the uabi, I'd suggest to at least make this 'u32 offset; u32
>> formats'.. we can keep the existing implementation in this patch and
>> always set 'offset' to zero, and let the first one to hit more than 32
>> formats deal with the implementation.  (Maybe a strategically placed
>> WARN_ON() if you go that route..)
>
> Isn't an implicit offset enough? As in first mask for a specific
> modifier is for format indexes 0-63, second mask for the same modifier
> is for 64-127, and so on.

hmm, hadn't thought of that approach.  Definitely if we go w/ implicit
then we want to have userspace support from the get-go.  For explicit,
I guess userspace could complain and ignore if it saw a non-zero
offset similar to what we do w/ pad and unknown flags in the other
direction?

Not sure if that would fly or not..  I guess it is not a *critical*
fail, it just means userspace won't realize that some modifiers are
supported on some formats.. otoh the implicit approach could confuse a
userspace that didn't realize the offset into thinking modifiers
*were* supported on formats where they are not.. that seems like a
bigger problem.

BR,
-R

> The bigger issue is the userspace side I think. If we don't add the
> userspace side code to handle this case from the get go, it's going to
> be hard to actually start doing it from the kernel side.
>
>>
>> Otherwise I guess it is just a couple years until getplane3 ;-)
>>
>> BR,
>> -R
>>
>> > +
>> > +       /* This modifier can be used with the format for this plane. */
>> > +       uint64_t modifier;
>> > +};
>> _______________________________________________
>> dri-devel mailing list
>> dri-devel@lists.freedesktop.org
>> https://lists.freedesktop.org/mailman/listinfo/dri-devel
>
> --
> Ville Syrjälä
> Intel OTC
Daniel Stone Jan. 12, 2017, 5:04 p.m. UTC | #5
Hi,

On 12 January 2017 at 14:56, Rob Clark <robdclark@gmail.com> wrote:
> On Thu, Jan 12, 2017 at 4:38 AM, Ville Syrjälä
> <ville.syrjala@linux.intel.com> wrote:
>> Isn't an implicit offset enough? As in first mask for a specific
>> modifier is for format indexes 0-63, second mask for the same modifier
>> is for 64-127, and so on.
>
> hmm, hadn't thought of that approach.  Definitely if we go w/ implicit
> then we want to have userspace support from the get-go.  For explicit,
> I guess userspace could complain and ignore if it saw a non-zero
> offset similar to what we do w/ pad and unknown flags in the other
> direction?

Implicit is clever but horrible. AFAICT, the only way to do it
properly would be to have a nested forwards loop walk when you first
hit a modifier, searching for further occurrences of that modifier to
collect the complete set of formats that modifier applies to.
Depending on what you did with the structures, you'd either have to
destroy the drm_format_modifiers in the GetPlane return so further
instances of your outer loop didn't hit them, or have a _second_
nested loop walk into wherever you copied the formats/modifiers,
searching for anything with that.

Too clever by half, and everyone will get it wrong. Just add an explicit offset.

Cheers,
Daniel
Ville Syrjala Jan. 12, 2017, 5:45 p.m. UTC | #6
On Thu, Jan 12, 2017 at 05:04:46PM +0000, Daniel Stone wrote:
> Hi,
> 
> On 12 January 2017 at 14:56, Rob Clark <robdclark@gmail.com> wrote:
> > On Thu, Jan 12, 2017 at 4:38 AM, Ville Syrjälä
> > <ville.syrjala@linux.intel.com> wrote:
> >> Isn't an implicit offset enough? As in first mask for a specific
> >> modifier is for format indexes 0-63, second mask for the same modifier
> >> is for 64-127, and so on.
> >
> > hmm, hadn't thought of that approach.  Definitely if we go w/ implicit
> > then we want to have userspace support from the get-go.  For explicit,
> > I guess userspace could complain and ignore if it saw a non-zero
> > offset similar to what we do w/ pad and unknown flags in the other
> > direction?
> 
> Implicit is clever but horrible. AFAICT, the only way to do it
> properly would be to have a nested forwards loop walk when you first
> hit a modifier, searching for further occurrences of that modifier to
> collect the complete set of formats that modifier applies to.

Not sure for what that is the "only way". In fact I can't right now
think of any operation that would require an extra loop necessarily.
For some things you might just want to look for a specific
format+modifier combo, for that it doesn't matter how many blocks there
are. And if you want to transform the reply into some less convoluted
form, well then you'd just need some modifier+dynamic format list thing,
or if you want to keep to bitmasks you'd either need a bitmask that can
grow when running out of bits or just make it big enough to handle a
sufficiently large worst case number of bits.

Dunno, maybe I just lack imagination. Then again, I'm not even sure if
we're talking about userspace of kernel code here, which might explain
my general confusion :)
Daniel Stone Jan. 12, 2017, 5:50 p.m. UTC | #7
Hi,

On 12 January 2017 at 17:45, Ville Syrjälä
<ville.syrjala@linux.intel.com> wrote:
> On Thu, Jan 12, 2017 at 05:04:46PM +0000, Daniel Stone wrote:
>> Implicit is clever but horrible. AFAICT, the only way to do it
>> properly would be to have a nested forwards loop walk when you first
>> hit a modifier, searching for further occurrences of that modifier to
>> collect the complete set of formats that modifier applies to.
>
> Not sure for what that is the "only way". In fact I can't right now
> think of any operation that would require an extra loop necessarily.
> For some things you might just want to look for a specific
> format+modifier combo, for that it doesn't matter how many blocks there
> are.

Right, that just needs a local variable to act as a counter.

> And if you want to transform the reply into some less convoluted
> form, well then you'd just need some modifier+dynamic format list thing,
> or if you want to keep to bitmasks you'd either need a bitmask that can
> grow when running out of bits or just make it big enough to handle a
> sufficiently large worst case number of bits.
>
> Dunno, maybe I just lack imagination. Then again, I'm not even sure if
> we're talking about userspace of kernel code here, which might explain
> my general confusion :)

I'm talking about userspace, where I want to have:
struct drm_plane {
    struct {
        uint32_t format;
        uint64_t modifiers[];
    } formats[];
}

Rather than keeping the original format around and doing the lookup every time.

Cheers,
Daniel
Ville Syrjala Jan. 12, 2017, 6:11 p.m. UTC | #8
On Thu, Jan 12, 2017 at 05:50:15PM +0000, Daniel Stone wrote:
> Hi,
> 
> On 12 January 2017 at 17:45, Ville Syrjälä
> <ville.syrjala@linux.intel.com> wrote:
> > On Thu, Jan 12, 2017 at 05:04:46PM +0000, Daniel Stone wrote:
> >> Implicit is clever but horrible. AFAICT, the only way to do it
> >> properly would be to have a nested forwards loop walk when you first
> >> hit a modifier, searching for further occurrences of that modifier to
> >> collect the complete set of formats that modifier applies to.
> >
> > Not sure for what that is the "only way". In fact I can't right now
> > think of any operation that would require an extra loop necessarily.
> > For some things you might just want to look for a specific
> > format+modifier combo, for that it doesn't matter how many blocks there
> > are.
> 
> Right, that just needs a local variable to act as a counter.
> 
> > And if you want to transform the reply into some less convoluted
> > form, well then you'd just need some modifier+dynamic format list thing,
> > or if you want to keep to bitmasks you'd either need a bitmask that can
> > grow when running out of bits or just make it big enough to handle a
> > sufficiently large worst case number of bits.
> >
> > Dunno, maybe I just lack imagination. Then again, I'm not even sure if
> > we're talking about userspace of kernel code here, which might explain
> > my general confusion :)
> 
> I'm talking about userspace, where I want to have:
> struct drm_plane {
>     struct {
>         uint32_t format;
>         uint64_t modifiers[];
>     } formats[];
> }

Flipping formats[] vs. modifiers[] here would seem like it should make
this easier with the proposed kernel API. And if the kernel will also
uarantee that multiple instances of the same modifier must be returned
contiguously, then it should be even easier.

Oh and flipping formats[] and modifiers[] should also save a quite a
bit of space since each format takes twice as much space as each
modifier. But I suppose that might come at a runtime cost if you have
to look for a specific format in each modifier's format list instead
of having to look at just the modifier list of a specific format. So
I suppose not flipping might be better after all, which I guess would
complicate populating the infromation somewhat.

Anyways, that's all a bit unrelated to the matter at hand, so I'll stop
now and just state that I don't mind having an explicit offset if
people really want it.
Daniel Stone Jan. 12, 2017, 7:27 p.m. UTC | #9
Hi,

On 12 January 2017 at 18:11, Ville Syrjälä
<ville.syrjala@linux.intel.com> wrote:
> On Thu, Jan 12, 2017 at 05:50:15PM +0000, Daniel Stone wrote:
>> struct drm_plane {
>>     struct {
>>         uint32_t format;
>>         uint64_t modifiers[];
>>     } formats[];
>> }
>
> Flipping formats[] vs. modifiers[] here would seem like it should make
> this easier with the proposed kernel API. And if the kernel will also
> uarantee that multiple instances of the same modifier must be returned
> contiguously, then it should be even easier.
>
> Oh and flipping formats[] and modifiers[] should also save a quite a
> bit of space since each format takes twice as much space as each
> modifier. But I suppose that might come at a runtime cost if you have
> to look for a specific format in each modifier's format list instead
> of having to look at just the modifier list of a specific format. So
> I suppose not flipping might be better after all, which I guess would
> complicate populating the infromation somewhat.
>
> Anyways, that's all a bit unrelated to the matter at hand, so I'll stop
> now and just state that I don't mind having an explicit offset if
> people really want it.

It would make sense, but then gbm_surface_create_with_modifiers takes
a fixed pixel format and a list of acceptable modifiers (which to me
seems like the right way around as an API), so whenever I was creating
a surface, I'd have to walk through and create a new list, flipped
back the other way.

Cheers,
Daniel
Ville Syrjala Jan. 13, 2017, 9:37 a.m. UTC | #10
On Thu, Jan 12, 2017 at 07:27:03PM +0000, Daniel Stone wrote:
> Hi,
> 
> On 12 January 2017 at 18:11, Ville Syrjälä
> <ville.syrjala@linux.intel.com> wrote:
> > On Thu, Jan 12, 2017 at 05:50:15PM +0000, Daniel Stone wrote:
> >> struct drm_plane {
> >>     struct {
> >>         uint32_t format;
> >>         uint64_t modifiers[];
> >>     } formats[];
> >> }
> >
> > Flipping formats[] vs. modifiers[] here would seem like it should make
> > this easier with the proposed kernel API. And if the kernel will also
> > uarantee that multiple instances of the same modifier must be returned
> > contiguously, then it should be even easier.
> >
> > Oh and flipping formats[] and modifiers[] should also save a quite a
> > bit of space since each format takes twice as much space as each
> > modifier. But I suppose that might come at a runtime cost if you have
> > to look for a specific format in each modifier's format list instead
> > of having to look at just the modifier list of a specific format. So
> > I suppose not flipping might be better after all, which I guess would
> > complicate populating the infromation somewhat.
> >
> > Anyways, that's all a bit unrelated to the matter at hand, so I'll stop
> > now and just state that I don't mind having an explicit offset if
> > people really want it.
> 
> It would make sense, but then gbm_surface_create_with_modifiers takes
> a fixed pixel format and a list of acceptable modifiers (which to me
> seems like the right way around as an API), so whenever I was creating
> a surface, I'd have to walk through and create a new list, flipped
> back the other way.

Yeah, for that your original order makes more sense, even if it
potentially uses more memory.
Daniel Stone Jan. 13, 2017, 9:45 a.m. UTC | #11
Hey,

On 13 January 2017 at 09:37, Ville Syrjälä
<ville.syrjala@linux.intel.com> wrote:
> On Thu, Jan 12, 2017 at 07:27:03PM +0000, Daniel Stone wrote:
>> It would make sense, but then gbm_surface_create_with_modifiers takes
>> a fixed pixel format and a list of acceptable modifiers (which to me
>> seems like the right way around as an API), so whenever I was creating
>> a surface, I'd have to walk through and create a new list, flipped
>> back the other way.
>
> Yeah, for that your original order makes more sense, even if it
> potentially uses more memory.

Given the size of framebuffers, I'm really not concerned by the
in-memory size of a format list. :)

Cheers,
Daniel
Andy Shevchenko Nov. 24, 2023, 3:08 p.m. UTC | #12
On Thu, Jan 12, 2017 at 12:23:16PM +0200, Ville Syrjälä wrote:
> On Wed, Jan 11, 2017 at 04:51:16PM -0800, Ben Widawsky wrote:

...

> > +	memcpy(plane->modifiers, format_modifiers,
> > +	       format_modifier_count * sizeof(format_modifiers[0]));
> 
> Looks all right since we do the same for formats anyway. But it did
> occur to me (twice at least) that a kmemdup_array() might a nice thing
> to have for things like this. But that's a separate topic.

JFYI:
https://lore.kernel.org/all/20231017052322.2636-2-kkartik@nvidia.com/
diff mbox

Patch

diff --git a/drivers/gpu/drm/arc/arcpgu_crtc.c b/drivers/gpu/drm/arc/arcpgu_crtc.c
index ad9a95916f1f..cd8a24c7c67d 100644
--- a/drivers/gpu/drm/arc/arcpgu_crtc.c
+++ b/drivers/gpu/drm/arc/arcpgu_crtc.c
@@ -218,6 +218,7 @@  static struct drm_plane *arc_pgu_plane_init(struct drm_device *drm)
 
 	ret = drm_universal_plane_init(drm, plane, 0xff, &arc_pgu_plane_funcs,
 				       formats, ARRAY_SIZE(formats),
+				       NULL,
 				       DRM_PLANE_TYPE_PRIMARY, NULL);
 	if (ret)
 		return ERR_PTR(ret);
diff --git a/drivers/gpu/drm/arm/hdlcd_crtc.c b/drivers/gpu/drm/arm/hdlcd_crtc.c
index 20ebfb4fbdfa..89fded880807 100644
--- a/drivers/gpu/drm/arm/hdlcd_crtc.c
+++ b/drivers/gpu/drm/arm/hdlcd_crtc.c
@@ -283,6 +283,7 @@  static struct drm_plane *hdlcd_plane_init(struct drm_device *drm)
 
 	ret = drm_universal_plane_init(drm, plane, 0xff, &hdlcd_plane_funcs,
 				       formats, ARRAY_SIZE(formats),
+				       NULL,
 				       DRM_PLANE_TYPE_PRIMARY, NULL);
 	if (ret) {
 		devm_kfree(drm->dev, plane);
diff --git a/drivers/gpu/drm/arm/malidp_planes.c b/drivers/gpu/drm/arm/malidp_planes.c
index eff2fe47e26a..94dbcbc9ad8f 100644
--- a/drivers/gpu/drm/arm/malidp_planes.c
+++ b/drivers/gpu/drm/arm/malidp_planes.c
@@ -283,7 +283,7 @@  int malidp_de_planes_init(struct drm_device *drm)
 					DRM_PLANE_TYPE_OVERLAY;
 		ret = drm_universal_plane_init(drm, &plane->base, crtcs,
 					       &malidp_de_plane_funcs, formats,
-					       n, plane_type, NULL);
+					       n, NULL, plane_type, NULL);
 		if (ret < 0)
 			goto cleanup;
 
diff --git a/drivers/gpu/drm/armada/armada_crtc.c b/drivers/gpu/drm/armada/armada_crtc.c
index e62ee4498ce4..39330145ba2a 100644
--- a/drivers/gpu/drm/armada/armada_crtc.c
+++ b/drivers/gpu/drm/armada/armada_crtc.c
@@ -1250,6 +1250,7 @@  static int armada_drm_crtc_create(struct drm_device *drm, struct device *dev,
 				       &armada_primary_plane_funcs,
 				       armada_primary_formats,
 				       ARRAY_SIZE(armada_primary_formats),
+				       NULL,
 				       DRM_PLANE_TYPE_PRIMARY, NULL);
 	if (ret) {
 		kfree(primary);
diff --git a/drivers/gpu/drm/armada/armada_overlay.c b/drivers/gpu/drm/armada/armada_overlay.c
index 34cb73d0db77..6a1ca8aa8e16 100644
--- a/drivers/gpu/drm/armada/armada_overlay.c
+++ b/drivers/gpu/drm/armada/armada_overlay.c
@@ -458,6 +458,7 @@  int armada_overlay_plane_create(struct drm_device *dev, unsigned long crtcs)
 				       &armada_ovl_plane_funcs,
 				       armada_ovl_formats,
 				       ARRAY_SIZE(armada_ovl_formats),
+				       NULL,
 				       DRM_PLANE_TYPE_OVERLAY, NULL);
 	if (ret) {
 		kfree(dplane);
diff --git a/drivers/gpu/drm/atmel-hlcdc/atmel_hlcdc_plane.c b/drivers/gpu/drm/atmel-hlcdc/atmel_hlcdc_plane.c
index bd2791c4b002..f12cec0bc58c 100644
--- a/drivers/gpu/drm/atmel-hlcdc/atmel_hlcdc_plane.c
+++ b/drivers/gpu/drm/atmel-hlcdc/atmel_hlcdc_plane.c
@@ -1038,7 +1038,9 @@  atmel_hlcdc_plane_create(struct drm_device *dev,
 	ret = drm_universal_plane_init(dev, &plane->base, 0,
 				       &layer_plane_funcs,
 				       desc->formats->formats,
-				       desc->formats->nformats, type, NULL);
+				       desc->formats->nformats,
+				       NULL,
+				       type, NULL);
 	if (ret)
 		return ERR_PTR(ret);
 
diff --git a/drivers/gpu/drm/drm_ioctl.c b/drivers/gpu/drm/drm_ioctl.c
index a7c61c23685a..c820d99d9231 100644
--- a/drivers/gpu/drm/drm_ioctl.c
+++ b/drivers/gpu/drm/drm_ioctl.c
@@ -614,7 +614,7 @@  static const struct drm_ioctl_desc drm_ioctls[] = {
 	DRM_IOCTL_DEF(DRM_IOCTL_MODE_GETPLANERESOURCES, drm_mode_getplane_res, DRM_CONTROL_ALLOW|DRM_UNLOCKED),
 	DRM_IOCTL_DEF(DRM_IOCTL_MODE_GETCRTC, drm_mode_getcrtc, DRM_CONTROL_ALLOW|DRM_UNLOCKED),
 	DRM_IOCTL_DEF(DRM_IOCTL_MODE_SETCRTC, drm_mode_setcrtc, DRM_MASTER|DRM_CONTROL_ALLOW|DRM_UNLOCKED),
-	DRM_IOCTL_DEF(DRM_IOCTL_MODE_GETPLANE, drm_mode_getplane, DRM_CONTROL_ALLOW|DRM_UNLOCKED),
+	DRM_IOCTL_DEF(DRM_IOCTL_MODE_GETPLANE2, drm_mode_getplane, DRM_CONTROL_ALLOW|DRM_UNLOCKED),
 	DRM_IOCTL_DEF(DRM_IOCTL_MODE_SETPLANE, drm_mode_setplane, DRM_MASTER|DRM_CONTROL_ALLOW|DRM_UNLOCKED),
 	DRM_IOCTL_DEF(DRM_IOCTL_MODE_CURSOR, drm_mode_cursor_ioctl, DRM_MASTER|DRM_CONTROL_ALLOW|DRM_UNLOCKED),
 	DRM_IOCTL_DEF(DRM_IOCTL_MODE_GETGAMMA, drm_mode_gamma_get_ioctl, DRM_UNLOCKED),
diff --git a/drivers/gpu/drm/drm_modeset_helper.c b/drivers/gpu/drm/drm_modeset_helper.c
index 2b33825f2f93..9cb1eede0b4d 100644
--- a/drivers/gpu/drm/drm_modeset_helper.c
+++ b/drivers/gpu/drm/drm_modeset_helper.c
@@ -124,6 +124,7 @@  static struct drm_plane *create_primary_plane(struct drm_device *dev)
 				       &drm_primary_helper_funcs,
 				       safe_modeset_formats,
 				       ARRAY_SIZE(safe_modeset_formats),
+				       NULL,
 				       DRM_PLANE_TYPE_PRIMARY, NULL);
 	if (ret) {
 		kfree(primary);
diff --git a/drivers/gpu/drm/drm_plane.c b/drivers/gpu/drm/drm_plane.c
index 7b7275f0c2df..2d4fad5db8ed 100644
--- a/drivers/gpu/drm/drm_plane.c
+++ b/drivers/gpu/drm/drm_plane.c
@@ -70,6 +70,7 @@  static unsigned int drm_num_planes(struct drm_device *dev)
  * @funcs: callbacks for the new plane
  * @formats: array of supported formats (DRM_FORMAT\_\*)
  * @format_count: number of elements in @formats
+ * @format_modifiers: array of struct drm_format modifiers terminated by INVALID
  * @type: type of plane (overlay, primary, cursor)
  * @name: printf style format string for the plane name, or NULL for default name
  *
@@ -82,10 +83,12 @@  int drm_universal_plane_init(struct drm_device *dev, struct drm_plane *plane,
 			     uint32_t possible_crtcs,
 			     const struct drm_plane_funcs *funcs,
 			     const uint32_t *formats, unsigned int format_count,
+			     const uint64_t *format_modifiers,
 			     enum drm_plane_type type,
 			     const char *name, ...)
 {
 	struct drm_mode_config *config = &dev->mode_config;
+	unsigned int format_modifier_count = 0;
 	int ret;
 
 	ret = drm_mode_object_get(dev, &plane->base, DRM_MODE_OBJECT_PLANE);
@@ -105,6 +108,28 @@  int drm_universal_plane_init(struct drm_device *dev, struct drm_plane *plane,
 		return -ENOMEM;
 	}
 
+	if (format_modifiers) {
+		const uint64_t *temp_modifiers = format_modifiers;
+		while (*temp_modifiers++ != DRM_FORMAT_MOD_INVALID)
+			format_modifier_count++;
+	}
+
+	if (format_modifier_count)
+		DRM_DEBUG_KMS("%d format modifiers added to list\n",
+			      format_modifier_count);
+
+	plane->modifier_count = format_modifier_count;
+	plane->modifiers = kmalloc_array(format_modifier_count,
+					 sizeof(format_modifiers[0]),
+					 GFP_KERNEL);
+
+	if (format_modifier_count && !plane->modifiers) {
+		DRM_DEBUG_KMS("out of memory when allocating plane\n");
+		kfree(plane->format_types);
+		drm_mode_object_unregister(dev, &plane->base);
+		return -ENOMEM;
+	}
+
 	if (name) {
 		va_list ap;
 
@@ -117,12 +142,15 @@  int drm_universal_plane_init(struct drm_device *dev, struct drm_plane *plane,
 	}
 	if (!plane->name) {
 		kfree(plane->format_types);
+		kfree(plane->modifiers);
 		drm_mode_object_unregister(dev, &plane->base);
 		return -ENOMEM;
 	}
 
 	memcpy(plane->format_types, formats, format_count * sizeof(uint32_t));
 	plane->format_count = format_count;
+	memcpy(plane->modifiers, format_modifiers,
+	       format_modifier_count * sizeof(format_modifiers[0]));
 	plane->possible_crtcs = possible_crtcs;
 	plane->type = type;
 
@@ -205,7 +233,8 @@  int drm_plane_init(struct drm_device *dev, struct drm_plane *plane,
 
 	type = is_primary ? DRM_PLANE_TYPE_PRIMARY : DRM_PLANE_TYPE_OVERLAY;
 	return drm_universal_plane_init(dev, plane, possible_crtcs, funcs,
-					formats, format_count, type, NULL);
+					formats, format_count,
+					NULL, type, NULL);
 }
 EXPORT_SYMBOL(drm_plane_init);
 
@@ -224,6 +253,7 @@  void drm_plane_cleanup(struct drm_plane *plane)
 	drm_modeset_lock_fini(&plane->mutex);
 
 	kfree(plane->format_types);
+	kfree(plane->modifiers);
 	drm_mode_object_unregister(dev, &plane->base);
 
 	BUG_ON(list_empty(&plane->head));
@@ -380,12 +410,15 @@  int drm_mode_getplane_res(struct drm_device *dev, void *data,
 int drm_mode_getplane(struct drm_device *dev, void *data,
 		      struct drm_file *file_priv)
 {
-	struct drm_mode_get_plane *plane_resp = data;
+	struct drm_mode_get_plane2 *plane_resp = data;
 	struct drm_plane *plane;
 	uint32_t __user *format_ptr;
+	struct drm_format_modifier __user *modifier_ptr;
 
 	if (!drm_core_check_feature(dev, DRIVER_MODESET))
 		return -EINVAL;
+	if (plane_resp->flags)
+		return -EINVAL;
 
 	plane = drm_plane_find(dev, plane_resp->plane_id);
 	if (!plane)
@@ -426,6 +459,36 @@  int drm_mode_getplane(struct drm_device *dev, void *data,
 	}
 	plane_resp->count_format_types = plane->format_count;
 
+	if (plane->modifier_count &&
+	    plane_resp->count_format_modifiers >= plane->modifier_count) {
+		struct drm_format_modifier mod;
+		int i;
+
+		modifier_ptr = (struct drm_format_modifier __user *)
+			(unsigned long)plane_resp->format_modifier_ptr;
+
+		/* Build the mask for each modifier */
+		for (i = 0; i < plane->modifier_count; i++) {
+			int j;
+			mod.modifier = plane->modifiers[i];
+			for (j = 0; j < plane->format_count; j++) {
+				if (plane->funcs->format_mod_supported &&
+				    plane->funcs->format_mod_supported(plane,
+								       plane->format_types[j],
+								       plane->modifiers[i])) {
+					mod.formats |= 1 << j;
+				}
+			}
+
+			if (copy_to_user(modifier_ptr, &mod, sizeof(mod)))
+				return -EFAULT;
+
+			modifier_ptr++;
+		}
+	}
+
+	plane_resp->count_format_modifiers = plane->modifier_count;
+
 	return 0;
 }
 
diff --git a/drivers/gpu/drm/drm_simple_kms_helper.c b/drivers/gpu/drm/drm_simple_kms_helper.c
index 35c5d99296b9..0406e71b38e8 100644
--- a/drivers/gpu/drm/drm_simple_kms_helper.c
+++ b/drivers/gpu/drm/drm_simple_kms_helper.c
@@ -193,6 +193,7 @@  EXPORT_SYMBOL(drm_simple_display_pipe_attach_bridge);
  * @funcs: callbacks for the display pipe (optional)
  * @formats: array of supported formats (DRM_FORMAT\_\*)
  * @format_count: number of elements in @formats
+ * @modifiers: array of formats modifiers
  * @connector: connector to attach and register (optional)
  *
  * Sets up a display pipeline which consist of a really simple
@@ -213,6 +214,7 @@  int drm_simple_display_pipe_init(struct drm_device *dev,
 			struct drm_simple_display_pipe *pipe,
 			const struct drm_simple_display_pipe_funcs *funcs,
 			const uint32_t *formats, unsigned int format_count,
+			const uint64_t *format_modifiers,
 			struct drm_connector *connector)
 {
 	struct drm_encoder *encoder = &pipe->encoder;
@@ -227,6 +229,7 @@  int drm_simple_display_pipe_init(struct drm_device *dev,
 	ret = drm_universal_plane_init(dev, plane, 0,
 				       &drm_simple_kms_plane_funcs,
 				       formats, format_count,
+				       format_modifiers,
 				       DRM_PLANE_TYPE_PRIMARY, NULL);
 	if (ret)
 		return ret;
diff --git a/drivers/gpu/drm/exynos/exynos_drm_plane.c b/drivers/gpu/drm/exynos/exynos_drm_plane.c
index c2f17f30afab..75d4928dd196 100644
--- a/drivers/gpu/drm/exynos/exynos_drm_plane.c
+++ b/drivers/gpu/drm/exynos/exynos_drm_plane.c
@@ -284,7 +284,7 @@  int exynos_plane_init(struct drm_device *dev,
 				       &exynos_plane_funcs,
 				       config->pixel_formats,
 				       config->num_pixel_formats,
-				       config->type, NULL);
+				       NULL, config->type, NULL);
 	if (err) {
 		DRM_ERROR("failed to initialize plane\n");
 		return err;
diff --git a/drivers/gpu/drm/fsl-dcu/fsl_dcu_drm_plane.c b/drivers/gpu/drm/fsl-dcu/fsl_dcu_drm_plane.c
index 0a20723aa6e1..9554b245746e 100644
--- a/drivers/gpu/drm/fsl-dcu/fsl_dcu_drm_plane.c
+++ b/drivers/gpu/drm/fsl-dcu/fsl_dcu_drm_plane.c
@@ -224,7 +224,7 @@  struct drm_plane *fsl_dcu_drm_primary_create_plane(struct drm_device *dev)
 				       &fsl_dcu_drm_plane_funcs,
 				       fsl_dcu_drm_plane_formats,
 				       ARRAY_SIZE(fsl_dcu_drm_plane_formats),
-				       DRM_PLANE_TYPE_PRIMARY, NULL);
+				       NULL, DRM_PLANE_TYPE_PRIMARY, NULL);
 	if (ret) {
 		kfree(primary);
 		primary = NULL;
diff --git a/drivers/gpu/drm/hisilicon/hibmc/hibmc_drm_de.c b/drivers/gpu/drm/hisilicon/hibmc/hibmc_drm_de.c
index c655883d3613..919cc807809f 100644
--- a/drivers/gpu/drm/hisilicon/hibmc/hibmc_drm_de.c
+++ b/drivers/gpu/drm/hisilicon/hibmc/hibmc_drm_de.c
@@ -181,6 +181,7 @@  static struct drm_plane *hibmc_plane_init(struct hibmc_drm_private *priv)
 	ret = drm_universal_plane_init(dev, plane, 1, &hibmc_plane_funcs,
 				       channel_formats1,
 				       ARRAY_SIZE(channel_formats1),
+				       NULL,
 				       DRM_PLANE_TYPE_PRIMARY,
 				       NULL);
 	if (ret) {
diff --git a/drivers/gpu/drm/hisilicon/kirin/kirin_drm_ade.c b/drivers/gpu/drm/hisilicon/kirin/kirin_drm_ade.c
index 307d460ab684..b433b8e7c9cd 100644
--- a/drivers/gpu/drm/hisilicon/kirin/kirin_drm_ade.c
+++ b/drivers/gpu/drm/hisilicon/kirin/kirin_drm_ade.c
@@ -911,7 +911,7 @@  static int ade_plane_init(struct drm_device *dev, struct ade_plane *aplane,
 		return ret;
 
 	ret = drm_universal_plane_init(dev, &aplane->base, 1, &ade_plane_funcs,
-				       fmts, fmts_cnt, type, NULL);
+				       fmts, fmts_cnt, NULL, type, NULL);
 	if (ret) {
 		DRM_ERROR("fail to init plane, ch=%d\n", aplane->ch);
 		return ret;
diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c
index b547332eeda1..8715b1083d1d 100644
--- a/drivers/gpu/drm/i915/intel_display.c
+++ b/drivers/gpu/drm/i915/intel_display.c
@@ -15303,6 +15303,8 @@  intel_legacy_cursor_update(struct drm_plane *plane,
 					      src_x, src_y, src_w, src_h);
 }
 
+
+
 static const struct drm_plane_funcs intel_cursor_plane_funcs = {
 	.update_plane = intel_legacy_cursor_update,
 	.disable_plane = drm_atomic_helper_disable_plane,
@@ -15387,18 +15389,21 @@  intel_primary_plane_create(struct drm_i915_private *dev_priv, enum pipe pipe)
 		ret = drm_universal_plane_init(&dev_priv->drm, &primary->base,
 					       0, &intel_plane_funcs,
 					       intel_primary_formats, num_formats,
+					       NULL,
 					       DRM_PLANE_TYPE_PRIMARY,
 					       "plane 1%c", pipe_name(pipe));
 	else if (INTEL_GEN(dev_priv) >= 5 || IS_G4X(dev_priv))
 		ret = drm_universal_plane_init(&dev_priv->drm, &primary->base,
 					       0, &intel_plane_funcs,
 					       intel_primary_formats, num_formats,
+					       NULL,
 					       DRM_PLANE_TYPE_PRIMARY,
 					       "primary %c", pipe_name(pipe));
 	else
 		ret = drm_universal_plane_init(&dev_priv->drm, &primary->base,
 					       0, &intel_plane_funcs,
 					       intel_primary_formats, num_formats,
+					       NULL,
 					       DRM_PLANE_TYPE_PRIMARY,
 					       "plane %c", plane_name(primary->plane));
 	if (ret)
@@ -15563,7 +15568,7 @@  intel_cursor_plane_create(struct drm_i915_private *dev_priv, enum pipe pipe)
 				       0, &intel_cursor_plane_funcs,
 				       intel_cursor_formats,
 				       ARRAY_SIZE(intel_cursor_formats),
-				       DRM_PLANE_TYPE_CURSOR,
+				       NULL, DRM_PLANE_TYPE_CURSOR,
 				       "cursor %c", pipe_name(pipe));
 	if (ret)
 		goto fail;
diff --git a/drivers/gpu/drm/i915/intel_sprite.c b/drivers/gpu/drm/i915/intel_sprite.c
index 063a994815d0..1c2f26d86f76 100644
--- a/drivers/gpu/drm/i915/intel_sprite.c
+++ b/drivers/gpu/drm/i915/intel_sprite.c
@@ -1127,13 +1127,13 @@  intel_sprite_plane_create(struct drm_i915_private *dev_priv,
 		ret = drm_universal_plane_init(&dev_priv->drm, &intel_plane->base,
 					       possible_crtcs, &intel_plane_funcs,
 					       plane_formats, num_plane_formats,
-					       DRM_PLANE_TYPE_OVERLAY,
+					       NULL, DRM_PLANE_TYPE_OVERLAY,
 					       "plane %d%c", plane + 2, pipe_name(pipe));
 	else
 		ret = drm_universal_plane_init(&dev_priv->drm, &intel_plane->base,
 					       possible_crtcs, &intel_plane_funcs,
 					       plane_formats, num_plane_formats,
-					       DRM_PLANE_TYPE_OVERLAY,
+					       NULL, DRM_PLANE_TYPE_OVERLAY,
 					       "sprite %c", sprite_name(pipe, plane));
 	if (ret)
 		goto fail;
diff --git a/drivers/gpu/drm/imx/ipuv3-plane.c b/drivers/gpu/drm/imx/ipuv3-plane.c
index 8b5294d47cee..ef9e11b98b3c 100644
--- a/drivers/gpu/drm/imx/ipuv3-plane.c
+++ b/drivers/gpu/drm/imx/ipuv3-plane.c
@@ -496,8 +496,8 @@  struct ipu_plane *ipu_plane_init(struct drm_device *dev, struct ipu_soc *ipu,
 
 	ret = drm_universal_plane_init(dev, &ipu_plane->base, possible_crtcs,
 				       &ipu_plane_funcs, ipu_plane_formats,
-				       ARRAY_SIZE(ipu_plane_formats), type,
-				       NULL);
+				       ARRAY_SIZE(ipu_plane_formats),
+				       NULL, type, NULL);
 	if (ret) {
 		DRM_ERROR("failed to initialize plane\n");
 		kfree(ipu_plane);
diff --git a/drivers/gpu/drm/mediatek/mtk_drm_plane.c b/drivers/gpu/drm/mediatek/mtk_drm_plane.c
index e405e89ed5e5..bec6d14dd070 100644
--- a/drivers/gpu/drm/mediatek/mtk_drm_plane.c
+++ b/drivers/gpu/drm/mediatek/mtk_drm_plane.c
@@ -173,7 +173,7 @@  int mtk_plane_init(struct drm_device *dev, struct drm_plane *plane,
 
 	err = drm_universal_plane_init(dev, plane, possible_crtcs,
 				       &mtk_plane_funcs, formats,
-				       ARRAY_SIZE(formats), type, NULL);
+				       ARRAY_SIZE(formats), NULL, type, NULL);
 	if (err) {
 		DRM_ERROR("failed to initialize plane\n");
 		return err;
diff --git a/drivers/gpu/drm/meson/meson_plane.c b/drivers/gpu/drm/meson/meson_plane.c
index 642b2fab42ff..13c7c2c1d4cc 100644
--- a/drivers/gpu/drm/meson/meson_plane.c
+++ b/drivers/gpu/drm/meson/meson_plane.c
@@ -220,6 +220,7 @@  int meson_plane_create(struct meson_drm *priv)
 				 &meson_plane_funcs,
 				 supported_drm_formats,
 				 ARRAY_SIZE(supported_drm_formats),
+				 NULL,
 				 DRM_PLANE_TYPE_PRIMARY, "meson_primary_plane");
 
 	drm_plane_helper_add(plane, &meson_plane_helper_funcs);
diff --git a/drivers/gpu/drm/msm/mdp/mdp4/mdp4_plane.c b/drivers/gpu/drm/msm/mdp/mdp4/mdp4_plane.c
index 53619d07677e..8f3417e45d4e 100644
--- a/drivers/gpu/drm/msm/mdp/mdp4/mdp4_plane.c
+++ b/drivers/gpu/drm/msm/mdp/mdp4/mdp4_plane.c
@@ -398,7 +398,7 @@  struct drm_plane *mdp4_plane_init(struct drm_device *dev,
 	type = private_plane ? DRM_PLANE_TYPE_PRIMARY : DRM_PLANE_TYPE_OVERLAY;
 	ret = drm_universal_plane_init(dev, plane, 0xff, &mdp4_plane_funcs,
 				 mdp4_plane->formats, mdp4_plane->nformats,
-				 type, NULL);
+				 NULL, type, NULL);
 	if (ret)
 		goto fail;
 
diff --git a/drivers/gpu/drm/msm/mdp/mdp5/mdp5_plane.c b/drivers/gpu/drm/msm/mdp/mdp5/mdp5_plane.c
index 75247ea4335b..c2068c30ca73 100644
--- a/drivers/gpu/drm/msm/mdp/mdp5/mdp5_plane.c
+++ b/drivers/gpu/drm/msm/mdp/mdp5/mdp5_plane.c
@@ -882,7 +882,7 @@  struct drm_plane *mdp5_plane_init(struct drm_device *dev, bool primary)
 	type = primary ? DRM_PLANE_TYPE_PRIMARY : DRM_PLANE_TYPE_OVERLAY;
 	ret = drm_universal_plane_init(dev, plane, 0xff, &mdp5_plane_funcs,
 				 mdp5_plane->formats, mdp5_plane->nformats,
-				 type, NULL);
+				 NULL, type, NULL);
 	if (ret)
 		goto fail;
 
diff --git a/drivers/gpu/drm/mxsfb/mxsfb_drv.c b/drivers/gpu/drm/mxsfb/mxsfb_drv.c
index 955441f71500..f29d463e03a4 100644
--- a/drivers/gpu/drm/mxsfb/mxsfb_drv.c
+++ b/drivers/gpu/drm/mxsfb/mxsfb_drv.c
@@ -186,7 +186,7 @@  static int mxsfb_load(struct drm_device *drm, unsigned long flags)
 	}
 
 	ret = drm_simple_display_pipe_init(drm, &mxsfb->pipe, &mxsfb_funcs,
-			mxsfb_formats, ARRAY_SIZE(mxsfb_formats),
+			mxsfb_formats, ARRAY_SIZE(mxsfb_formats), NULL, 0,
 			&mxsfb->connector);
 	if (ret < 0) {
 		dev_err(drm->dev, "Cannot setup simple display pipe\n");
diff --git a/drivers/gpu/drm/nouveau/nv50_display.c b/drivers/gpu/drm/nouveau/nv50_display.c
index cb85cb72dc1c..cf5ed98caed4 100644
--- a/drivers/gpu/drm/nouveau/nv50_display.c
+++ b/drivers/gpu/drm/nouveau/nv50_display.c
@@ -1079,8 +1079,9 @@  nv50_wndw_ctor(const struct nv50_wndw_func *func, struct drm_device *dev,
 	wndw->func = func;
 	wndw->dmac = dmac;
 
-	ret = drm_universal_plane_init(dev, &wndw->plane, 0, &nv50_wndw, format,
-				       nformat, type, "%s-%d", name, index);
+	ret = drm_universal_plane_init(dev, &wndw->plane, 0, &nv50_wndw,
+				       format, nformat, NULL,
+				       type, "%s-%d", name, index);
 	if (ret)
 		return ret;
 
diff --git a/drivers/gpu/drm/omapdrm/omap_plane.c b/drivers/gpu/drm/omapdrm/omap_plane.c
index 82b2c23d6769..b4a575cf7e71 100644
--- a/drivers/gpu/drm/omapdrm/omap_plane.c
+++ b/drivers/gpu/drm/omapdrm/omap_plane.c
@@ -383,7 +383,8 @@  struct drm_plane *omap_plane_init(struct drm_device *dev,
 
 	ret = drm_universal_plane_init(dev, plane, possible_crtcs,
 				       &omap_plane_funcs, omap_plane->formats,
-				       omap_plane->nformats, type, NULL);
+				       omap_plane->nformats,
+				       NULL, type, NULL);
 	if (ret < 0)
 		goto error;
 
diff --git a/drivers/gpu/drm/rcar-du/rcar_du_plane.c b/drivers/gpu/drm/rcar-du/rcar_du_plane.c
index dcde6288da6c..2b02eccbfb70 100644
--- a/drivers/gpu/drm/rcar-du/rcar_du_plane.c
+++ b/drivers/gpu/drm/rcar-du/rcar_du_plane.c
@@ -743,8 +743,8 @@  int rcar_du_planes_init(struct rcar_du_group *rgrp)
 
 		ret = drm_universal_plane_init(rcdu->ddev, &plane->plane, crtcs,
 					       &rcar_du_plane_funcs, formats,
-					       ARRAY_SIZE(formats), type,
-					       NULL);
+					       ARRAY_SIZE(formats),
+					       NULL, type, NULL);
 		if (ret < 0)
 			return ret;
 
diff --git a/drivers/gpu/drm/rcar-du/rcar_du_vsp.c b/drivers/gpu/drm/rcar-du/rcar_du_vsp.c
index b5bfbe50bd87..744be029436b 100644
--- a/drivers/gpu/drm/rcar-du/rcar_du_vsp.c
+++ b/drivers/gpu/drm/rcar-du/rcar_du_vsp.c
@@ -364,8 +364,9 @@  int rcar_du_vsp_init(struct rcar_du_vsp *vsp)
 					       1 << vsp->index,
 					       &rcar_du_vsp_plane_funcs,
 					       formats_kms,
-					       ARRAY_SIZE(formats_kms), type,
-					       NULL);
+					       ARRAY_SIZE(formats_kms),
+					       NULL,
+					       type, NULL);
 		if (ret < 0)
 			return ret;
 
diff --git a/drivers/gpu/drm/rockchip/rockchip_drm_vop.c b/drivers/gpu/drm/rockchip/rockchip_drm_vop.c
index fb5f001f51c3..826b376114f6 100644
--- a/drivers/gpu/drm/rockchip/rockchip_drm_vop.c
+++ b/drivers/gpu/drm/rockchip/rockchip_drm_vop.c
@@ -1221,7 +1221,7 @@  static int vop_create_crtc(struct vop *vop)
 					       0, &vop_plane_funcs,
 					       win_data->phy->data_formats,
 					       win_data->phy->nformats,
-					       win_data->type, NULL);
+					       NULL, win_data->type, NULL);
 		if (ret) {
 			DRM_DEV_ERROR(vop->dev, "failed to init plane %d\n",
 				      ret);
@@ -1260,7 +1260,7 @@  static int vop_create_crtc(struct vop *vop)
 					       &vop_plane_funcs,
 					       win_data->phy->data_formats,
 					       win_data->phy->nformats,
-					       win_data->type, NULL);
+					       NULL, win_data->type, NULL);
 		if (ret) {
 			DRM_DEV_ERROR(vop->dev, "failed to init overlay %d\n",
 				      ret);
diff --git a/drivers/gpu/drm/sti/sti_cursor.c b/drivers/gpu/drm/sti/sti_cursor.c
index cca75bddb9ad..f61808b419a5 100644
--- a/drivers/gpu/drm/sti/sti_cursor.c
+++ b/drivers/gpu/drm/sti/sti_cursor.c
@@ -393,6 +393,7 @@  struct drm_plane *sti_cursor_create(struct drm_device *drm_dev,
 				       &sti_cursor_plane_helpers_funcs,
 				       cursor_supported_formats,
 				       ARRAY_SIZE(cursor_supported_formats),
+				       NULL,
 				       DRM_PLANE_TYPE_CURSOR, NULL);
 	if (res) {
 		DRM_ERROR("Failed to initialize universal plane\n");
diff --git a/drivers/gpu/drm/sti/sti_gdp.c b/drivers/gpu/drm/sti/sti_gdp.c
index 877d053d86f4..6bf1a592cee4 100644
--- a/drivers/gpu/drm/sti/sti_gdp.c
+++ b/drivers/gpu/drm/sti/sti_gdp.c
@@ -921,7 +921,7 @@  struct drm_plane *sti_gdp_create(struct drm_device *drm_dev,
 				       &sti_gdp_plane_helpers_funcs,
 				       gdp_supported_formats,
 				       ARRAY_SIZE(gdp_supported_formats),
-				       type, NULL);
+				       NULL, type, NULL);
 	if (res) {
 		DRM_ERROR("Failed to initialize universal plane\n");
 		goto err;
diff --git a/drivers/gpu/drm/sti/sti_hqvdp.c b/drivers/gpu/drm/sti/sti_hqvdp.c
index becf10d255c4..911e8f0f69a6 100644
--- a/drivers/gpu/drm/sti/sti_hqvdp.c
+++ b/drivers/gpu/drm/sti/sti_hqvdp.c
@@ -1277,7 +1277,7 @@  static struct drm_plane *sti_hqvdp_create(struct drm_device *drm_dev,
 				       &sti_hqvdp_plane_helpers_funcs,
 				       hqvdp_supported_formats,
 				       ARRAY_SIZE(hqvdp_supported_formats),
-				       DRM_PLANE_TYPE_OVERLAY, NULL);
+				       NULL, DRM_PLANE_TYPE_OVERLAY, NULL);
 	if (res) {
 		DRM_ERROR("Failed to initialize universal plane\n");
 		return NULL;
diff --git a/drivers/gpu/drm/sun4i/sun4i_layer.c b/drivers/gpu/drm/sun4i/sun4i_layer.c
index 5d53c977bca5..827f45364654 100644
--- a/drivers/gpu/drm/sun4i/sun4i_layer.c
+++ b/drivers/gpu/drm/sun4i/sun4i_layer.c
@@ -117,6 +117,7 @@  static struct sun4i_layer *sun4i_layer_init_one(struct drm_device *drm,
 	ret = drm_universal_plane_init(drm, &layer->plane, BIT(0),
 				       &sun4i_backend_layer_funcs,
 				       plane->formats, plane->nformats,
+				       NULL,
 				       plane->type, NULL);
 	if (ret) {
 		dev_err(drm->dev, "Couldn't initialize layer\n");
diff --git a/drivers/gpu/drm/tegra/dc.c b/drivers/gpu/drm/tegra/dc.c
index 7561a95a54e3..9ddea84208c4 100644
--- a/drivers/gpu/drm/tegra/dc.c
+++ b/drivers/gpu/drm/tegra/dc.c
@@ -655,8 +655,8 @@  static struct drm_plane *tegra_dc_primary_plane_create(struct drm_device *drm,
 
 	err = drm_universal_plane_init(drm, &plane->base, possible_crtcs,
 				       &tegra_primary_plane_funcs, formats,
-				       num_formats, DRM_PLANE_TYPE_PRIMARY,
-				       NULL);
+				       num_formats, NULL,
+				       DRM_PLANE_TYPE_PRIMARY, NULL);
 	if (err < 0) {
 		kfree(plane);
 		return ERR_PTR(err);
@@ -821,8 +821,8 @@  static struct drm_plane *tegra_dc_cursor_plane_create(struct drm_device *drm,
 
 	err = drm_universal_plane_init(drm, &plane->base, 1 << dc->pipe,
 				       &tegra_cursor_plane_funcs, formats,
-				       num_formats, DRM_PLANE_TYPE_CURSOR,
-				       NULL);
+				       num_formats, NULL,
+				       DRM_PLANE_TYPE_CURSOR, NULL);
 	if (err < 0) {
 		kfree(plane);
 		return ERR_PTR(err);
@@ -883,8 +883,8 @@  static struct drm_plane *tegra_dc_overlay_plane_create(struct drm_device *drm,
 
 	err = drm_universal_plane_init(drm, &plane->base, 1 << dc->pipe,
 				       &tegra_overlay_plane_funcs, formats,
-				       num_formats, DRM_PLANE_TYPE_OVERLAY,
-				       NULL);
+				       num_formats, NULL,
+				       DRM_PLANE_TYPE_OVERLAY, NULL);
 	if (err < 0) {
 		kfree(plane);
 		return ERR_PTR(err);
diff --git a/drivers/gpu/drm/vc4/vc4_plane.c b/drivers/gpu/drm/vc4/vc4_plane.c
index 110d1518f5d5..487bb40e0e4e 100644
--- a/drivers/gpu/drm/vc4/vc4_plane.c
+++ b/drivers/gpu/drm/vc4/vc4_plane.c
@@ -861,7 +861,7 @@  struct drm_plane *vc4_plane_init(struct drm_device *dev,
 	ret = drm_universal_plane_init(dev, plane, 0xff,
 				       &vc4_plane_funcs,
 				       formats, num_formats,
-				       type, NULL);
+				       NULL, type, NULL);
 
 	drm_plane_helper_add(plane, &vc4_plane_helper_funcs);
 
diff --git a/drivers/gpu/drm/virtio/virtgpu_plane.c b/drivers/gpu/drm/virtio/virtgpu_plane.c
index 11288ffa4af6..00f368cde0b8 100644
--- a/drivers/gpu/drm/virtio/virtgpu_plane.c
+++ b/drivers/gpu/drm/virtio/virtgpu_plane.c
@@ -225,7 +225,7 @@  struct drm_plane *virtio_gpu_plane_init(struct virtio_gpu_device *vgdev,
 	ret = drm_universal_plane_init(dev, plane, 1 << index,
 				       &virtio_gpu_plane_funcs,
 				       formats, nformats,
-				       type, NULL);
+				       NULL, type, NULL);
 	if (ret)
 		goto err_plane_init;
 
diff --git a/drivers/gpu/drm/zte/zx_plane.c b/drivers/gpu/drm/zte/zx_plane.c
index b634b090cdc1..c57d4fe5da18 100644
--- a/drivers/gpu/drm/zte/zx_plane.c
+++ b/drivers/gpu/drm/zte/zx_plane.c
@@ -287,7 +287,7 @@  struct drm_plane *zx_plane_init(struct drm_device *drm, struct device *dev,
 
 	ret = drm_universal_plane_init(drm, plane, VOU_CRTC_MASK,
 				       &zx_plane_funcs, formats, format_count,
-				       type, NULL);
+				       NULL, type, NULL);
 	if (ret) {
 		DRM_DEV_ERROR(dev, "failed to init universal plane: %d\n", ret);
 		return ERR_PTR(ret);
diff --git a/include/drm/drm_plane.h b/include/drm/drm_plane.h
index e049bc52fb07..108a955438e5 100644
--- a/include/drm/drm_plane.h
+++ b/include/drm/drm_plane.h
@@ -389,6 +389,20 @@  struct drm_plane_funcs {
 	 */
 	void (*atomic_print_state)(struct drm_printer *p,
 				   const struct drm_plane_state *state);
+
+	/**
+	 * @format_mod_supported:
+	 *
+	 * This optional hook is used for the DRM to determine if the given
+	 * format/modifier combination is valid for the plane. This allows the
+	 * DRM to generate the correct format bitmask (which formats apply to
+	 * which modifier).
+	 *
+	 * True if the given modifier is valid for that format on the plane.
+	 * False otherwise.
+	 */
+	bool (*format_mod_supported)(struct drm_plane *plane, uint32_t format,
+				     uint64_t modifier);
 };
 
 /**
@@ -483,6 +497,10 @@  struct drm_plane {
 	unsigned int format_count;
 	bool format_default;
 
+	uint32_t *formats;
+	uint64_t *modifiers;
+	unsigned int modifier_count;
+
 	struct drm_crtc *crtc;
 	struct drm_framebuffer *fb;
 
@@ -510,13 +528,14 @@  struct drm_plane {
 
 #define obj_to_plane(x) container_of(x, struct drm_plane, base)
 
-extern __printf(8, 9)
+extern __printf(9, 10)
 int drm_universal_plane_init(struct drm_device *dev,
 			     struct drm_plane *plane,
 			     uint32_t possible_crtcs,
 			     const struct drm_plane_funcs *funcs,
 			     const uint32_t *formats,
 			     unsigned int format_count,
+			     const uint64_t *format_modifiers,
 			     enum drm_plane_type type,
 			     const char *name, ...);
 extern int drm_plane_init(struct drm_device *dev,
diff --git a/include/drm/drm_simple_kms_helper.h b/include/drm/drm_simple_kms_helper.h
index fe8c4ba905ac..059cd592a127 100644
--- a/include/drm/drm_simple_kms_helper.h
+++ b/include/drm/drm_simple_kms_helper.h
@@ -118,6 +118,7 @@  int drm_simple_display_pipe_init(struct drm_device *dev,
 			struct drm_simple_display_pipe *pipe,
 			const struct drm_simple_display_pipe_funcs *funcs,
 			const uint32_t *formats, unsigned int format_count,
+			const uint64_t *format_modifiers,
 			struct drm_connector *connector);
 
 #endif /* __LINUX_DRM_SIMPLE_KMS_HELPER_H */
diff --git a/include/uapi/drm/drm.h b/include/uapi/drm/drm.h
index b2c52843bc70..487e0f17113f 100644
--- a/include/uapi/drm/drm.h
+++ b/include/uapi/drm/drm.h
@@ -805,6 +805,7 @@  extern "C" {
 #define DRM_IOCTL_MODE_DESTROY_DUMB    DRM_IOWR(0xB4, struct drm_mode_destroy_dumb)
 #define DRM_IOCTL_MODE_GETPLANERESOURCES DRM_IOWR(0xB5, struct drm_mode_get_plane_res)
 #define DRM_IOCTL_MODE_GETPLANE	DRM_IOWR(0xB6, struct drm_mode_get_plane)
+#define DRM_IOCTL_MODE_GETPLANE2	DRM_IOWR(0xB6, struct drm_mode_get_plane2)
 #define DRM_IOCTL_MODE_SETPLANE	DRM_IOWR(0xB7, struct drm_mode_set_plane)
 #define DRM_IOCTL_MODE_ADDFB2		DRM_IOWR(0xB8, struct drm_mode_fb_cmd2)
 #define DRM_IOCTL_MODE_OBJ_GETPROPERTIES	DRM_IOWR(0xB9, struct drm_mode_obj_get_properties)
diff --git a/include/uapi/drm/drm_fourcc.h b/include/uapi/drm/drm_fourcc.h
index 4581e3d41e5c..5859014d93c7 100644
--- a/include/uapi/drm/drm_fourcc.h
+++ b/include/uapi/drm/drm_fourcc.h
@@ -162,6 +162,8 @@  extern "C" {
 #define DRM_FORMAT_MOD_VENDOR_QCOM    0x05
 /* add more to the end as needed */
 
+#define DRM_FORMAT_RESERVED	      ((1ULL << 56) - 1)
+
 #define fourcc_mod_code(vendor, val) \
 	((((__u64)DRM_FORMAT_MOD_VENDOR_## vendor) << 56) | (val & 0x00ffffffffffffffULL))
 
@@ -174,6 +176,15 @@  extern "C" {
  */
 
 /*
+ * Invalid Modifier
+ *
+ * This modifier can be used as a sentinel to terminate list, or to initialize a
+ * variable with an invalid modifier. It might also be used to report an error
+ * back to userspace for certain APIs.
+ */
+#define DRM_FORMAT_MOD_INVALID	fourcc_mod_code(NONE, DRM_FORMAT_RESERVED)
+
+/*
  * Linear Layout
  *
  * Just plain linear layout. Note that this is different from no specifying any
diff --git a/include/uapi/drm/drm_mode.h b/include/uapi/drm/drm_mode.h
index ce7efe2e8a5e..cea3de3aa301 100644
--- a/include/uapi/drm/drm_mode.h
+++ b/include/uapi/drm/drm_mode.h
@@ -209,6 +209,33 @@  struct drm_mode_get_plane {
 	__u64 format_type_ptr;
 };
 
+struct drm_format_modifier {
+	/* Bitmask of formats in get_plane format list this info
+	 * applies to. */
+	uint64_t formats;
+
+	/* This modifier can be used with the format for this plane. */
+	uint64_t modifier;
+};
+
+struct drm_mode_get_plane2 {
+	__u32 plane_id;
+
+	__u32 crtc_id;
+	__u32 fb_id;
+
+	__u32 possible_crtcs;
+	__u32 gamma_size;
+
+	__u32 count_format_types;
+	__u64 format_type_ptr;
+
+	/* New in v2 */
+	__u32 count_format_modifiers;
+	__u32 flags;
+	__u64 format_modifier_ptr;
+};
+
 struct drm_mode_get_plane_res {
 	__u64 plane_id_ptr;
 	__u32 count_planes;