| Message ID | 20221129101030.57499-1-randy.li@synaptics.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | [RFC] drm/fourcc: Add a modifier for contiguous memory | expand |
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.
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
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?
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 > *
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 --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 *