diff mbox

[1/2] drm: Add new DRM_IOCTL_MODE_GETPLANE2

Message ID 20161221001222.21864-1-hoegsberg@gmail.com (mailing list archive)
State New, archived
Headers show

Commit Message

Kristian Høgsberg Dec. 21, 2016, 12:12 a.m. UTC
From: "Kristian H. Kristensen" <hoegsberg@google.com>

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

Signed-off-by: Kristian H. Kristensen <hoegsberg@chromium.org>
---
 drivers/gpu/drm/arm/hdlcd_crtc.c                |  1 +
 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                     | 40 +++++++++++++++++++++++--
 drivers/gpu/drm/drm_simple_kms_helper.c         |  5 ++++
 drivers/gpu/drm/exynos/exynos_drm_plane.c       |  2 +-
 drivers/gpu/drm/fsl-dcu/fsl_dcu_drm_plane.c     |  2 +-
 drivers/gpu/drm/i915/intel_display.c            |  5 +++-
 drivers/gpu/drm/imx/ipuv3-plane.c               |  4 +--
 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/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/tegra/dc.c                      | 12 ++++----
 drivers/gpu/drm/vc4/vc4_plane.c                 |  2 +-
 drivers/gpu/drm/virtio/virtgpu_plane.c          |  2 +-
 include/drm/drm_plane.h                         |  7 ++++-
 include/drm/drm_simple_kms_helper.h             |  2 ++
 include/uapi/drm/drm.h                          |  1 +
 include/uapi/drm/drm_mode.h                     | 27 +++++++++++++++++
 27 files changed, 116 insertions(+), 28 deletions(-)

Comments

Rob Clark Dec. 21, 2016, 12:46 a.m. UTC | #1
On Tue, Dec 20, 2016 at 7:12 PM, Kristian H. Kristensen
<hoegsberg@gmail.com> wrote:
> diff --git a/include/uapi/drm/drm_mode.h b/include/uapi/drm/drm_mode.h
> index ce7efe2..cea3de3 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;

Hmm, this seems a bit clunky/brittle when you have a lot of supported
formats (esp. if format order changes or new formats are not added at
end).  I guess fine when you support a four or five different formats,
but I think you'll start to hate maintaining those tables on hw that
supports more.

Also I guess it limits you to modifiers only with the first 64
formats.. maybe not a problem right away, but a quick look and drm/msm
is already at 23 formats (and there are probably some more it could
do.. without even starting to get into "exotic" float/etc formats and
whatever else might come in the future.

Not that I really have a better idea..  maybe just instead of
getplane2 we just add a querymodifiers ioctl (ie. fourcc in, list of
modifiers out), with the idea that userspace probably knows what
format/fourcc it wants to use, and only just cares about modifiers for
that particular fourcc..

BR,
-R

> +       /* This modifier can be used with the format for this plane. */
> +       uint64_t modifier;
> +};
> +
Daniel Vetter Dec. 21, 2016, 9:15 a.m. UTC | #2
On Tue, Dec 20, 2016 at 07:46:12PM -0500, Rob Clark wrote:
> On Tue, Dec 20, 2016 at 7:12 PM, Kristian H. Kristensen
> <hoegsberg@gmail.com> wrote:
> > diff --git a/include/uapi/drm/drm_mode.h b/include/uapi/drm/drm_mode.h
> > index ce7efe2..cea3de3 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;
> 
> Hmm, this seems a bit clunky/brittle when you have a lot of supported
> formats (esp. if format order changes or new formats are not added at
> end).  I guess fine when you support a four or five different formats,
> but I think you'll start to hate maintaining those tables on hw that
> supports more.
> 
> Also I guess it limits you to modifiers only with the first 64
> formats.. maybe not a problem right away, but a quick look and drm/msm
> is already at 23 formats (and there are probably some more it could
> do.. without even starting to get into "exotic" float/etc formats and
> whatever else might come in the future.

Hm, I'd have said with max 23 currently used 64 is good enough.
> 
> Not that I really have a better idea..  maybe just instead of
> getplane2 we just add a querymodifiers ioctl (ie. fourcc in, list of
> modifiers out), with the idea that userspace probably knows what
> format/fourcc it wants to use, and only just cares about modifiers for
> that particular fourcc..

Could we do a table of tables instead, at least internally? So

struct drm_format_support {
	u64 modifiers;
	u32 *formats;
};

And then supply an array of those? Maybe with some magic to convert it in
the ioctl since the proposed ioctl struct is easier to transport to
userspace. Could also switch over to terminating arrays with a final 0
element (like with pci tables and everything else used in the kernel) to
get rid of all those silly ARRAY_SIZE lines. Totalling up:

u32 format_list_untiled[] = { RGBX8888, RGBA8888, 0 };
u32 format_list_tiled[] = { RGBX8888, 0 };

struct drm_format_support formats[] = {
	{ MODE_NODE, format_untiled },
	{ MODE_TILED, format_tiled },
	{ 0, NULL }
};

Not sure that's any better really.
-Daniel

> 
> 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ä Dec. 21, 2016, 9:19 a.m. UTC | #3
On Wed, Dec 21, 2016 at 10:15:15AM +0100, Daniel Vetter wrote:
> On Tue, Dec 20, 2016 at 07:46:12PM -0500, Rob Clark wrote:
> > On Tue, Dec 20, 2016 at 7:12 PM, Kristian H. Kristensen
> > <hoegsberg@gmail.com> wrote:
> > > diff --git a/include/uapi/drm/drm_mode.h b/include/uapi/drm/drm_mode.h
> > > index ce7efe2..cea3de3 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;
> > 
> > Hmm, this seems a bit clunky/brittle when you have a lot of supported
> > formats (esp. if format order changes or new formats are not added at
> > end).  I guess fine when you support a four or five different formats,
> > but I think you'll start to hate maintaining those tables on hw that
> > supports more.
> > 
> > Also I guess it limits you to modifiers only with the first 64
> > formats.. maybe not a problem right away, but a quick look and drm/msm
> > is already at 23 formats (and there are probably some more it could
> > do.. without even starting to get into "exotic" float/etc formats and
> > whatever else might come in the future.
> 
> Hm, I'd have said with max 23 currently used 64 is good enough.
> > 
> > Not that I really have a better idea..  maybe just instead of
> > getplane2 we just add a querymodifiers ioctl (ie. fourcc in, list of
> > modifiers out), with the idea that userspace probably knows what
> > format/fourcc it wants to use, and only just cares about modifiers for
> > that particular fourcc..
> 
> Could we do a table of tables instead, at least internally? So
> 
> struct drm_format_support {
> 	u64 modifiers;
> 	u32 *formats;
> };
> 
> And then supply an array of those? Maybe with some magic to convert it in
> the ioctl since the proposed ioctl struct is easier to transport to
> userspace. Could also switch over to terminating arrays with a final 0
> element (like with pci tables and everything else used in the kernel) to
> get rid of all those silly ARRAY_SIZE lines. Totalling up:
> 
> u32 format_list_untiled[] = { RGBX8888, RGBA8888, 0 };
> u32 format_list_tiled[] = { RGBX8888, 0 };
> 
> struct drm_format_support formats[] = {
> 	{ MODE_NODE, format_untiled },
> 	{ MODE_TILED, format_tiled },
> 	{ 0, NULL }
> };
> 
> Not sure that's any better really.

I think what we might want is:

u32 formats[]
u64 modifiers[]
bool (*format_mod_supported)(u32 format, u64 modifier);

The driver provides those, and the core will then just go through the
combinations and build up the masks. We could then also reuse that stuff
for addfb2 so that the core will call that same hook to check whether
the format+modifier is supported. That way the driver .fb_create() will
never see any unsupported combinations and we avoid having to duplicate
any logic there to see which hardware supports which formats.


> -Daniel
> 
> > 
> > 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
> 
> -- 
> Daniel Vetter
> Software Engineer, Intel Corporation
> http://blog.ffwll.ch
> _______________________________________________
> dri-devel mailing list
> dri-devel@lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/dri-devel
Rob Clark Dec. 21, 2016, 3:41 p.m. UTC | #4
On Wed, Dec 21, 2016 at 4:15 AM, Daniel Vetter <daniel@ffwll.ch> wrote:
>> Also I guess it limits you to modifiers only with the first 64
>> formats.. maybe not a problem right away, but a quick look and drm/msm
>> is already at 23 formats (and there are probably some more it could
>> do.. without even starting to get into "exotic" float/etc formats and
>> whatever else might come in the future.
>
> Hm, I'd have said with max 23 currently used 64 is good enough.

The fact that the struct has no room to grow worries me a bit, when we
are nearly half way through using the available space, without even
adding permutations of float/sRGB/etc..

(and note: just a quick look at the trm/hrd for 8016 and it looks like
we could easily add things like permutations of 5551, 4444, etc..
those 23 formats aren't even trying to cover everything the hw could
do)

Maybe something like:

  struct drm_format_support {
      u16 base;
      u16 pad;
      u32 mask;   /* bitmask relative to base */
      u64 modifier;
  };

would be a bit more future proof..

BR,
-R
Rob Clark Dec. 21, 2016, 3:42 p.m. UTC | #5
On Wed, Dec 21, 2016 at 4:19 AM, Ville Syrjälä
<ville.syrjala@linux.intel.com> wrote:
> On Wed, Dec 21, 2016 at 10:15:15AM +0100, Daniel Vetter wrote:
>> On Tue, Dec 20, 2016 at 07:46:12PM -0500, Rob Clark wrote:
>> > On Tue, Dec 20, 2016 at 7:12 PM, Kristian H. Kristensen
>> > <hoegsberg@gmail.com> wrote:
>> > > diff --git a/include/uapi/drm/drm_mode.h b/include/uapi/drm/drm_mode.h
>> > > index ce7efe2..cea3de3 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;
>> >
>> > Hmm, this seems a bit clunky/brittle when you have a lot of supported
>> > formats (esp. if format order changes or new formats are not added at
>> > end).  I guess fine when you support a four or five different formats,
>> > but I think you'll start to hate maintaining those tables on hw that
>> > supports more.
>> >
>> > Also I guess it limits you to modifiers only with the first 64
>> > formats.. maybe not a problem right away, but a quick look and drm/msm
>> > is already at 23 formats (and there are probably some more it could
>> > do.. without even starting to get into "exotic" float/etc formats and
>> > whatever else might come in the future.
>>
>> Hm, I'd have said with max 23 currently used 64 is good enough.
>> >
>> > Not that I really have a better idea..  maybe just instead of
>> > getplane2 we just add a querymodifiers ioctl (ie. fourcc in, list of
>> > modifiers out), with the idea that userspace probably knows what
>> > format/fourcc it wants to use, and only just cares about modifiers for
>> > that particular fourcc..
>>
>> Could we do a table of tables instead, at least internally? So
>>
>> struct drm_format_support {
>>       u64 modifiers;
>>       u32 *formats;
>> };
>>
>> And then supply an array of those? Maybe with some magic to convert it in
>> the ioctl since the proposed ioctl struct is easier to transport to
>> userspace. Could also switch over to terminating arrays with a final 0
>> element (like with pci tables and everything else used in the kernel) to
>> get rid of all those silly ARRAY_SIZE lines. Totalling up:
>>
>> u32 format_list_untiled[] = { RGBX8888, RGBA8888, 0 };
>> u32 format_list_tiled[] = { RGBX8888, 0 };
>>
>> struct drm_format_support formats[] = {
>>       { MODE_NODE, format_untiled },
>>       { MODE_TILED, format_tiled },
>>       { 0, NULL }
>> };
>>
>> Not sure that's any better really.
>
> I think what we might want is:
>
> u32 formats[]
> u64 modifiers[]
> bool (*format_mod_supported)(u32 format, u64 modifier);
>
> The driver provides those, and the core will then just go through the
> combinations and build up the masks. We could then also reuse that stuff
> for addfb2 so that the core will call that same hook to check whether
> the format+modifier is supported. That way the driver .fb_create() will
> never see any unsupported combinations and we avoid having to duplicate
> any logic there to see which hardware supports which formats.
>

I do like this for internal API better, rather than driver building up
tables itself.

BR,
-R
Kristian Høgsberg Dec. 21, 2016, 5:26 p.m. UTC | #6
On Wed, Dec 21, 2016 at 7:42 AM Rob Clark <robdclark@gmail.com> wrote:

> On Wed, Dec 21, 2016 at 4:19 AM, Ville Syrjälä
> <ville.syrjala@linux.intel.com> wrote:
> > On Wed, Dec 21, 2016 at 10:15:15AM +0100, Daniel Vetter wrote:
> >> On Tue, Dec 20, 2016 at 07:46:12PM -0500, Rob Clark wrote:
> >> > On Tue, Dec 20, 2016 at 7:12 PM, Kristian H. Kristensen
> >> > <hoegsberg@gmail.com> wrote:
> >> > > diff --git a/include/uapi/drm/drm_mode.h
> b/include/uapi/drm/drm_mode.h
> >> > > index ce7efe2..cea3de3 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;
> >> >
> >> > Hmm, this seems a bit clunky/brittle when you have a lot of supported
> >> > formats (esp. if format order changes or new formats are not added at
> >> > end).  I guess fine when you support a four or five different formats,
> >> > but I think you'll start to hate maintaining those tables on hw that
> >> > supports more.
> >> >
> >> > Also I guess it limits you to modifiers only with the first 64
> >> > formats.. maybe not a problem right away, but a quick look and drm/msm
> >> > is already at 23 formats (and there are probably some more it could
> >> > do.. without even starting to get into "exotic" float/etc formats and
> >> > whatever else might come in the future.
> >>
> >> Hm, I'd have said with max 23 currently used 64 is good enough.
> >> >
> >> > Not that I really have a better idea..  maybe just instead of
> >> > getplane2 we just add a querymodifiers ioctl (ie. fourcc in, list of
> >> > modifiers out), with the idea that userspace probably knows what
> >> > format/fourcc it wants to use, and only just cares about modifiers for
> >> > that particular fourcc..
> >>
> >> Could we do a table of tables instead, at least internally? So
> >>
> >> struct drm_format_support {
> >>       u64 modifiers;
> >>       u32 *formats;
> >> };
> >>
> >> And then supply an array of those? Maybe with some magic to convert it
> in
> >> the ioctl since the proposed ioctl struct is easier to transport to
> >> userspace. Could also switch over to terminating arrays with a final 0
> >> element (like with pci tables and everything else used in the kernel) to
> >> get rid of all those silly ARRAY_SIZE lines. Totalling up:
> >>
> >> u32 format_list_untiled[] = { RGBX8888, RGBA8888, 0 };
> >> u32 format_list_tiled[] = { RGBX8888, 0 };
> >>
> >> struct drm_format_support formats[] = {
> >>       { MODE_NODE, format_untiled },
> >>       { MODE_TILED, format_tiled },
> >>       { 0, NULL }
> >> };
> >>
> >> Not sure that's any better really.
> >
> > I think what we might want is:
> >
> > u32 formats[]
> > u64 modifiers[]
> > bool (*format_mod_supported)(u32 format, u64 modifier);
> >
> > The driver provides those, and the core will then just go through the
> > combinations and build up the masks. We could then also reuse that stuff
> > for addfb2 so that the core will call that same hook to check whether
> > the format+modifier is supported. That way the driver .fb_create() will
> > never see any unsupported combinations and we avoid having to duplicate
> > any logic there to see which hardware supports which formats.
> >
>
> I do like this for internal API better, rather than driver building up
> tables itself.
>

Yeah, I like Ville's suggestion too, I'll give it a try.

Kristian


> BR,
> -R
>
Kristian Hogsberg Dec. 21, 2016, 6:03 p.m. UTC | #7
Rob Clark <robdclark@gmail.com> writes:

> On Tue, Dec 20, 2016 at 7:12 PM, Kristian H. Kristensen
> <hoegsberg@gmail.com> wrote:
>> diff --git a/include/uapi/drm/drm_mode.h b/include/uapi/drm/drm_mode.h
>> index ce7efe2..cea3de3 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;
>
> Hmm, this seems a bit clunky/brittle when you have a lot of supported
> formats (esp. if format order changes or new formats are not added at
> end).  I guess fine when you support a four or five different formats,
> but I think you'll start to hate maintaining those tables on hw that
> supports more.

I can't disagree, it was pretty finicky getting the bit masks right for
the Intel patch. On the other hand, in userspace, the interface is
easier to work with as you create the bitmask programatically, and
there's precedence for bitmask already (possible_crtcs).

> Also I guess it limits you to modifiers only with the first 64
> formats.. maybe not a problem right away, but a quick look and drm/msm
> is already at 23 formats (and there are probably some more it could
> do.. without even starting to get into "exotic" float/etc formats and
> whatever else might come in the future.
>
> Not that I really have a better idea..  maybe just instead of
> getplane2 we just add a querymodifiers ioctl (ie. fourcc in, list of
> modifiers out), with the idea that userspace probably knows what
> format/fourcc it wants to use, and only just cares about modifiers for
> that particular fourcc..

I'll reworking this with your base idea and Ville's suggestion and see
how it comes out. Thanks for the feedback.

Kristian

> BR,
> -R
>
>> +       /* This modifier can be used with the format for this plane. */
>> +       uint64_t modifier;
>> +};
>> +
Daniel Vetter Dec. 22, 2016, 7:04 a.m. UTC | #8
On Wed, Dec 21, 2016 at 05:26:21PM +0000, Kristian Høgsberg wrote:
> On Wed, Dec 21, 2016 at 7:42 AM Rob Clark <robdclark@gmail.com> wrote:
> 
> > On Wed, Dec 21, 2016 at 4:19 AM, Ville Syrjälä
> > <ville.syrjala@linux.intel.com> wrote:
> > > On Wed, Dec 21, 2016 at 10:15:15AM +0100, Daniel Vetter wrote:
> > >> On Tue, Dec 20, 2016 at 07:46:12PM -0500, Rob Clark wrote:
> > >> > On Tue, Dec 20, 2016 at 7:12 PM, Kristian H. Kristensen
> > >> > <hoegsberg@gmail.com> wrote:
> > >> > > diff --git a/include/uapi/drm/drm_mode.h
> > b/include/uapi/drm/drm_mode.h
> > >> > > index ce7efe2..cea3de3 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;
> > >> >
> > >> > Hmm, this seems a bit clunky/brittle when you have a lot of supported
> > >> > formats (esp. if format order changes or new formats are not added at
> > >> > end).  I guess fine when you support a four or five different formats,
> > >> > but I think you'll start to hate maintaining those tables on hw that
> > >> > supports more.
> > >> >
> > >> > Also I guess it limits you to modifiers only with the first 64
> > >> > formats.. maybe not a problem right away, but a quick look and drm/msm
> > >> > is already at 23 formats (and there are probably some more it could
> > >> > do.. without even starting to get into "exotic" float/etc formats and
> > >> > whatever else might come in the future.
> > >>
> > >> Hm, I'd have said with max 23 currently used 64 is good enough.
> > >> >
> > >> > Not that I really have a better idea..  maybe just instead of
> > >> > getplane2 we just add a querymodifiers ioctl (ie. fourcc in, list of
> > >> > modifiers out), with the idea that userspace probably knows what
> > >> > format/fourcc it wants to use, and only just cares about modifiers for
> > >> > that particular fourcc..
> > >>
> > >> Could we do a table of tables instead, at least internally? So
> > >>
> > >> struct drm_format_support {
> > >>       u64 modifiers;
> > >>       u32 *formats;
> > >> };
> > >>
> > >> And then supply an array of those? Maybe with some magic to convert it
> > in
> > >> the ioctl since the proposed ioctl struct is easier to transport to
> > >> userspace. Could also switch over to terminating arrays with a final 0
> > >> element (like with pci tables and everything else used in the kernel) to
> > >> get rid of all those silly ARRAY_SIZE lines. Totalling up:
> > >>
> > >> u32 format_list_untiled[] = { RGBX8888, RGBA8888, 0 };
> > >> u32 format_list_tiled[] = { RGBX8888, 0 };
> > >>
> > >> struct drm_format_support formats[] = {
> > >>       { MODE_NODE, format_untiled },
> > >>       { MODE_TILED, format_tiled },
> > >>       { 0, NULL }
> > >> };
> > >>
> > >> Not sure that's any better really.
> > >
> > > I think what we might want is:
> > >
> > > u32 formats[]
> > > u64 modifiers[]
> > > bool (*format_mod_supported)(u32 format, u64 modifier);

bool (*format_mod_supported)(struct drm_device *dev, u32 format, u64 modifier);

... since the point is to apply device restrictions.

> > >
> > > The driver provides those, and the core will then just go through the
> > > combinations and build up the masks. We could then also reuse that stuff
> > > for addfb2 so that the core will call that same hook to check whether
> > > the format+modifier is supported. That way the driver .fb_create() will
> > > never see any unsupported combinations and we avoid having to duplicate
> > > any logic there to see which hardware supports which formats.
> > >
> >
> > I do like this for internal API better, rather than driver building up
> > tables itself.
> >
> 
> Yeah, I like Ville's suggestion too, I'll give it a try.

+1, I like too. Especially if we reuse this in addfb2 to filter out
unsupported combinations.
-Daniel
Daniel Kurtz Jan. 3, 2017, 6:42 a.m. UTC | #9
Hi Kristian,

On Wed, Dec 21, 2016 at 8:12 AM, Kristian H. Kristensen
<hoegsberg@gmail.com> wrote:
> From: "Kristian H. Kristensen" <hoegsberg@google.com>
>
> This new ioctl exctends DRM_IOCTL_MODE_GETPLANE, by returning
> information about the modifiers that will work with each format.
>
> Signed-off-by: Kristian H. Kristensen <hoegsberg@chromium.org>
> ---
>  drivers/gpu/drm/arm/hdlcd_crtc.c                |  1 +
>  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                     | 40 +++++++++++++++++++++++--
>  drivers/gpu/drm/drm_simple_kms_helper.c         |  5 ++++
>  drivers/gpu/drm/exynos/exynos_drm_plane.c       |  2 +-
>  drivers/gpu/drm/fsl-dcu/fsl_dcu_drm_plane.c     |  2 +-
>  drivers/gpu/drm/i915/intel_display.c            |  5 +++-
>  drivers/gpu/drm/imx/ipuv3-plane.c               |  4 +--

Looks like this missed drivers/gpu/drm/mediatek.

>  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/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/tegra/dc.c                      | 12 ++++----
>  drivers/gpu/drm/vc4/vc4_plane.c                 |  2 +-
>  drivers/gpu/drm/virtio/virtgpu_plane.c          |  2 +-
>  include/drm/drm_plane.h                         |  7 ++++-
>  include/drm/drm_simple_kms_helper.h             |  2 ++
>  include/uapi/drm/drm.h                          |  1 +
>  include/uapi/drm/drm_mode.h                     | 27 +++++++++++++++++
>  27 files changed, 116 insertions(+), 28 deletions(-)
Rob Clark April 1, 2017, 6:47 p.m. UTC | #10
On Tue, Dec 20, 2016 at 7:12 PM, Kristian H. Kristensen
<hoegsberg@gmail.com> wrote:
> From: "Kristian H. Kristensen" <hoegsberg@google.com>
>
> This new ioctl exctends DRM_IOCTL_MODE_GETPLANE, by returning
> information about the modifiers that will work with each format.

So, just to toss out a completely different idea..

What if instead of a new ioctl, we had a read-only blob property
(which encoded the info basically the same way, plus the fourcc's)?

If we do writeback via some sort of OUT_FB_ID property on plane/crtc,
we will need the same sort of format information so userspace knows
what output formats (and modifiers) are supported.  So re-using the
same blob property layout (and userspace parsing) seems useful.

BR,
-R

> Signed-off-by: Kristian H. Kristensen <hoegsberg@chromium.org>
> ---
>  drivers/gpu/drm/arm/hdlcd_crtc.c                |  1 +
>  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                     | 40 +++++++++++++++++++++++--
>  drivers/gpu/drm/drm_simple_kms_helper.c         |  5 ++++
>  drivers/gpu/drm/exynos/exynos_drm_plane.c       |  2 +-
>  drivers/gpu/drm/fsl-dcu/fsl_dcu_drm_plane.c     |  2 +-
>  drivers/gpu/drm/i915/intel_display.c            |  5 +++-
>  drivers/gpu/drm/imx/ipuv3-plane.c               |  4 +--
>  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/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/tegra/dc.c                      | 12 ++++----
>  drivers/gpu/drm/vc4/vc4_plane.c                 |  2 +-
>  drivers/gpu/drm/virtio/virtgpu_plane.c          |  2 +-
>  include/drm/drm_plane.h                         |  7 ++++-
>  include/drm/drm_simple_kms_helper.h             |  2 ++
>  include/uapi/drm/drm.h                          |  1 +
>  include/uapi/drm/drm_mode.h                     | 27 +++++++++++++++++
>  27 files changed, 116 insertions(+), 28 deletions(-)
>
> diff --git a/drivers/gpu/drm/arm/hdlcd_crtc.c b/drivers/gpu/drm/arm/hdlcd_crtc.c
> index 20ebfb4..6062578 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, 0,
>                                        DRM_PLANE_TYPE_PRIMARY, NULL);
>         if (ret) {
>                 devm_kfree(drm->dev, plane);
> diff --git a/drivers/gpu/drm/armada/armada_crtc.c b/drivers/gpu/drm/armada/armada_crtc.c
> index e62ee44..e1a6170 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, 0,
>                                        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 34cb73d..32fb8f3 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, 0,
>                                        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 bd2791c..ac9e4db 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, 0,
> +                                      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 59b6911..9724209 100644
> --- a/drivers/gpu/drm/drm_ioctl.c
> +++ b/drivers/gpu/drm/drm_ioctl.c
> @@ -615,7 +615,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 b4f3665..94b2717 100644
> --- a/drivers/gpu/drm/drm_modeset_helper.c
> +++ b/drivers/gpu/drm/drm_modeset_helper.c
> @@ -122,6 +122,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, 0,
>                                        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 8ad20af..53a9821 100644
> --- a/drivers/gpu/drm/drm_plane.c
> +++ b/drivers/gpu/drm/drm_plane.c
> @@ -70,6 +70,8 @@ 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
> + * @format_modifier_count: number of elements in @format_modifiers
>   * @type: type of plane (overlay, primary, cursor)
>   * @name: printf style format string for the plane name, or NULL for default name
>   *
> @@ -82,6 +84,8 @@ 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 struct drm_format_modifier *format_modifiers,
> +                            unsigned int format_modifier_count,
>                              enum drm_plane_type type,
>                              const char *name, ...)
>  {
> @@ -105,6 +109,16 @@ int drm_universal_plane_init(struct drm_device *dev, struct drm_plane *plane,
>                 return -ENOMEM;
>         }
>
> +       plane->format_modifiers =
> +               kmalloc_array(format_modifier_count,
> +                             sizeof(format_modifiers[0]), GFP_KERNEL);
> +       if (!plane->format_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 +131,16 @@ int drm_universal_plane_init(struct drm_device *dev, struct drm_plane *plane,
>         }
>         if (!plane->name) {
>                 kfree(plane->format_types);
> +               kfree(plane->format_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->format_modifiers, format_modifiers,
> +              format_modifier_count * sizeof(format_modifiers[0]));
> +       plane->format_modifier_count = format_modifier_count;
>         plane->possible_crtcs = possible_crtcs;
>         plane->type = type;
>
> @@ -205,7 +223,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, 0, type, NULL);
>  }
>  EXPORT_SYMBOL(drm_plane_init);
>
> @@ -224,6 +243,7 @@ void drm_plane_cleanup(struct drm_plane *plane)
>         drm_modeset_lock_fini(&plane->mutex);
>
>         kfree(plane->format_types);
> +       kfree(plane->format_modifiers);
>         drm_mode_object_unregister(dev, &plane->base);
>
>         BUG_ON(list_empty(&plane->head));
> @@ -380,12 +400,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 +449,19 @@ int drm_mode_getplane(struct drm_device *dev, void *data,
>         }
>         plane_resp->count_format_types = plane->format_count;
>
> +       if (plane->format_modifier_count &&
> +           (plane_resp->count_format_modifiers >= plane->format_modifier_count)) {
> +               modifier_ptr = (struct drm_format_modifier __user *)
> +                       (unsigned long)plane_resp->format_modifier_ptr;
> +               if (copy_to_user(modifier_ptr,
> +                                plane->format_modifiers,
> +                                sizeof(plane->format_modifiers[0]) *
> +                                plane->format_modifier_count)) {
> +                       return -EFAULT;
> +               }
> +       }
> +       plane_resp->count_format_modifiers = plane->format_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 7bae08c..f79f4c5 100644
> --- a/drivers/gpu/drm/drm_simple_kms_helper.c
> +++ b/drivers/gpu/drm/drm_simple_kms_helper.c
> @@ -212,6 +212,8 @@ EXPORT_SYMBOL(drm_simple_display_pipe_detach_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
> + * @modifier_count: number of elements in @modifiers
>   * @connector: connector to attach and register (optional)
>   *
>   * Sets up a display pipeline which consist of a really simple
> @@ -232,6 +234,8 @@ 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 struct drm_format_modifier *format_modifiers,
> +                       unsigned int format_modifier_count,
>                         struct drm_connector *connector)
>  {
>         struct drm_encoder *encoder = &pipe->encoder;
> @@ -246,6 +250,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, format_modifier_count,
>                                        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 c2f17f3..174bec5 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, 0, 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 0a20723..1b719e0 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, 0, DRM_PLANE_TYPE_PRIMARY, NULL);
>         if (ret) {
>                 kfree(primary);
>                 primary = NULL;
> diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c
> index 135c601..9c198fb 100644
> --- a/drivers/gpu/drm/i915/intel_display.c
> +++ b/drivers/gpu/drm/i915/intel_display.c
> @@ -15026,18 +15026,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, 0,
>                                                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, 0,
>                                                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, 0,
>                                                DRM_PLANE_TYPE_PRIMARY,
>                                                "plane %c", plane_name(primary->plane));
>         if (ret)
> @@ -15201,7 +15204,7 @@ intel_cursor_plane_create(struct drm_i915_private *dev_priv, enum pipe pipe)
>                                        0, &intel_plane_funcs,
>                                        intel_cursor_formats,
>                                        ARRAY_SIZE(intel_cursor_formats),
> -                                      DRM_PLANE_TYPE_CURSOR,
> +                                      NULL, 0, DRM_PLANE_TYPE_CURSOR,
>                                        "cursor %c", pipe_name(pipe));
>         if (ret)
>                 goto fail;
> diff --git a/drivers/gpu/drm/imx/ipuv3-plane.c b/drivers/gpu/drm/imx/ipuv3-plane.c
> index 8b5294d..da2c6a7 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, 0, type, NULL);
>         if (ret) {
>                 DRM_ERROR("failed to initialize plane\n");
>                 kfree(ipu_plane);
> diff --git a/drivers/gpu/drm/mxsfb/mxsfb_drv.c b/drivers/gpu/drm/mxsfb/mxsfb_drv.c
> index 955441f..f29d463 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 cb85cb7..8b2a005 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, 0,
> +                                      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 82b2c23..0e37777 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, 0, 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 dcde628..ad611b2 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, 0, 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 fb5f001..a32281b 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, 0, 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, 0, 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 cca75bd..5463faa 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, 0,
>                                        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 877d053..8ff7e30 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, 0, 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 becf10d..7e88092 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, 0, DRM_PLANE_TYPE_OVERLAY, NULL);
>         if (res) {
>                 DRM_ERROR("Failed to initialize universal plane\n");
>                 return NULL;
> diff --git a/drivers/gpu/drm/tegra/dc.c b/drivers/gpu/drm/tegra/dc.c
> index 7561a95..9543c8c 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, 0,
> +                                      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, 0,
> +                                      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, 0,
> +                                      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 110d151..d6802f1 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, 0, 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 cb75f06..b9c9866 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, 0, type, NULL);
>         if (ret)
>                 goto err_plane_init;
>
> diff --git a/include/drm/drm_plane.h b/include/drm/drm_plane.h
> index db3bbde..7feb463 100644
> --- a/include/drm/drm_plane.h
> +++ b/include/drm/drm_plane.h
> @@ -483,6 +483,9 @@ struct drm_plane {
>         unsigned int format_count;
>         bool format_default;
>
> +       struct drm_format_modifier *format_modifiers;
> +       unsigned int format_modifier_count;
> +
>         struct drm_crtc *crtc;
>         struct drm_framebuffer *fb;
>
> @@ -510,13 +513,15 @@ struct drm_plane {
>
>  #define obj_to_plane(x) container_of(x, struct drm_plane, base)
>
> -extern __printf(8, 9)
> +extern __printf(10, 11)
>  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 struct drm_format_modifier *format_modifiers,
> +                            unsigned int format_modifier_count,
>                              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 01a8436..98997e0 100644
> --- a/include/drm/drm_simple_kms_helper.h
> +++ b/include/drm/drm_simple_kms_helper.h
> @@ -120,6 +120,8 @@ 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 struct drm_format_modifier *format_modifiers,
> +                       unsigned int format_modifier_count,
>                         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 b2c5284..487e0f1 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_mode.h b/include/uapi/drm/drm_mode.h
> index ce7efe2..cea3de3 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;
> --
> 2.9.3
>
> _______________________________________________
> dri-devel mailing list
> dri-devel@lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/dri-devel
Rob Clark April 3, 2017, 9:44 p.m. UTC | #11
On Sat, Apr 1, 2017 at 2:47 PM, Rob Clark <robdclark@gmail.com> wrote:
> On Tue, Dec 20, 2016 at 7:12 PM, Kristian H. Kristensen
> <hoegsberg@gmail.com> wrote:
>> From: "Kristian H. Kristensen" <hoegsberg@google.com>
>>
>> This new ioctl exctends DRM_IOCTL_MODE_GETPLANE, by returning
>> information about the modifiers that will work with each format.
>
> So, just to toss out a completely different idea..
>
> What if instead of a new ioctl, we had a read-only blob property
> (which encoded the info basically the same way, plus the fourcc's)?
>
> If we do writeback via some sort of OUT_FB_ID property on plane/crtc,
> we will need the same sort of format information so userspace knows
> what output formats (and modifiers) are supported.  So re-using the
> same blob property layout (and userspace parsing) seems useful.

re-sending w/ correct email for Ben..

> BR,
> -R
>
>> Signed-off-by: Kristian H. Kristensen <hoegsberg@chromium.org>
>> ---
>>  drivers/gpu/drm/arm/hdlcd_crtc.c                |  1 +
>>  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                     | 40 +++++++++++++++++++++++--
>>  drivers/gpu/drm/drm_simple_kms_helper.c         |  5 ++++
>>  drivers/gpu/drm/exynos/exynos_drm_plane.c       |  2 +-
>>  drivers/gpu/drm/fsl-dcu/fsl_dcu_drm_plane.c     |  2 +-
>>  drivers/gpu/drm/i915/intel_display.c            |  5 +++-
>>  drivers/gpu/drm/imx/ipuv3-plane.c               |  4 +--
>>  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/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/tegra/dc.c                      | 12 ++++----
>>  drivers/gpu/drm/vc4/vc4_plane.c                 |  2 +-
>>  drivers/gpu/drm/virtio/virtgpu_plane.c          |  2 +-
>>  include/drm/drm_plane.h                         |  7 ++++-
>>  include/drm/drm_simple_kms_helper.h             |  2 ++
>>  include/uapi/drm/drm.h                          |  1 +
>>  include/uapi/drm/drm_mode.h                     | 27 +++++++++++++++++
>>  27 files changed, 116 insertions(+), 28 deletions(-)
>>
>> diff --git a/drivers/gpu/drm/arm/hdlcd_crtc.c b/drivers/gpu/drm/arm/hdlcd_crtc.c
>> index 20ebfb4..6062578 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, 0,
>>                                        DRM_PLANE_TYPE_PRIMARY, NULL);
>>         if (ret) {
>>                 devm_kfree(drm->dev, plane);
>> diff --git a/drivers/gpu/drm/armada/armada_crtc.c b/drivers/gpu/drm/armada/armada_crtc.c
>> index e62ee44..e1a6170 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, 0,
>>                                        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 34cb73d..32fb8f3 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, 0,
>>                                        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 bd2791c..ac9e4db 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, 0,
>> +                                      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 59b6911..9724209 100644
>> --- a/drivers/gpu/drm/drm_ioctl.c
>> +++ b/drivers/gpu/drm/drm_ioctl.c
>> @@ -615,7 +615,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 b4f3665..94b2717 100644
>> --- a/drivers/gpu/drm/drm_modeset_helper.c
>> +++ b/drivers/gpu/drm/drm_modeset_helper.c
>> @@ -122,6 +122,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, 0,
>>                                        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 8ad20af..53a9821 100644
>> --- a/drivers/gpu/drm/drm_plane.c
>> +++ b/drivers/gpu/drm/drm_plane.c
>> @@ -70,6 +70,8 @@ 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
>> + * @format_modifier_count: number of elements in @format_modifiers
>>   * @type: type of plane (overlay, primary, cursor)
>>   * @name: printf style format string for the plane name, or NULL for default name
>>   *
>> @@ -82,6 +84,8 @@ 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 struct drm_format_modifier *format_modifiers,
>> +                            unsigned int format_modifier_count,
>>                              enum drm_plane_type type,
>>                              const char *name, ...)
>>  {
>> @@ -105,6 +109,16 @@ int drm_universal_plane_init(struct drm_device *dev, struct drm_plane *plane,
>>                 return -ENOMEM;
>>         }
>>
>> +       plane->format_modifiers =
>> +               kmalloc_array(format_modifier_count,
>> +                             sizeof(format_modifiers[0]), GFP_KERNEL);
>> +       if (!plane->format_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 +131,16 @@ int drm_universal_plane_init(struct drm_device *dev, struct drm_plane *plane,
>>         }
>>         if (!plane->name) {
>>                 kfree(plane->format_types);
>> +               kfree(plane->format_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->format_modifiers, format_modifiers,
>> +              format_modifier_count * sizeof(format_modifiers[0]));
>> +       plane->format_modifier_count = format_modifier_count;
>>         plane->possible_crtcs = possible_crtcs;
>>         plane->type = type;
>>
>> @@ -205,7 +223,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, 0, type, NULL);
>>  }
>>  EXPORT_SYMBOL(drm_plane_init);
>>
>> @@ -224,6 +243,7 @@ void drm_plane_cleanup(struct drm_plane *plane)
>>         drm_modeset_lock_fini(&plane->mutex);
>>
>>         kfree(plane->format_types);
>> +       kfree(plane->format_modifiers);
>>         drm_mode_object_unregister(dev, &plane->base);
>>
>>         BUG_ON(list_empty(&plane->head));
>> @@ -380,12 +400,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 +449,19 @@ int drm_mode_getplane(struct drm_device *dev, void *data,
>>         }
>>         plane_resp->count_format_types = plane->format_count;
>>
>> +       if (plane->format_modifier_count &&
>> +           (plane_resp->count_format_modifiers >= plane->format_modifier_count)) {
>> +               modifier_ptr = (struct drm_format_modifier __user *)
>> +                       (unsigned long)plane_resp->format_modifier_ptr;
>> +               if (copy_to_user(modifier_ptr,
>> +                                plane->format_modifiers,
>> +                                sizeof(plane->format_modifiers[0]) *
>> +                                plane->format_modifier_count)) {
>> +                       return -EFAULT;
>> +               }
>> +       }
>> +       plane_resp->count_format_modifiers = plane->format_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 7bae08c..f79f4c5 100644
>> --- a/drivers/gpu/drm/drm_simple_kms_helper.c
>> +++ b/drivers/gpu/drm/drm_simple_kms_helper.c
>> @@ -212,6 +212,8 @@ EXPORT_SYMBOL(drm_simple_display_pipe_detach_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
>> + * @modifier_count: number of elements in @modifiers
>>   * @connector: connector to attach and register (optional)
>>   *
>>   * Sets up a display pipeline which consist of a really simple
>> @@ -232,6 +234,8 @@ 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 struct drm_format_modifier *format_modifiers,
>> +                       unsigned int format_modifier_count,
>>                         struct drm_connector *connector)
>>  {
>>         struct drm_encoder *encoder = &pipe->encoder;
>> @@ -246,6 +250,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, format_modifier_count,
>>                                        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 c2f17f3..174bec5 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, 0, 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 0a20723..1b719e0 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, 0, DRM_PLANE_TYPE_PRIMARY, NULL);
>>         if (ret) {
>>                 kfree(primary);
>>                 primary = NULL;
>> diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c
>> index 135c601..9c198fb 100644
>> --- a/drivers/gpu/drm/i915/intel_display.c
>> +++ b/drivers/gpu/drm/i915/intel_display.c
>> @@ -15026,18 +15026,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, 0,
>>                                                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, 0,
>>                                                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, 0,
>>                                                DRM_PLANE_TYPE_PRIMARY,
>>                                                "plane %c", plane_name(primary->plane));
>>         if (ret)
>> @@ -15201,7 +15204,7 @@ intel_cursor_plane_create(struct drm_i915_private *dev_priv, enum pipe pipe)
>>                                        0, &intel_plane_funcs,
>>                                        intel_cursor_formats,
>>                                        ARRAY_SIZE(intel_cursor_formats),
>> -                                      DRM_PLANE_TYPE_CURSOR,
>> +                                      NULL, 0, DRM_PLANE_TYPE_CURSOR,
>>                                        "cursor %c", pipe_name(pipe));
>>         if (ret)
>>                 goto fail;
>> diff --git a/drivers/gpu/drm/imx/ipuv3-plane.c b/drivers/gpu/drm/imx/ipuv3-plane.c
>> index 8b5294d..da2c6a7 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, 0, type, NULL);
>>         if (ret) {
>>                 DRM_ERROR("failed to initialize plane\n");
>>                 kfree(ipu_plane);
>> diff --git a/drivers/gpu/drm/mxsfb/mxsfb_drv.c b/drivers/gpu/drm/mxsfb/mxsfb_drv.c
>> index 955441f..f29d463 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 cb85cb7..8b2a005 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, 0,
>> +                                      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 82b2c23..0e37777 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, 0, 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 dcde628..ad611b2 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, 0, 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 fb5f001..a32281b 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, 0, 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, 0, 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 cca75bd..5463faa 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, 0,
>>                                        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 877d053..8ff7e30 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, 0, 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 becf10d..7e88092 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, 0, DRM_PLANE_TYPE_OVERLAY, NULL);
>>         if (res) {
>>                 DRM_ERROR("Failed to initialize universal plane\n");
>>                 return NULL;
>> diff --git a/drivers/gpu/drm/tegra/dc.c b/drivers/gpu/drm/tegra/dc.c
>> index 7561a95..9543c8c 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, 0,
>> +                                      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, 0,
>> +                                      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, 0,
>> +                                      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 110d151..d6802f1 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, 0, 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 cb75f06..b9c9866 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, 0, type, NULL);
>>         if (ret)
>>                 goto err_plane_init;
>>
>> diff --git a/include/drm/drm_plane.h b/include/drm/drm_plane.h
>> index db3bbde..7feb463 100644
>> --- a/include/drm/drm_plane.h
>> +++ b/include/drm/drm_plane.h
>> @@ -483,6 +483,9 @@ struct drm_plane {
>>         unsigned int format_count;
>>         bool format_default;
>>
>> +       struct drm_format_modifier *format_modifiers;
>> +       unsigned int format_modifier_count;
>> +
>>         struct drm_crtc *crtc;
>>         struct drm_framebuffer *fb;
>>
>> @@ -510,13 +513,15 @@ struct drm_plane {
>>
>>  #define obj_to_plane(x) container_of(x, struct drm_plane, base)
>>
>> -extern __printf(8, 9)
>> +extern __printf(10, 11)
>>  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 struct drm_format_modifier *format_modifiers,
>> +                            unsigned int format_modifier_count,
>>                              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 01a8436..98997e0 100644
>> --- a/include/drm/drm_simple_kms_helper.h
>> +++ b/include/drm/drm_simple_kms_helper.h
>> @@ -120,6 +120,8 @@ 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 struct drm_format_modifier *format_modifiers,
>> +                       unsigned int format_modifier_count,
>>                         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 b2c5284..487e0f1 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_mode.h b/include/uapi/drm/drm_mode.h
>> index ce7efe2..cea3de3 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;
>> --
>> 2.9.3
>>
>> _______________________________________________
>> dri-devel mailing list
>> dri-devel@lists.freedesktop.org
>> https://lists.freedesktop.org/mailman/listinfo/dri-devel
Daniel Stone April 4, 2017, 10:07 a.m. UTC | #12
Hi,

On 1 April 2017 at 19:47, Rob Clark <robdclark@gmail.com> wrote:
> On Tue, Dec 20, 2016 at 7:12 PM, Kristian H. Kristensen
> <hoegsberg@gmail.com> wrote:
>> This new ioctl exctends DRM_IOCTL_MODE_GETPLANE, by returning
>> information about the modifiers that will work with each format.
>
> So, just to toss out a completely different idea..
>
> What if instead of a new ioctl, we had a read-only blob property
> (which encoded the info basically the same way, plus the fourcc's)?
>
> If we do writeback via some sort of OUT_FB_ID property on plane/crtc,
> we will need the same sort of format information so userspace knows
> what output formats (and modifiers) are supported.  So re-using the
> same blob property layout (and userspace parsing) seems useful.

I'd definitely be cool with this. It doesn't make our lives any easier
or more difficult in terms of parsing, and it also avoids a dep on new
libdrm API/ABI, which is always nice. If anyone types this up, I'll
happily port the Weston WIP.

Cheers,
Daniel
Ben Widawsky April 4, 2017, 7:41 p.m. UTC | #13
On 17-04-04 11:07:26, Daniel Stone wrote:
>Hi,
>
>On 1 April 2017 at 19:47, Rob Clark <robdclark@gmail.com> wrote:
>> On Tue, Dec 20, 2016 at 7:12 PM, Kristian H. Kristensen
>> <hoegsberg@gmail.com> wrote:
>>> This new ioctl exctends DRM_IOCTL_MODE_GETPLANE, by returning
>>> information about the modifiers that will work with each format.
>>
>> So, just to toss out a completely different idea..
>>
>> What if instead of a new ioctl, we had a read-only blob property
>> (which encoded the info basically the same way, plus the fourcc's)?
>>
>> If we do writeback via some sort of OUT_FB_ID property on plane/crtc,
>> we will need the same sort of format information so userspace knows
>> what output formats (and modifiers) are supported.  So re-using the
>> same blob property layout (and userspace parsing) seems useful.
>
>I'd definitely be cool with this. It doesn't make our lives any easier
>or more difficult in terms of parsing, and it also avoids a dep on new
>libdrm API/ABI, which is always nice. If anyone types this up, I'll
>happily port the Weston WIP.
>
>Cheers,
>Daniel

Okay. Sounds like we have consensus. I'm working on it now. I think like Rob
said on IRC, a good amount of it will be reusable from GET_PLANE2. I'm a bit new
to the binary blob props, so if anyone has a strong opinion on how it should
look, please speak now. Otherwise, I'll just wire up something.
Lucas Stach April 28, 2017, 12:11 p.m. UTC | #14
Hi Ben,

Am Dienstag, den 04.04.2017, 12:41 -0700 schrieb Ben Widawsky:
> On 17-04-04 11:07:26, Daniel Stone wrote:
> >Hi,
> >
> >On 1 April 2017 at 19:47, Rob Clark <robdclark@gmail.com> wrote:
> >> On Tue, Dec 20, 2016 at 7:12 PM, Kristian H. Kristensen
> >> <hoegsberg@gmail.com> wrote:
> >>> This new ioctl exctends DRM_IOCTL_MODE_GETPLANE, by returning
> >>> information about the modifiers that will work with each format.
> >>
> >> So, just to toss out a completely different idea..
> >>
> >> What if instead of a new ioctl, we had a read-only blob property
> >> (which encoded the info basically the same way, plus the fourcc's)?
> >>
> >> If we do writeback via some sort of OUT_FB_ID property on plane/crtc,
> >> we will need the same sort of format information so userspace knows
> >> what output formats (and modifiers) are supported.  So re-using the
> >> same blob property layout (and userspace parsing) seems useful.
> >
> >I'd definitely be cool with this. It doesn't make our lives any easier
> >or more difficult in terms of parsing, and it also avoids a dep on new
> >libdrm API/ABI, which is always nice. If anyone types this up, I'll
> >happily port the Weston WIP.
> >
> >Cheers,
> >Daniel
> 
> Okay. Sounds like we have consensus. I'm working on it now. I think like Rob
> said on IRC, a good amount of it will be reusable from GET_PLANE2. I'm a bit new
> to the binary blob props, so if anyone has a strong opinion on how it should
> look, please speak now. Otherwise, I'll just wire up something.
> 
Can you tell me the status of this?
I'm looking at adding tiled scanout to imx-drm and just want to know if
it's worth to hold my breath for the reworked patch to arrive.

Regards,
Lucas
Ben Widawsky April 28, 2017, 6:33 p.m. UTC | #15
On 17-04-28 14:11:56, Lucas Stach wrote:
>Hi Ben,
>
>Am Dienstag, den 04.04.2017, 12:41 -0700 schrieb Ben Widawsky:
>> On 17-04-04 11:07:26, Daniel Stone wrote:
>> >Hi,
>> >
>> >On 1 April 2017 at 19:47, Rob Clark <robdclark@gmail.com> wrote:
>> >> On Tue, Dec 20, 2016 at 7:12 PM, Kristian H. Kristensen
>> >> <hoegsberg@gmail.com> wrote:
>> >>> This new ioctl exctends DRM_IOCTL_MODE_GETPLANE, by returning
>> >>> information about the modifiers that will work with each format.
>> >>
>> >> So, just to toss out a completely different idea..
>> >>
>> >> What if instead of a new ioctl, we had a read-only blob property
>> >> (which encoded the info basically the same way, plus the fourcc's)?
>> >>
>> >> If we do writeback via some sort of OUT_FB_ID property on plane/crtc,
>> >> we will need the same sort of format information so userspace knows
>> >> what output formats (and modifiers) are supported.  So re-using the
>> >> same blob property layout (and userspace parsing) seems useful.
>> >
>> >I'd definitely be cool with this. It doesn't make our lives any easier
>> >or more difficult in terms of parsing, and it also avoids a dep on new
>> >libdrm API/ABI, which is always nice. If anyone types this up, I'll
>> >happily port the Weston WIP.
>> >
>> >Cheers,
>> >Daniel
>>
>> Okay. Sounds like we have consensus. I'm working on it now. I think like Rob
>> said on IRC, a good amount of it will be reusable from GET_PLANE2. I'm a bit new
>> to the binary blob props, so if anyone has a strong opinion on how it should
>> look, please speak now. Otherwise, I'll just wire up something.
>>
>Can you tell me the status of this?
>I'm looking at adding tiled scanout to imx-drm and just want to know if
>it's worth to hold my breath for the reworked patch to arrive.
>
>Regards,
>Lucas
>

The agreement was to use a blob property instead of GET_PLANE2. I wrote the
patches:
https://cgit.freedesktop.org/~bwidawsk/drm-intel/log/?h=blobifier

However, my test vehicle, kmscube has broken on i915, so I haven't yet been able
to test and therefore I didn't send them yet. Daniel said he'd try to wire it up
to weston next week.

I can go back and try to make it work with legacy kmscube as well, unless
someone wants to fix kmscube/i915/i965 first?
Lucas Stach May 2, 2017, 12:48 p.m. UTC | #16
Am Freitag, den 28.04.2017, 11:33 -0700 schrieb Ben Widawsky:
> On 17-04-28 14:11:56, Lucas Stach wrote:
> >Hi Ben,
> >
> >Am Dienstag, den 04.04.2017, 12:41 -0700 schrieb Ben Widawsky:
> >> On 17-04-04 11:07:26, Daniel Stone wrote:
> >> >Hi,
> >> >
> >> >On 1 April 2017 at 19:47, Rob Clark <robdclark@gmail.com> wrote:
> >> >> On Tue, Dec 20, 2016 at 7:12 PM, Kristian H. Kristensen
> >> >> <hoegsberg@gmail.com> wrote:
> >> >>> This new ioctl exctends DRM_IOCTL_MODE_GETPLANE, by returning
> >> >>> information about the modifiers that will work with each format.
> >> >>
> >> >> So, just to toss out a completely different idea..
> >> >>
> >> >> What if instead of a new ioctl, we had a read-only blob property
> >> >> (which encoded the info basically the same way, plus the fourcc's)?
> >> >>
> >> >> If we do writeback via some sort of OUT_FB_ID property on plane/crtc,
> >> >> we will need the same sort of format information so userspace knows
> >> >> what output formats (and modifiers) are supported.  So re-using the
> >> >> same blob property layout (and userspace parsing) seems useful.
> >> >
> >> >I'd definitely be cool with this. It doesn't make our lives any easier
> >> >or more difficult in terms of parsing, and it also avoids a dep on new
> >> >libdrm API/ABI, which is always nice. If anyone types this up, I'll
> >> >happily port the Weston WIP.
> >> >
> >> >Cheers,
> >> >Daniel
> >>
> >> Okay. Sounds like we have consensus. I'm working on it now. I think like Rob
> >> said on IRC, a good amount of it will be reusable from GET_PLANE2. I'm a bit new
> >> to the binary blob props, so if anyone has a strong opinion on how it should
> >> look, please speak now. Otherwise, I'll just wire up something.
> >>
> >Can you tell me the status of this?
> >I'm looking at adding tiled scanout to imx-drm and just want to know if
> >it's worth to hold my breath for the reworked patch to arrive.
> >
> >Regards,
> >Lucas
> >
> 
> The agreement was to use a blob property instead of GET_PLANE2. I wrote the
> patches:
> https://cgit.freedesktop.org/~bwidawsk/drm-intel/log/?h=blobifier
> 
> However, my test vehicle, kmscube has broken on i915, so I haven't yet been able
> to test and therefore I didn't send them yet. Daniel said he'd try to wire it up
> to weston next week.
> 
> I can go back and try to make it work with legacy kmscube as well, unless
> someone wants to fix kmscube/i915/i965 first?
> 
Thanks. I'll pull those in an try to get things working with imx-drm and
kmscube.

Regards,
Lucas
diff mbox

Patch

diff --git a/drivers/gpu/drm/arm/hdlcd_crtc.c b/drivers/gpu/drm/arm/hdlcd_crtc.c
index 20ebfb4..6062578 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, 0,
 				       DRM_PLANE_TYPE_PRIMARY, NULL);
 	if (ret) {
 		devm_kfree(drm->dev, plane);
diff --git a/drivers/gpu/drm/armada/armada_crtc.c b/drivers/gpu/drm/armada/armada_crtc.c
index e62ee44..e1a6170 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, 0,
 				       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 34cb73d..32fb8f3 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, 0,
 				       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 bd2791c..ac9e4db 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, 0,
+				       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 59b6911..9724209 100644
--- a/drivers/gpu/drm/drm_ioctl.c
+++ b/drivers/gpu/drm/drm_ioctl.c
@@ -615,7 +615,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 b4f3665..94b2717 100644
--- a/drivers/gpu/drm/drm_modeset_helper.c
+++ b/drivers/gpu/drm/drm_modeset_helper.c
@@ -122,6 +122,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, 0,
 				       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 8ad20af..53a9821 100644
--- a/drivers/gpu/drm/drm_plane.c
+++ b/drivers/gpu/drm/drm_plane.c
@@ -70,6 +70,8 @@  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
+ * @format_modifier_count: number of elements in @format_modifiers
  * @type: type of plane (overlay, primary, cursor)
  * @name: printf style format string for the plane name, or NULL for default name
  *
@@ -82,6 +84,8 @@  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 struct drm_format_modifier *format_modifiers,
+			     unsigned int format_modifier_count,
 			     enum drm_plane_type type,
 			     const char *name, ...)
 {
@@ -105,6 +109,16 @@  int drm_universal_plane_init(struct drm_device *dev, struct drm_plane *plane,
 		return -ENOMEM;
 	}
 
+	plane->format_modifiers =
+		kmalloc_array(format_modifier_count,
+			      sizeof(format_modifiers[0]), GFP_KERNEL);
+	if (!plane->format_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 +131,16 @@  int drm_universal_plane_init(struct drm_device *dev, struct drm_plane *plane,
 	}
 	if (!plane->name) {
 		kfree(plane->format_types);
+		kfree(plane->format_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->format_modifiers, format_modifiers,
+	       format_modifier_count * sizeof(format_modifiers[0]));
+	plane->format_modifier_count = format_modifier_count;
 	plane->possible_crtcs = possible_crtcs;
 	plane->type = type;
 
@@ -205,7 +223,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, 0, type, NULL);
 }
 EXPORT_SYMBOL(drm_plane_init);
 
@@ -224,6 +243,7 @@  void drm_plane_cleanup(struct drm_plane *plane)
 	drm_modeset_lock_fini(&plane->mutex);
 
 	kfree(plane->format_types);
+	kfree(plane->format_modifiers);
 	drm_mode_object_unregister(dev, &plane->base);
 
 	BUG_ON(list_empty(&plane->head));
@@ -380,12 +400,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 +449,19 @@  int drm_mode_getplane(struct drm_device *dev, void *data,
 	}
 	plane_resp->count_format_types = plane->format_count;
 
+	if (plane->format_modifier_count &&
+	    (plane_resp->count_format_modifiers >= plane->format_modifier_count)) {
+		modifier_ptr = (struct drm_format_modifier __user *)
+			(unsigned long)plane_resp->format_modifier_ptr;
+		if (copy_to_user(modifier_ptr,
+				 plane->format_modifiers,
+				 sizeof(plane->format_modifiers[0]) *
+				 plane->format_modifier_count)) {
+			return -EFAULT;
+		}
+	}
+	plane_resp->count_format_modifiers = plane->format_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 7bae08c..f79f4c5 100644
--- a/drivers/gpu/drm/drm_simple_kms_helper.c
+++ b/drivers/gpu/drm/drm_simple_kms_helper.c
@@ -212,6 +212,8 @@  EXPORT_SYMBOL(drm_simple_display_pipe_detach_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
+ * @modifier_count: number of elements in @modifiers
  * @connector: connector to attach and register (optional)
  *
  * Sets up a display pipeline which consist of a really simple
@@ -232,6 +234,8 @@  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 struct drm_format_modifier *format_modifiers,
+			unsigned int format_modifier_count,
 			struct drm_connector *connector)
 {
 	struct drm_encoder *encoder = &pipe->encoder;
@@ -246,6 +250,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, format_modifier_count,
 				       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 c2f17f3..174bec5 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, 0, 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 0a20723..1b719e0 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, 0, DRM_PLANE_TYPE_PRIMARY, NULL);
 	if (ret) {
 		kfree(primary);
 		primary = NULL;
diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c
index 135c601..9c198fb 100644
--- a/drivers/gpu/drm/i915/intel_display.c
+++ b/drivers/gpu/drm/i915/intel_display.c
@@ -15026,18 +15026,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, 0,
 					       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, 0,
 					       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, 0,
 					       DRM_PLANE_TYPE_PRIMARY,
 					       "plane %c", plane_name(primary->plane));
 	if (ret)
@@ -15201,7 +15204,7 @@  intel_cursor_plane_create(struct drm_i915_private *dev_priv, enum pipe pipe)
 				       0, &intel_plane_funcs,
 				       intel_cursor_formats,
 				       ARRAY_SIZE(intel_cursor_formats),
-				       DRM_PLANE_TYPE_CURSOR,
+				       NULL, 0, DRM_PLANE_TYPE_CURSOR,
 				       "cursor %c", pipe_name(pipe));
 	if (ret)
 		goto fail;
diff --git a/drivers/gpu/drm/imx/ipuv3-plane.c b/drivers/gpu/drm/imx/ipuv3-plane.c
index 8b5294d..da2c6a7 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, 0, type, NULL);
 	if (ret) {
 		DRM_ERROR("failed to initialize plane\n");
 		kfree(ipu_plane);
diff --git a/drivers/gpu/drm/mxsfb/mxsfb_drv.c b/drivers/gpu/drm/mxsfb/mxsfb_drv.c
index 955441f..f29d463 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 cb85cb7..8b2a005 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, 0,
+				       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 82b2c23..0e37777 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, 0, 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 dcde628..ad611b2 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, 0, 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 fb5f001..a32281b 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, 0, 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, 0, 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 cca75bd..5463faa 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, 0,
 				       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 877d053..8ff7e30 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, 0, 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 becf10d..7e88092 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, 0, DRM_PLANE_TYPE_OVERLAY, NULL);
 	if (res) {
 		DRM_ERROR("Failed to initialize universal plane\n");
 		return NULL;
diff --git a/drivers/gpu/drm/tegra/dc.c b/drivers/gpu/drm/tegra/dc.c
index 7561a95..9543c8c 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, 0,
+				       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, 0,
+				       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, 0,
+				       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 110d151..d6802f1 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, 0, 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 cb75f06..b9c9866 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, 0, type, NULL);
 	if (ret)
 		goto err_plane_init;
 
diff --git a/include/drm/drm_plane.h b/include/drm/drm_plane.h
index db3bbde..7feb463 100644
--- a/include/drm/drm_plane.h
+++ b/include/drm/drm_plane.h
@@ -483,6 +483,9 @@  struct drm_plane {
 	unsigned int format_count;
 	bool format_default;
 
+	struct drm_format_modifier *format_modifiers;
+	unsigned int format_modifier_count;
+
 	struct drm_crtc *crtc;
 	struct drm_framebuffer *fb;
 
@@ -510,13 +513,15 @@  struct drm_plane {
 
 #define obj_to_plane(x) container_of(x, struct drm_plane, base)
 
-extern __printf(8, 9)
+extern __printf(10, 11)
 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 struct drm_format_modifier *format_modifiers,
+			     unsigned int format_modifier_count,
 			     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 01a8436..98997e0 100644
--- a/include/drm/drm_simple_kms_helper.h
+++ b/include/drm/drm_simple_kms_helper.h
@@ -120,6 +120,8 @@  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 struct drm_format_modifier *format_modifiers,
+			unsigned int format_modifier_count,
 			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 b2c5284..487e0f1 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_mode.h b/include/uapi/drm/drm_mode.h
index ce7efe2..cea3de3 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;