diff mbox series

[RFC] drm/fourcc: Add a modifier for contiguous memory

Message ID 20221129101030.57499-1-randy.li@synaptics.com
State New
Headers
Series [RFC] drm/fourcc: Add a modifier for contiguous memory |

Commit Message

Hsia-Jun Li Nov. 29, 2022, 10:10 a.m. UTC 
  From: "Hsia-Jun(Randy) Li" <randy.li@synaptics.com>

Hello All

Currently, we assume all the pixel formats are multiple planes, devices
could support each component has its own memory plane.
But that may not apply for any device in the world. We could have a
device without IOMMU then this is not impossible.

Besides, when we export an handle through the PRIME, the upstream
device(likes a capture card or camera) may not support non-contiguous
memory. It would be better to allocate the handle in contiguous memory
at the first time.

We may think the memory allocation is done in user space, we could do
the trick there. But the dumb_create() sometimes is not the right API
for that.

"Note that userspace is not allowed to use such objects for render
acceleration - drivers must create their own private ioctls for such a
use case."
"Note that dumb objects may not be used for gpu acceleration, as has
been attempted on some ARM embedded platforms. Such drivers really must
have a hardware-specific ioctl to allocate suitable buffer objects."

We need to relay on those device custom APIs then. It would be helpful
for their library to calculate the right size for contiguous memory. It
would be useful for the driver supports rendering dumb buffer as well.

Signed-off-by: Hsia-Jun(Randy) Li <randy.li@synaptics.com>
---
 include/uapi/drm/drm_fourcc.h | 5 +++++
 1 file changed, 5 insertions(+)

Comments

Simon Ser Nov. 29, 2022, 10:18 a.m. UTC | #1 
Format modifiers are for the buffer layout only, not for the allocation
parameters, placement, etc. See the doc comment at the top of
drm_fourcc.h.
Daniel Stone Nov. 29, 2022, 10:42 a.m. UTC | #2 
Hi Randy,

On Tue, 29 Nov 2022 at 10:11, Hsia-Jun Li <randy.li@synaptics.com> wrote:
> Currently, we assume all the pixel formats are multiple planes, devices
> could support each component has its own memory plane.
> But that may not apply for any device in the world. We could have a
> device without IOMMU then this is not impossible.
>
> Besides, when we export an handle through the PRIME, the upstream
> device(likes a capture card or camera) may not support non-contiguous
> memory. It would be better to allocate the handle in contiguous memory
> at the first time.
>
> We may think the memory allocation is done in user space, we could do
> the trick there. But the dumb_create() sometimes is not the right API
> for that.
>
> "Note that userspace is not allowed to use such objects for render
> acceleration - drivers must create their own private ioctls for such a
> use case."
> "Note that dumb objects may not be used for gpu acceleration, as has
> been attempted on some ARM embedded platforms. Such drivers really must
> have a hardware-specific ioctl to allocate suitable buffer objects."
>
> We need to relay on those device custom APIs then. It would be helpful
> for their library to calculate the right size for contiguous memory. It
> would be useful for the driver supports rendering dumb buffer as well.

As a buffer can only have a single modifier, this isn't practical.
Contiguous needs to be negotiated separately and out of band. See e.g.
dma-heaps for this.

Cheers,
Daniel
Hsia-Jun Li Nov. 29, 2022, 10:46 a.m. UTC | #3 
On 11/29/22 18:18, Simon Ser wrote:
> CAUTION: Email originated externally, do not click links or open attachments unless you recognize the sender and know the content is safe.
> 
> 
> Format modifiers are for the buffer layout only, not for the allocation
> parameters, placement, etc. See the doc comment at the top of
> drm_fourcc.h.
In the v4l2 mail list, we have such proposal that dropping the pixel 
formats(not the codec formats) from v4l2 header completely, as the 
growing of tile pixel formats.
But we can't get rid of those variants about non-contiguous(the same 
value FOURCC in v4l2 are all for the contiguous memory).

Before I solve this problem, I believe the support for tile formats in 
v4l2 would never be stable.

The most common way here is to hack the pixel format modifier, then 
userspace library could be aware this in allocation and get properties 
of the drm_planes.

Or another way, we could add a common plane property, indicated that 
whether the driver requests contiguous memory plane for a format?
Pekka Paalanen Nov. 29, 2022, 10:48 a.m. UTC | #4 
On Tue, 29 Nov 2022 18:10:30 +0800
Hsia-Jun Li <randy.li@synaptics.com> wrote:

> From: "Hsia-Jun(Randy) Li" <randy.li@synaptics.com>
> 
> Hello All
> 
> Currently, we assume all the pixel formats are multiple planes,

Hi,

that's not true for any definition of "multiple planes" that I know of.

For example, DRM_FORMAT_XRGB8888 is a single-plane format by definition.

From below it sounds like you mean "physically non-contiguous". But no,
pixel formats make no such assumption at all. Contiguous or not is
independent of pixel formats.

> devices
> could support each component has its own memory plane.
> But that may not apply for any device in the world. We could have a
> device without IOMMU then this is not impossible.
> 
> Besides, when we export an handle through the PRIME, the upstream
> device(likes a capture card or camera) may not support non-contiguous
> memory. It would be better to allocate the handle in contiguous memory
> at the first time.
> 
> We may think the memory allocation is done in user space, we could do
> the trick there. But the dumb_create() sometimes is not the right API
> for that.
> 
> "Note that userspace is not allowed to use such objects for render
> acceleration - drivers must create their own private ioctls for such a
> use case."
> "Note that dumb objects may not be used for gpu acceleration, as has
> been attempted on some ARM embedded platforms. Such drivers really must
> have a hardware-specific ioctl to allocate suitable buffer objects."
> 
> We need to relay on those device custom APIs then. It would be helpful
> for their library to calculate the right size for contiguous memory. It
> would be useful for the driver supports rendering dumb buffer as well.
> 
> Signed-off-by: Hsia-Jun(Randy) Li <randy.li@synaptics.com>
> ---
>  include/uapi/drm/drm_fourcc.h | 5 +++++
>  1 file changed, 5 insertions(+)
> 
> diff --git a/include/uapi/drm/drm_fourcc.h b/include/uapi/drm/drm_fourcc.h
> index bc056f2d537d..ec039ced8257 100644
> --- a/include/uapi/drm/drm_fourcc.h
> +++ b/include/uapi/drm/drm_fourcc.h
> @@ -473,6 +473,11 @@ extern "C" {
>   */
>  #define DRM_FORMAT_MOD_LINEAR	fourcc_mod_code(NONE, 0)
>  
> +/*
> + * Contiguous memory
> + */
> +#define DRM_FORMAT_MOD_CONTIG_MEM	fourcc_mod_code(NONE, 1)

NAK. This is not what modifiers are for.

This also would not work in practise, because if this was a modifier,
you would not be able to use the actual modifiers.


Thanks,
pq

> +
>  /*
>   * Deprecated: use DRM_FORMAT_MOD_LINEAR instead
>   *
Hsia-Jun Li Nov. 29, 2022, 10:57 a.m. UTC | #5 
On 11/29/22 18:42, Daniel Stone wrote:
> CAUTION: Email originated externally, do not click links or open attachments unless you recognize the sender and know the content is safe.
> 
> 
> Hi Randy,
> 
> On Tue, 29 Nov 2022 at 10:11, Hsia-Jun Li <randy.li@synaptics.com> wrote:
>> Currently, we assume all the pixel formats are multiple planes, devices
>> could support each component has its own memory plane.
>> But that may not apply for any device in the world. We could have a
>> device without IOMMU then this is not impossible.
>>
>> Besides, when we export an handle through the PRIME, the upstream
>> device(likes a capture card or camera) may not support non-contiguous
>> memory. It would be better to allocate the handle in contiguous memory
>> at the first time.
>>
>> We may think the memory allocation is done in user space, we could do
>> the trick there. But the dumb_create() sometimes is not the right API
>> for that.
>>
>> "Note that userspace is not allowed to use such objects for render
>> acceleration - drivers must create their own private ioctls for such a
>> use case."
>> "Note that dumb objects may not be used for gpu acceleration, as has
>> been attempted on some ARM embedded platforms. Such drivers really must
>> have a hardware-specific ioctl to allocate suitable buffer objects."
>>
>> We need to relay on those device custom APIs then. It would be helpful
>> for their library to calculate the right size for contiguous memory. It
>> would be useful for the driver supports rendering dumb buffer as well.
> 
> As a buffer can only have a single modifier, this isn't practical.
Usually only those legacy or low cost devices would request this 
modifier. Unlikely they would support tile format(or we would add 
support for them).

But yes, we would be better not set a trap for us.
> Contiguous needs to be negotiated separately and out of band. See e.g.
> dma-heaps for this.
I don't really like the Android way here. If we are in a world of no 
hot-plug. That would be fine.

V4L2 has had a way to negotiate the memory layout it could support. 
Android gralloc would use the fixed platform sentences to decide the 
memory layout and buffer size. That is not flexible.

So would it be better that I add a common property(a list be the same 
length of the formats property) in drm_plane ?
> 
> Cheers,
> Daniel
diff mbox series

Patch

diff --git a/include/uapi/drm/drm_fourcc.h b/include/uapi/drm/drm_fourcc.h
index bc056f2d537d..ec039ced8257 100644
--- a/include/uapi/drm/drm_fourcc.h
+++ b/include/uapi/drm/drm_fourcc.h
@@ -473,6 +473,11 @@  extern "C" {
  */
 #define DRM_FORMAT_MOD_LINEAR	fourcc_mod_code(NONE, 0)
 
+/*
+ * Contiguous memory
+ */
+#define DRM_FORMAT_MOD_CONTIG_MEM	fourcc_mod_code(NONE, 1)
+
 /*
  * Deprecated: use DRM_FORMAT_MOD_LINEAR instead
  *