Hi,
This patch series adds rgb input buffer range property to exynos_mixer.
It applies on top of drm-exynos/exynos-drm-next with patches:
- [PATCH 1/2] drm/edid: Add colorimetry helper function,
- [PATCH 2/2] drm/exynos: mixer: Use colorimetry helper function
Patch 1 Add apropriate labels
Patch 2 Split color properties function
Patch 3 Add default range field to exynos_drm_plane config
Patch 4 Minor code cleanup
Patch 5 Add optional range property to exynos plane
Patch 6 Make input rgb range configurable in exynos_mixer
Regards,
Chris
Christoph Manszewski (6):
include/drm: color_mgmt: Add enum labels
drm: color_mgmt: Split create_color_properties function
drm/exynos: drm_drv: Extend exynos_drm_plane_config
drm/exynos: plane: Minor cleanup
drm/exynos: plane: Add range property to exynos plane
drm/exynos: mixer: Make input buffer color range configurable
drivers/gpu/drm/drm_color_mgmt.c | 93 ++++++++++++++++++++++++-------
drivers/gpu/drm/exynos/exynos_drm_drv.h | 2 +
drivers/gpu/drm/exynos/exynos_drm_plane.c | 15 ++++-
drivers/gpu/drm/exynos/exynos_mixer.c | 57 +++++++++++++++++--
drivers/gpu/drm/exynos/regs-mixer.h | 2 +
include/drm/drm_color_mgmt.h | 10 ++++
6 files changed, 152 insertions(+), 27 deletions(-)
--
2.7.4
Use available drm_plane pointer, for consistency.
Signed-off-by: Christoph Manszewski <[email protected]>
---
drivers/gpu/drm/exynos/exynos_drm_plane.c | 6 +++---
1 file changed, 3 insertions(+), 3 deletions(-)
diff --git a/drivers/gpu/drm/exynos/exynos_drm_plane.c b/drivers/gpu/drm/exynos/exynos_drm_plane.c
index df0508e0e49e..cf352d980eee 100644
--- a/drivers/gpu/drm/exynos/exynos_drm_plane.c
+++ b/drivers/gpu/drm/exynos/exynos_drm_plane.c
@@ -303,7 +303,7 @@ int exynos_plane_init(struct drm_device *dev,
BIT(DRM_MODE_BLEND_COVERAGE);
struct drm_plane *plane = &exynos_plane->base;
- err = drm_universal_plane_init(dev, &exynos_plane->base,
+ err = drm_universal_plane_init(dev, plane,
1 << dev->mode_config.num_crtc,
&exynos_plane_funcs,
config->pixel_formats,
@@ -314,12 +314,12 @@ int exynos_plane_init(struct drm_device *dev,
return err;
}
- drm_plane_helper_add(&exynos_plane->base, &plane_helper_funcs);
+ drm_plane_helper_add(plane, &plane_helper_funcs);
exynos_plane->index = index;
exynos_plane->config = config;
- exynos_plane_attach_zpos_property(&exynos_plane->base, config->zpos,
+ exynos_plane_attach_zpos_property(plane, config->zpos,
!(config->capabilities & EXYNOS_DRM_PLANE_CAP_ZPOS));
if (config->capabilities & EXYNOS_DRM_PLANE_CAP_PIX_BLEND)
--
2.7.4
Create range property within exynos_drm_plane_init and set to default value
in exynos_drm_plane_reset.
Signed-off-by: Christoph Manszewski <[email protected]>
---
drivers/gpu/drm/exynos/exynos_drm_plane.c | 9 +++++++++
1 file changed, 9 insertions(+)
diff --git a/drivers/gpu/drm/exynos/exynos_drm_plane.c b/drivers/gpu/drm/exynos/exynos_drm_plane.c
index cf352d980eee..a6ab0e7277f0 100644
--- a/drivers/gpu/drm/exynos/exynos_drm_plane.c
+++ b/drivers/gpu/drm/exynos/exynos_drm_plane.c
@@ -140,6 +140,7 @@ static void exynos_drm_plane_reset(struct drm_plane *plane)
if (exynos_state) {
__drm_atomic_helper_plane_reset(plane, &exynos_state->base);
plane->state->zpos = exynos_plane->config->zpos;
+ plane->state->color_range = exynos_plane->config->range;
}
}
@@ -301,6 +302,10 @@ int exynos_plane_init(struct drm_device *dev,
unsigned int supported_modes = BIT(DRM_MODE_BLEND_PIXEL_NONE) |
BIT(DRM_MODE_BLEND_PREMULTI) |
BIT(DRM_MODE_BLEND_COVERAGE);
+
+ u32 supported_ranges = BIT(DRM_COLOR_LIMITED_RANGE) |
+ BIT(DRM_COLOR_FULL_RANGE);
+
struct drm_plane *plane = &exynos_plane->base;
err = drm_universal_plane_init(dev, plane,
@@ -328,5 +333,9 @@ int exynos_plane_init(struct drm_device *dev,
if (config->capabilities & EXYNOS_DRM_PLANE_CAP_WIN_BLEND)
drm_plane_create_alpha_property(plane);
+ if (config->capabilities & EXYNOS_DRM_PLANE_CAP_RANGE)
+ drm_plane_create_range_property(plane, supported_ranges,
+ config->range);
+
return 0;
}
--
2.7.4
Define new range capability and add range value to
exynos_drm_plane_config.
Signed-off-by: Christoph Manszewski <[email protected]>
---
drivers/gpu/drm/exynos/exynos_drm_drv.h | 2 ++
1 file changed, 2 insertions(+)
diff --git a/drivers/gpu/drm/exynos/exynos_drm_drv.h b/drivers/gpu/drm/exynos/exynos_drm_drv.h
index 5e61e707f955..1bc334d846c5 100644
--- a/drivers/gpu/drm/exynos/exynos_drm_drv.h
+++ b/drivers/gpu/drm/exynos/exynos_drm_drv.h
@@ -94,6 +94,7 @@ struct exynos_drm_plane {
#define EXYNOS_DRM_PLANE_CAP_TILE (1 << 3)
#define EXYNOS_DRM_PLANE_CAP_PIX_BLEND (1 << 4)
#define EXYNOS_DRM_PLANE_CAP_WIN_BLEND (1 << 5)
+#define EXYNOS_DRM_PLANE_CAP_RANGE (1 << 6)
/*
* Exynos DRM plane configuration structure.
@@ -111,6 +112,7 @@ struct exynos_drm_plane_config {
const uint32_t *pixel_formats;
unsigned int num_pixel_formats;
unsigned int capabilities;
+ enum drm_color_range range;
};
/*
--
2.7.4
Mixer hardware supports RGB input buffers with full and limited range.
Set the csc matrix accordingly to chosen range. Note that range setting
has to be equal for both graphic layers.
Signed-off-by: Christoph Manszewski <[email protected]>
---
drivers/gpu/drm/exynos/exynos_mixer.c | 57 ++++++++++++++++++++++++++++++++---
drivers/gpu/drm/exynos/regs-mixer.h | 2 ++
2 files changed, 55 insertions(+), 4 deletions(-)
diff --git a/drivers/gpu/drm/exynos/exynos_mixer.c b/drivers/gpu/drm/exynos/exynos_mixer.c
index 895c6268025d..035d635188d7 100644
--- a/drivers/gpu/drm/exynos/exynos_mixer.c
+++ b/drivers/gpu/drm/exynos/exynos_mixer.c
@@ -35,6 +35,8 @@
#include <linux/component.h>
#include <drm/exynos_drm.h>
+#include <drm/drm_atomic.h>
+#include <drm/drm_atomic_helper.h>
#include "exynos_drm_drv.h"
#include "exynos_drm_crtc.h"
@@ -133,7 +135,9 @@ static const struct exynos_drm_plane_config plane_configs[MIXER_WIN_NR] = {
.capabilities = EXYNOS_DRM_PLANE_CAP_DOUBLE |
EXYNOS_DRM_PLANE_CAP_ZPOS |
EXYNOS_DRM_PLANE_CAP_PIX_BLEND |
- EXYNOS_DRM_PLANE_CAP_WIN_BLEND,
+ EXYNOS_DRM_PLANE_CAP_WIN_BLEND |
+ EXYNOS_DRM_PLANE_CAP_RANGE,
+ .range = DRM_COLOR_FULL_RANGE,
}, {
.zpos = 1,
.type = DRM_PLANE_TYPE_CURSOR,
@@ -142,7 +146,9 @@ static const struct exynos_drm_plane_config plane_configs[MIXER_WIN_NR] = {
.capabilities = EXYNOS_DRM_PLANE_CAP_DOUBLE |
EXYNOS_DRM_PLANE_CAP_ZPOS |
EXYNOS_DRM_PLANE_CAP_PIX_BLEND |
- EXYNOS_DRM_PLANE_CAP_WIN_BLEND,
+ EXYNOS_DRM_PLANE_CAP_WIN_BLEND |
+ EXYNOS_DRM_PLANE_CAP_RANGE,
+ .range = DRM_COLOR_FULL_RANGE,
}, {
.zpos = 2,
.type = DRM_PLANE_TYPE_OVERLAY,
@@ -389,13 +395,19 @@ static void mixer_cfg_rgb_fmt(struct mixer_context *ctx, struct drm_display_mode
if (format == HDMI_COLORIMETRY_ITU_601) {
val = MXR_CFG_RGB601;
+
+ mixer_reg_write(ctx, MXR_CM_COEFF_Y,
+ MXR_CSC_CT( 0.257, 0.504, 0.098));
+ mixer_reg_write(ctx, MXR_CM_COEFF_CB,
+ MXR_CSC_CT(-0.148, -0.291, 0.439));
+ mixer_reg_write(ctx, MXR_CM_COEFF_CR,
+ MXR_CSC_CT( 0.439, -0.368, -0.071));
} else {
val = MXR_CFG_RGB709;
/* Configure the BT.709 CSC matrix for full range RGB. */
mixer_reg_write(ctx, MXR_CM_COEFF_Y,
- MXR_CSC_CT( 0.184, 0.614, 0.063) |
- MXR_CM_COEFF_RGB_FULL);
+ MXR_CSC_CT( 0.184, 0.614, 0.063));
mixer_reg_write(ctx, MXR_CM_COEFF_CB,
MXR_CSC_CT(-0.102, -0.338, 0.440));
mixer_reg_write(ctx, MXR_CM_COEFF_CR,
@@ -560,6 +572,20 @@ static void mixer_layer_update(struct mixer_context *ctx)
mixer_reg_writemask(ctx, MXR_CFG, ~0, MXR_CFG_LAYER_UPDATE);
}
+static void mixer_set_input_buffer_range(struct mixer_context *ctx,
+ struct exynos_drm_plane *plane)
+{
+ u32 cm_coeff;
+
+ if (plane->base.state->color_range == DRM_COLOR_LIMITED_RANGE)
+ cm_coeff = MXR_CM_COEFF_RGB_LIMITED;
+ else
+ cm_coeff = MXR_CM_COEFF_RGB_FULL;
+
+ mixer_reg_writemask(ctx, MXR_CM_COEFF_Y, cm_coeff,
+ MXR_CM_COEFF_RGB_RANGE_MASK);
+}
+
static void mixer_graph_buffer(struct mixer_context *ctx,
struct exynos_drm_plane *plane)
{
@@ -647,6 +673,9 @@ static void mixer_graph_buffer(struct mixer_context *ctx,
ctx->mxr_ver == MXR_VER_128_0_0_184)
mixer_layer_update(ctx);
+ /* set the input buffer rgb range */
+ mixer_set_input_buffer_range(ctx, plane);
+
spin_unlock_irqrestore(&ctx->reg_slock, flags);
mixer_regs_dump(ctx);
@@ -917,6 +946,25 @@ static void mixer_disable_vblank(struct exynos_drm_crtc *crtc)
mixer_reg_writemask(mixer_ctx, MXR_INT_EN, 0, MXR_INT_EN_VSYNC);
}
+static int mixer_atomic_check(struct exynos_drm_crtc *crtc,
+ struct drm_crtc_state *state)
+{
+ const struct drm_plane_state *pstate;
+ enum drm_color_range range_val[2];
+ struct drm_plane *plane;
+ unsigned int cnt = 0;
+
+ drm_atomic_crtc_state_for_each_plane_state(plane, pstate, state) {
+ if (plane->color_range_property && cnt < 2) {
+ range_val[cnt] = pstate->color_range;
+ ++cnt;
+ }
+ }
+ if (cnt == 2 && range_val[0] != range_val[1])
+ return -EINVAL;
+ return 0;
+}
+
static void mixer_atomic_begin(struct exynos_drm_crtc *crtc)
{
struct mixer_context *mixer_ctx = crtc->ctx;
@@ -1096,6 +1144,7 @@ static const struct exynos_drm_crtc_ops mixer_crtc_ops = {
.disable = mixer_disable,
.enable_vblank = mixer_enable_vblank,
.disable_vblank = mixer_disable_vblank,
+ .atomic_check = mixer_atomic_check,
.atomic_begin = mixer_atomic_begin,
.update_plane = mixer_update_plane,
.disable_plane = mixer_disable_plane,
diff --git a/drivers/gpu/drm/exynos/regs-mixer.h b/drivers/gpu/drm/exynos/regs-mixer.h
index 5ff095b0c1b3..7d558e3dd46c 100644
--- a/drivers/gpu/drm/exynos/regs-mixer.h
+++ b/drivers/gpu/drm/exynos/regs-mixer.h
@@ -158,7 +158,9 @@
#define MXR_LAYER_CFG_VP_MASK MXR_LAYER_CFG_VP_VAL(~0)
/* bits for MXR_CM_COEFF_Y */
+#define MXR_CM_COEFF_RGB_LIMITED (0 << 30)
#define MXR_CM_COEFF_RGB_FULL (1 << 30)
+#define MXR_CM_COEFF_RGB_RANGE_MASK (1 << 30)
#endif /* SAMSUNG_REGS_MIXER_H */
--
2.7.4
Split drm_plane_create_color_properties into two separate functions.
This allows to create range and encoding property independently.
Signed-off-by: Christoph Manszewski <[email protected]>
---
drivers/gpu/drm/drm_color_mgmt.c | 93 +++++++++++++++++++++++++++++++---------
include/drm/drm_color_mgmt.h | 8 ++++
2 files changed, 81 insertions(+), 20 deletions(-)
diff --git a/drivers/gpu/drm/drm_color_mgmt.c b/drivers/gpu/drm/drm_color_mgmt.c
index 581cc3788223..416cc9c8560b 100644
--- a/drivers/gpu/drm/drm_color_mgmt.c
+++ b/drivers/gpu/drm/drm_color_mgmt.c
@@ -396,29 +396,24 @@ const char *drm_get_color_range_name(enum drm_color_range range)
}
/**
- * drm_plane_create_color_properties - color encoding related plane properties
+ * drm_plane_create_encoding_property - color encoding related plane property
* @plane: plane object
* @supported_encodings: bitfield indicating supported color encodings
- * @supported_ranges: bitfileld indicating supported color ranges
* @default_encoding: default color encoding
- * @default_range: default color range
*
- * Create and attach plane specific COLOR_ENCODING and COLOR_RANGE
- * properties to @plane. The supported encodings and ranges should
- * be provided in supported_encodings and supported_ranges bitmasks.
- * Each bit set in the bitmask indicates that its number as enum
- * value is supported.
+ * Create and attach plane specific COLOR_ENCODING property to @plane.
+ * The supported encodings be provided in supported_encodings bitmask.
+ * Each bit set in the bitmask indicates that its number as enum value
+ * is supported.
*/
-int drm_plane_create_color_properties(struct drm_plane *plane,
- u32 supported_encodings,
- u32 supported_ranges,
- enum drm_color_encoding default_encoding,
- enum drm_color_range default_range)
+
+int drm_plane_create_encoding_property(struct drm_plane *plane,
+ u32 supported_encodings,
+ enum drm_color_encoding default_encoding)
{
struct drm_device *dev = plane->dev;
struct drm_property *prop;
- struct drm_prop_enum_list enum_list[max_t(int, DRM_COLOR_ENCODING_MAX,
- DRM_COLOR_RANGE_MAX)];
+ struct drm_prop_enum_list enum_list[DRM_COLOR_ENCODING_MAX];
int i, len;
if (WARN_ON(supported_encodings == 0 ||
@@ -426,11 +421,6 @@ int drm_plane_create_color_properties(struct drm_plane *plane,
(supported_encodings & BIT(default_encoding)) == 0))
return -EINVAL;
- if (WARN_ON(supported_ranges == 0 ||
- (supported_ranges & -BIT(DRM_COLOR_RANGE_MAX)) != 0 ||
- (supported_ranges & BIT(default_range)) == 0))
- return -EINVAL;
-
len = 0;
for (i = 0; i < DRM_COLOR_ENCODING_MAX; i++) {
if ((supported_encodings & BIT(i)) == 0)
@@ -450,6 +440,36 @@ int drm_plane_create_color_properties(struct drm_plane *plane,
if (plane->state)
plane->state->color_encoding = default_encoding;
+ return 0;
+}
+EXPORT_SYMBOL(drm_plane_create_encoding_property);
+
+/**
+ * drm_plane_create_range_property - color range related plane property
+ * @plane: plane object
+ * @supported_ranges: bitfileld indicating supported color ranges
+ * @default_range: default color range
+ *
+ * Create and attach plane specific COLOR_RANGE property to @plane.
+ * The supported ranges should be provided in supported_ranges bitmask.
+ * Each bit set in the bitmask indicates that its number as enum value
+ * is supported.
+ */
+
+int drm_plane_create_range_property(struct drm_plane *plane,
+ u32 supported_ranges,
+ enum drm_color_range default_range)
+{
+ struct drm_device *dev = plane->dev;
+ struct drm_property *prop;
+ struct drm_prop_enum_list enum_list[DRM_COLOR_RANGE_MAX];
+ int i, len;
+
+ if (WARN_ON(supported_ranges == 0 ||
+ (supported_ranges & -BIT(DRM_COLOR_RANGE_MAX)) != 0 ||
+ (supported_ranges & BIT(default_range)) == 0))
+ return -EINVAL;
+
len = 0;
for (i = 0; i < DRM_COLOR_RANGE_MAX; i++) {
if ((supported_ranges & BIT(i)) == 0)
@@ -471,4 +491,37 @@ int drm_plane_create_color_properties(struct drm_plane *plane,
return 0;
}
+EXPORT_SYMBOL(drm_plane_create_range_property);
+
+/**
+ * drm_plane_create_color_properties - color encoding related plane properties
+ * @plane: plane object
+ * @supported_encodings: bitfield indicating supported color encodings
+ * @supported_ranges: bitfileld indicating supported color ranges
+ * @default_encoding: default color encoding
+ * @default_range: default color range
+ *
+ * Create and attach plane specific COLOR_ENCODING and COLOR_RANGE
+ * properties to @plane. The supported encodings and ranges should
+ * be provided in supported_encodings and supported_ranges bitmasks.
+ * Each bit set in the bitmask indicates that its number as enum
+ * value is supported.
+ */
+
+int drm_plane_create_color_properties(struct drm_plane *plane,
+ u32 supported_encodings,
+ u32 supported_ranges,
+ enum drm_color_encoding default_encoding,
+ enum drm_color_range default_range)
+{
+ int ret;
+
+ ret = drm_plane_create_encoding_property(plane, supported_encodings,
+ default_encoding);
+ if (ret)
+ return ret;
+
+ return drm_plane_create_range_property(plane, supported_ranges,
+ default_range);
+}
EXPORT_SYMBOL(drm_plane_create_color_properties);
diff --git a/include/drm/drm_color_mgmt.h b/include/drm/drm_color_mgmt.h
index 52f6d5221a0d..fca8d3fabcd9 100644
--- a/include/drm/drm_color_mgmt.h
+++ b/include/drm/drm_color_mgmt.h
@@ -66,6 +66,14 @@ enum drm_color_range {
DRM_COLOR_FULL_RANGE = DRM_COLOR_YCBCR_FULL_RANGE,
};
+int drm_plane_create_encoding_property(struct drm_plane *plane,
+ u32 supported_encodings,
+ enum drm_color_encoding default_encoding);
+
+int drm_plane_create_range_property(struct drm_plane *plane,
+ u32 supported_ranges,
+ enum drm_color_range default_range);
+
int drm_plane_create_color_properties(struct drm_plane *plane,
u32 supported_encodings,
u32 supported_ranges,
--
2.7.4
Range setting makes sense for YCbCr and RGB buffers. Current
drm_color_range enum labels suggest use with YCbCr buffers.
Create enum labels without colorspace specification.
Signed-off-by: Christoph Manszewski <[email protected]>
---
include/drm/drm_color_mgmt.h | 2 ++
1 file changed, 2 insertions(+)
diff --git a/include/drm/drm_color_mgmt.h b/include/drm/drm_color_mgmt.h
index 90ef9996d9a4..52f6d5221a0d 100644
--- a/include/drm/drm_color_mgmt.h
+++ b/include/drm/drm_color_mgmt.h
@@ -62,6 +62,8 @@ enum drm_color_range {
DRM_COLOR_YCBCR_LIMITED_RANGE,
DRM_COLOR_YCBCR_FULL_RANGE,
DRM_COLOR_RANGE_MAX,
+ DRM_COLOR_LIMITED_RANGE = DRM_COLOR_YCBCR_LIMITED_RANGE,
+ DRM_COLOR_FULL_RANGE = DRM_COLOR_YCBCR_FULL_RANGE,
};
int drm_plane_create_color_properties(struct drm_plane *plane,
--
2.7.4
Hi,
18. 12. 14. 오후 9:10에 Christoph Manszewski 이(가) 쓴 글:
> Range setting makes sense for YCbCr and RGB buffers. Current
> drm_color_range enum labels suggest use with YCbCr buffers.
> Create enum labels without colorspace specification.
>
> Signed-off-by: Christoph Manszewski <[email protected]>
> ---
> include/drm/drm_color_mgmt.h | 2 ++
> 1 file changed, 2 insertions(+)
>
> diff --git a/include/drm/drm_color_mgmt.h b/include/drm/drm_color_mgmt.h
> index 90ef9996d9a4..52f6d5221a0d 100644
> --- a/include/drm/drm_color_mgmt.h
> +++ b/include/drm/drm_color_mgmt.h
> @@ -62,6 +62,8 @@ enum drm_color_range {
> DRM_COLOR_YCBCR_LIMITED_RANGE,
> DRM_COLOR_YCBCR_FULL_RANGE,
> DRM_COLOR_RANGE_MAX,
> + DRM_COLOR_LIMITED_RANGE = DRM_COLOR_YCBCR_LIMITED_RANGE,
> + DRM_COLOR_FULL_RANGE = DRM_COLOR_YCBCR_FULL_RANGE,
I don't see why above two types are needed. Is there any case that the range checking behavior should be different according to the color space?
And even if there is that case, I think you have to put DRM_COLOR_RANG_MAX under DRM_COLOR_FULL_RANGE.
Thanks,
Inki Dae
> };
>
> int drm_plane_create_color_properties(struct drm_plane *plane,
>
On Fri, Dec 14, 2018 at 01:10:16PM +0100, Christoph Manszewski wrote:
> Range setting makes sense for YCbCr and RGB buffers. Current
> drm_color_range enum labels suggest use with YCbCr buffers.
> Create enum labels without colorspace specification.
>
> Signed-off-by: Christoph Manszewski <[email protected]>
> ---
> include/drm/drm_color_mgmt.h | 2 ++
> 1 file changed, 2 insertions(+)
>
> diff --git a/include/drm/drm_color_mgmt.h b/include/drm/drm_color_mgmt.h
> index 90ef9996d9a4..52f6d5221a0d 100644
> --- a/include/drm/drm_color_mgmt.h
> +++ b/include/drm/drm_color_mgmt.h
> @@ -62,6 +62,8 @@ enum drm_color_range {
> DRM_COLOR_YCBCR_LIMITED_RANGE,
> DRM_COLOR_YCBCR_FULL_RANGE,
> DRM_COLOR_RANGE_MAX,
> + DRM_COLOR_LIMITED_RANGE = DRM_COLOR_YCBCR_LIMITED_RANGE,
> + DRM_COLOR_FULL_RANGE = DRM_COLOR_YCBCR_FULL_RANGE,
These enum values don't mean anything really. The strings are what
matter for the uapi.
The default for YCbCr needs to be limited range, the default for RGB
needs to be full range. So I think this would really require a separate
prop for each.
But is there an actual usecase for this stuff?
> };
>
> int drm_plane_create_color_properties(struct drm_plane *plane,
> --
> 2.7.4
>
> _______________________________________________
> dri-devel mailing list
> [email protected]
> https://lists.freedesktop.org/mailman/listinfo/dri-devel
--
Ville Syrj?l?
Intel
+CC: Hans
On 17.01.2019 20:47, Ville Syrjälä wrote:
> On Fri, Dec 14, 2018 at 01:10:16PM +0100, Christoph Manszewski wrote:
>> Range setting makes sense for YCbCr and RGB buffers. Current
>> drm_color_range enum labels suggest use with YCbCr buffers.
>> Create enum labels without colorspace specification.
>>
>> Signed-off-by: Christoph Manszewski <[email protected]>
>> ---
>> include/drm/drm_color_mgmt.h | 2 ++
>> 1 file changed, 2 insertions(+)
>>
>> diff --git a/include/drm/drm_color_mgmt.h b/include/drm/drm_color_mgmt.h
>> index 90ef9996d9a4..52f6d5221a0d 100644
>> --- a/include/drm/drm_color_mgmt.h
>> +++ b/include/drm/drm_color_mgmt.h
>> @@ -62,6 +62,8 @@ enum drm_color_range {
>> DRM_COLOR_YCBCR_LIMITED_RANGE,
>> DRM_COLOR_YCBCR_FULL_RANGE,
>> DRM_COLOR_RANGE_MAX,
>> + DRM_COLOR_LIMITED_RANGE = DRM_COLOR_YCBCR_LIMITED_RANGE,
>> + DRM_COLOR_FULL_RANGE = DRM_COLOR_YCBCR_FULL_RANGE,
> These enum values don't mean anything really. The strings are what
> matter for the uapi.
>
> The default for YCbCr needs to be limited range, the default for RGB
> needs to be full range. So I think this would really require a separate
> prop for each.
>
> But is there an actual usecase for this stuff?
Yes, for example HDMI path (Display Controller and HDMI encoder) in
Exynos5433 works on RGB buffers and do not perform any conversion if the
output is also RGB, on the other side CEA861 specs says that in case of
CEA modes output RGB should be in limited range, in case of non CEA
modes it should be full range.
The idea was to allow userspace to check (atomic_check) which ranges are
acceptable in given mode and to set desired one if possible.
Any better solution is welcome?
Regards
Andrzej
>
>> };
>>
>> int drm_plane_create_color_properties(struct drm_plane *plane,
>> --
>> 2.7.4
>>
>> _______________________________________________
>> dri-devel mailing list
>> [email protected]
>> https://lists.freedesktop.org/mailman/listinfo/dri-devel
On Fri, Jan 18, 2019 at 03:34:18PM +0100, Andrzej Hajda wrote:
> +CC: Hans
>
> On 17.01.2019 20:47, Ville Syrj?l? wrote:
> > On Fri, Dec 14, 2018 at 01:10:16PM +0100, Christoph Manszewski wrote:
> >> Range setting makes sense for YCbCr and RGB buffers. Current
> >> drm_color_range enum labels suggest use with YCbCr buffers.
> >> Create enum labels without colorspace specification.
> >>
> >> Signed-off-by: Christoph Manszewski <[email protected]>
> >> ---
> >> include/drm/drm_color_mgmt.h | 2 ++
> >> 1 file changed, 2 insertions(+)
> >>
> >> diff --git a/include/drm/drm_color_mgmt.h b/include/drm/drm_color_mgmt.h
> >> index 90ef9996d9a4..52f6d5221a0d 100644
> >> --- a/include/drm/drm_color_mgmt.h
> >> +++ b/include/drm/drm_color_mgmt.h
> >> @@ -62,6 +62,8 @@ enum drm_color_range {
> >> DRM_COLOR_YCBCR_LIMITED_RANGE,
> >> DRM_COLOR_YCBCR_FULL_RANGE,
> >> DRM_COLOR_RANGE_MAX,
> >> + DRM_COLOR_LIMITED_RANGE = DRM_COLOR_YCBCR_LIMITED_RANGE,
> >> + DRM_COLOR_FULL_RANGE = DRM_COLOR_YCBCR_FULL_RANGE,
> > These enum values don't mean anything really. The strings are what
> > matter for the uapi.
> >
> > The default for YCbCr needs to be limited range, the default for RGB
> > needs to be full range. So I think this would really require a separate
> > prop for each.
> >
> > But is there an actual usecase for this stuff?
>
>
> Yes, for example HDMI path (Display Controller and HDMI encoder) in
> Exynos5433 works on RGB buffers and do not perform any conversion if the
> output is also RGB, on the other side CEA861 specs says that in case of
> CEA modes output RGB should be in limited range, in case of non CEA
> modes it should be full range.
>
> The idea was to allow userspace to check (atomic_check) which ranges are
> acceptable in given mode and to set desired one if possible.
>
> Any better solution is welcome?
i915 has the "Broadcast RGB" property for the CEA-861 stuff.
You don't have a way to implement that?
--
Ville Syrj?l?
Intel