All of lore.kernel.org
 help / color / mirror / Atom feed
* [PATCH 0/1] [RFC] drm/fourcc: Add new unsigned R16_UINT/RG1616_UINT formats
@ 2022-06-27 13:40 ` Dennis Tsiang
  0 siblings, 0 replies; 15+ messages in thread
From: Dennis Tsiang @ 2022-06-27 13:40 UTC (permalink / raw)
  To: dri-devel
  Cc: maarten.lankhorst, mripard, tzimmermann, airlied, daniel,
	sumit.semwal, christian.koenig, linux-kernel, linux-media,
	linaro-mm-sig, Liviu Dudau, Brian Starkey, Lisa Wu,
	Normunds Rieksts, Dennis Tsiang, nd

This patch is an early RFC to discuss the viable options and
alternatives for inclusion of unsigned integer formats for the DRM API.

This patch adds a new single component 16-bit and a two component 32-bit
DRM fourcc’s that represent unsigned integer formats. The use case for
needing UINT formats, in our case, would be to support using raw buffers
for camera ISPs.

For images imported with DRM fourcc + modifier combination, the GPU
driver needs a way to determine the datatype of the format which
currently the DRM API does not provide explicitly with a notable
exception of the floating-point fourccs such as DRM_FORMAT_XRGB16161616F
as an example. As the DRM fourccs do not currently define the
interpretation of the data, should the information be made explicit in
the DRM API similarly to how it is already done in Vulkan?

The reason for introducing datatype to the DRM fourcc's is that the
alternative, for any API (e.g., EGL) that lacks the format datatype
information for fourcc/modifier combination for dma_buf interop would be
to introduce explicit additional metadata/attributes that encode this
information which then would be passed to the GPU driver but the
drawback of this is that it would require extending multiple graphics
APIs to support every single platform.

By having the DRM API expose the datatype information for formats saves
a lot of integration/verification work for all of the different graphics
APIs and platforms as this information could be determined by the DRM
triple alone for dma_buf interop.

It would be good to gather some opinions on what others think about
introducing datatypes to the DRM API.

Any feedback and suggestions are highly appreciated.

Dennis Tsiang (1):
  [RFC] drm/fourcc: Add new unsigned R16_UINT/RG1616_UINT formats

 include/uapi/drm/drm_fourcc.h | 6 ++++--
 1 file changed, 4 insertions(+), 2 deletions(-)

--
2.36.1

IMPORTANT NOTICE: The contents of this email and any attachments are confidential and may also be privileged. If you are not the intended recipient, please notify the sender immediately and do not disclose the contents to any other person, use it for any purpose, or store or copy the information in any medium. Thank you.

^ permalink raw reply	[flat|nested] 15+ messages in thread

* [PATCH 0/1] [RFC] drm/fourcc: Add new unsigned R16_UINT/RG1616_UINT formats
@ 2022-06-27 13:40 ` Dennis Tsiang
  0 siblings, 0 replies; 15+ messages in thread
From: Dennis Tsiang @ 2022-06-27 13:40 UTC (permalink / raw)
  To: dri-devel
  Cc: Normunds Rieksts, tzimmermann, airlied, Liviu Dudau,
	Dennis Tsiang, linux-kernel, christian.koenig, linaro-mm-sig,
	Lisa Wu, nd, sumit.semwal, linux-media

This patch is an early RFC to discuss the viable options and
alternatives for inclusion of unsigned integer formats for the DRM API.

This patch adds a new single component 16-bit and a two component 32-bit
DRM fourcc’s that represent unsigned integer formats. The use case for
needing UINT formats, in our case, would be to support using raw buffers
for camera ISPs.

For images imported with DRM fourcc + modifier combination, the GPU
driver needs a way to determine the datatype of the format which
currently the DRM API does not provide explicitly with a notable
exception of the floating-point fourccs such as DRM_FORMAT_XRGB16161616F
as an example. As the DRM fourccs do not currently define the
interpretation of the data, should the information be made explicit in
the DRM API similarly to how it is already done in Vulkan?

The reason for introducing datatype to the DRM fourcc's is that the
alternative, for any API (e.g., EGL) that lacks the format datatype
information for fourcc/modifier combination for dma_buf interop would be
to introduce explicit additional metadata/attributes that encode this
information which then would be passed to the GPU driver but the
drawback of this is that it would require extending multiple graphics
APIs to support every single platform.

By having the DRM API expose the datatype information for formats saves
a lot of integration/verification work for all of the different graphics
APIs and platforms as this information could be determined by the DRM
triple alone for dma_buf interop.

It would be good to gather some opinions on what others think about
introducing datatypes to the DRM API.

Any feedback and suggestions are highly appreciated.

Dennis Tsiang (1):
  [RFC] drm/fourcc: Add new unsigned R16_UINT/RG1616_UINT formats

 include/uapi/drm/drm_fourcc.h | 6 ++++--
 1 file changed, 4 insertions(+), 2 deletions(-)

--
2.36.1

IMPORTANT NOTICE: The contents of this email and any attachments are confidential and may also be privileged. If you are not the intended recipient, please notify the sender immediately and do not disclose the contents to any other person, use it for any purpose, or store or copy the information in any medium. Thank you.

^ permalink raw reply	[flat|nested] 15+ messages in thread

* [PATCH 1/1] [RFC] drm/fourcc: Add new unsigned R16_UINT/RG1616_UINT formats
  2022-06-27 13:40 ` Dennis Tsiang
@ 2022-06-27 13:41   ` Dennis Tsiang
  -1 siblings, 0 replies; 15+ messages in thread
From: Dennis Tsiang @ 2022-06-27 13:41 UTC (permalink / raw)
  To: dri-devel
  Cc: maarten.lankhorst, mripard, tzimmermann, airlied, daniel,
	sumit.semwal, christian.koenig, linux-kernel, linux-media,
	linaro-mm-sig, Liviu Dudau, Brian Starkey, Lisa Wu,
	Normunds Rieksts, nd

Adds R16_UINT and RG1616_UINT DRM formats that represent unsigned
integer formats.

Although these formats are not used at this moment, they would need to
be exposed in the future for applications that need to use raw formats
suitable for camera ISPs

Signed-off-by: Dennis Tsiang <dennis.tsiang@arm.com>
Signed-off-by: Normunds Rieksts <normunds.rieksts@arm.com>
---
 include/uapi/drm/drm_fourcc.h | 6 ++++--
 1 file changed, 4 insertions(+), 2 deletions(-)

diff --git a/include/uapi/drm/drm_fourcc.h b/include/uapi/drm/drm_fourcc.h
index f1972154a594..fdb7d2a76507 100644
--- a/include/uapi/drm/drm_fourcc.h
+++ b/include/uapi/drm/drm_fourcc.h
@@ -112,14 +112,16 @@ extern "C" {

 /* 16 bpp Red */
 #define DRM_FORMAT_R16         fourcc_code('R', '1', '6', ' ') /* [15:0] R little endian */
+#define DRM_FORMAT_R16_UINT    fourcc_code('R', '1', '6', 'U') /* [15:0] R little endian, unsigned */

 /* 16 bpp RG */
 #define DRM_FORMAT_RG88                fourcc_code('R', 'G', '8', '8') /* [15:0] R:G 8:8 little endian */
 #define DRM_FORMAT_GR88                fourcc_code('G', 'R', '8', '8') /* [15:0] G:R 8:8 little endian */

 /* 32 bpp RG */
-#define DRM_FORMAT_RG1616      fourcc_code('R', 'G', '3', '2') /* [31:0] R:G 16:16 little endian */
-#define DRM_FORMAT_GR1616      fourcc_code('G', 'R', '3', '2') /* [31:0] G:R 16:16 little endian */
+#define DRM_FORMAT_RG1616              fourcc_code('R', 'G', '3', '2') /* [31:0] R:G 16:16 little endian */
+#define DRM_FORMAT_RG1616_UINT         fourcc_code('R', 'G', '3', 'U') /* [31:0] R:G 16:16 little endian, unsigned */
+#define DRM_FORMAT_GR1616              fourcc_code('G', 'R', '3', '2') /* [31:0] G:R 16:16 little endian */

 /* 8 bpp RGB */
 #define DRM_FORMAT_RGB332      fourcc_code('R', 'G', 'B', '8') /* [7:0] R:G:B 3:3:2 */
--
2.36.1




From: Dennis Tsiang
Sent: Monday, June 27, 2022 2:40 PM
To: dri-devel@lists.freedesktop.org <dri-devel@lists.freedesktop.org>
Cc: maarten.lankhorst@linux.intel.com <maarten.lankhorst@linux.intel.com>; mripard@kernel.org <mripard@kernel.org>; tzimmermann@suse.de <tzimmermann@suse.de>; airlied@linux.ie <airlied@linux.ie>; daniel@ffwll.ch <daniel@ffwll.ch>; sumit.semwal@linaro.org <sumit.semwal@linaro.org>; christian.koenig@amd.com <christian.koenig@amd.com>; linux-kernel@vger.kernel.org <linux-kernel@vger.kernel.org>; linux-media@vger.kernel.org <linux-media@vger.kernel.org>; linaro-mm-sig@lists.linaro.org <linaro-mm-sig@lists.linaro.org>; Liviu Dudau <Liviu.Dudau@arm.com>; Brian Starkey <Brian.Starkey@arm.com>; Lisa Wu <lisa.wu@arm.com>; Normunds Rieksts <Normunds.Rieksts@arm.com>; Dennis Tsiang <Dennis.Tsiang@arm.com>; nd <nd@arm.com>
Subject: [PATCH 0/1] [RFC] drm/fourcc: Add new unsigned R16_UINT/RG1616_UINT formats

This patch is an early RFC to discuss the viable options and
alternatives for inclusion of unsigned integer formats for the DRM API.

This patch adds a new single component 16-bit and a two component 32-bit
DRM fourcc’s that represent unsigned integer formats. The use case for
needing UINT formats, in our case, would be to support using raw buffers
for camera ISPs.

For images imported with DRM fourcc + modifier combination, the GPU
driver needs a way to determine the datatype of the format which
currently the DRM API does not provide explicitly with a notable
exception of the floating-point fourccs such as DRM_FORMAT_XRGB16161616F
as an example. As the DRM fourccs do not currently define the
interpretation of the data, should the information be made explicit in
the DRM API similarly to how it is already done in Vulkan?

The reason for introducing datatype to the DRM fourcc's is that the
alternative, for any API (e.g., EGL) that lacks the format datatype
information for fourcc/modifier combination for dma_buf interop would be
to introduce explicit additional metadata/attributes that encode this
information which then would be passed to the GPU driver but the
drawback of this is that it would require extending multiple graphics
APIs to support every single platform.

By having the DRM API expose the datatype information for formats saves
a lot of integration/verification work for all of the different graphics
APIs and platforms as this information could be determined by the DRM
triple alone for dma_buf interop.

It would be good to gather some opinions on what others think about
introducing datatypes to the DRM API.

Any feedback and suggestions are highly appreciated.

Dennis Tsiang (1):
  [RFC] drm/fourcc: Add new unsigned R16_UINT/RG1616_UINT formats

 include/uapi/drm/drm_fourcc.h | 6 ++++--
 1 file changed, 4 insertions(+), 2 deletions(-)

--
2.36.1
IMPORTANT NOTICE: The contents of this email and any attachments are confidential and may also be privileged. If you are not the intended recipient, please notify the sender immediately and do not disclose the contents to any other person, use it for any purpose, or store or copy the information in any medium. Thank you.

^ permalink raw reply related	[flat|nested] 15+ messages in thread

* [PATCH 1/1] [RFC] drm/fourcc: Add new unsigned R16_UINT/RG1616_UINT formats
@ 2022-06-27 13:41   ` Dennis Tsiang
  0 siblings, 0 replies; 15+ messages in thread
From: Dennis Tsiang @ 2022-06-27 13:41 UTC (permalink / raw)
  To: dri-devel
  Cc: Normunds Rieksts, tzimmermann, airlied, Liviu Dudau,
	linux-kernel, christian.koenig, linaro-mm-sig, Lisa Wu, nd,
	sumit.semwal, linux-media

Adds R16_UINT and RG1616_UINT DRM formats that represent unsigned
integer formats.

Although these formats are not used at this moment, they would need to
be exposed in the future for applications that need to use raw formats
suitable for camera ISPs

Signed-off-by: Dennis Tsiang <dennis.tsiang@arm.com>
Signed-off-by: Normunds Rieksts <normunds.rieksts@arm.com>
---
 include/uapi/drm/drm_fourcc.h | 6 ++++--
 1 file changed, 4 insertions(+), 2 deletions(-)

diff --git a/include/uapi/drm/drm_fourcc.h b/include/uapi/drm/drm_fourcc.h
index f1972154a594..fdb7d2a76507 100644
--- a/include/uapi/drm/drm_fourcc.h
+++ b/include/uapi/drm/drm_fourcc.h
@@ -112,14 +112,16 @@ extern "C" {

 /* 16 bpp Red */
 #define DRM_FORMAT_R16         fourcc_code('R', '1', '6', ' ') /* [15:0] R little endian */
+#define DRM_FORMAT_R16_UINT    fourcc_code('R', '1', '6', 'U') /* [15:0] R little endian, unsigned */

 /* 16 bpp RG */
 #define DRM_FORMAT_RG88                fourcc_code('R', 'G', '8', '8') /* [15:0] R:G 8:8 little endian */
 #define DRM_FORMAT_GR88                fourcc_code('G', 'R', '8', '8') /* [15:0] G:R 8:8 little endian */

 /* 32 bpp RG */
-#define DRM_FORMAT_RG1616      fourcc_code('R', 'G', '3', '2') /* [31:0] R:G 16:16 little endian */
-#define DRM_FORMAT_GR1616      fourcc_code('G', 'R', '3', '2') /* [31:0] G:R 16:16 little endian */
+#define DRM_FORMAT_RG1616              fourcc_code('R', 'G', '3', '2') /* [31:0] R:G 16:16 little endian */
+#define DRM_FORMAT_RG1616_UINT         fourcc_code('R', 'G', '3', 'U') /* [31:0] R:G 16:16 little endian, unsigned */
+#define DRM_FORMAT_GR1616              fourcc_code('G', 'R', '3', '2') /* [31:0] G:R 16:16 little endian */

 /* 8 bpp RGB */
 #define DRM_FORMAT_RGB332      fourcc_code('R', 'G', 'B', '8') /* [7:0] R:G:B 3:3:2 */
--
2.36.1




From: Dennis Tsiang
Sent: Monday, June 27, 2022 2:40 PM
To: dri-devel@lists.freedesktop.org <dri-devel@lists.freedesktop.org>
Cc: maarten.lankhorst@linux.intel.com <maarten.lankhorst@linux.intel.com>; mripard@kernel.org <mripard@kernel.org>; tzimmermann@suse.de <tzimmermann@suse.de>; airlied@linux.ie <airlied@linux.ie>; daniel@ffwll.ch <daniel@ffwll.ch>; sumit.semwal@linaro.org <sumit.semwal@linaro.org>; christian.koenig@amd.com <christian.koenig@amd.com>; linux-kernel@vger.kernel.org <linux-kernel@vger.kernel.org>; linux-media@vger.kernel.org <linux-media@vger.kernel.org>; linaro-mm-sig@lists.linaro.org <linaro-mm-sig@lists.linaro.org>; Liviu Dudau <Liviu.Dudau@arm.com>; Brian Starkey <Brian.Starkey@arm.com>; Lisa Wu <lisa.wu@arm.com>; Normunds Rieksts <Normunds.Rieksts@arm.com>; Dennis Tsiang <Dennis.Tsiang@arm.com>; nd <nd@arm.com>
Subject: [PATCH 0/1] [RFC] drm/fourcc: Add new unsigned R16_UINT/RG1616_UINT formats

This patch is an early RFC to discuss the viable options and
alternatives for inclusion of unsigned integer formats for the DRM API.

This patch adds a new single component 16-bit and a two component 32-bit
DRM fourcc’s that represent unsigned integer formats. The use case for
needing UINT formats, in our case, would be to support using raw buffers
for camera ISPs.

For images imported with DRM fourcc + modifier combination, the GPU
driver needs a way to determine the datatype of the format which
currently the DRM API does not provide explicitly with a notable
exception of the floating-point fourccs such as DRM_FORMAT_XRGB16161616F
as an example. As the DRM fourccs do not currently define the
interpretation of the data, should the information be made explicit in
the DRM API similarly to how it is already done in Vulkan?

The reason for introducing datatype to the DRM fourcc's is that the
alternative, for any API (e.g., EGL) that lacks the format datatype
information for fourcc/modifier combination for dma_buf interop would be
to introduce explicit additional metadata/attributes that encode this
information which then would be passed to the GPU driver but the
drawback of this is that it would require extending multiple graphics
APIs to support every single platform.

By having the DRM API expose the datatype information for formats saves
a lot of integration/verification work for all of the different graphics
APIs and platforms as this information could be determined by the DRM
triple alone for dma_buf interop.

It would be good to gather some opinions on what others think about
introducing datatypes to the DRM API.

Any feedback and suggestions are highly appreciated.

Dennis Tsiang (1):
  [RFC] drm/fourcc: Add new unsigned R16_UINT/RG1616_UINT formats

 include/uapi/drm/drm_fourcc.h | 6 ++++--
 1 file changed, 4 insertions(+), 2 deletions(-)

--
2.36.1
IMPORTANT NOTICE: The contents of this email and any attachments are confidential and may also be privileged. If you are not the intended recipient, please notify the sender immediately and do not disclose the contents to any other person, use it for any purpose, or store or copy the information in any medium. Thank you.

^ permalink raw reply related	[flat|nested] 15+ messages in thread

* Re: [PATCH 0/1] [RFC] drm/fourcc: Add new unsigned R16_UINT/RG1616_UINT formats
  2022-06-27 13:40 ` Dennis Tsiang
  (?)
  (?)
@ 2022-06-27 14:50 ` Pekka Paalanen
  2022-06-29 14:46     ` Dennis Tsiang
  -1 siblings, 1 reply; 15+ messages in thread
From: Pekka Paalanen @ 2022-06-27 14:50 UTC (permalink / raw)
  To: Dennis Tsiang
  Cc:  <tzimmermann@suse.de>, ,
	airlied,  <christian.koenig@amd.com>,  ,
	linux-kernel,  <linux-kernel@vger.kernel.org>,  ,
	linux-media, linaro-mm-sig, Liviu Dudau, Brian Starkey, Lisa Wu,
	Normunds Rieksts, nd,  <daniel@ffwll.ch>, ,
	sumit.semwal, dri-devel,  <sumit.semwal@linaro.org>, ,
	christian.koenig,   <mripard@kernel.org>, ,
	tzimmermann,  <airlied@linux.ie>,  ,
	daniel,  <linux-media@vger.kernel.org>,   ,
	linaro-mm-sig,
	maarten.lankhorst@linux.intel.com"         
	<maarten.lankhorst@linux.intel.com>, ,
	mripard

[-- Attachment #1: Type: text/plain, Size: 4449 bytes --]

On Mon, 27 Jun 2022 13:40:04 +0000
Dennis Tsiang <Dennis.Tsiang@arm.com> wrote:

> This patch is an early RFC to discuss the viable options and
> alternatives for inclusion of unsigned integer formats for the DRM API.
> 
> This patch adds a new single component 16-bit and a two component 32-bit
> DRM fourcc’s that represent unsigned integer formats. The use case for
> needing UINT formats, in our case, would be to support using raw buffers
> for camera ISPs.
> 
> For images imported with DRM fourcc + modifier combination, the GPU
> driver needs a way to determine the datatype of the format which
> currently the DRM API does not provide explicitly with a notable
> exception of the floating-point fourccs such as DRM_FORMAT_XRGB16161616F
> as an example. As the DRM fourccs do not currently define the
> interpretation of the data, should the information be made explicit in
> the DRM API similarly to how it is already done in Vulkan?
> 
> The reason for introducing datatype to the DRM fourcc's is that the
> alternative, for any API (e.g., EGL) that lacks the format datatype
> information for fourcc/modifier combination for dma_buf interop would be
> to introduce explicit additional metadata/attributes that encode this
> information which then would be passed to the GPU driver but the
> drawback of this is that it would require extending multiple graphics
> APIs to support every single platform.
> 
> By having the DRM API expose the datatype information for formats saves
> a lot of integration/verification work for all of the different graphics
> APIs and platforms as this information could be determined by the DRM
> triple alone for dma_buf interop.
> 
> It would be good to gather some opinions on what others think about
> introducing datatypes to the DRM API.

Hi,

I didn't quite grasp where this information is necessary, and when it
is necessary, is it that simple to communicate? Does it even belong
with the pixel format at all?

Let us consider the existing problems.

All traditional integer formats in drm_fourcc.h right now are unsigned.
They get interpreted as being in the range [0.0, 1.0] for color
operations, which means converting to another bit depth works
implicitly. That's where the simplicity ends. We assume to have full
quantization range unless otherwise noted in some auxiliary data, like
KMS properties (I forget if there even was a property to say DRM
framebuffer uses limited quantization range). We assume all pixel data
is non-linearly encoded. There is no color space information. YUV-RGB
conversion matrix coefficients are handled by a KMS property IIRC.

Coming back to the nominal range [0.0, 1.0]. That's an implicit
assumption that allows us to apply things like LUTs. It already
breaks down if you choose a floating-point format instead of unsigned
integer format. Is FP pixel value 1.0 the same as nominal 1.0? Or is
the FP pixel value 255.0 the same as nominal 1.0? KMS has no way to
know or control that AFAIK.

If you had UINT format, meaning the nominal value range is [0.0, 65535.0]
(e.g. for 16 bpc) instead of [0.0, 1.0], then how does that work with a
LUT element in the color pipeline? How would it be both meaningful and
different to existing 16 bpc integer format?

What's the actual difference between R16 and R16_UINT, what difference
does it make to a GPU driver?

Maybe that is intimately dependent on the API where the pixel formats
are used?

Maybe for KMS R16 and R16_UINT would be completely equivalent, but not
for some other API?

We also need to be very careful to not load pixel format with meaning
that does not belong there. Non-linear encoding (transfer function) is
obviously something that is completely unrelated to the pixel format,
as long as the pixel format defines a conversion to nominal value
range. Color space (primaries and white point) are another thing that
has nothing to do with pixel format, and so must not be in any way
implied by pixel format.

Should a pixel format define how the raw pixel values are converted to
nominal values?

No, because we have quantization range as a separate property,
currently with "full" and "limited" understood, where "limited" means
different things depending on the color model.

Color model is defined by the pixel formats: we have RGB and YUV
formats, likewise is chroma sub-sampling.

Hmm.


Thanks,
pq

[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 833 bytes --]

^ permalink raw reply	[flat|nested] 15+ messages in thread

* Re: [PATCH 0/1] [RFC] drm/fourcc: Add new unsigned R16_UINT/RG1616_UINT formats
  2022-06-27 14:50 ` [PATCH 0/1] " Pekka Paalanen
@ 2022-06-29 14:46     ` Dennis Tsiang
  0 siblings, 0 replies; 15+ messages in thread
From: Dennis Tsiang @ 2022-06-29 14:46 UTC (permalink / raw)
  To: Pekka Paalanen
  Cc: christian.koenig, linux-kernel, linux-media, linaro-mm-sig,
	Liviu Dudau, Brian Starkey, Lisa Wu, Normunds Rieksts, nd,
	daniel, sumit.semwal, dri-devel, sumit.semwal, christian.koenig,
	mripard, tzimmermann, airlied, daniel, maarten.lankhorst,
	mripard, david.harvey-macaulay

On 27/06/2022 15:50, Pekka Paalanen wrote:
> On Mon, 27 Jun 2022 13:40:04 +0000
> Dennis Tsiang <Dennis.Tsiang@arm.com> wrote:
>
>> This patch is an early RFC to discuss the viable options and
>> alternatives for inclusion of unsigned integer formats for the DRM API.
>>
>> This patch adds a new single component 16-bit and a two component 32-bit
>> DRM fourcc’s that represent unsigned integer formats. The use case for
>> needing UINT formats, in our case, would be to support using raw buffers
>> for camera ISPs.
>>
>> For images imported with DRM fourcc + modifier combination, the GPU
>> driver needs a way to determine the datatype of the format which
>> currently the DRM API does not provide explicitly with a notable
>> exception of the floating-point fourccs such as DRM_FORMAT_XRGB16161616F
>> as an example. As the DRM fourccs do not currently define the
>> interpretation of the data, should the information be made explicit in
>> the DRM API similarly to how it is already done in Vulkan?
>>
>> The reason for introducing datatype to the DRM fourcc's is that the
>> alternative, for any API (e.g., EGL) that lacks the format datatype
>> information for fourcc/modifier combination for dma_buf interop would be
>> to introduce explicit additional metadata/attributes that encode this
>> information which then would be passed to the GPU driver but the
>> drawback of this is that it would require extending multiple graphics
>> APIs to support every single platform.
>>
>> By having the DRM API expose the datatype information for formats saves
>> a lot of integration/verification work for all of the different graphics
>> APIs and platforms as this information could be determined by the DRM
>> triple alone for dma_buf interop.
>>
>> It would be good to gather some opinions on what others think about
>> introducing datatypes to the DRM API.
>
> Hi,
>
> I didn't quite grasp where this information is necessary, and when it
> is necessary, is it that simple to communicate? Does it even belong
> with the pixel format at all?
>
> Let us consider the existing problems.
>
> All traditional integer formats in drm_fourcc.h right now are unsigned.
> They get interpreted as being in the range [0.0, 1.0] for color
> operations, which means converting to another bit depth works
> implicitly. That's where the simplicity ends. We assume to have full
> quantization range unless otherwise noted in some auxiliary data, like
> KMS properties (I forget if there even was a property to say DRM
> framebuffer uses limited quantization range). We assume all pixel data
> is non-linearly encoded. There is no color space information. YUV-RGB
> conversion matrix coefficients are handled by a KMS property IIRC.
>
> Coming back to the nominal range [0.0, 1.0]. That's an implicit
> assumption that allows us to apply things like LUTs. It already
> breaks down if you choose a floating-point format instead of unsigned
> integer format. Is FP pixel value 1.0 the same as nominal 1.0? Or is
> the FP pixel value 255.0 the same as nominal 1.0? KMS has no way to
> know or control that AFAIK.
>
> If you had UINT format, meaning the nominal value range is [0.0, 65535.0]
> (e.g. for 16 bpc) instead of [0.0, 1.0], then how does that work with a
> LUT element in the color pipeline? How would it be both meaningful and
> different to existing 16 bpc integer format?
>
> What's the actual difference between R16 and R16_UINT, what difference
> does it make to a GPU driver?
>
> Maybe that is intimately dependent on the API where the pixel formats
> are used?
>
> Maybe for KMS R16 and R16_UINT would be completely equivalent, but not
> for some other API?
>
> We also need to be very careful to not load pixel format with meaning
> that does not belong there. Non-linear encoding (transfer function) is
> obviously something that is completely unrelated to the pixel format,
> as long as the pixel format defines a conversion to nominal value
> range. Color space (primaries and white point) are another thing that
> has nothing to do with pixel format, and so must not be in any way
> implied by pixel format.
>
> Should a pixel format define how the raw pixel values are converted to
> nominal values?
>
> No, because we have quantization range as a separate property,
> currently with "full" and "limited" understood, where "limited" means
> different things depending on the color model.
>
> Color model is defined by the pixel formats: we have RGB and YUV
> formats, likewise is chroma sub-sampling.
>
> Hmm.
>
>
> Thanks,
> pq

Hi Pekka,

Thanks for your comments. This is not intended to be used for KMS, where
indeed there would be no difference. This proposal is for other Graphics
APIs such as Vulkan, which requires the application to be explicit
upfront about how they will interpret the data, whether that be UNORM,
UINT .etc. We want to be able to import dma_bufs which create a VkImage
with a "_UINT" VkFormat. However there is currently no explicit mapping
between the DRM fourccs + modifiers combos to "_UINT" VkFormats. One
solution is to encode that into the fourccs, which is what this RFC is
proposing.

Thanks,
Dennis
IMPORTANT NOTICE: The contents of this email and any attachments are confidential and may also be privileged. If you are not the intended recipient, please notify the sender immediately and do not disclose the contents to any other person, use it for any purpose, or store or copy the information in any medium. Thank you.

^ permalink raw reply	[flat|nested] 15+ messages in thread

* Re: [PATCH 0/1] [RFC] drm/fourcc: Add new unsigned R16_UINT/RG1616_UINT formats
@ 2022-06-29 14:46     ` Dennis Tsiang
  0 siblings, 0 replies; 15+ messages in thread
From: Dennis Tsiang @ 2022-06-29 14:46 UTC (permalink / raw)
  To: Pekka Paalanen
  Cc: Normunds Rieksts, airlied, tzimmermann, Liviu Dudau,
	linux-kernel, dri-devel, christian.koenig, linaro-mm-sig,
	david.harvey-macaulay, Lisa Wu, nd, sumit.semwal, linux-media

On 27/06/2022 15:50, Pekka Paalanen wrote:
> On Mon, 27 Jun 2022 13:40:04 +0000
> Dennis Tsiang <Dennis.Tsiang@arm.com> wrote:
>
>> This patch is an early RFC to discuss the viable options and
>> alternatives for inclusion of unsigned integer formats for the DRM API.
>>
>> This patch adds a new single component 16-bit and a two component 32-bit
>> DRM fourcc’s that represent unsigned integer formats. The use case for
>> needing UINT formats, in our case, would be to support using raw buffers
>> for camera ISPs.
>>
>> For images imported with DRM fourcc + modifier combination, the GPU
>> driver needs a way to determine the datatype of the format which
>> currently the DRM API does not provide explicitly with a notable
>> exception of the floating-point fourccs such as DRM_FORMAT_XRGB16161616F
>> as an example. As the DRM fourccs do not currently define the
>> interpretation of the data, should the information be made explicit in
>> the DRM API similarly to how it is already done in Vulkan?
>>
>> The reason for introducing datatype to the DRM fourcc's is that the
>> alternative, for any API (e.g., EGL) that lacks the format datatype
>> information for fourcc/modifier combination for dma_buf interop would be
>> to introduce explicit additional metadata/attributes that encode this
>> information which then would be passed to the GPU driver but the
>> drawback of this is that it would require extending multiple graphics
>> APIs to support every single platform.
>>
>> By having the DRM API expose the datatype information for formats saves
>> a lot of integration/verification work for all of the different graphics
>> APIs and platforms as this information could be determined by the DRM
>> triple alone for dma_buf interop.
>>
>> It would be good to gather some opinions on what others think about
>> introducing datatypes to the DRM API.
>
> Hi,
>
> I didn't quite grasp where this information is necessary, and when it
> is necessary, is it that simple to communicate? Does it even belong
> with the pixel format at all?
>
> Let us consider the existing problems.
>
> All traditional integer formats in drm_fourcc.h right now are unsigned.
> They get interpreted as being in the range [0.0, 1.0] for color
> operations, which means converting to another bit depth works
> implicitly. That's where the simplicity ends. We assume to have full
> quantization range unless otherwise noted in some auxiliary data, like
> KMS properties (I forget if there even was a property to say DRM
> framebuffer uses limited quantization range). We assume all pixel data
> is non-linearly encoded. There is no color space information. YUV-RGB
> conversion matrix coefficients are handled by a KMS property IIRC.
>
> Coming back to the nominal range [0.0, 1.0]. That's an implicit
> assumption that allows us to apply things like LUTs. It already
> breaks down if you choose a floating-point format instead of unsigned
> integer format. Is FP pixel value 1.0 the same as nominal 1.0? Or is
> the FP pixel value 255.0 the same as nominal 1.0? KMS has no way to
> know or control that AFAIK.
>
> If you had UINT format, meaning the nominal value range is [0.0, 65535.0]
> (e.g. for 16 bpc) instead of [0.0, 1.0], then how does that work with a
> LUT element in the color pipeline? How would it be both meaningful and
> different to existing 16 bpc integer format?
>
> What's the actual difference between R16 and R16_UINT, what difference
> does it make to a GPU driver?
>
> Maybe that is intimately dependent on the API where the pixel formats
> are used?
>
> Maybe for KMS R16 and R16_UINT would be completely equivalent, but not
> for some other API?
>
> We also need to be very careful to not load pixel format with meaning
> that does not belong there. Non-linear encoding (transfer function) is
> obviously something that is completely unrelated to the pixel format,
> as long as the pixel format defines a conversion to nominal value
> range. Color space (primaries and white point) are another thing that
> has nothing to do with pixel format, and so must not be in any way
> implied by pixel format.
>
> Should a pixel format define how the raw pixel values are converted to
> nominal values?
>
> No, because we have quantization range as a separate property,
> currently with "full" and "limited" understood, where "limited" means
> different things depending on the color model.
>
> Color model is defined by the pixel formats: we have RGB and YUV
> formats, likewise is chroma sub-sampling.
>
> Hmm.
>
>
> Thanks,
> pq

Hi Pekka,

Thanks for your comments. This is not intended to be used for KMS, where
indeed there would be no difference. This proposal is for other Graphics
APIs such as Vulkan, which requires the application to be explicit
upfront about how they will interpret the data, whether that be UNORM,
UINT .etc. We want to be able to import dma_bufs which create a VkImage
with a "_UINT" VkFormat. However there is currently no explicit mapping
between the DRM fourccs + modifiers combos to "_UINT" VkFormats. One
solution is to encode that into the fourccs, which is what this RFC is
proposing.

Thanks,
Dennis
IMPORTANT NOTICE: The contents of this email and any attachments are confidential and may also be privileged. If you are not the intended recipient, please notify the sender immediately and do not disclose the contents to any other person, use it for any purpose, or store or copy the information in any medium. Thank you.

^ permalink raw reply	[flat|nested] 15+ messages in thread

* Re: [PATCH 0/1] [RFC] drm/fourcc: Add new unsigned R16_UINT/RG1616_UINT formats
  2022-06-29 14:46     ` Dennis Tsiang
@ 2022-06-29 14:53       ` Simon Ser
  -1 siblings, 0 replies; 15+ messages in thread
From: Simon Ser @ 2022-06-29 14:53 UTC (permalink / raw)
  To: Dennis Tsiang
  Cc: Pekka Paalanen, Normunds Rieksts, airlied, tzimmermann,
	Liviu Dudau, linux-kernel, dri-devel, christian.koenig,
	linaro-mm-sig, david.harvey-macaulay, Lisa Wu, nd, sumit.semwal,
	linux-media

On Wednesday, June 29th, 2022 at 16:46, Dennis Tsiang <dennis.tsiang@arm.com> wrote:

> Thanks for your comments. This is not intended to be used for KMS, where
> indeed there would be no difference. This proposal is for other Graphics
> APIs such as Vulkan, which requires the application to be explicit
> upfront about how they will interpret the data, whether that be UNORM,
> UINT .etc. We want to be able to import dma_bufs which create a VkImage
> with a "_UINT" VkFormat. However there is currently no explicit mapping
> between the DRM fourccs + modifiers combos to "_UINT" VkFormats. One
> solution is to encode that into the fourccs, which is what this RFC is
> proposing.

As a general comment, I don't think it's reasonable to encode all of the
VkFormat information inside DRM FourCC. For instance, VkFormat has SRGB/UNORM
variants which describe whether pixel values are electrical or optical
(IOW, EOTF-encoded or not). Moreover, other APIs may encode different
information in their format enums.

^ permalink raw reply	[flat|nested] 15+ messages in thread

* Re: [PATCH 0/1] [RFC] drm/fourcc: Add new unsigned R16_UINT/RG1616_UINT formats
@ 2022-06-29 14:53       ` Simon Ser
  0 siblings, 0 replies; 15+ messages in thread
From: Simon Ser @ 2022-06-29 14:53 UTC (permalink / raw)
  To: Dennis Tsiang
  Cc: Normunds Rieksts, airlied, Liviu Dudau, linux-kernel, dri-devel,
	sumit.semwal, linaro-mm-sig, david.harvey-macaulay,
	Pekka Paalanen, Lisa Wu, tzimmermann, nd, christian.koenig,
	linux-media

On Wednesday, June 29th, 2022 at 16:46, Dennis Tsiang <dennis.tsiang@arm.com> wrote:

> Thanks for your comments. This is not intended to be used for KMS, where
> indeed there would be no difference. This proposal is for other Graphics
> APIs such as Vulkan, which requires the application to be explicit
> upfront about how they will interpret the data, whether that be UNORM,
> UINT .etc. We want to be able to import dma_bufs which create a VkImage
> with a "_UINT" VkFormat. However there is currently no explicit mapping
> between the DRM fourccs + modifiers combos to "_UINT" VkFormats. One
> solution is to encode that into the fourccs, which is what this RFC is
> proposing.

As a general comment, I don't think it's reasonable to encode all of the
VkFormat information inside DRM FourCC. For instance, VkFormat has SRGB/UNORM
variants which describe whether pixel values are electrical or optical
(IOW, EOTF-encoded or not). Moreover, other APIs may encode different
information in their format enums.

^ permalink raw reply	[flat|nested] 15+ messages in thread

* Re: [PATCH 0/1] [RFC] drm/fourcc: Add new unsigned R16_UINT/RG1616_UINT formats
  2022-06-29 14:53       ` Simon Ser
@ 2022-06-30  7:47         ` Pekka Paalanen
  -1 siblings, 0 replies; 15+ messages in thread
From: Pekka Paalanen @ 2022-06-30  7:47 UTC (permalink / raw)
  To: Dennis Tsiang
  Cc: Simon Ser, Normunds Rieksts, airlied, tzimmermann, Liviu Dudau,
	linux-kernel, dri-devel, christian.koenig, linaro-mm-sig,
	david.harvey-macaulay, Lisa Wu, nd, sumit.semwal, linux-media

[-- Attachment #1: Type: text/plain, Size: 3462 bytes --]

On Wed, 29 Jun 2022 14:53:49 +0000
Simon Ser <contact@emersion.fr> wrote:

> On Wednesday, June 29th, 2022 at 16:46, Dennis Tsiang <dennis.tsiang@arm.com> wrote:
> 
> > Thanks for your comments. This is not intended to be used for KMS, where
> > indeed there would be no difference. This proposal is for other Graphics
> > APIs such as Vulkan, which requires the application to be explicit
> > upfront about how they will interpret the data, whether that be UNORM,
> > UINT .etc. We want to be able to import dma_bufs which create a VkImage
> > with a "_UINT" VkFormat. However there is currently no explicit mapping
> > between the DRM fourccs + modifiers combos to "_UINT" VkFormats. One
> > solution is to encode that into the fourccs, which is what this RFC is
> > proposing.  
> 
> As a general comment, I don't think it's reasonable to encode all of the
> VkFormat information inside DRM FourCC. For instance, VkFormat has SRGB/UNORM
> variants which describe whether pixel values are electrical or optical
> (IOW, EOTF-encoded or not). Moreover, other APIs may encode different
> information in their format enums.

Yeah, do not add any of that information to the DRM pixel format codes.

There is *so much* other stuff you also need to define than what's
already mentioned, and which bits you need for the API at hand depends
totally on the API at hand. After the API has defined some parts of the
metadata, the API user has to take care of the remaining parts of the
metadata in other ways, like dynamic range or color space.

Besides, when you deal with dmabuf, you already need to pass a lot of
metadata explicitly, like the pixel format, width, height, stride,
modifier, etc. so it's better to add more of those (like we will be
doing in Wayland, and not specific to dmabuf even) than to try make
pixel formats a huge mess through combinatorial explosion and sometimes
partial and sometimes conflicting image metadata.

You might be able to get a glimpse of what all metadata there could be
by reading
https://gitlab.freedesktop.org/pq/color-and-hdr/-/blob/main/doc/pixels_color.md
.

Compare Vulkan formats to e.g.
https://docs.microsoft.com/en-us/windows/win32/api/dxgicommon/ne-dxgicommon-dxgi_color_space_type
and you'll see that while DXGI color space enumeration is mostly about
other stuff, it also has overlap with Vulkan formats I think, at least
the SRGB vs. not part.

Btw. practically all buffers you see used, especially if they are 8
bpc, they are almost guaranteed to be "SRGB" non-linearly encoded, but
do you ever see that fact being explicitly communicated?

Then there is the question that if you have an SRGB-encoded buffer, do
you want to read out SRGB-encoded or linear values? That depends on
what you are doing with the buffer, so if you always mapped dmabuf to
Vulkan SRGB formats (or always to non-SRGB formats), then you need some
other way in Vulkan for the app to say whether to sample encoded or
linear (electrical or optical) values. And whether texture filtering is
done in encoded or linear space, because that makes a difference too.

IOW, there are cases where the format mapping depends on the user of the
buffer and not only on the contents of the buffer.

Therefore you simply cannot create a static mapping table between two
format definition systems when the two systems are fundamentally
different, like Vulkan and DRM fourcc.


Thanks,
pq

[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 833 bytes --]

^ permalink raw reply	[flat|nested] 15+ messages in thread

* Re: [PATCH 0/1] [RFC] drm/fourcc: Add new unsigned R16_UINT/RG1616_UINT formats
@ 2022-06-30  7:47         ` Pekka Paalanen
  0 siblings, 0 replies; 15+ messages in thread
From: Pekka Paalanen @ 2022-06-30  7:47 UTC (permalink / raw)
  To: Dennis Tsiang
  Cc: Normunds Rieksts, airlied, Liviu Dudau, linux-kernel, dri-devel,
	sumit.semwal, linaro-mm-sig, david.harvey-macaulay, Lisa Wu,
	tzimmermann, nd, christian.koenig, linux-media

[-- Attachment #1: Type: text/plain, Size: 3462 bytes --]

On Wed, 29 Jun 2022 14:53:49 +0000
Simon Ser <contact@emersion.fr> wrote:

> On Wednesday, June 29th, 2022 at 16:46, Dennis Tsiang <dennis.tsiang@arm.com> wrote:
> 
> > Thanks for your comments. This is not intended to be used for KMS, where
> > indeed there would be no difference. This proposal is for other Graphics
> > APIs such as Vulkan, which requires the application to be explicit
> > upfront about how they will interpret the data, whether that be UNORM,
> > UINT .etc. We want to be able to import dma_bufs which create a VkImage
> > with a "_UINT" VkFormat. However there is currently no explicit mapping
> > between the DRM fourccs + modifiers combos to "_UINT" VkFormats. One
> > solution is to encode that into the fourccs, which is what this RFC is
> > proposing.  
> 
> As a general comment, I don't think it's reasonable to encode all of the
> VkFormat information inside DRM FourCC. For instance, VkFormat has SRGB/UNORM
> variants which describe whether pixel values are electrical or optical
> (IOW, EOTF-encoded or not). Moreover, other APIs may encode different
> information in their format enums.

Yeah, do not add any of that information to the DRM pixel format codes.

There is *so much* other stuff you also need to define than what's
already mentioned, and which bits you need for the API at hand depends
totally on the API at hand. After the API has defined some parts of the
metadata, the API user has to take care of the remaining parts of the
metadata in other ways, like dynamic range or color space.

Besides, when you deal with dmabuf, you already need to pass a lot of
metadata explicitly, like the pixel format, width, height, stride,
modifier, etc. so it's better to add more of those (like we will be
doing in Wayland, and not specific to dmabuf even) than to try make
pixel formats a huge mess through combinatorial explosion and sometimes
partial and sometimes conflicting image metadata.

You might be able to get a glimpse of what all metadata there could be
by reading
https://gitlab.freedesktop.org/pq/color-and-hdr/-/blob/main/doc/pixels_color.md
.

Compare Vulkan formats to e.g.
https://docs.microsoft.com/en-us/windows/win32/api/dxgicommon/ne-dxgicommon-dxgi_color_space_type
and you'll see that while DXGI color space enumeration is mostly about
other stuff, it also has overlap with Vulkan formats I think, at least
the SRGB vs. not part.

Btw. practically all buffers you see used, especially if they are 8
bpc, they are almost guaranteed to be "SRGB" non-linearly encoded, but
do you ever see that fact being explicitly communicated?

Then there is the question that if you have an SRGB-encoded buffer, do
you want to read out SRGB-encoded or linear values? That depends on
what you are doing with the buffer, so if you always mapped dmabuf to
Vulkan SRGB formats (or always to non-SRGB formats), then you need some
other way in Vulkan for the app to say whether to sample encoded or
linear (electrical or optical) values. And whether texture filtering is
done in encoded or linear space, because that makes a difference too.

IOW, there are cases where the format mapping depends on the user of the
buffer and not only on the contents of the buffer.

Therefore you simply cannot create a static mapping table between two
format definition systems when the two systems are fundamentally
different, like Vulkan and DRM fourcc.


Thanks,
pq

[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 833 bytes --]

^ permalink raw reply	[flat|nested] 15+ messages in thread

* Re: [PATCH 0/1] [RFC] drm/fourcc: Add new unsigned R16_UINT/RG1616_UINT formats
  2022-06-30  7:47         ` Pekka Paalanen
@ 2022-07-15 10:20           ` Dennis Tsiang
  -1 siblings, 0 replies; 15+ messages in thread
From: Dennis Tsiang @ 2022-07-15 10:20 UTC (permalink / raw)
  To: Pekka Paalanen
  Cc: Simon Ser, Normunds Rieksts, airlied, tzimmermann, Liviu Dudau,
	linux-kernel, dri-devel, christian.koenig, linaro-mm-sig,
	david.harvey-macaulay, Lisa Wu, nd, sumit.semwal, linux-media

On 30/06/2022 08:47, Pekka Paalanen wrote:
> On Wed, 29 Jun 2022 14:53:49 +0000
> Simon Ser <contact@emersion.fr> wrote:
>
>> On Wednesday, June 29th, 2022 at 16:46, Dennis Tsiang <dennis.tsiang@arm.com> wrote:
>>
>>> Thanks for your comments. This is not intended to be used for KMS, where
>>> indeed there would be no difference. This proposal is for other Graphics
>>> APIs such as Vulkan, which requires the application to be explicit
>>> upfront about how they will interpret the data, whether that be UNORM,
>>> UINT .etc. We want to be able to import dma_bufs which create a VkImage
>>> with a "_UINT" VkFormat. However there is currently no explicit mapping
>>> between the DRM fourccs + modifiers combos to "_UINT" VkFormats. One
>>> solution is to encode that into the fourccs, which is what this RFC is
>>> proposing.
>>
>> As a general comment, I don't think it's reasonable to encode all of the
>> VkFormat information inside DRM FourCC. For instance, VkFormat has SRGB/UNORM
>> variants which describe whether pixel values are electrical or optical
>> (IOW, EOTF-encoded or not). Moreover, other APIs may encode different
>> information in their format enums.
>
> Yeah, do not add any of that information to the DRM pixel format codes.
>
> There is *so much* other stuff you also need to define than what's
> already mentioned, and which bits you need for the API at hand depends
> totally on the API at hand. After the API has defined some parts of the
> metadata, the API user has to take care of the remaining parts of the
> metadata in other ways, like dynamic range or color space.
>
> Besides, when you deal with dmabuf, you already need to pass a lot of
> metadata explicitly, like the pixel format, width, height, stride,
> modifier, etc. so it's better to add more of those (like we will be
> doing in Wayland, and not specific to dmabuf even) than to try make
> pixel formats a huge mess through combinatorial explosion and sometimes
> partial and sometimes conflicting image metadata.
>
> You might be able to get a glimpse of what all metadata there could be
> by reading
> https://gitlab.freedesktop.org/pq/color-and-hdr/-/blob/main/doc/pixels_color.md
> .
>
> Compare Vulkan formats to e.g.
> https://docs.microsoft.com/en-us/windows/win32/api/dxgicommon/ne-dxgicommon-dxgi_color_space_type
> and you'll see that while DXGI color space enumeration is mostly about
> other stuff, it also has overlap with Vulkan formats I think, at least
> the SRGB vs. not part.
>
> Btw. practically all buffers you see used, especially if they are 8
> bpc, they are almost guaranteed to be "SRGB" non-linearly encoded, but
> do you ever see that fact being explicitly communicated?
>
> Then there is the question that if you have an SRGB-encoded buffer, do
> you want to read out SRGB-encoded or linear values? That depends on
> what you are doing with the buffer, so if you always mapped dmabuf to
> Vulkan SRGB formats (or always to non-SRGB formats), then you need some
> other way in Vulkan for the app to say whether to sample encoded or
> linear (electrical or optical) values. And whether texture filtering is
> done in encoded or linear space, because that makes a difference too.
>
> IOW, there are cases where the format mapping depends on the user of the
> buffer and not only on the contents of the buffer.
>
> Therefore you simply cannot create a static mapping table between two
> format definition systems when the two systems are fundamentally
> different, like Vulkan and DRM fourcc.
>
>
> Thanks,
> pq

Thanks all for your comments. We'll look into an alternative approach to
achieve what we need.
IMPORTANT NOTICE: The contents of this email and any attachments are confidential and may also be privileged. If you are not the intended recipient, please notify the sender immediately and do not disclose the contents to any other person, use it for any purpose, or store or copy the information in any medium. Thank you.

^ permalink raw reply	[flat|nested] 15+ messages in thread

* Re: [PATCH 0/1] [RFC] drm/fourcc: Add new unsigned R16_UINT/RG1616_UINT formats
@ 2022-07-15 10:20           ` Dennis Tsiang
  0 siblings, 0 replies; 15+ messages in thread
From: Dennis Tsiang @ 2022-07-15 10:20 UTC (permalink / raw)
  To: Pekka Paalanen
  Cc: Normunds Rieksts, airlied, Liviu Dudau, linux-kernel, dri-devel,
	sumit.semwal, linaro-mm-sig, david.harvey-macaulay, Lisa Wu,
	tzimmermann, nd, christian.koenig, linux-media

On 30/06/2022 08:47, Pekka Paalanen wrote:
> On Wed, 29 Jun 2022 14:53:49 +0000
> Simon Ser <contact@emersion.fr> wrote:
>
>> On Wednesday, June 29th, 2022 at 16:46, Dennis Tsiang <dennis.tsiang@arm.com> wrote:
>>
>>> Thanks for your comments. This is not intended to be used for KMS, where
>>> indeed there would be no difference. This proposal is for other Graphics
>>> APIs such as Vulkan, which requires the application to be explicit
>>> upfront about how they will interpret the data, whether that be UNORM,
>>> UINT .etc. We want to be able to import dma_bufs which create a VkImage
>>> with a "_UINT" VkFormat. However there is currently no explicit mapping
>>> between the DRM fourccs + modifiers combos to "_UINT" VkFormats. One
>>> solution is to encode that into the fourccs, which is what this RFC is
>>> proposing.
>>
>> As a general comment, I don't think it's reasonable to encode all of the
>> VkFormat information inside DRM FourCC. For instance, VkFormat has SRGB/UNORM
>> variants which describe whether pixel values are electrical or optical
>> (IOW, EOTF-encoded or not). Moreover, other APIs may encode different
>> information in their format enums.
>
> Yeah, do not add any of that information to the DRM pixel format codes.
>
> There is *so much* other stuff you also need to define than what's
> already mentioned, and which bits you need for the API at hand depends
> totally on the API at hand. After the API has defined some parts of the
> metadata, the API user has to take care of the remaining parts of the
> metadata in other ways, like dynamic range or color space.
>
> Besides, when you deal with dmabuf, you already need to pass a lot of
> metadata explicitly, like the pixel format, width, height, stride,
> modifier, etc. so it's better to add more of those (like we will be
> doing in Wayland, and not specific to dmabuf even) than to try make
> pixel formats a huge mess through combinatorial explosion and sometimes
> partial and sometimes conflicting image metadata.
>
> You might be able to get a glimpse of what all metadata there could be
> by reading
> https://gitlab.freedesktop.org/pq/color-and-hdr/-/blob/main/doc/pixels_color.md
> .
>
> Compare Vulkan formats to e.g.
> https://docs.microsoft.com/en-us/windows/win32/api/dxgicommon/ne-dxgicommon-dxgi_color_space_type
> and you'll see that while DXGI color space enumeration is mostly about
> other stuff, it also has overlap with Vulkan formats I think, at least
> the SRGB vs. not part.
>
> Btw. practically all buffers you see used, especially if they are 8
> bpc, they are almost guaranteed to be "SRGB" non-linearly encoded, but
> do you ever see that fact being explicitly communicated?
>
> Then there is the question that if you have an SRGB-encoded buffer, do
> you want to read out SRGB-encoded or linear values? That depends on
> what you are doing with the buffer, so if you always mapped dmabuf to
> Vulkan SRGB formats (or always to non-SRGB formats), then you need some
> other way in Vulkan for the app to say whether to sample encoded or
> linear (electrical or optical) values. And whether texture filtering is
> done in encoded or linear space, because that makes a difference too.
>
> IOW, there are cases where the format mapping depends on the user of the
> buffer and not only on the contents of the buffer.
>
> Therefore you simply cannot create a static mapping table between two
> format definition systems when the two systems are fundamentally
> different, like Vulkan and DRM fourcc.
>
>
> Thanks,
> pq

Thanks all for your comments. We'll look into an alternative approach to
achieve what we need.
IMPORTANT NOTICE: The contents of this email and any attachments are confidential and may also be privileged. If you are not the intended recipient, please notify the sender immediately and do not disclose the contents to any other person, use it for any purpose, or store or copy the information in any medium. Thank you.

^ permalink raw reply	[flat|nested] 15+ messages in thread

* Re: [PATCH 0/1] [RFC] drm/fourcc: Add new unsigned R16_UINT/RG1616_UINT formats
  2022-07-15 10:20           ` Dennis Tsiang
@ 2022-08-09 16:32             ` Jason Ekstrand
  -1 siblings, 0 replies; 15+ messages in thread
From: Jason Ekstrand @ 2022-08-09 16:32 UTC (permalink / raw)
  To: Dennis Tsiang, Pekka Paalanen
  Cc: Normunds Rieksts, airlied, Liviu Dudau, linux-kernel, dri-devel,
	sumit.semwal, linaro-mm-sig, david.harvey-macaulay, Lisa Wu,
	tzimmermann, nd, christian.koenig, linux-media

On Fri, 2022-07-15 at 11:20 +0100, Dennis Tsiang wrote:
> On 30/06/2022 08:47, Pekka Paalanen wrote:
> > On Wed, 29 Jun 2022 14:53:49 +0000
> > Simon Ser <contact@emersion.fr> wrote:
> > 
> > > On Wednesday, June 29th, 2022 at 16:46, Dennis Tsiang
> > > <dennis.tsiang@arm.com> wrote:
> > > 
> > > > Thanks for your comments. This is not intended to be used for
> > > > KMS, where
> > > > indeed there would be no difference. This proposal is for other
> > > > Graphics
> > > > APIs such as Vulkan, which requires the application to be
> > > > explicit
> > > > upfront about how they will interpret the data, whether that be
> > > > UNORM,
> > > > UINT .etc. We want to be able to import dma_bufs which create a
> > > > VkImage
> > > > with a "_UINT" VkFormat. However there is currently no explicit
> > > > mapping
> > > > between the DRM fourccs + modifiers combos to "_UINT"
> > > > VkFormats. One
> > > > solution is to encode that into the fourccs, which is what this
> > > > RFC is
> > > > proposing.
> > > 
> > > As a general comment, I don't think it's reasonable to encode all
> > > of the
> > > VkFormat information inside DRM FourCC. For instance, VkFormat
> > > has SRGB/UNORM
> > > variants which describe whether pixel values are electrical or
> > > optical
> > > (IOW, EOTF-encoded or not). Moreover, other APIs may encode
> > > different
> > > information in their format enums.
> > 
> > Yeah, do not add any of that information to the DRM pixel format
> > codes.
> > 
> > There is *so much* other stuff you also need to define than what's
> > already mentioned, and which bits you need for the API at hand
> > depends
> > totally on the API at hand. After the API has defined some parts of
> > the
> > metadata, the API user has to take care of the remaining parts of
> > the
> > metadata in other ways, like dynamic range or color space.
> > 
> > Besides, when you deal with dmabuf, you already need to pass a lot
> > of
> > metadata explicitly, like the pixel format, width, height, stride,
> > modifier, etc. so it's better to add more of those (like we will be
> > doing in Wayland, and not specific to dmabuf even) than to try make
> > pixel formats a huge mess through combinatorial explosion and
> > sometimes
> > partial and sometimes conflicting image metadata.
> > 
> > You might be able to get a glimpse of what all metadata there could
> > be
> > by reading
> > https://gitlab.freedesktop.org/pq/color-and-hdr/-/blob/main/doc/pixels_color.md
> > .
> > 
> > Compare Vulkan formats to e.g.
> > https://docs.microsoft.com/en-us/windows/win32/api/dxgicommon/ne-dxgicommon-dxgi_color_space_type
> > and you'll see that while DXGI color space enumeration is mostly
> > about
> > other stuff, it also has overlap with Vulkan formats I think, at
> > least
> > the SRGB vs. not part.
> > 
> > Btw. practically all buffers you see used, especially if they are 8
> > bpc, they are almost guaranteed to be "SRGB" non-linearly encoded,
> > but
> > do you ever see that fact being explicitly communicated?
> > 
> > Then there is the question that if you have an SRGB-encoded buffer,
> > do
> > you want to read out SRGB-encoded or linear values? That depends on
> > what you are doing with the buffer, so if you always mapped dmabuf
> > to
> > Vulkan SRGB formats (or always to non-SRGB formats), then you need
> > some
> > other way in Vulkan for the app to say whether to sample encoded or
> > linear (electrical or optical) values. And whether texture
> > filtering is
> > done in encoded or linear space, because that makes a difference
> > too.
> > 
> > IOW, there are cases where the format mapping depends on the user
> > of the
> > buffer and not only on the contents of the buffer.
> > 
> > Therefore you simply cannot create a static mapping table between
> > two
> > format definition systems when the two systems are fundamentally
> > different, like Vulkan and DRM fourcc.
> > 
> > 
> > Thanks,
> > pq
> 
> Thanks all for your comments. We'll look into an alternative approach
> to
> achieve what we need.

I mostly agree with Pekka here.  The fourcc formats as they currently
are defined only specify a bit pattern and channel order, not an
interpretation.  Vulkan formats, on the other hand, have everything you
need in order to know how to convert float vec4s to/from that format in
a shader.  That's not the same as knowing what the data represents
(colorspace, wite values, etc.) but it's a lot more than fourcc.

That said, the Vulkan APIs for querying modifier support will give you
much more fine-grained information about exactly the Vulkan formats you
request.  So if you ask for modifier support for VK_FORMAT_R16G16_UINT,
that's what you'll get.  I *think* it should be fine to use
VK_FORMAT_R16G16_UINT with DRM_FOURCC_GR1616.  Of course, the API on
the other side will also need a more precise format specifier than
fourcc if it's to know the difference between R16G16_UINT and
R16G16_UNORM.

--Jason


^ permalink raw reply	[flat|nested] 15+ messages in thread

* Re: [PATCH 0/1] [RFC] drm/fourcc: Add new unsigned R16_UINT/RG1616_UINT formats
@ 2022-08-09 16:32             ` Jason Ekstrand
  0 siblings, 0 replies; 15+ messages in thread
From: Jason Ekstrand @ 2022-08-09 16:32 UTC (permalink / raw)
  To: Dennis Tsiang, Pekka Paalanen
  Cc: Normunds Rieksts, airlied, Liviu Dudau, linux-kernel, dri-devel,
	christian.koenig, linaro-mm-sig, david.harvey-macaulay, Lisa Wu,
	tzimmermann, nd, sumit.semwal, linux-media

On Fri, 2022-07-15 at 11:20 +0100, Dennis Tsiang wrote:
> On 30/06/2022 08:47, Pekka Paalanen wrote:
> > On Wed, 29 Jun 2022 14:53:49 +0000
> > Simon Ser <contact@emersion.fr> wrote:
> > 
> > > On Wednesday, June 29th, 2022 at 16:46, Dennis Tsiang
> > > <dennis.tsiang@arm.com> wrote:
> > > 
> > > > Thanks for your comments. This is not intended to be used for
> > > > KMS, where
> > > > indeed there would be no difference. This proposal is for other
> > > > Graphics
> > > > APIs such as Vulkan, which requires the application to be
> > > > explicit
> > > > upfront about how they will interpret the data, whether that be
> > > > UNORM,
> > > > UINT .etc. We want to be able to import dma_bufs which create a
> > > > VkImage
> > > > with a "_UINT" VkFormat. However there is currently no explicit
> > > > mapping
> > > > between the DRM fourccs + modifiers combos to "_UINT"
> > > > VkFormats. One
> > > > solution is to encode that into the fourccs, which is what this
> > > > RFC is
> > > > proposing.
> > > 
> > > As a general comment, I don't think it's reasonable to encode all
> > > of the
> > > VkFormat information inside DRM FourCC. For instance, VkFormat
> > > has SRGB/UNORM
> > > variants which describe whether pixel values are electrical or
> > > optical
> > > (IOW, EOTF-encoded or not). Moreover, other APIs may encode
> > > different
> > > information in their format enums.
> > 
> > Yeah, do not add any of that information to the DRM pixel format
> > codes.
> > 
> > There is *so much* other stuff you also need to define than what's
> > already mentioned, and which bits you need for the API at hand
> > depends
> > totally on the API at hand. After the API has defined some parts of
> > the
> > metadata, the API user has to take care of the remaining parts of
> > the
> > metadata in other ways, like dynamic range or color space.
> > 
> > Besides, when you deal with dmabuf, you already need to pass a lot
> > of
> > metadata explicitly, like the pixel format, width, height, stride,
> > modifier, etc. so it's better to add more of those (like we will be
> > doing in Wayland, and not specific to dmabuf even) than to try make
> > pixel formats a huge mess through combinatorial explosion and
> > sometimes
> > partial and sometimes conflicting image metadata.
> > 
> > You might be able to get a glimpse of what all metadata there could
> > be
> > by reading
> > https://gitlab.freedesktop.org/pq/color-and-hdr/-/blob/main/doc/pixels_color.md
> > .
> > 
> > Compare Vulkan formats to e.g.
> > https://docs.microsoft.com/en-us/windows/win32/api/dxgicommon/ne-dxgicommon-dxgi_color_space_type
> > and you'll see that while DXGI color space enumeration is mostly
> > about
> > other stuff, it also has overlap with Vulkan formats I think, at
> > least
> > the SRGB vs. not part.
> > 
> > Btw. practically all buffers you see used, especially if they are 8
> > bpc, they are almost guaranteed to be "SRGB" non-linearly encoded,
> > but
> > do you ever see that fact being explicitly communicated?
> > 
> > Then there is the question that if you have an SRGB-encoded buffer,
> > do
> > you want to read out SRGB-encoded or linear values? That depends on
> > what you are doing with the buffer, so if you always mapped dmabuf
> > to
> > Vulkan SRGB formats (or always to non-SRGB formats), then you need
> > some
> > other way in Vulkan for the app to say whether to sample encoded or
> > linear (electrical or optical) values. And whether texture
> > filtering is
> > done in encoded or linear space, because that makes a difference
> > too.
> > 
> > IOW, there are cases where the format mapping depends on the user
> > of the
> > buffer and not only on the contents of the buffer.
> > 
> > Therefore you simply cannot create a static mapping table between
> > two
> > format definition systems when the two systems are fundamentally
> > different, like Vulkan and DRM fourcc.
> > 
> > 
> > Thanks,
> > pq
> 
> Thanks all for your comments. We'll look into an alternative approach
> to
> achieve what we need.

I mostly agree with Pekka here.  The fourcc formats as they currently
are defined only specify a bit pattern and channel order, not an
interpretation.  Vulkan formats, on the other hand, have everything you
need in order to know how to convert float vec4s to/from that format in
a shader.  That's not the same as knowing what the data represents
(colorspace, wite values, etc.) but it's a lot more than fourcc.

That said, the Vulkan APIs for querying modifier support will give you
much more fine-grained information about exactly the Vulkan formats you
request.  So if you ask for modifier support for VK_FORMAT_R16G16_UINT,
that's what you'll get.  I *think* it should be fine to use
VK_FORMAT_R16G16_UINT with DRM_FOURCC_GR1616.  Of course, the API on
the other side will also need a more precise format specifier than
fourcc if it's to know the difference between R16G16_UINT and
R16G16_UNORM.

--Jason


^ permalink raw reply	[flat|nested] 15+ messages in thread

end of thread, other threads:[~2022-08-09 16:33 UTC | newest]

Thread overview: 15+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2022-06-27 13:40 [PATCH 0/1] [RFC] drm/fourcc: Add new unsigned R16_UINT/RG1616_UINT formats Dennis Tsiang
2022-06-27 13:40 ` Dennis Tsiang
2022-06-27 13:41 ` [PATCH 1/1] " Dennis Tsiang
2022-06-27 13:41   ` Dennis Tsiang
2022-06-27 14:50 ` [PATCH 0/1] " Pekka Paalanen
2022-06-29 14:46   ` Dennis Tsiang
2022-06-29 14:46     ` Dennis Tsiang
2022-06-29 14:53     ` Simon Ser
2022-06-29 14:53       ` Simon Ser
2022-06-30  7:47       ` Pekka Paalanen
2022-06-30  7:47         ` Pekka Paalanen
2022-07-15 10:20         ` Dennis Tsiang
2022-07-15 10:20           ` Dennis Tsiang
2022-08-09 16:32           ` Jason Ekstrand
2022-08-09 16:32             ` Jason Ekstrand

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.