All of lore.kernel.org
 help / color / mirror / Atom feed
From: Harry Wentland <harry.wentland@amd.com>
To: "Ville Syrjälä" <ville.syrjala@linux.intel.com>
Cc: Deepak.Sharma@amd.com, Krunoslav.Kovac@amd.com,
	mcasas@google.com, Shashank.Sharma@amd.com,
	dri-devel@lists.freedesktop.org, Shirish.S@amd.com,
	sebastian@sebastianwick.net, Uma Shankar <uma.shankar@intel.com>,
	hersenxs.wu@amd.com, amd-gfx@lists.freedesktop.org,
	laurentiu.palcu@oss.nxp.com, Bhawanpreet.Lakha@amd.com,
	Nicholas.Kazlauskas@amd.com, Vitaly.Prosyak@amd.com
Subject: Re: [RFC PATCH 1/3] drm/color: Add RGB Color encodings
Date: Mon, 26 Apr 2021 14:56:26 -0400	[thread overview]
Message-ID: <0090ce07-6102-59e7-bc8c-3528297aa5ae@amd.com> (raw)
In-Reply-To: <YIcBUl+94sHJsT8B@intel.com>

On 2021-04-26 2:07 p.m., Ville Syrjälä wrote:
> On Mon, Apr 26, 2021 at 01:38:50PM -0400, Harry Wentland wrote:
>> From: Bhawanpreet Lakha <Bhawanpreet.Lakha@amd.com>
>>
>> Add the following color encodings
>> - RGB versions for BT601, BT709, BT2020
>> - DCI-P3: Used for digital movies
>>
>> Signed-off-by: Bhawanpreet Lakha <Bhawanpreet.Lakha@amd.com>
>> Signed-off-by: Harry Wentland <harry.wentland@amd.com>
>> ---
>>   drivers/gpu/drm/drm_color_mgmt.c | 4 ++++
>>   include/drm/drm_color_mgmt.h     | 4 ++++
>>   2 files changed, 8 insertions(+)
>>
>> diff --git a/drivers/gpu/drm/drm_color_mgmt.c b/drivers/gpu/drm/drm_color_mgmt.c
>> index bb14f488c8f6..a183ebae2941 100644
>> --- a/drivers/gpu/drm/drm_color_mgmt.c
>> +++ b/drivers/gpu/drm/drm_color_mgmt.c
>> @@ -469,6 +469,10 @@ static const char * const color_encoding_name[] = {
>>   	[DRM_COLOR_YCBCR_BT601] = "ITU-R BT.601 YCbCr",
>>   	[DRM_COLOR_YCBCR_BT709] = "ITU-R BT.709 YCbCr",
>>   	[DRM_COLOR_YCBCR_BT2020] = "ITU-R BT.2020 YCbCr",
>> +	[DRM_COLOR_RGB_BT601] = "ITU-R BT.601 RGB",
>> +	[DRM_COLOR_RGB_BT709] = "ITU-R BT.709 RGB",
>> +	[DRM_COLOR_RGB_BT2020] = "ITU-R BT.2020 RGB",
>> +	[DRM_COLOR_P3] = "DCI-P3",
> 
> These are a totally different thing than the YCbCr stuff.
> The YCbCr stuff just specifies the YCbCr<->RGB converison matrix,
> whereas these are I guess supposed to specify the primaries/whitepoint?
> But without specifying what we're converting *to* these mean absolutely
> nothing. Ie. I don't think they belong in this property.
> 

If this is the intention I don't see it documented.

I might have overlooked something but do we have a way to explicitly 
specify today what *to* format the YCbCr color encodings convert into? 
Would that be a combination of the output color encoding specified via 
colorspace_property and the color space encoded in the primaries and 
whitepoint of the hdr_output_metadata?

Fundamentally I don't see how the use of this property differs, whether 
you translate from YCbCr or from RGB. In either case you're converting 
from the defined input color space and pixel format to an output color 
space and pixel format.

> The previous proposals around this topic have suggested a new
> property to specify a conversion matrix either explicitly, or
> via a separate enum (which would specify both the src and dst
> colorspaces). I've always argued the enum approach is needed
> anyway since not all hardware has a programmable matrix for
> this. But a fully programmable matrix could be nice for tone
> mapping purposes/etc, so we may want to make sure both are
> possible.
> 
> As for the transfer func, the proposals so far have mostly just
> been to expose a programmable degamma/gamma LUTs for each plane.
> But considering how poor the current gamma uapi is we've thrown
> around some ideas how to allow the kernel to properly expose the
> hw capabilities. This is one of those ideas:
> https://lists.freedesktop.org/archives/dri-devel/2019-April/212886.html>> I think that basic idea could be also be extended to allow fixed
> curves in case the hw doesn't have a fully programmable LUT. But
> dunno if that's relevant for your hw.
> 

The problem with exposing gamma, whether per-plane or per-crtc, is that 
it is hard to define an API that works for all the HW out there. The 
capabilities for different HW differ a lot, not just between vendors but 
also between generations of a vendor's HW.

Another reason I'm proposing to define the color space (and gamma) of a 
plane is to make this explicit. Up until the color space and gamma of a 
plane or framebuffer are not well defined, which leads to drivers 
assuming the color space and gamma of a buffer (for blending and other 
purposes) and might lead to sub-optimal outcomes.

Harry

_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

WARNING: multiple messages have this Message-ID (diff)
From: Harry Wentland <harry.wentland@amd.com>
To: "Ville Syrjälä" <ville.syrjala@linux.intel.com>
Cc: Deepak.Sharma@amd.com, aric.cyr@amd.com, Krunoslav.Kovac@amd.com,
	mcasas@google.com, Shashank.Sharma@amd.com, ppaalanen@gmail.com,
	dri-devel@lists.freedesktop.org, Shirish.S@amd.com,
	sebastian@sebastianwick.net, Uma Shankar <uma.shankar@intel.com>,
	hersenxs.wu@amd.com, amd-gfx@lists.freedesktop.org,
	laurentiu.palcu@oss.nxp.com, Bhawanpreet.Lakha@amd.com,
	Nicholas.Kazlauskas@amd.com, Vitaly.Prosyak@amd.com
Subject: Re: [RFC PATCH 1/3] drm/color: Add RGB Color encodings
Date: Mon, 26 Apr 2021 14:56:26 -0400	[thread overview]
Message-ID: <0090ce07-6102-59e7-bc8c-3528297aa5ae@amd.com> (raw)
In-Reply-To: <YIcBUl+94sHJsT8B@intel.com>

On 2021-04-26 2:07 p.m., Ville Syrjälä wrote:
> On Mon, Apr 26, 2021 at 01:38:50PM -0400, Harry Wentland wrote:
>> From: Bhawanpreet Lakha <Bhawanpreet.Lakha@amd.com>
>>
>> Add the following color encodings
>> - RGB versions for BT601, BT709, BT2020
>> - DCI-P3: Used for digital movies
>>
>> Signed-off-by: Bhawanpreet Lakha <Bhawanpreet.Lakha@amd.com>
>> Signed-off-by: Harry Wentland <harry.wentland@amd.com>
>> ---
>>   drivers/gpu/drm/drm_color_mgmt.c | 4 ++++
>>   include/drm/drm_color_mgmt.h     | 4 ++++
>>   2 files changed, 8 insertions(+)
>>
>> diff --git a/drivers/gpu/drm/drm_color_mgmt.c b/drivers/gpu/drm/drm_color_mgmt.c
>> index bb14f488c8f6..a183ebae2941 100644
>> --- a/drivers/gpu/drm/drm_color_mgmt.c
>> +++ b/drivers/gpu/drm/drm_color_mgmt.c
>> @@ -469,6 +469,10 @@ static const char * const color_encoding_name[] = {
>>   	[DRM_COLOR_YCBCR_BT601] = "ITU-R BT.601 YCbCr",
>>   	[DRM_COLOR_YCBCR_BT709] = "ITU-R BT.709 YCbCr",
>>   	[DRM_COLOR_YCBCR_BT2020] = "ITU-R BT.2020 YCbCr",
>> +	[DRM_COLOR_RGB_BT601] = "ITU-R BT.601 RGB",
>> +	[DRM_COLOR_RGB_BT709] = "ITU-R BT.709 RGB",
>> +	[DRM_COLOR_RGB_BT2020] = "ITU-R BT.2020 RGB",
>> +	[DRM_COLOR_P3] = "DCI-P3",
> 
> These are a totally different thing than the YCbCr stuff.
> The YCbCr stuff just specifies the YCbCr<->RGB converison matrix,
> whereas these are I guess supposed to specify the primaries/whitepoint?
> But without specifying what we're converting *to* these mean absolutely
> nothing. Ie. I don't think they belong in this property.
> 

If this is the intention I don't see it documented.

I might have overlooked something but do we have a way to explicitly 
specify today what *to* format the YCbCr color encodings convert into? 
Would that be a combination of the output color encoding specified via 
colorspace_property and the color space encoded in the primaries and 
whitepoint of the hdr_output_metadata?

Fundamentally I don't see how the use of this property differs, whether 
you translate from YCbCr or from RGB. In either case you're converting 
from the defined input color space and pixel format to an output color 
space and pixel format.

> The previous proposals around this topic have suggested a new
> property to specify a conversion matrix either explicitly, or
> via a separate enum (which would specify both the src and dst
> colorspaces). I've always argued the enum approach is needed
> anyway since not all hardware has a programmable matrix for
> this. But a fully programmable matrix could be nice for tone
> mapping purposes/etc, so we may want to make sure both are
> possible.
> 
> As for the transfer func, the proposals so far have mostly just
> been to expose a programmable degamma/gamma LUTs for each plane.
> But considering how poor the current gamma uapi is we've thrown
> around some ideas how to allow the kernel to properly expose the
> hw capabilities. This is one of those ideas:
> https://lists.freedesktop.org/archives/dri-devel/2019-April/212886.html>> I think that basic idea could be also be extended to allow fixed
> curves in case the hw doesn't have a fully programmable LUT. But
> dunno if that's relevant for your hw.
> 

The problem with exposing gamma, whether per-plane or per-crtc, is that 
it is hard to define an API that works for all the HW out there. The 
capabilities for different HW differ a lot, not just between vendors but 
also between generations of a vendor's HW.

Another reason I'm proposing to define the color space (and gamma) of a 
plane is to make this explicit. Up until the color space and gamma of a 
plane or framebuffer are not well defined, which leads to drivers 
assuming the color space and gamma of a buffer (for blending and other 
purposes) and might lead to sub-optimal outcomes.

Harry

_______________________________________________
amd-gfx mailing list
amd-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/amd-gfx

  reply	other threads:[~2021-04-26 18:56 UTC|newest]

Thread overview: 64+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2021-04-26 17:38 [RFC PATCH 0/3] A drm_plane API to support HDR planes Harry Wentland
2021-04-26 17:38 ` Harry Wentland
2021-04-26 17:38 ` [RFC PATCH 1/3] drm/color: Add RGB Color encodings Harry Wentland
2021-04-26 17:38   ` Harry Wentland
2021-04-26 18:07   ` Ville Syrjälä
2021-04-26 18:07     ` Ville Syrjälä
2021-04-26 18:56     ` Harry Wentland [this message]
2021-04-26 18:56       ` Harry Wentland
2021-04-26 19:08       ` Ville Syrjälä
2021-04-26 19:08         ` Ville Syrjälä
2021-04-30  9:04         ` Pekka Paalanen
2021-04-30  9:04           ` Pekka Paalanen
2021-05-01  0:53       ` Sebastian Wick
2021-05-01  0:53         ` Sebastian Wick
2021-05-14 21:04         ` Harry Wentland
2021-05-14 21:04           ` Harry Wentland
2021-05-17  8:34           ` Pekka Paalanen
2021-05-17  8:34             ` Pekka Paalanen
2021-05-18 14:32             ` Harry Wentland
2021-05-18 14:32               ` Harry Wentland
2021-05-19  7:56               ` Pekka Paalanen
2021-05-19  7:56                 ` Pekka Paalanen
2021-04-26 17:38 ` [RFC PATCH 2/3] drm/color: Add Color transfer functions for HDR/SDR Harry Wentland
2021-04-26 17:38   ` Harry Wentland
2021-04-26 19:03   ` kernel test robot
2021-04-26 20:27   ` kernel test robot
2021-04-26 20:27   ` [RFC PATCH] drm/color: drm_get_color_transfer_function_name() can be static kernel test robot
2021-04-26 21:29   ` [RFC PATCH 2/3] drm/color: Add Color transfer functions for HDR/SDR kernel test robot
2021-04-26 22:00   ` kernel test robot
2021-04-26 17:38 ` [RFC PATCH 3/3] drm/color: Add sdr boost property Harry Wentland
2021-04-26 17:38   ` Harry Wentland
2021-04-26 20:38   ` kernel test robot
2021-04-26 21:45   ` kernel test robot
2021-04-26 21:45   ` [RFC PATCH] drm/color: drm_plane_create_sdr_white_level_property() can be static kernel test robot
2021-04-27  9:09 ` [RFC PATCH 0/3] A drm_plane API to support HDR planes Daniel Vetter
2021-04-27  9:09   ` Daniel Vetter
2021-04-27 14:50 ` Pekka Paalanen
2021-04-27 14:50   ` Pekka Paalanen
2021-04-28  7:54   ` Shashank Sharma
2021-04-28  7:54     ` Shashank Sharma
2021-04-30  9:43     ` Pekka Paalanen
2021-04-30  9:43       ` Pekka Paalanen
2021-04-30 10:39       ` Shashank Sharma
2021-04-30 10:39         ` Shashank Sharma
2021-05-14 21:01         ` Harry Wentland
2021-05-14 21:01           ` Harry Wentland
2021-05-14 21:05   ` Harry Wentland
2021-05-14 21:05     ` Harry Wentland
2021-05-17  8:57     ` Pekka Paalanen
2021-05-17  8:57       ` Pekka Paalanen
2021-05-17 16:48       ` Sebastian Wick
2021-05-17 16:48         ` Sebastian Wick
2021-05-17 19:39         ` Vitaly Prosyak
2021-05-17 19:39           ` Vitaly Prosyak
2021-05-18  7:56           ` Pekka Paalanen
2021-05-18  7:56             ` Pekka Paalanen
2021-05-18 14:19             ` Harry Wentland
2021-05-18 14:19               ` Harry Wentland
2021-05-18 23:00               ` Sebastian Wick
2021-05-18 23:00                 ` Sebastian Wick
2021-05-19  8:53               ` Pekka Paalanen
2021-05-19  8:53                 ` Pekka Paalanen
2021-05-19 10:02                 ` Pekka Paalanen
2021-05-19 10:02                   ` Pekka Paalanen

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=0090ce07-6102-59e7-bc8c-3528297aa5ae@amd.com \
    --to=harry.wentland@amd.com \
    --cc=Bhawanpreet.Lakha@amd.com \
    --cc=Deepak.Sharma@amd.com \
    --cc=Krunoslav.Kovac@amd.com \
    --cc=Nicholas.Kazlauskas@amd.com \
    --cc=Shashank.Sharma@amd.com \
    --cc=Shirish.S@amd.com \
    --cc=Vitaly.Prosyak@amd.com \
    --cc=amd-gfx@lists.freedesktop.org \
    --cc=dri-devel@lists.freedesktop.org \
    --cc=hersenxs.wu@amd.com \
    --cc=laurentiu.palcu@oss.nxp.com \
    --cc=mcasas@google.com \
    --cc=sebastian@sebastianwick.net \
    --cc=uma.shankar@intel.com \
    --cc=ville.syrjala@linux.intel.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
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.