* [v3 0/7] Add Multi Segment Gamma Support
@ 2019-04-12 10:20 Uma Shankar
2019-04-12 10:20 ` [v3 1/7] drm: Add gamma mode property Uma Shankar
` (9 more replies)
0 siblings, 10 replies; 24+ messages in thread
From: Uma Shankar @ 2019-04-12 10:20 UTC (permalink / raw)
To: intel-gfx, dri-devel
Cc: dcastagna, sam, emil.l.velikov, Uma Shankar, seanpaul,
ville.syrjala, maarten.lankhorst
This series adds support for programmable gamma modes and
exposes a property interface for the same. Also added,
support for multi segment gamma mode introduced in ICL+
It creates GAMMA_MODE property interface. This is an enum
property with values as blob_id's and exposes
the various gamma modes supported and the lut ranges Getting the
blob id in userspace, user can get the mode supported and
also the range of gamma mode supported with number of lut
coefficients. It can then set one of the modes using this
enum property.
Lut values will be sent through already available GAMMA_LUT
blob property.
It also introduces a CLIENT CAP for advanced GAMMA_MODE.
This is for user to set the and use advance gamma mode and older
userspace can continue using the legacy paths.
v2: Used Ville's design and approach to define the interfaces.
Addressed Matt Roper's review feedback and re-ordered the
patches.
v3: Converged to 1 property interface and introduced a Client cap
as suggested by Ville. Fixed review comments received.
Uma Shankar (5):
drm/i915/icl: Add register definitions for Multi Segmented gamma
drm/i915/icl: Add support for multi segmented gamma mode
drm/i915: Attach gamma mode property
drm: Add Client Cap for advance gamma mode
drm/i915: Enable advance gamma mode
Ville Syrjälä (2):
drm: Add gamma mode property
drm/i915: Define color lut range structure
drivers/gpu/drm/drm_atomic_uapi.c | 8 +
drivers/gpu/drm/drm_color_mgmt.c | 77 ++++
drivers/gpu/drm/drm_ioctl.c | 5 +
drivers/gpu/drm/i915/i915_reg.h | 17 +
drivers/gpu/drm/i915/intel_color.c | 735 ++++++++++++++++++++++++++++++++++-
drivers/gpu/drm/i915/intel_display.c | 3 +
include/drm/drm_atomic.h | 1 +
include/drm/drm_color_mgmt.h | 8 +
include/drm/drm_crtc.h | 17 +
include/drm/drm_file.h | 8 +
include/drm/drm_mode_config.h | 6 +
include/uapi/drm/drm.h | 2 +
include/uapi/drm/drm_mode.h | 38 ++
13 files changed, 918 insertions(+), 7 deletions(-)
--
1.9.1
_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel
^ permalink raw reply [flat|nested] 24+ messages in thread
* [v3 1/7] drm: Add gamma mode property
2019-04-12 10:20 [v3 0/7] Add Multi Segment Gamma Support Uma Shankar
@ 2019-04-12 10:20 ` Uma Shankar
2019-04-16 7:28 ` Daniel Vetter
2019-04-12 10:20 ` [v3 2/7] drm/i915: Define color lut range structure Uma Shankar
` (8 subsequent siblings)
9 siblings, 1 reply; 24+ messages in thread
From: Uma Shankar @ 2019-04-12 10:20 UTC (permalink / raw)
To: intel-gfx, dri-devel
Cc: dcastagna, sam, seanpaul, ville.syrjala, maarten.lankhorst
From: Ville Syrjälä <ville.syrjala@linux.intel.com>
Add a gamma mode property to enable various kind of
gamma modes supported by platforms like: Interpolated, Split,
Multi Segmented etc. Userspace can get this property and
should be able to get the platform capabilties wrt various
gamma modes possible and the possible ranges.
It can select one of the modes exposed as blob_id as an
enum and set the respective mode.
It can then create the LUT and send it to driver using
already available GAMMA_LUT property as blob.
v2: Addressed Sam Ravnborg's review comments. Implemented
gamma mode with just one property and renamed the current
one to GAMMA_MODE property as recommended by Ville.
Signed-off-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
Signed-off-by: Uma Shankar <uma.shankar@intel.com>
---
drivers/gpu/drm/drm_atomic_uapi.c | 5 +++
drivers/gpu/drm/drm_color_mgmt.c | 77 +++++++++++++++++++++++++++++++++++++++
include/drm/drm_color_mgmt.h | 8 ++++
include/drm/drm_crtc.h | 7 ++++
include/drm/drm_mode_config.h | 6 +++
include/uapi/drm/drm_mode.h | 38 +++++++++++++++++++
6 files changed, 141 insertions(+)
diff --git a/drivers/gpu/drm/drm_atomic_uapi.c b/drivers/gpu/drm/drm_atomic_uapi.c
index ea797d4..d85e0c9 100644
--- a/drivers/gpu/drm/drm_atomic_uapi.c
+++ b/drivers/gpu/drm/drm_atomic_uapi.c
@@ -459,6 +459,9 @@ static int drm_atomic_crtc_set_property(struct drm_crtc *crtc,
&replaced);
state->color_mgmt_changed |= replaced;
return ret;
+ } else if (property == config->gamma_mode_property) {
+ state->gamma_mode = val;
+ state->color_mgmt_changed |= replaced;
} else if (property == config->prop_out_fence_ptr) {
s32 __user *fence_ptr = u64_to_user_ptr(val);
@@ -495,6 +498,8 @@ static int drm_atomic_crtc_set_property(struct drm_crtc *crtc,
*val = (state->mode_blob) ? state->mode_blob->base.id : 0;
else if (property == config->prop_vrr_enabled)
*val = state->vrr_enabled;
+ else if (property == config->gamma_mode_property)
+ *val = state->gamma_mode;
else if (property == config->degamma_lut_property)
*val = (state->degamma_lut) ? state->degamma_lut->base.id : 0;
else if (property == config->ctm_property)
diff --git a/drivers/gpu/drm/drm_color_mgmt.c b/drivers/gpu/drm/drm_color_mgmt.c
index d5d34d0..4d6792d 100644
--- a/drivers/gpu/drm/drm_color_mgmt.c
+++ b/drivers/gpu/drm/drm_color_mgmt.c
@@ -176,6 +176,83 @@ void drm_crtc_enable_color_mgmt(struct drm_crtc *crtc,
}
EXPORT_SYMBOL(drm_crtc_enable_color_mgmt);
+void drm_crtc_attach_gamma_mode_property(struct drm_crtc *crtc)
+{
+ struct drm_device *dev = crtc->dev;
+ struct drm_mode_config *config = &dev->mode_config;
+
+ if (!config->gamma_mode_property)
+ return;
+
+ drm_object_attach_property(&crtc->base,
+ config->gamma_mode_property, 0);
+}
+EXPORT_SYMBOL(drm_crtc_attach_gamma_mode_property);
+
+int drm_color_create_gamma_mode_property(struct drm_device *dev,
+ int num_values)
+{
+ struct drm_mode_config *config = &dev->mode_config;
+ struct drm_property *prop;
+
+ prop = drm_property_create(dev,
+ DRM_MODE_PROP_ENUM,
+ "GAMMA_MODE", num_values);
+ if (!prop)
+ return -ENOMEM;
+
+ config->gamma_mode_property = prop;
+
+ return 0;
+}
+EXPORT_SYMBOL(drm_color_create_gamma_mode_property);
+
+int drm_color_add_gamma_mode_range(struct drm_device *dev,
+ const char *name,
+ const struct drm_color_lut_range *ranges,
+ size_t length)
+{
+ struct drm_mode_config *config = &dev->mode_config;
+ struct drm_property_blob *blob;
+ struct drm_property *prop;
+ int num_ranges = length / sizeof(ranges[0]);
+ int i, ret, num_types_0;
+
+ if (WARN_ON(length == 0 || length % sizeof(ranges[0]) != 0))
+ return -EINVAL;
+
+ num_types_0 = hweight8(ranges[0].flags & (DRM_MODE_LUT_GAMMA |
+ DRM_MODE_LUT_DEGAMMA));
+ if (num_types_0 == 0)
+ return -EINVAL;
+
+ for (i = 1; i < num_ranges; i++) {
+ int num_types = hweight8(ranges[i].flags & (DRM_MODE_LUT_GAMMA |
+ DRM_MODE_LUT_DEGAMMA));
+
+ /* either all ranges have DEGAMMA|GAMMA or none have it */
+ if (num_types_0 != num_types)
+ return -EINVAL;
+ }
+
+ prop = config->gamma_mode_property;
+ if (!prop)
+ return -EINVAL;
+
+ blob = drm_property_create_blob(dev, length, ranges);
+ if (IS_ERR(blob))
+ return PTR_ERR(blob);
+
+ ret = drm_property_add_enum(prop, blob->base.id, name);
+ if (ret) {
+ drm_property_blob_put(blob);
+ return ret;
+ }
+
+ return 0;
+}
+EXPORT_SYMBOL(drm_color_add_gamma_mode_range);
+
/**
* drm_mode_crtc_set_gamma_size - set the gamma table size
* @crtc: CRTC to set the gamma table size for
diff --git a/include/drm/drm_color_mgmt.h b/include/drm/drm_color_mgmt.h
index d1c662d..f18e9b8 100644
--- a/include/drm/drm_color_mgmt.h
+++ b/include/drm/drm_color_mgmt.h
@@ -51,6 +51,14 @@ static inline int drm_color_lut_size(const struct drm_property_blob *blob)
return blob->length / sizeof(struct drm_color_lut);
}
+int drm_color_create_gamma_mode_property(struct drm_device *dev,
+ int num_values);
+void drm_crtc_attach_gamma_mode_property(struct drm_crtc *crtc);
+int drm_color_add_gamma_mode_range(struct drm_device *dev,
+ const char *name,
+ const struct drm_color_lut_range *ranges,
+ size_t length);
+
enum drm_color_encoding {
DRM_COLOR_YCBCR_BT601,
DRM_COLOR_YCBCR_BT709,
diff --git a/include/drm/drm_crtc.h b/include/drm/drm_crtc.h
index 58ad983..f2e60bd 100644
--- a/include/drm/drm_crtc.h
+++ b/include/drm/drm_crtc.h
@@ -249,6 +249,13 @@ struct drm_crtc_state {
struct drm_property_blob *mode_blob;
/**
+ * @gamma_mode: This is a blob_id and exposes the platform capabilties
+ * wrt to various gamma modes and the respective lut ranges. This also
+ * helps user select a gamma mode amongst the supported ones.
+ */
+ u32 gamma_mode;
+
+ /**
* @degamma_lut:
*
* Lookup table for converting framebuffer pixel data before apply the
diff --git a/include/drm/drm_mode_config.h b/include/drm/drm_mode_config.h
index 7f60e8e..8f961c5b 100644
--- a/include/drm/drm_mode_config.h
+++ b/include/drm/drm_mode_config.h
@@ -761,6 +761,12 @@ struct drm_mode_config {
*/
struct drm_property *content_type_property;
/**
+ * @gamma_mode_property: Optional CRTC property to enumerate and
+ * select the mode of the crtc gamma/degmama LUTs. This also exposes
+ * the lut ranges of the various supported gamma modes to userspace.
+ */
+ struct drm_property *gamma_mode_property;
+ /**
* @degamma_lut_property: Optional CRTC property to set the LUT used to
* convert the framebuffer's colors to linear gamma.
*/
diff --git a/include/uapi/drm/drm_mode.h b/include/uapi/drm/drm_mode.h
index 83cd163..e70b7f8 100644
--- a/include/uapi/drm/drm_mode.h
+++ b/include/uapi/drm/drm_mode.h
@@ -630,6 +630,44 @@ struct drm_color_lut {
__u16 reserved;
};
+/*
+ * DRM_MODE_LUT_GAMMA|DRM_MODE_LUT_DEGAMMA is legal and means the LUT
+ * can be used for either purpose, but not simultaneously. To expose
+ * modes that support gamma and degamma simultaneously the gamma mode
+ * must declare distinct DRM_MODE_LUT_GAMMA and DRM_MODE_LUT_DEGAMMA
+ * ranges.
+ */
+/* LUT is for gamma (after CTM) */
+#define DRM_MODE_LUT_GAMMA BIT(0)
+/* LUT is for degamma (before CTM) */
+#define DRM_MODE_LUT_DEGAMMA BIT(1)
+/* linearly interpolate between the points */
+#define DRM_MODE_LUT_INTERPOLATE BIT(2)
+/*
+ * the last value of the previous range is the
+ * first value of the current range.
+ */
+#define DRM_MODE_LUT_REUSE_LAST BIT(3)
+/* the curve must be non-decreasing */
+#define DRM_MODE_LUT_NON_DECREASING BIT(4)
+/* the curve is reflected across origin for negative inputs */
+#define DRM_MODE_LUT_REFLECT_NEGATIVE BIT(5)
+/* the same curve (red) is used for blue and green channels as well */
+#define DRM_MODE_LUT_SINGLE_CHANNEL BIT(6)
+
+struct drm_color_lut_range {
+ /* DRM_MODE_LUT_* */
+ __u32 flags;
+ /* number of points on the curve */
+ __u16 count;
+ /* input/output bits per component */
+ __u8 input_bpc, output_bpc;
+ /* input start/end values */
+ __s32 start, end;
+ /* output min/max values */
+ __s32 min, max;
+};
+
#define DRM_MODE_PAGE_FLIP_EVENT 0x01
#define DRM_MODE_PAGE_FLIP_ASYNC 0x02
#define DRM_MODE_PAGE_FLIP_TARGET_ABSOLUTE 0x4
--
1.9.1
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
^ permalink raw reply related [flat|nested] 24+ messages in thread
* [v3 2/7] drm/i915: Define color lut range structure
2019-04-12 10:20 [v3 0/7] Add Multi Segment Gamma Support Uma Shankar
2019-04-12 10:20 ` [v3 1/7] drm: Add gamma mode property Uma Shankar
@ 2019-04-12 10:20 ` Uma Shankar
2019-04-12 10:20 ` [v3 3/7] drm/i915/icl: Add register definitions for Multi Segmented gamma Uma Shankar
` (7 subsequent siblings)
9 siblings, 0 replies; 24+ messages in thread
From: Uma Shankar @ 2019-04-12 10:20 UTC (permalink / raw)
To: intel-gfx, dri-devel
Cc: dcastagna, sam, seanpaul, ville.syrjala, maarten.lankhorst
From: Ville Syrjälä <ville.syrjala@linux.intel.com>
This defines the color lut ranges for 10bit and multi
segmented gamma range for ICL.
v2: Defined and advertise the gamma modes supported on
various platforms as suggested by Ville.
Signed-off-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
Signed-off-by: Uma Shankar <uma.shankar@intel.com>
---
drivers/gpu/drm/i915/intel_color.c | 566 ++++++++++++++++++++++++++++++++++++-
1 file changed, 562 insertions(+), 4 deletions(-)
diff --git a/drivers/gpu/drm/i915/intel_color.c b/drivers/gpu/drm/i915/intel_color.c
index ca341a9..c433215 100644
--- a/drivers/gpu/drm/i915/intel_color.c
+++ b/drivers/gpu/drm/i915/intel_color.c
@@ -383,6 +383,20 @@ static u32 ilk_lut_10(const struct drm_color_lut *color)
drm_color_lut_extract(color->blue, 10);
}
+static bool i9xx_has_10bit_lut(struct drm_i915_private *dev_priv)
+{
+ /*
+ * Bspec:
+ " "NOTE: The 8-bit (non-10-bit) mode is the only
+ * mode supported by BrookDale-G and Springdale-G."
+ * and
+ * "NOTE: The 8-bit (non-10-bit) mode is the only
+ * mode supported by Alviso and Grantsdale."
+ */
+ return !IS_I845G(dev_priv) && !IS_I865G(dev_priv) &&
+ !IS_I915G(dev_priv) && !IS_I915GM(dev_priv);
+}
+
/* Loads the legacy palette/gamma unit for the CRTC. */
static void i9xx_load_luts_internal(const struct intel_crtc_state *crtc_state,
const struct drm_property_blob *blob)
@@ -1221,10 +1235,420 @@ static int icl_color_check(struct intel_crtc_state *crtc_state)
return 0;
}
+enum {
+ I9XX_LUT_SIZE_8BIT = 256,
+ I9XX_LUT_SIZE_10BIT = 129,
+
+ ILK_LUT_SIZE_10BIT = 1024,
+ ILK_LUT_SIZE_12BIT = 513,
+
+ IVB_LUT_SIZE_SPLIT = 512,
+
+ CHV_LUT_SIZE_CGM_DEGAMMA = 65,
+ CHV_LUT_SIZE_CGM_GAMMA = 257,
+};
+
+#define I9XX_GAMMA_8 \
+ { \
+ .flags = DRM_MODE_LUT_GAMMA, \
+ .count = 256, \
+ .input_bpc = 8, .output_bpc = 8, \
+ .start = 0, .end = (1 << 8) - 1, \
+ .min = 0, .max = (1 << 8) - 1, \
+ }
+
+static const struct drm_color_lut_range i9xx_gamma_8[] = {
+ I9XX_GAMMA_8,
+};
+
+static const struct drm_color_lut_range i9xx_gamma_10_slope[] = {
+ {
+ .flags = (DRM_MODE_LUT_GAMMA |
+ DRM_MODE_LUT_INTERPOLATE |
+ DRM_MODE_LUT_NON_DECREASING),
+ .count = 129,
+ .input_bpc = 10, .output_bpc = 10,
+ .start = 0, .end = 1 << 10,
+ .min = 0, .max = (1 << 10) - 1,
+ },
+};
+
+#define I965_GAMMA_10 \
+ { \
+ .flags = (DRM_MODE_LUT_GAMMA | \
+ DRM_MODE_LUT_INTERPOLATE | \
+ DRM_MODE_LUT_NON_DECREASING), \
+ .count = 128, \
+ .input_bpc = 10, .output_bpc = 16, \
+ .start = 0, .end = (1 << 10) - (1 << 10) / 128, \
+ .min = 0, .max = (1 << 16) - 1, \
+ }, \
+ /* PIPEGCMAX */ \
+ { \
+ .flags = (DRM_MODE_LUT_GAMMA | \
+ DRM_MODE_LUT_INTERPOLATE | \
+ DRM_MODE_LUT_REUSE_LAST | \
+ DRM_MODE_LUT_NON_DECREASING), \
+ .count = 1, \
+ .input_bpc = 10, .output_bpc = 16, \
+ .start = (1 << 10) - (1 << 10) / 128, .end = 1 << 10, \
+ .min = 0, .max = 1 << 16, \
+ }
+
+static const struct drm_color_lut_range i965_gamma_10[] = {
+ I965_GAMMA_10,
+};
+
+#define CHV_CGM_DEGAMMA \
+ { \
+ .flags = (DRM_MODE_LUT_DEGAMMA | \
+ DRM_MODE_LUT_INTERPOLATE | \
+ DRM_MODE_LUT_NON_DECREASING), \
+ .count = 65, \
+ .input_bpc = 10, .output_bpc = 14, \
+ .start = 0, .end = 1 << 10, \
+ .min = 0, .max = (1 << 14) - 1, \
+ }
+#define CHV_CGM_GAMMA \
+ { \
+ .flags = (DRM_MODE_LUT_GAMMA | \
+ DRM_MODE_LUT_INTERPOLATE | \
+ DRM_MODE_LUT_NON_DECREASING), \
+ .count = 257, \
+ .input_bpc = 14, .output_bpc = 10, \
+ .start = 0, .end = 1 << 14, \
+ .min = 0, .max = (1 << 10) - 1, \
+ }
+
+static const struct drm_color_lut_range chv_cgm_degamma[] = {
+ CHV_CGM_DEGAMMA,
+};
+
+static const struct drm_color_lut_range chv_cgm_gamma[] = {
+ CHV_CGM_GAMMA,
+};
+
+static const struct drm_color_lut_range chv_cgm_degamma_i9xx_gamma_8[] = {
+ CHV_CGM_DEGAMMA,
+ I9XX_GAMMA_8,
+};
+
+static const struct drm_color_lut_range chv_cgm_degamma_i965_gamma_10[] = {
+ CHV_CGM_DEGAMMA,
+ I965_GAMMA_10,
+};
+
+static const struct drm_color_lut_range chv_cgm_degamma_cgm_degamma[] = {
+ CHV_CGM_DEGAMMA,
+ CHV_CGM_GAMMA,
+};
+
+static const struct drm_color_lut_range ilk_gamma_degamma_8[] = {
+ {
+ .flags = (DRM_MODE_LUT_GAMMA |
+ DRM_MODE_LUT_DEGAMMA),
+ .count = 256,
+ .input_bpc = 8, .output_bpc = 8,
+ .start = 0, .end = (1 << 8) - 1,
+ .min = 0, .max = (1 << 8) - 1,
+ },
+};
+
+static const struct drm_color_lut_range ilk_gamma_degamma_10[] = {
+ {
+ .flags = (DRM_MODE_LUT_GAMMA |
+ DRM_MODE_LUT_DEGAMMA),
+ .count = 1024,
+ .input_bpc = 10, .output_bpc = 10,
+ .start = 0, .end = (1 << 10) - 1,
+ .min = 0, .max = (1 << 10) - 1,
+ },
+};
+
+static const struct drm_color_lut_range ilk_gamma_degamma_12p4[] = {
+ {
+ .flags = (DRM_MODE_LUT_GAMMA |
+ DRM_MODE_LUT_DEGAMMA |
+ DRM_MODE_LUT_INTERPOLATE |
+ DRM_MODE_LUT_NON_DECREASING),
+ .count = 512,
+ .input_bpc = 12, .output_bpc = 16,
+ .start = 0, .end = (1 << 12) - (1 << 12) / 512,
+ .min = 0, .max = (1 << 16) - 1,
+ },
+ /* PIPEGCMAX */
+ {
+ .flags = (DRM_MODE_LUT_GAMMA |
+ DRM_MODE_LUT_DEGAMMA |
+ DRM_MODE_LUT_INTERPOLATE |
+ DRM_MODE_LUT_REUSE_LAST |
+ DRM_MODE_LUT_NON_DECREASING),
+ .count = 1,
+ .input_bpc = 10, .output_bpc = 16,
+ .start = (1 << 12) - (1 << 12) / 512, .end = 1 << 12,
+ .min = 0, .max = 1 << 16,
+ },
+};
+
+static const struct drm_color_lut_range ivb_gamma_degamma_10[] = {
+ {
+ .flags = (DRM_MODE_LUT_GAMMA |
+ DRM_MODE_LUT_DEGAMMA |
+ DRM_MODE_LUT_REFLECT_NEGATIVE),
+ .count = 1024,
+ .input_bpc = 10, .output_bpc = 10,
+ .start = 0, .end = (1 << 10) - 1,
+ .min = 0, .max = (1 << 10) - 1,
+ },
+ /* PAL_EXT_GC_MAX */
+ {
+ .flags = (DRM_MODE_LUT_GAMMA |
+ DRM_MODE_LUT_DEGAMMA |
+ DRM_MODE_LUT_REFLECT_NEGATIVE |
+ DRM_MODE_LUT_INTERPOLATE |
+ DRM_MODE_LUT_REUSE_LAST |
+ DRM_MODE_LUT_NON_DECREASING),
+ .count = 1,
+ .input_bpc = 10, .output_bpc = 16,
+ .start = 1 << 10, .end = 3 << 10,
+ .min = 0, .max = (8 << 16) - 1,
+ }
+};
+
+static const struct drm_color_lut_range glk_gamma_10[] = {
+ {
+ .flags = (DRM_MODE_LUT_GAMMA |
+ DRM_MODE_LUT_REFLECT_NEGATIVE),
+ .count = 1024,
+ .input_bpc = 10, .output_bpc = 10,
+ .start = 0, .end = (1 << 10) - 1,
+ .min = 0, .max = (1 << 10) - 1,
+ },
+ /* PAL_EXT_GC_MAX */
+ {
+ .flags = (DRM_MODE_LUT_GAMMA |
+ DRM_MODE_LUT_REFLECT_NEGATIVE |
+ DRM_MODE_LUT_INTERPOLATE |
+ DRM_MODE_LUT_REUSE_LAST |
+ DRM_MODE_LUT_NON_DECREASING),
+ .count = 1,
+ .input_bpc = 10, .output_bpc = 16,
+ .start = 1 << 10, .end = 3 << 10,
+ .min = 0, .max = (8 << 16) - 1,
+ },
+ /* PAL_EXT2_GC_MAX */
+ {
+ .flags = (DRM_MODE_LUT_GAMMA |
+ DRM_MODE_LUT_REFLECT_NEGATIVE |
+ DRM_MODE_LUT_INTERPOLATE |
+ DRM_MODE_LUT_REUSE_LAST |
+ DRM_MODE_LUT_NON_DECREASING),
+ .count = 1,
+ .input_bpc = 10, .output_bpc = 16,
+ .start = 3 << 12, .end = 7 << 12,
+ .min = 0, .max = (8 << 16) - 1,
+ },
+};
+
+/* FIXME input bpc? */
+static const struct drm_color_lut_range glk_gamma_12p4[] = {
+ {
+ .flags = (DRM_MODE_LUT_GAMMA |
+ DRM_MODE_LUT_REFLECT_NEGATIVE |
+ DRM_MODE_LUT_INTERPOLATE |
+ DRM_MODE_LUT_NON_DECREASING),
+ .count = 512,
+ .input_bpc = 16, .output_bpc = 16,
+ .start = 0, .end = (1 << 16) - (1 << 16) / 512,
+ .min = 0, .max = (1 << 16) - 1,
+ },
+ /* PAL_GC_MAX */
+ {
+ .flags = (DRM_MODE_LUT_GAMMA |
+ DRM_MODE_LUT_REFLECT_NEGATIVE |
+ DRM_MODE_LUT_INTERPOLATE |
+ DRM_MODE_LUT_REUSE_LAST |
+ DRM_MODE_LUT_NON_DECREASING),
+ .count = 1,
+ .input_bpc = 16, .output_bpc = 16,
+ .start = (1 << 16) - (1 << 16) / 512, .end = 1 << 16,
+ .min = 0, .max = 1 << 16,
+ },
+ /* PAL_EXT_GC_MAX */
+ {
+ .flags = (DRM_MODE_LUT_GAMMA |
+ DRM_MODE_LUT_REFLECT_NEGATIVE |
+ DRM_MODE_LUT_INTERPOLATE |
+ DRM_MODE_LUT_REUSE_LAST |
+ DRM_MODE_LUT_NON_DECREASING),
+ .count = 1,
+ .input_bpc = 16, .output_bpc = 16,
+ .start = 1 << 16, .end = 3 << 16,
+ .min = 0, .max = (8 << 16) - 1,
+ },
+ /* PAL_EXT2_GC_MAX */
+ {
+ .flags = (DRM_MODE_LUT_GAMMA |
+ DRM_MODE_LUT_REFLECT_NEGATIVE |
+ DRM_MODE_LUT_INTERPOLATE |
+ DRM_MODE_LUT_REUSE_LAST |
+ DRM_MODE_LUT_NON_DECREASING),
+ .count = 1,
+ .input_bpc = 16, .output_bpc = 16,
+ .start = 3 << 16, .end = 7 << 16,
+ .min = 0, .max = (8 << 16) - 1,
+ },
+};
+
+static const struct drm_color_lut_range ivb_split_gamma[] = {
+ {
+ .flags = (DRM_MODE_LUT_DEGAMMA |
+ DRM_MODE_LUT_REFLECT_NEGATIVE),
+ .count = 512,
+ .input_bpc = 9, .output_bpc = 10,
+ .start = 0, .end = (1 << 9) - 1,
+ .min = 0, .max = (1 << 10) - 1,
+ },
+ /* PAL_EXT_GC_MAX */
+ {
+ .flags = (DRM_MODE_LUT_DEGAMMA |
+ DRM_MODE_LUT_REFLECT_NEGATIVE |
+ DRM_MODE_LUT_INTERPOLATE |
+ DRM_MODE_LUT_REUSE_LAST |
+ DRM_MODE_LUT_NON_DECREASING),
+ .count = 1,
+ .input_bpc = 10, .output_bpc = 16,
+ .start = 1 << 9, .end = 3 << 9,
+ .min = 0, .max = (8 << 16) - 1,
+ },
+ {
+ .flags = DRM_MODE_LUT_GAMMA,
+ .count = 512,
+ .input_bpc = 9, .output_bpc = 10,
+ .start = 0, .end = (1 << 9) - 1,
+ .min = 0, .max = (1 << 10) - 1,
+ },
+};
+
+/* FIXME input bpc? */
+static const struct drm_color_lut_range ivb_gamma_degamma_12p4[] = {
+ {
+ .flags = (DRM_MODE_LUT_GAMMA |
+ DRM_MODE_LUT_DEGAMMA |
+ DRM_MODE_LUT_REFLECT_NEGATIVE |
+ DRM_MODE_LUT_INTERPOLATE |
+ DRM_MODE_LUT_NON_DECREASING),
+ .count = 512,
+ .input_bpc = 12, .output_bpc = 16,
+ .start = 0, .end = (1 << 12) - (1 << 12) / 512,
+ .min = 0, .max = (1 << 16) - 1,
+ },
+ /* PAL_GC_MAX */
+ {
+ .flags = (DRM_MODE_LUT_GAMMA |
+ DRM_MODE_LUT_DEGAMMA |
+ DRM_MODE_LUT_REFLECT_NEGATIVE |
+ DRM_MODE_LUT_INTERPOLATE |
+ DRM_MODE_LUT_REUSE_LAST |
+ DRM_MODE_LUT_NON_DECREASING),
+ .count = 1,
+ .input_bpc = 12, .output_bpc = 16,
+ .start = (1 << 12) - (1 << 12) / 512, .end = 1 << 12,
+ .min = 0, .max = 1 << 16,
+ },
+ /* PAL_EXT_GC_MAX */
+ {
+ .flags = (DRM_MODE_LUT_GAMMA |
+ DRM_MODE_LUT_DEGAMMA |
+ DRM_MODE_LUT_REFLECT_NEGATIVE |
+ DRM_MODE_LUT_INTERPOLATE |
+ DRM_MODE_LUT_REUSE_LAST |
+ DRM_MODE_LUT_NON_DECREASING),
+ .count = 1,
+ .input_bpc = 12, .output_bpc = 16,
+ .start = 1 << 12, .end = 3 << 12,
+ .min = 0, .max = (8 << 16) - 1,
+ },
+};
+
+ /* FIXME input bpc? */
+static const struct drm_color_lut_range icl_multi_seg_gamma[] = {
+ /* segment 1 aka. super fine segment */
+ {
+ .flags = (DRM_MODE_LUT_GAMMA |
+ DRM_MODE_LUT_REFLECT_NEGATIVE |
+ DRM_MODE_LUT_INTERPOLATE |
+ DRM_MODE_LUT_NON_DECREASING),
+ .count = 9,
+ .input_bpc = 24, .output_bpc = 16,
+ .start = 0, .end = (1 << 24) / (128 * 256),
+ .min = 0, .max = (1 << 16) - 1,
+ },
+ /* segment 2 aka. fine segment */
+ {
+ .flags = (DRM_MODE_LUT_GAMMA |
+ DRM_MODE_LUT_REFLECT_NEGATIVE |
+ DRM_MODE_LUT_INTERPOLATE |
+ DRM_MODE_LUT_NON_DECREASING),
+ .count = 257,
+ .input_bpc = 24, .output_bpc = 16,
+ .start = 0, .end = (1 << 24) / 128,
+ .min = 0, .max = (1 << 16) - 1,
+ },
+ /* segment 3 aka. coarse segment */
+ {
+ .flags = (DRM_MODE_LUT_GAMMA |
+ DRM_MODE_LUT_REFLECT_NEGATIVE |
+ DRM_MODE_LUT_INTERPOLATE |
+ DRM_MODE_LUT_NON_DECREASING),
+ .count = 257,
+ .input_bpc = 24, .output_bpc = 16,
+ .start = 0, .end = (1 << 24) - (1 << 24) / 256,
+ .min = 0, .max = (1 << 16) - 1,
+ },
+ /* segment 3 aka. coarse segment / PAL_GC_MAX */
+ {
+ .flags = (DRM_MODE_LUT_GAMMA |
+ DRM_MODE_LUT_REFLECT_NEGATIVE |
+ DRM_MODE_LUT_INTERPOLATE |
+ DRM_MODE_LUT_REUSE_LAST |
+ DRM_MODE_LUT_NON_DECREASING),
+ .count = 1,
+ .input_bpc = 24, .output_bpc = 16,
+ .start = (1 << 24) - (1 << 24) / 256, .end = 1 << 24,
+ .min = 0, .max = 1 << 16,
+ },
+ /* PAL_EXT_GC_MAX */
+ {
+ .flags = (DRM_MODE_LUT_GAMMA |
+ DRM_MODE_LUT_REFLECT_NEGATIVE |
+ DRM_MODE_LUT_INTERPOLATE |
+ DRM_MODE_LUT_REUSE_LAST |
+ DRM_MODE_LUT_NON_DECREASING),
+ .count = 1,
+ .input_bpc = 24, .output_bpc = 16,
+ .start = 1 << 24, .end = 3 << 24,
+ .min = 0, .max = (8 << 16) - 1,
+ },
+ /* PAL_EXT2_GC_MAX */
+ {
+ .flags = (DRM_MODE_LUT_GAMMA |
+ DRM_MODE_LUT_REFLECT_NEGATIVE |
+ DRM_MODE_LUT_INTERPOLATE |
+ DRM_MODE_LUT_REUSE_LAST |
+ DRM_MODE_LUT_NON_DECREASING),
+ .count = 1,
+ .input_bpc = 24, .output_bpc = 16,
+ .start = 3 << 24, .end = 7 << 24,
+ .min = 0, .max = (8 << 16) - 1,
+ },
+};
+
void intel_color_init(struct intel_crtc *crtc)
{
struct drm_i915_private *dev_priv = to_i915(crtc->base.dev);
bool has_ctm = INTEL_INFO(dev_priv)->color.degamma_lut_size != 0;
+ int degamma_lut_size, gamma_lut_size;
drm_mode_crtc_set_gamma_size(&crtc->base, 256);
@@ -1233,25 +1657,159 @@ void intel_color_init(struct intel_crtc *crtc)
dev_priv->display.color_check = chv_color_check;
dev_priv->display.color_commit = i9xx_color_commit;
dev_priv->display.load_luts = cherryview_load_luts;
+
+ drm_color_add_gamma_mode_range(&dev_priv->drm,
+ "8bit gamma",
+ i9xx_gamma_8,
+ sizeof(i9xx_gamma_8));
+ drm_color_add_gamma_mode_range(&dev_priv->drm,
+ "interpolated gamma",
+ i965_gamma_10,
+ sizeof(i965_gamma_10));
+ drm_color_add_gamma_mode_range(&dev_priv->drm,
+ "CGM gamma",
+ chv_cgm_gamma,
+ sizeof(chv_cgm_gamma));
+ drm_color_add_gamma_mode_range(&dev_priv->drm,
+ "CGM degamma",
+ chv_cgm_degamma,
+ sizeof(chv_cgm_degamma));
+ drm_color_add_gamma_mode_range(&dev_priv->drm,
+ "CGM degamma with 8bit gamma",
+ chv_cgm_degamma_i9xx_gamma_8,
+ sizeof(chv_cgm_degamma));
+ drm_color_add_gamma_mode_range(&dev_priv->drm,
+ "CGM degamma with 10bit interpolated gamma",
+ chv_cgm_degamma_i965_gamma_10,
+ sizeof(chv_cgm_degamma));
+ drm_color_add_gamma_mode_range(&dev_priv->drm,
+ "CGM degamma with CGM gamma",
+ chv_cgm_degamma_cgm_degamma,
+ sizeof(chv_cgm_degamma));
} else if (INTEL_GEN(dev_priv) >= 4) {
dev_priv->display.color_check = i9xx_color_check;
dev_priv->display.color_commit = i9xx_color_commit;
dev_priv->display.load_luts = i965_load_luts;
+
+ drm_color_add_gamma_mode_range(&dev_priv->drm,
+ "8bit gamma",
+ i9xx_gamma_8,
+ sizeof(i9xx_gamma_8));
+ drm_color_add_gamma_mode_range(&dev_priv->drm,
+ "interpolated gamma",
+ i965_gamma_10,
+ sizeof(i965_gamma_10));
} else {
dev_priv->display.color_check = i9xx_color_check;
dev_priv->display.color_commit = i9xx_color_commit;
dev_priv->display.load_luts = i9xx_load_luts;
+
+ degamma_lut_size = 0;
+ gamma_lut_size = 0;
+ has_ctm = false;
+
+ drm_color_add_gamma_mode_range(&dev_priv->drm,
+ "8bit gamma",
+ i9xx_gamma_8,
+ sizeof(i9xx_gamma_8));
+
+ if (i9xx_has_10bit_lut(dev_priv)) {
+ /* 10bit interpolated gamma */
+ gamma_lut_size = I9XX_LUT_SIZE_10BIT;
+
+ drm_color_add_gamma_mode_range(&dev_priv->drm,
+ "interpolated gamma",
+ i9xx_gamma_10_slope,
+ sizeof(i9xx_gamma_10_slope));
+ }
}
} else {
- if (INTEL_GEN(dev_priv) >= 11)
+ if (INTEL_GEN(dev_priv) >= 11) {
dev_priv->display.color_check = icl_color_check;
- else if (INTEL_GEN(dev_priv) >= 10 || IS_GEMINILAKE(dev_priv))
+ /* don't advertize the >= 1.0 entries */
+ degamma_lut_size = 0;
+ gamma_lut_size = ILK_LUT_SIZE_10BIT;
+
+ drm_color_add_gamma_mode_range(&dev_priv->drm,
+ "8bit gamma",
+ i9xx_gamma_8,
+ sizeof(i9xx_gamma_8));
+ drm_color_add_gamma_mode_range(&dev_priv->drm,
+ "10bit gamma",
+ glk_gamma_10,
+ sizeof(glk_gamma_10));
+ drm_color_add_gamma_mode_range(&dev_priv->drm,
+ "interpolated gamma",
+ glk_gamma_12p4,
+ sizeof(glk_gamma_12p4));
+ drm_color_add_gamma_mode_range(&dev_priv->drm,
+ "multi-segmented gamma",
+ icl_multi_seg_gamma,
+ sizeof(icl_multi_seg_gamma));
+ } else if (INTEL_GEN(dev_priv) >= 10 || IS_GEMINILAKE(dev_priv)) {
dev_priv->display.color_check = glk_color_check;
- else if (INTEL_GEN(dev_priv) >= 7)
+ /* don't advertize the >= 1.0 entries */
+ degamma_lut_size = 0;
+ gamma_lut_size = ILK_LUT_SIZE_10BIT;
+ has_ctm = true;
+
+ drm_color_add_gamma_mode_range(&dev_priv->drm,
+ "8bit gamma",
+ i9xx_gamma_8,
+ sizeof(i9xx_gamma_8));
+ drm_color_add_gamma_mode_range(&dev_priv->drm,
+ "10bit gamma",
+ glk_gamma_10,
+ sizeof(glk_gamma_10));
+ drm_color_add_gamma_mode_range(&dev_priv->drm,
+ "interpolated gamma",
+ glk_gamma_12p4,
+ sizeof(glk_gamma_12p4));
+ } else if (INTEL_GEN(dev_priv) >= 7) {
dev_priv->display.color_check = ivb_color_check;
- else
+
+ /* don't advertize the >= 1.0 entries */
+ degamma_lut_size = IVB_LUT_SIZE_SPLIT;
+ gamma_lut_size = IVB_LUT_SIZE_SPLIT;
+ has_ctm = true;
+
+ drm_color_add_gamma_mode_range(&dev_priv->drm,
+ "8bit gamma or degamma",
+ ilk_gamma_degamma_8,
+ sizeof(ilk_gamma_degamma_8));
+ drm_color_add_gamma_mode_range(&dev_priv->drm,
+ "split gamma",
+ ivb_split_gamma,
+ sizeof(ivb_split_gamma));
+ drm_color_add_gamma_mode_range(&dev_priv->drm,
+ "10bit gamma or degamma",
+ ivb_gamma_degamma_10,
+ sizeof(ivb_gamma_degamma_10));
+ drm_color_add_gamma_mode_range(&dev_priv->drm,
+ "interpolated gamma or degamma",
+ ivb_gamma_degamma_12p4,
+ sizeof(ivb_gamma_degamma_12p4));
+ } else {
dev_priv->display.color_check = ilk_color_check;
+ degamma_lut_size = 0;
+ gamma_lut_size = ILK_LUT_SIZE_10BIT;
+ has_ctm = true;
+
+ drm_color_add_gamma_mode_range(&dev_priv->drm,
+ "8bit gamma or degamma",
+ ilk_gamma_degamma_8,
+ sizeof(ilk_gamma_degamma_8));
+ drm_color_add_gamma_mode_range(&dev_priv->drm,
+ "10bit gamma or degamma",
+ ilk_gamma_degamma_10,
+ sizeof(ilk_gamma_degamma_10));
+ drm_color_add_gamma_mode_range(&dev_priv->drm,
+ "interpolated gamma or degamma",
+ ilk_gamma_degamma_12p4,
+ sizeof(ilk_gamma_degamma_12p4));
+ }
+
if (INTEL_GEN(dev_priv) >= 9)
dev_priv->display.color_commit = skl_color_commit;
else if (IS_BROADWELL(dev_priv) || IS_HASWELL(dev_priv))
--
1.9.1
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
^ permalink raw reply related [flat|nested] 24+ messages in thread
* [v3 3/7] drm/i915/icl: Add register definitions for Multi Segmented gamma
2019-04-12 10:20 [v3 0/7] Add Multi Segment Gamma Support Uma Shankar
2019-04-12 10:20 ` [v3 1/7] drm: Add gamma mode property Uma Shankar
2019-04-12 10:20 ` [v3 2/7] drm/i915: Define color lut range structure Uma Shankar
@ 2019-04-12 10:20 ` Uma Shankar
2019-04-12 10:21 ` [v3 4/7] drm/i915/icl: Add support for multi segmented gamma mode Uma Shankar
` (6 subsequent siblings)
9 siblings, 0 replies; 24+ messages in thread
From: Uma Shankar @ 2019-04-12 10:20 UTC (permalink / raw)
To: intel-gfx, dri-devel
Cc: dcastagna, sam, seanpaul, ville.syrjala, maarten.lankhorst
Add macros to define multi segmented gamma registers
Signed-off-by: Uma Shankar <uma.shankar@intel.com>
---
drivers/gpu/drm/i915/i915_reg.h | 17 +++++++++++++++++
1 file changed, 17 insertions(+)
diff --git a/drivers/gpu/drm/i915/i915_reg.h b/drivers/gpu/drm/i915/i915_reg.h
index 9c206e8..5554b0c 100644
--- a/drivers/gpu/drm/i915/i915_reg.h
+++ b/drivers/gpu/drm/i915/i915_reg.h
@@ -7196,6 +7196,7 @@ enum {
#define GAMMA_MODE_MODE_10BIT (1 << 0)
#define GAMMA_MODE_MODE_12BIT (2 << 0)
#define GAMMA_MODE_MODE_SPLIT (3 << 0)
+#define GAMMA_MODE_MODE_12BIT_MULTI_SEGMENTED (3 << 0)
/* DMC/CSR */
#define CSR_PROGRAM(i) _MMIO(0x80000 + (i) * 4)
@@ -10136,6 +10137,22 @@ enum skl_power_gate {
#define PRE_CSC_GAMC_INDEX(pipe) _MMIO_PIPE(pipe, _PRE_CSC_GAMC_INDEX_A, _PRE_CSC_GAMC_INDEX_B)
#define PRE_CSC_GAMC_DATA(pipe) _MMIO_PIPE(pipe, _PRE_CSC_GAMC_DATA_A, _PRE_CSC_GAMC_DATA_B)
+/* Add registers for Gen11 Multi Segmented Gamma Mode */
+#define _PAL_PREC_MULTI_SEG_INDEX_A 0x4A408
+#define _PAL_PREC_MULTI_SEG_INDEX_B 0x4AC08
+#define PAL_PREC_MULTI_SEGMENT_AUTO_INCREMENT BIT(15)
+#define PAL_PREC_MULTI_SEGMENT_INDEX_VALUE_MASK (0x1f << 0)
+
+#define _PAL_PREC_MULTI_SEG_DATA_A 0x4A40C
+#define _PAL_PREC_MULTI_SEG_DATA_B 0x4AC0C
+
+#define PREC_PAL_MULTI_SEG_INDEX(pipe) _MMIO_PIPE(pipe, \
+ _PAL_PREC_MULTI_SEG_INDEX_A, \
+ _PAL_PREC_MULTI_SEG_INDEX_B)
+#define PREC_PAL_MULTI_SEG_DATA(pipe) _MMIO_PIPE(pipe, \
+ _PAL_PREC_MULTI_SEG_DATA_A, \
+ _PAL_PREC_MULTI_SEG_DATA_B)
+
/* pipe CSC & degamma/gamma LUTs on CHV */
#define _CGM_PIPE_A_CSC_COEFF01 (VLV_DISPLAY_BASE + 0x67900)
#define _CGM_PIPE_A_CSC_COEFF23 (VLV_DISPLAY_BASE + 0x67904)
--
1.9.1
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
^ permalink raw reply related [flat|nested] 24+ messages in thread
* [v3 4/7] drm/i915/icl: Add support for multi segmented gamma mode
2019-04-12 10:20 [v3 0/7] Add Multi Segment Gamma Support Uma Shankar
` (2 preceding siblings ...)
2019-04-12 10:20 ` [v3 3/7] drm/i915/icl: Add register definitions for Multi Segmented gamma Uma Shankar
@ 2019-04-12 10:21 ` Uma Shankar
2019-04-12 10:21 ` [v3 5/7] drm/i915: Attach gamma mode property Uma Shankar
` (5 subsequent siblings)
9 siblings, 0 replies; 24+ messages in thread
From: Uma Shankar @ 2019-04-12 10:21 UTC (permalink / raw)
To: intel-gfx, dri-devel
Cc: dcastagna, sam, seanpaul, ville.syrjala, maarten.lankhorst
Gen11 introduced a new gamma mode i.e, multi segmented
gamma mode. Added support for the same.
v2: Aligned to just 1 property interface as suggested by
Ville. Fixed Ville's review comments.
Signed-off-by: Uma Shankar <uma.shankar@intel.com>
---
drivers/gpu/drm/i915/intel_color.c | 166 ++++++++++++++++++++++++++++++++++++-
include/drm/drm_crtc.h | 3 +
2 files changed, 165 insertions(+), 4 deletions(-)
diff --git a/drivers/gpu/drm/i915/intel_color.c b/drivers/gpu/drm/i915/intel_color.c
index c433215..d4ce1ed 100644
--- a/drivers/gpu/drm/i915/intel_color.c
+++ b/drivers/gpu/drm/i915/intel_color.c
@@ -58,6 +58,12 @@
#define ILK_CSC_POSTOFF_LIMITED_RANGE (16 * (1 << 12) / 255)
+#define LEGACY_PALETTE_MODE_8BIT BIT(0)
+#define PRECISION_PALETTE_MODE_10BIT BIT(1)
+#define INTERPOLATED_GAMMA_MODE_12BIT BIT(2)
+#define MULTI_SEGMENTED_GAMMA_MODE_12BIT BIT(3)
+#define SPLIT_GAMMA_MODE_12BIT BIT(4)
+
static const u16 ilk_csc_off_zero[3] = {};
static const u16 ilk_csc_coeff_identity[9] = {
@@ -93,6 +99,22 @@
0x0800, 0x0100, 0x0800,
};
+/* ilk+ "12.4" interpolated format (high 10 bits) */
+static u32 ilk_lut_12p4_ldw(const struct drm_color_lut *color)
+{
+ return (color->red >> 6) << 20 |
+ (color->green >> 6) << 10 |
+ (color->blue >> 6);
+}
+
+/* ilk+ "12.4" interpolated format (low 6 bits) */
+static u32 ilk_lut_12p4_udw(const struct drm_color_lut *color)
+{
+ return (color->red & 0x3f) << 24 |
+ (color->green & 0x3f) << 14 |
+ (color->blue & 0x3f);
+}
+
static bool lut_is_legacy(const struct drm_property_blob *lut)
{
return drm_color_lut_size(lut) == LEGACY_LUT_LENGTH;
@@ -781,6 +803,118 @@ static void glk_load_luts(const struct intel_crtc_state *crtc_state)
}
}
+static void icl_program_coarse_segment_lut(const struct intel_crtc_state
+ *crtc_state,
+ struct drm_color_lut *gamma_lut,
+ u32 offset)
+{
+ struct intel_crtc *crtc = to_intel_crtc(crtc_state->base.crtc);
+ struct drm_i915_private *dev_priv = to_i915(crtc->base.dev);
+ const struct drm_color_lut *lut = gamma_lut;
+ enum pipe pipe = crtc->pipe;
+ u32 i, lut_size, word;
+
+ WARN_ON(offset & ~PAL_PREC_INDEX_VALUE_MASK);
+
+ I915_WRITE(PREC_PAL_INDEX(pipe),
+ (offset ? PAL_PREC_SPLIT_MODE : 0) |
+ PAL_PREC_AUTO_INCREMENT |
+ offset);
+
+ if (lut && crtc_state->base.gamma_mode_type ==
+ MULTI_SEGMENTED_GAMMA_MODE_12BIT) {
+ lut_size = 9 + 514;
+ for (i = 9; i < lut_size; i++) {
+ /* For Even Index */
+ word = ilk_lut_12p4_udw(&lut[i]);
+
+ I915_WRITE(PREC_PAL_DATA(pipe), word);
+
+ /* For ODD index */
+ word = ilk_lut_12p4_ldw(&lut[i]);
+
+ I915_WRITE(PREC_PAL_DATA(pipe), word);
+ }
+ }
+
+ /*
+ * Program the max register to clamp values > 1.0.
+ * ToDo: Extend the ABI to be able to program values
+ * from 1.0
+ */
+ I915_WRITE(PREC_PAL_GC_MAX(pipe, 0), (1 << 16));
+ I915_WRITE(PREC_PAL_GC_MAX(pipe, 1), (1 << 16));
+ I915_WRITE(PREC_PAL_GC_MAX(pipe, 2), (1 << 16));
+
+ /*
+ * Program the max register to clamp values > 1.0.
+ * ToDo: Extend the ABI to be able to program values
+ * from 1.0 to 3.0
+ */
+ I915_WRITE(PREC_PAL_EXT_GC_MAX(pipe, 0), (1 << 16));
+ I915_WRITE(PREC_PAL_EXT_GC_MAX(pipe, 1), (1 << 16));
+ I915_WRITE(PREC_PAL_EXT_GC_MAX(pipe, 2), (1 << 16));
+
+ /*
+ * Program the gc max 2 register to clamp values > 1.0.
+ * ToDo: Extend the ABI to be able to program values
+ * from 3.0 to 7.0
+ */
+ if (INTEL_GEN(dev_priv) >= 10 || IS_GEMINILAKE(dev_priv)) {
+ I915_WRITE(PREC_PAL_EXT2_GC_MAX(pipe, 0), (1 << 16));
+ I915_WRITE(PREC_PAL_EXT2_GC_MAX(pipe, 1), (1 << 16));
+ I915_WRITE(PREC_PAL_EXT2_GC_MAX(pipe, 2), (1 << 16));
+ }
+}
+
+static void icl_program_fine_segment_lut(const struct intel_crtc_state
+ *crtc_state,
+ struct drm_color_lut *gamma_lut,
+ u32 offset)
+{
+ struct drm_crtc *crtc = crtc_state->base.crtc;
+ struct drm_device *dev = crtc_state->base.crtc->dev;
+ struct drm_i915_private *dev_priv = to_i915(dev);
+ enum pipe pipe = to_intel_crtc(crtc)->pipe;
+ u32 i, word, lut_size = 9;
+
+ WARN_ON(offset & ~PAL_PREC_MULTI_SEGMENT_INDEX_VALUE_MASK);
+
+ I915_WRITE(PREC_PAL_MULTI_SEG_INDEX(pipe),
+ (PAL_PREC_AUTO_INCREMENT | offset));
+
+ if (gamma_lut) {
+ struct drm_color_lut *lut =
+ (struct drm_color_lut *)gamma_lut;
+
+ for (i = 0; i < lut_size; i++) {
+ /* For Even Index */
+ word = ilk_lut_12p4_udw(&lut[i]);
+
+ I915_WRITE(PREC_PAL_MULTI_SEG_DATA(pipe), word);
+
+ /* For ODD index */
+ word = ilk_lut_12p4_ldw(&lut[i]);
+
+ I915_WRITE(PREC_PAL_MULTI_SEG_DATA(pipe), word);
+ }
+ }
+}
+
+static void icl_load_gamma_multi_segmented_lut(const struct intel_crtc_state
+ *crtc_state, u32 offset)
+{
+ const struct drm_property_blob *gamma_lut_blob =
+ crtc_state->base.gamma_lut;
+ struct drm_color_lut *gamma_lut = NULL;
+
+ if (gamma_lut_blob)
+ gamma_lut = gamma_lut_blob->data;
+
+ icl_program_fine_segment_lut(crtc_state, gamma_lut, 0);
+ icl_program_coarse_segment_lut(crtc_state, gamma_lut, 0);
+}
+
static void icl_load_luts(const struct intel_crtc_state *crtc_state)
{
const struct drm_property_blob *gamma_lut = crtc_state->base.gamma_lut;
@@ -792,6 +926,9 @@ static void icl_load_luts(const struct intel_crtc_state *crtc_state)
if ((crtc_state->gamma_mode & GAMMA_MODE_MODE_MASK) ==
GAMMA_MODE_MODE_8BIT) {
i9xx_load_luts(crtc_state);
+ } else if (crtc_state->base.gamma_mode_type ==
+ MULTI_SEGMENTED_GAMMA_MODE_12BIT) {
+ icl_load_gamma_multi_segmented_lut(crtc_state, 0);
} else {
bdw_load_lut_10(crtc, gamma_lut, PAL_PREC_INDEX_VALUE(0));
ivb_load_lut_10_max(crtc);
@@ -1186,10 +1323,28 @@ static int glk_color_check(struct intel_crtc_state *crtc_state)
return 0;
}
-static u32 icl_gamma_mode(const struct intel_crtc_state *crtc_state)
+static u32 icl_gamma_mode(struct intel_crtc_state *crtc_state)
{
+ struct drm_device *dev = crtc_state->base.crtc->dev;
+ struct drm_mode_config *config = &dev->mode_config;
+ struct drm_property *property = config->gamma_mode_property;
+ struct drm_property_enum *prop_enum;
+ u32 index = 0;
u32 gamma_mode = 0;
+ list_for_each_entry(prop_enum, &property->enum_list, head) {
+ if (prop_enum->value == crtc_state->base.gamma_mode) {
+ if (!strcmp(prop_enum->name,
+ "multi-segmented gamma")) {
+ crtc_state->base.gamma_mode_type =
+ MULTI_SEGMENTED_GAMMA_MODE_12BIT;
+ DRM_INFO("multi-segment enabled\n");
+ }
+ break;
+ }
+ index++;
+ }
+
if (crtc_state->base.degamma_lut)
gamma_mode |= PRE_CSC_GAMMA_ENABLE;
@@ -1200,6 +1355,9 @@ static u32 icl_gamma_mode(const struct intel_crtc_state *crtc_state)
if (!crtc_state->base.gamma_lut ||
crtc_state_is_legacy_gamma(crtc_state))
gamma_mode |= GAMMA_MODE_MODE_8BIT;
+ else if (crtc_state->base.gamma_mode_type ==
+ MULTI_SEGMENTED_GAMMA_MODE_12BIT)
+ gamma_mode |= GAMMA_MODE_MODE_12BIT_MULTI_SEGMENTED;
else
gamma_mode |= GAMMA_MODE_MODE_10BIT;
@@ -1755,7 +1913,7 @@ void intel_color_init(struct intel_crtc *crtc)
drm_color_add_gamma_mode_range(&dev_priv->drm,
"8bit gamma",
- i9xx_gamma_8,
+ i9xx_gamma_8,
sizeof(i9xx_gamma_8));
drm_color_add_gamma_mode_range(&dev_priv->drm,
"10bit gamma",
@@ -1798,8 +1956,8 @@ void intel_color_init(struct intel_crtc *crtc)
drm_color_add_gamma_mode_range(&dev_priv->drm,
"8bit gamma or degamma",
- ilk_gamma_degamma_8,
- sizeof(ilk_gamma_degamma_8));
+ ilk_gamma_degamma_8,
+ sizeof(ilk_gamma_degamma_8));
drm_color_add_gamma_mode_range(&dev_priv->drm,
"10bit gamma or degamma",
ilk_gamma_degamma_10,
diff --git a/include/drm/drm_crtc.h b/include/drm/drm_crtc.h
index f2e60bd..f789359 100644
--- a/include/drm/drm_crtc.h
+++ b/include/drm/drm_crtc.h
@@ -255,6 +255,9 @@ struct drm_crtc_state {
*/
u32 gamma_mode;
+ /* Gamma mode type programmed on the pipe */
+ u32 gamma_mode_type;
+
/**
* @degamma_lut:
*
--
1.9.1
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
^ permalink raw reply related [flat|nested] 24+ messages in thread
* [v3 5/7] drm/i915: Attach gamma mode property
2019-04-12 10:20 [v3 0/7] Add Multi Segment Gamma Support Uma Shankar
` (3 preceding siblings ...)
2019-04-12 10:21 ` [v3 4/7] drm/i915/icl: Add support for multi segmented gamma mode Uma Shankar
@ 2019-04-12 10:21 ` Uma Shankar
2019-04-12 10:21 ` [v3 6/7] drm: Add Client Cap for advance gamma mode Uma Shankar
` (4 subsequent siblings)
9 siblings, 0 replies; 24+ messages in thread
From: Uma Shankar @ 2019-04-12 10:21 UTC (permalink / raw)
To: intel-gfx, dri-devel
Cc: dcastagna, sam, seanpaul, ville.syrjala, maarten.lankhorst
Attach the gamma mode property to allow userspace
set the gamma mode and provide the luts for the
same.
v2: Enabled just 1 property interface for gamma_mode,
as suggested by Ville.
Signed-off-by: Uma Shankar <uma.shankar@intel.com>
---
drivers/gpu/drm/i915/intel_color.c | 3 +++
drivers/gpu/drm/i915/intel_display.c | 3 +++
2 files changed, 6 insertions(+)
diff --git a/drivers/gpu/drm/i915/intel_color.c b/drivers/gpu/drm/i915/intel_color.c
index d4ce1ed..edf5ff8 100644
--- a/drivers/gpu/drm/i915/intel_color.c
+++ b/drivers/gpu/drm/i915/intel_color.c
@@ -1884,6 +1884,9 @@ void intel_color_init(struct intel_crtc *crtc)
} else {
if (INTEL_GEN(dev_priv) >= 11) {
dev_priv->display.color_check = icl_color_check;
+
+ drm_crtc_attach_gamma_mode_property(&crtc->base);
+
/* don't advertize the >= 1.0 entries */
degamma_lut_size = 0;
gamma_lut_size = ILK_LUT_SIZE_10BIT;
diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c
index f29a348..3e47935 100644
--- a/drivers/gpu/drm/i915/intel_display.c
+++ b/drivers/gpu/drm/i915/intel_display.c
@@ -15680,6 +15680,9 @@ int intel_modeset_init(struct drm_device *dev)
INTEL_INFO(dev_priv)->num_pipes,
INTEL_INFO(dev_priv)->num_pipes > 1 ? "s" : "");
+ if (INTEL_GEN(dev_priv) >= 11)
+ drm_color_create_gamma_mode_property(&dev_priv->drm, 4);
+
for_each_pipe(dev_priv, pipe) {
ret = intel_crtc_init(dev_priv, pipe);
if (ret) {
--
1.9.1
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
^ permalink raw reply related [flat|nested] 24+ messages in thread
* [v3 6/7] drm: Add Client Cap for advance gamma mode
2019-04-12 10:20 [v3 0/7] Add Multi Segment Gamma Support Uma Shankar
` (4 preceding siblings ...)
2019-04-12 10:21 ` [v3 5/7] drm/i915: Attach gamma mode property Uma Shankar
@ 2019-04-12 10:21 ` Uma Shankar
2019-04-15 10:57 ` Lankhorst, Maarten
2019-04-12 10:21 ` [v3 7/7] drm/i915: Enable " Uma Shankar
` (3 subsequent siblings)
9 siblings, 1 reply; 24+ messages in thread
From: Uma Shankar @ 2019-04-12 10:21 UTC (permalink / raw)
To: intel-gfx, dri-devel
Cc: dcastagna, sam, seanpaul, ville.syrjala, maarten.lankhorst
Introduced a client cap for advance cap mode
capability. Userspace should set this to get
to be able to use the new gamma_mode property.
If this is not set, driver will work in legacy
mode.
Suggested-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
Signed-off-by: Uma Shankar <uma.shankar@intel.com>
---
drivers/gpu/drm/drm_atomic_uapi.c | 3 +++
drivers/gpu/drm/drm_ioctl.c | 5 +++++
include/drm/drm_atomic.h | 1 +
include/drm/drm_crtc.h | 7 +++++++
include/drm/drm_file.h | 8 ++++++++
include/uapi/drm/drm.h | 2 ++
6 files changed, 26 insertions(+)
diff --git a/drivers/gpu/drm/drm_atomic_uapi.c b/drivers/gpu/drm/drm_atomic_uapi.c
index d85e0c9..590c87a 100644
--- a/drivers/gpu/drm/drm_atomic_uapi.c
+++ b/drivers/gpu/drm/drm_atomic_uapi.c
@@ -974,6 +974,8 @@ int drm_atomic_set_property(struct drm_atomic_state *state,
break;
}
+ crtc_state->advance_gamma_mode_active =
+ state->advance_gamma_mode_active;
ret = drm_atomic_crtc_set_property(crtc,
crtc_state, prop, prop_value);
break;
@@ -1297,6 +1299,7 @@ int drm_mode_atomic_ioctl(struct drm_device *dev,
drm_modeset_acquire_init(&ctx, DRM_MODESET_ACQUIRE_INTERRUPTIBLE);
state->acquire_ctx = &ctx;
state->allow_modeset = !!(arg->flags & DRM_MODE_ATOMIC_ALLOW_MODESET);
+ state->advance_gamma_mode_active = file_priv->advance_gamma_mode_active;
retry:
copied_objs = 0;
diff --git a/drivers/gpu/drm/drm_ioctl.c b/drivers/gpu/drm/drm_ioctl.c
index d337f16..e593a4c 100644
--- a/drivers/gpu/drm/drm_ioctl.c
+++ b/drivers/gpu/drm/drm_ioctl.c
@@ -348,6 +348,11 @@ static int drm_getcap(struct drm_device *dev, void *data, struct drm_file *file_
return -EINVAL;
file_priv->writeback_connectors = req->value;
break;
+ case DRM_CLIENT_CAP_ADVANCE_GAMMA_MODES:
+ if (req->value > 1)
+ return -EINVAL;
+ file_priv->advance_gamma_mode_active = req->value;
+ break;
default:
return -EINVAL;
}
diff --git a/include/drm/drm_atomic.h b/include/drm/drm_atomic.h
index 824a5ed..02c1a68 100644
--- a/include/drm/drm_atomic.h
+++ b/include/drm/drm_atomic.h
@@ -338,6 +338,7 @@ struct drm_atomic_state {
* states.
*/
bool duplicated : 1;
+ bool advance_gamma_mode_active : 1;
struct __drm_planes_state *planes;
struct __drm_crtcs_state *crtcs;
int num_connector;
diff --git a/include/drm/drm_crtc.h b/include/drm/drm_crtc.h
index f789359..f11dc25 100644
--- a/include/drm/drm_crtc.h
+++ b/include/drm/drm_crtc.h
@@ -170,6 +170,11 @@ struct drm_crtc_state {
bool color_mgmt_changed : 1;
/**
+ * This is to indicate advance gamma mode support
+ */
+ bool advance_gamma_mode_active : 1;
+
+ /**
* @no_vblank:
*
* Reflects the ability of a CRTC to send VBLANK events. This state
@@ -952,6 +957,8 @@ struct drm_crtc {
*/
bool enabled;
+ bool advance_gamma_mode_active : 1;
+
/**
* @mode:
*
diff --git a/include/drm/drm_file.h b/include/drm/drm_file.h
index 6710b61..b5aa24e 100644
--- a/include/drm/drm_file.h
+++ b/include/drm/drm_file.h
@@ -201,6 +201,14 @@ struct drm_file {
bool writeback_connectors;
/**
+ * This is to enable advance gamma modes using
+ * gamma_mode property
+ *
+ * True if client understands advance gamma
+ */
+ bool advance_gamma_mode_active : 1;
+
+ /**
* @is_master:
*
* This client is the creator of @master. Protected by struct
diff --git a/include/uapi/drm/drm.h b/include/uapi/drm/drm.h
index 236b01a..abef966 100644
--- a/include/uapi/drm/drm.h
+++ b/include/uapi/drm/drm.h
@@ -696,6 +696,8 @@ struct drm_get_cap {
*/
#define DRM_CLIENT_CAP_WRITEBACK_CONNECTORS 5
+#define DRM_CLIENT_CAP_ADVANCE_GAMMA_MODES 6
+
/** DRM_IOCTL_SET_CLIENT_CAP ioctl argument type */
struct drm_set_client_cap {
__u64 capability;
--
1.9.1
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
^ permalink raw reply related [flat|nested] 24+ messages in thread
* [v3 7/7] drm/i915: Enable advance gamma mode
2019-04-12 10:20 [v3 0/7] Add Multi Segment Gamma Support Uma Shankar
` (5 preceding siblings ...)
2019-04-12 10:21 ` [v3 6/7] drm: Add Client Cap for advance gamma mode Uma Shankar
@ 2019-04-12 10:21 ` Uma Shankar
2019-04-12 10:39 ` ✗ Fi.CI.CHECKPATCH: warning for Add Multi Segment Gamma Support (rev3) Patchwork
` (2 subsequent siblings)
9 siblings, 0 replies; 24+ messages in thread
From: Uma Shankar @ 2019-04-12 10:21 UTC (permalink / raw)
To: intel-gfx, dri-devel
Cc: dcastagna, sam, emil.l.velikov, Uma Shankar, seanpaul,
ville.syrjala, maarten.lankhorst
Enable advance gamma modes based on client
caps.
Signed-off-by: Uma Shankar <uma.shankar@intel.com>
---
drivers/gpu/drm/i915/intel_color.c | 16 +++++++++-------
1 file changed, 9 insertions(+), 7 deletions(-)
diff --git a/drivers/gpu/drm/i915/intel_color.c b/drivers/gpu/drm/i915/intel_color.c
index edf5ff8..36604c16 100644
--- a/drivers/gpu/drm/i915/intel_color.c
+++ b/drivers/gpu/drm/i915/intel_color.c
@@ -926,8 +926,9 @@ static void icl_load_luts(const struct intel_crtc_state *crtc_state)
if ((crtc_state->gamma_mode & GAMMA_MODE_MODE_MASK) ==
GAMMA_MODE_MODE_8BIT) {
i9xx_load_luts(crtc_state);
- } else if (crtc_state->base.gamma_mode_type ==
- MULTI_SEGMENTED_GAMMA_MODE_12BIT) {
+ } else if ((crtc_state->base.gamma_mode_type ==
+ MULTI_SEGMENTED_GAMMA_MODE_12BIT) &&
+ crtc_state->base.advance_gamma_mode_active) {
icl_load_gamma_multi_segmented_lut(crtc_state, 0);
} else {
bdw_load_lut_10(crtc, gamma_lut, PAL_PREC_INDEX_VALUE(0));
@@ -1352,12 +1353,13 @@ static u32 icl_gamma_mode(struct intel_crtc_state *crtc_state)
!crtc_state->c8_planes)
gamma_mode |= POST_CSC_GAMMA_ENABLE;
- if (!crtc_state->base.gamma_lut ||
- crtc_state_is_legacy_gamma(crtc_state))
- gamma_mode |= GAMMA_MODE_MODE_8BIT;
- else if (crtc_state->base.gamma_mode_type ==
- MULTI_SEGMENTED_GAMMA_MODE_12BIT)
+ if (crtc_state->base.gamma_mode_type ==
+ MULTI_SEGMENTED_GAMMA_MODE_12BIT &&
+ crtc_state->base.advance_gamma_mode_active)
gamma_mode |= GAMMA_MODE_MODE_12BIT_MULTI_SEGMENTED;
+ else if (!crtc_state->base.gamma_lut ||
+ crtc_state_is_legacy_gamma(crtc_state))
+ gamma_mode |= GAMMA_MODE_MODE_8BIT;
else
gamma_mode |= GAMMA_MODE_MODE_10BIT;
--
1.9.1
_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel
^ permalink raw reply related [flat|nested] 24+ messages in thread
* ✗ Fi.CI.CHECKPATCH: warning for Add Multi Segment Gamma Support (rev3)
2019-04-12 10:20 [v3 0/7] Add Multi Segment Gamma Support Uma Shankar
` (6 preceding siblings ...)
2019-04-12 10:21 ` [v3 7/7] drm/i915: Enable " Uma Shankar
@ 2019-04-12 10:39 ` Patchwork
2019-04-12 12:50 ` ✗ Fi.CI.BAT: failure " Patchwork
2019-04-17 7:28 ` [v3 0/7] Add Multi Segment Gamma Support Daniel Vetter
9 siblings, 0 replies; 24+ messages in thread
From: Patchwork @ 2019-04-12 10:39 UTC (permalink / raw)
To: Uma Shankar; +Cc: intel-gfx
== Series Details ==
Series: Add Multi Segment Gamma Support (rev3)
URL : https://patchwork.freedesktop.org/series/58169/
State : warning
== Summary ==
$ dim checkpatch origin/drm-tip
d60e638b9241 drm: Add gamma mode property
938ac10de262 drm/i915: Define color lut range structure
-:28: ERROR:CODE_INDENT: code indent should use tabs where possible
#28: FILE: drivers/gpu/drm/i915/intel_color.c:388:
+ /*$
-:29: ERROR:CODE_INDENT: code indent should use tabs where possible
#29: FILE: drivers/gpu/drm/i915/intel_color.c:389:
+ * Bspec:$
-:30: ERROR:CODE_INDENT: code indent should use tabs where possible
#30: FILE: drivers/gpu/drm/i915/intel_color.c:390:
+ " "NOTE: The 8-bit (non-10-bit) mode is the only$
-:31: ERROR:CODE_INDENT: code indent should use tabs where possible
#31: FILE: drivers/gpu/drm/i915/intel_color.c:391:
+ * mode supported by BrookDale-G and Springdale-G."$
-:32: ERROR:CODE_INDENT: code indent should use tabs where possible
#32: FILE: drivers/gpu/drm/i915/intel_color.c:392:
+ * and$
-:33: ERROR:CODE_INDENT: code indent should use tabs where possible
#33: FILE: drivers/gpu/drm/i915/intel_color.c:393:
+ * "NOTE: The 8-bit (non-10-bit) mode is the only$
-:34: ERROR:CODE_INDENT: code indent should use tabs where possible
#34: FILE: drivers/gpu/drm/i915/intel_color.c:394:
+ * mode supported by Alviso and Grantsdale."$
-:35: ERROR:CODE_INDENT: code indent should use tabs where possible
#35: FILE: drivers/gpu/drm/i915/intel_color.c:395:
+ */$
-:36: ERROR:CODE_INDENT: code indent should use tabs where possible
#36: FILE: drivers/gpu/drm/i915/intel_color.c:396:
+ return !IS_I845G(dev_priv) && !IS_I865G(dev_priv) &&$
-:36: WARNING:LEADING_SPACE: please, no spaces at the start of a line
#36: FILE: drivers/gpu/drm/i915/intel_color.c:396:
+ return !IS_I845G(dev_priv) && !IS_I865G(dev_priv) &&$
-:37: ERROR:CODE_INDENT: code indent should use tabs where possible
#37: FILE: drivers/gpu/drm/i915/intel_color.c:397:
+ !IS_I915G(dev_priv) && !IS_I915GM(dev_priv);$
-:37: WARNING:LEADING_SPACE: please, no spaces at the start of a line
#37: FILE: drivers/gpu/drm/i915/intel_color.c:397:
+ !IS_I915G(dev_priv) && !IS_I915GM(dev_priv);$
-:85: ERROR:COMPLEX_MACRO: Macros with complex values should be enclosed in parentheses
#85: FILE: drivers/gpu/drm/i915/intel_color.c:1276:
+#define I965_GAMMA_10 \
+ { \
+ .flags = (DRM_MODE_LUT_GAMMA | \
+ DRM_MODE_LUT_INTERPOLATE | \
+ DRM_MODE_LUT_NON_DECREASING), \
+ .count = 128, \
+ .input_bpc = 10, .output_bpc = 16, \
+ .start = 0, .end = (1 << 10) - (1 << 10) / 128, \
+ .min = 0, .max = (1 << 16) - 1, \
+ }, \
+ /* PIPEGCMAX */ \
+ { \
+ .flags = (DRM_MODE_LUT_GAMMA | \
+ DRM_MODE_LUT_INTERPOLATE | \
+ DRM_MODE_LUT_REUSE_LAST | \
+ DRM_MODE_LUT_NON_DECREASING), \
+ .count = 1, \
+ .input_bpc = 10, .output_bpc = 16, \
+ .start = (1 << 10) - (1 << 10) / 128, .end = 1 << 10, \
+ .min = 0, .max = 1 << 16, \
+ }
-:112: ERROR:CODE_INDENT: code indent should use tabs where possible
#112: FILE: drivers/gpu/drm/i915/intel_color.c:1303:
+ { \$
-:112: WARNING:LEADING_SPACE: please, no spaces at the start of a line
#112: FILE: drivers/gpu/drm/i915/intel_color.c:1303:
+ { \$
-:113: ERROR:CODE_INDENT: code indent should use tabs where possible
#113: FILE: drivers/gpu/drm/i915/intel_color.c:1304:
+ .flags = (DRM_MODE_LUT_DEGAMMA | \$
-:113: WARNING:LEADING_SPACE: please, no spaces at the start of a line
#113: FILE: drivers/gpu/drm/i915/intel_color.c:1304:
+ .flags = (DRM_MODE_LUT_DEGAMMA | \$
-:114: ERROR:CODE_INDENT: code indent should use tabs where possible
#114: FILE: drivers/gpu/drm/i915/intel_color.c:1305:
+ DRM_MODE_LUT_INTERPOLATE | \$
-:114: WARNING:LEADING_SPACE: please, no spaces at the start of a line
#114: FILE: drivers/gpu/drm/i915/intel_color.c:1305:
+ DRM_MODE_LUT_INTERPOLATE | \$
-:115: ERROR:CODE_INDENT: code indent should use tabs where possible
#115: FILE: drivers/gpu/drm/i915/intel_color.c:1306:
+ DRM_MODE_LUT_NON_DECREASING), \$
-:115: WARNING:LEADING_SPACE: please, no spaces at the start of a line
#115: FILE: drivers/gpu/drm/i915/intel_color.c:1306:
+ DRM_MODE_LUT_NON_DECREASING), \$
-:116: ERROR:CODE_INDENT: code indent should use tabs where possible
#116: FILE: drivers/gpu/drm/i915/intel_color.c:1307:
+ .count = 65, \$
-:116: WARNING:LEADING_SPACE: please, no spaces at the start of a line
#116: FILE: drivers/gpu/drm/i915/intel_color.c:1307:
+ .count = 65, \$
-:117: ERROR:CODE_INDENT: code indent should use tabs where possible
#117: FILE: drivers/gpu/drm/i915/intel_color.c:1308:
+ .input_bpc = 10, .output_bpc = 14, \$
-:117: WARNING:LEADING_SPACE: please, no spaces at the start of a line
#117: FILE: drivers/gpu/drm/i915/intel_color.c:1308:
+ .input_bpc = 10, .output_bpc = 14, \$
-:118: ERROR:CODE_INDENT: code indent should use tabs where possible
#118: FILE: drivers/gpu/drm/i915/intel_color.c:1309:
+ .start = 0, .end = 1 << 10, \$
-:118: WARNING:LEADING_SPACE: please, no spaces at the start of a line
#118: FILE: drivers/gpu/drm/i915/intel_color.c:1309:
+ .start = 0, .end = 1 << 10, \$
-:119: ERROR:CODE_INDENT: code indent should use tabs where possible
#119: FILE: drivers/gpu/drm/i915/intel_color.c:1310:
+ .min = 0, .max = (1 << 14) - 1, \$
-:119: WARNING:LEADING_SPACE: please, no spaces at the start of a line
#119: FILE: drivers/gpu/drm/i915/intel_color.c:1310:
+ .min = 0, .max = (1 << 14) - 1, \$
-:120: ERROR:CODE_INDENT: code indent should use tabs where possible
#120: FILE: drivers/gpu/drm/i915/intel_color.c:1311:
+ }$
-:120: WARNING:LEADING_SPACE: please, no spaces at the start of a line
#120: FILE: drivers/gpu/drm/i915/intel_color.c:1311:
+ }$
-:122: ERROR:CODE_INDENT: code indent should use tabs where possible
#122: FILE: drivers/gpu/drm/i915/intel_color.c:1313:
+ { \$
-:122: WARNING:LEADING_SPACE: please, no spaces at the start of a line
#122: FILE: drivers/gpu/drm/i915/intel_color.c:1313:
+ { \$
-:123: ERROR:CODE_INDENT: code indent should use tabs where possible
#123: FILE: drivers/gpu/drm/i915/intel_color.c:1314:
+ .flags = (DRM_MODE_LUT_GAMMA | \$
-:123: WARNING:LEADING_SPACE: please, no spaces at the start of a line
#123: FILE: drivers/gpu/drm/i915/intel_color.c:1314:
+ .flags = (DRM_MODE_LUT_GAMMA | \$
-:124: ERROR:CODE_INDENT: code indent should use tabs where possible
#124: FILE: drivers/gpu/drm/i915/intel_color.c:1315:
+ DRM_MODE_LUT_INTERPOLATE | \$
-:124: WARNING:LEADING_SPACE: please, no spaces at the start of a line
#124: FILE: drivers/gpu/drm/i915/intel_color.c:1315:
+ DRM_MODE_LUT_INTERPOLATE | \$
-:125: ERROR:CODE_INDENT: code indent should use tabs where possible
#125: FILE: drivers/gpu/drm/i915/intel_color.c:1316:
+ DRM_MODE_LUT_NON_DECREASING), \$
-:125: WARNING:LEADING_SPACE: please, no spaces at the start of a line
#125: FILE: drivers/gpu/drm/i915/intel_color.c:1316:
+ DRM_MODE_LUT_NON_DECREASING), \$
-:126: ERROR:CODE_INDENT: code indent should use tabs where possible
#126: FILE: drivers/gpu/drm/i915/intel_color.c:1317:
+ .count = 257, \$
-:126: WARNING:LEADING_SPACE: please, no spaces at the start of a line
#126: FILE: drivers/gpu/drm/i915/intel_color.c:1317:
+ .count = 257, \$
-:127: ERROR:CODE_INDENT: code indent should use tabs where possible
#127: FILE: drivers/gpu/drm/i915/intel_color.c:1318:
+ .input_bpc = 14, .output_bpc = 10, \$
-:127: WARNING:LEADING_SPACE: please, no spaces at the start of a line
#127: FILE: drivers/gpu/drm/i915/intel_color.c:1318:
+ .input_bpc = 14, .output_bpc = 10, \$
-:128: ERROR:CODE_INDENT: code indent should use tabs where possible
#128: FILE: drivers/gpu/drm/i915/intel_color.c:1319:
+ .start = 0, .end = 1 << 14, \$
-:128: WARNING:LEADING_SPACE: please, no spaces at the start of a line
#128: FILE: drivers/gpu/drm/i915/intel_color.c:1319:
+ .start = 0, .end = 1 << 14, \$
-:129: ERROR:CODE_INDENT: code indent should use tabs where possible
#129: FILE: drivers/gpu/drm/i915/intel_color.c:1320:
+ .min = 0, .max = (1 << 10) - 1, \$
-:129: WARNING:LEADING_SPACE: please, no spaces at the start of a line
#129: FILE: drivers/gpu/drm/i915/intel_color.c:1320:
+ .min = 0, .max = (1 << 10) - 1, \$
-:130: ERROR:CODE_INDENT: code indent should use tabs where possible
#130: FILE: drivers/gpu/drm/i915/intel_color.c:1321:
+ }$
-:130: WARNING:LEADING_SPACE: please, no spaces at the start of a line
#130: FILE: drivers/gpu/drm/i915/intel_color.c:1321:
+ }$
-:133: ERROR:CODE_INDENT: code indent should use tabs where possible
#133: FILE: drivers/gpu/drm/i915/intel_color.c:1324:
+ CHV_CGM_DEGAMMA,$
-:133: WARNING:LEADING_SPACE: please, no spaces at the start of a line
#133: FILE: drivers/gpu/drm/i915/intel_color.c:1324:
+ CHV_CGM_DEGAMMA,$
-:137: ERROR:CODE_INDENT: code indent should use tabs where possible
#137: FILE: drivers/gpu/drm/i915/intel_color.c:1328:
+ CHV_CGM_GAMMA,$
-:137: WARNING:LEADING_SPACE: please, no spaces at the start of a line
#137: FILE: drivers/gpu/drm/i915/intel_color.c:1328:
+ CHV_CGM_GAMMA,$
-:141: ERROR:CODE_INDENT: code indent should use tabs where possible
#141: FILE: drivers/gpu/drm/i915/intel_color.c:1332:
+ CHV_CGM_DEGAMMA,$
-:141: WARNING:LEADING_SPACE: please, no spaces at the start of a line
#141: FILE: drivers/gpu/drm/i915/intel_color.c:1332:
+ CHV_CGM_DEGAMMA,$
-:142: ERROR:CODE_INDENT: code indent should use tabs where possible
#142: FILE: drivers/gpu/drm/i915/intel_color.c:1333:
+ I9XX_GAMMA_8,$
-:142: WARNING:LEADING_SPACE: please, no spaces at the start of a line
#142: FILE: drivers/gpu/drm/i915/intel_color.c:1333:
+ I9XX_GAMMA_8,$
-:146: ERROR:CODE_INDENT: code indent should use tabs where possible
#146: FILE: drivers/gpu/drm/i915/intel_color.c:1337:
+ CHV_CGM_DEGAMMA,$
-:146: WARNING:LEADING_SPACE: please, no spaces at the start of a line
#146: FILE: drivers/gpu/drm/i915/intel_color.c:1337:
+ CHV_CGM_DEGAMMA,$
-:147: ERROR:CODE_INDENT: code indent should use tabs where possible
#147: FILE: drivers/gpu/drm/i915/intel_color.c:1338:
+ I965_GAMMA_10,$
-:147: WARNING:LEADING_SPACE: please, no spaces at the start of a line
#147: FILE: drivers/gpu/drm/i915/intel_color.c:1338:
+ I965_GAMMA_10,$
-:151: ERROR:CODE_INDENT: code indent should use tabs where possible
#151: FILE: drivers/gpu/drm/i915/intel_color.c:1342:
+ CHV_CGM_DEGAMMA,$
-:151: WARNING:LEADING_SPACE: please, no spaces at the start of a line
#151: FILE: drivers/gpu/drm/i915/intel_color.c:1342:
+ CHV_CGM_DEGAMMA,$
-:152: ERROR:CODE_INDENT: code indent should use tabs where possible
#152: FILE: drivers/gpu/drm/i915/intel_color.c:1343:
+ CHV_CGM_GAMMA,$
-:152: WARNING:LEADING_SPACE: please, no spaces at the start of a line
#152: FILE: drivers/gpu/drm/i915/intel_color.c:1343:
+ CHV_CGM_GAMMA,$
-:203: ERROR:CODE_INDENT: code indent should use tabs where possible
#203: FILE: drivers/gpu/drm/i915/intel_color.c:1394:
+ {$
-:203: WARNING:LEADING_SPACE: please, no spaces at the start of a line
#203: FILE: drivers/gpu/drm/i915/intel_color.c:1394:
+ {$
-:204: ERROR:CODE_INDENT: code indent should use tabs where possible
#204: FILE: drivers/gpu/drm/i915/intel_color.c:1395:
+ .flags = (DRM_MODE_LUT_GAMMA |$
-:204: WARNING:LEADING_SPACE: please, no spaces at the start of a line
#204: FILE: drivers/gpu/drm/i915/intel_color.c:1395:
+ .flags = (DRM_MODE_LUT_GAMMA |$
-:205: ERROR:CODE_INDENT: code indent should use tabs where possible
#205: FILE: drivers/gpu/drm/i915/intel_color.c:1396:
+ DRM_MODE_LUT_DEGAMMA |$
-:205: WARNING:LEADING_SPACE: please, no spaces at the start of a line
#205: FILE: drivers/gpu/drm/i915/intel_color.c:1396:
+ DRM_MODE_LUT_DEGAMMA |$
-:206: ERROR:CODE_INDENT: code indent should use tabs where possible
#206: FILE: drivers/gpu/drm/i915/intel_color.c:1397:
+ DRM_MODE_LUT_REFLECT_NEGATIVE),$
-:206: WARNING:LEADING_SPACE: please, no spaces at the start of a line
#206: FILE: drivers/gpu/drm/i915/intel_color.c:1397:
+ DRM_MODE_LUT_REFLECT_NEGATIVE),$
-:207: ERROR:CODE_INDENT: code indent should use tabs where possible
#207: FILE: drivers/gpu/drm/i915/intel_color.c:1398:
+ .count = 1024,$
-:207: WARNING:LEADING_SPACE: please, no spaces at the start of a line
#207: FILE: drivers/gpu/drm/i915/intel_color.c:1398:
+ .count = 1024,$
-:208: ERROR:CODE_INDENT: code indent should use tabs where possible
#208: FILE: drivers/gpu/drm/i915/intel_color.c:1399:
+ .input_bpc = 10, .output_bpc = 10,$
-:208: WARNING:LEADING_SPACE: please, no spaces at the start of a line
#208: FILE: drivers/gpu/drm/i915/intel_color.c:1399:
+ .input_bpc = 10, .output_bpc = 10,$
-:209: ERROR:CODE_INDENT: code indent should use tabs where possible
#209: FILE: drivers/gpu/drm/i915/intel_color.c:1400:
+ .start = 0, .end = (1 << 10) - 1,$
-:209: WARNING:LEADING_SPACE: please, no spaces at the start of a line
#209: FILE: drivers/gpu/drm/i915/intel_color.c:1400:
+ .start = 0, .end = (1 << 10) - 1,$
-:210: ERROR:CODE_INDENT: code indent should use tabs where possible
#210: FILE: drivers/gpu/drm/i915/intel_color.c:1401:
+ .min = 0, .max = (1 << 10) - 1,$
-:210: WARNING:LEADING_SPACE: please, no spaces at the start of a line
#210: FILE: drivers/gpu/drm/i915/intel_color.c:1401:
+ .min = 0, .max = (1 << 10) - 1,$
-:211: ERROR:CODE_INDENT: code indent should use tabs where possible
#211: FILE: drivers/gpu/drm/i915/intel_color.c:1402:
+ },$
-:211: WARNING:LEADING_SPACE: please, no spaces at the start of a line
#211: FILE: drivers/gpu/drm/i915/intel_color.c:1402:
+ },$
-:212: ERROR:CODE_INDENT: code indent should use tabs where possible
#212: FILE: drivers/gpu/drm/i915/intel_color.c:1403:
+ /* PAL_EXT_GC_MAX */$
-:213: ERROR:CODE_INDENT: code indent should use tabs where possible
#213: FILE: drivers/gpu/drm/i915/intel_color.c:1404:
+ {$
-:213: WARNING:LEADING_SPACE: please, no spaces at the start of a line
#213: FILE: drivers/gpu/drm/i915/intel_color.c:1404:
+ {$
-:214: ERROR:CODE_INDENT: code indent should use tabs where possible
#214: FILE: drivers/gpu/drm/i915/intel_color.c:1405:
+ .flags = (DRM_MODE_LUT_GAMMA |$
-:214: WARNING:LEADING_SPACE: please, no spaces at the start of a line
#214: FILE: drivers/gpu/drm/i915/intel_color.c:1405:
+ .flags = (DRM_MODE_LUT_GAMMA |$
-:215: ERROR:CODE_INDENT: code indent should use tabs where possible
#215: FILE: drivers/gpu/drm/i915/intel_color.c:1406:
+ DRM_MODE_LUT_DEGAMMA |$
-:215: WARNING:LEADING_SPACE: please, no spaces at the start of a line
#215: FILE: drivers/gpu/drm/i915/intel_color.c:1406:
+ DRM_MODE_LUT_DEGAMMA |$
-:216: ERROR:CODE_INDENT: code indent should use tabs where possible
#216: FILE: drivers/gpu/drm/i915/intel_color.c:1407:
+ DRM_MODE_LUT_REFLECT_NEGATIVE |$
-:216: WARNING:LEADING_SPACE: please, no spaces at the start of a line
#216: FILE: drivers/gpu/drm/i915/intel_color.c:1407:
+ DRM_MODE_LUT_REFLECT_NEGATIVE |$
-:217: ERROR:CODE_INDENT: code indent should use tabs where possible
#217: FILE: drivers/gpu/drm/i915/intel_color.c:1408:
+ DRM_MODE_LUT_INTERPOLATE |$
-:217: WARNING:LEADING_SPACE: please, no spaces at the start of a line
#217: FILE: drivers/gpu/drm/i915/intel_color.c:1408:
+ DRM_MODE_LUT_INTERPOLATE |$
-:218: ERROR:CODE_INDENT: code indent should use tabs where possible
#218: FILE: drivers/gpu/drm/i915/intel_color.c:1409:
+ DRM_MODE_LUT_REUSE_LAST |$
-:218: WARNING:LEADING_SPACE: please, no spaces at the start of a line
#218: FILE: drivers/gpu/drm/i915/intel_color.c:1409:
+ DRM_MODE_LUT_REUSE_LAST |$
-:219: ERROR:CODE_INDENT: code indent should use tabs where possible
#219: FILE: drivers/gpu/drm/i915/intel_color.c:1410:
+ DRM_MODE_LUT_NON_DECREASING),$
-:219: WARNING:LEADING_SPACE: please, no spaces at the start of a line
#219: FILE: drivers/gpu/drm/i915/intel_color.c:1410:
+ DRM_MODE_LUT_NON_DECREASING),$
-:220: ERROR:CODE_INDENT: code indent should use tabs where possible
#220: FILE: drivers/gpu/drm/i915/intel_color.c:1411:
+ .count = 1,$
-:220: WARNING:LEADING_SPACE: please, no spaces at the start of a line
#220: FILE: drivers/gpu/drm/i915/intel_color.c:1411:
+ .count = 1,$
-:221: ERROR:CODE_INDENT: code indent should use tabs where possible
#221: FILE: drivers/gpu/drm/i915/intel_color.c:1412:
+ .input_bpc = 10, .output_bpc = 16,$
-:221: WARNING:LEADING_SPACE: please, no spaces at the start of a line
#221: FILE: drivers/gpu/drm/i915/intel_color.c:1412:
+ .input_bpc = 10, .output_bpc = 16,$
-:222: ERROR:CODE_INDENT: code indent should use tabs where possible
#222: FILE: drivers/gpu/drm/i915/intel_color.c:1413:
+ .start = 1 << 10, .end = 3 << 10,$
-:222: WARNING:LEADING_SPACE: please, no spaces at the start of a line
#222: FILE: drivers/gpu/drm/i915/intel_color.c:1413:
+ .start = 1 << 10, .end = 3 << 10,$
-:223: ERROR:CODE_INDENT: code indent should use tabs where possible
#223: FILE: drivers/gpu/drm/i915/intel_color.c:1414:
+ .min = 0, .max = (8 << 16) - 1,$
-:223: WARNING:LEADING_SPACE: please, no spaces at the start of a line
#223: FILE: drivers/gpu/drm/i915/intel_color.c:1414:
+ .min = 0, .max = (8 << 16) - 1,$
-:224: ERROR:CODE_INDENT: code indent should use tabs where possible
#224: FILE: drivers/gpu/drm/i915/intel_color.c:1415:
+ }$
-:224: WARNING:LEADING_SPACE: please, no spaces at the start of a line
#224: FILE: drivers/gpu/drm/i915/intel_color.c:1415:
+ }$
-:313: ERROR:CODE_INDENT: code indent should use tabs where possible
#313: FILE: drivers/gpu/drm/i915/intel_color.c:1504:
+ {$
-:313: WARNING:LEADING_SPACE: please, no spaces at the start of a line
#313: FILE: drivers/gpu/drm/i915/intel_color.c:1504:
+ {$
-:314: ERROR:CODE_INDENT: code indent should use tabs where possible
#314: FILE: drivers/gpu/drm/i915/intel_color.c:1505:
+ .flags = (DRM_MODE_LUT_DEGAMMA |$
-:314: WARNING:LEADING_SPACE: please, no spaces at the start of a line
#314: FILE: drivers/gpu/drm/i915/intel_color.c:1505:
+ .flags = (DRM_MODE_LUT_DEGAMMA |$
-:315: ERROR:CODE_INDENT: code indent should use tabs where possible
#315: FILE: drivers/gpu/drm/i915/intel_color.c:1506:
+ DRM_MODE_LUT_REFLECT_NEGATIVE),$
-:315: WARNING:LEADING_SPACE: please, no spaces at the start of a line
#315: FILE: drivers/gpu/drm/i915/intel_color.c:1506:
+ DRM_MODE_LUT_REFLECT_NEGATIVE),$
-:316: ERROR:CODE_INDENT: code indent should use tabs where possible
#316: FILE: drivers/gpu/drm/i915/intel_color.c:1507:
+ .count = 512,$
-:316: WARNING:LEADING_SPACE: please, no spaces at the start of a line
#316: FILE: drivers/gpu/drm/i915/intel_color.c:1507:
+ .count = 512,$
-:317: ERROR:CODE_INDENT: code indent should use tabs where possible
#317: FILE: drivers/gpu/drm/i915/intel_color.c:1508:
+ .input_bpc = 9, .output_bpc = 10,$
-:317: WARNING:LEADING_SPACE: please, no spaces at the start o
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
^ permalink raw reply [flat|nested] 24+ messages in thread
* ✗ Fi.CI.BAT: failure for Add Multi Segment Gamma Support (rev3)
2019-04-12 10:20 [v3 0/7] Add Multi Segment Gamma Support Uma Shankar
` (7 preceding siblings ...)
2019-04-12 10:39 ` ✗ Fi.CI.CHECKPATCH: warning for Add Multi Segment Gamma Support (rev3) Patchwork
@ 2019-04-12 12:50 ` Patchwork
2019-04-17 7:28 ` [v3 0/7] Add Multi Segment Gamma Support Daniel Vetter
9 siblings, 0 replies; 24+ messages in thread
From: Patchwork @ 2019-04-12 12:50 UTC (permalink / raw)
To: Uma Shankar; +Cc: intel-gfx
== Series Details ==
Series: Add Multi Segment Gamma Support (rev3)
URL : https://patchwork.freedesktop.org/series/58169/
State : failure
== Summary ==
CI Bug Log - changes from CI_DRM_5922 -> Patchwork_12776
====================================================
Summary
-------
**FAILURE**
Serious unknown changes coming with Patchwork_12776 absolutely need to be
verified manually.
If you think the reported changes have nothing to do with the changes
introduced in Patchwork_12776, please notify your bug team to allow them
to document this new failure mode, which will reduce false positives in CI.
External URL: https://patchwork.freedesktop.org/api/1.0/series/58169/revisions/3/mbox/
Possible new issues
-------------------
Here are the unknown changes that may have been introduced in Patchwork_12776:
### IGT changes ###
#### Possible regressions ####
* igt@runner@aborted:
- fi-icl-u3: NOTRUN -> FAIL
- fi-icl-y: NOTRUN -> FAIL
Known issues
------------
Here are the changes found in Patchwork_12776 that come from known issues:
### IGT changes ###
#### Issues hit ####
* igt@gem_exec_suspend@basic-s3:
- fi-blb-e6850: PASS -> INCOMPLETE [fdo#107718]
* igt@i915_selftest@live_execlists:
- fi-apl-guc: PASS -> INCOMPLETE [fdo#103927] / [fdo#109720]
#### Possible fixes ####
* igt@i915_pm_rpm@module-reload:
- fi-skl-6770hq: FAIL [fdo#108511] -> PASS
* igt@kms_pipe_crc_basic@hang-read-crc-pipe-a:
- fi-glk-dsi: FAIL [fdo#103191] -> PASS
{name}: This element is suppressed. This means it is ignored when computing
the status of the difference (SUCCESS, WARNING, or FAILURE).
[fdo#103191]: https://bugs.freedesktop.org/show_bug.cgi?id=103191
[fdo#103927]: https://bugs.freedesktop.org/show_bug.cgi?id=103927
[fdo#107718]: https://bugs.freedesktop.org/show_bug.cgi?id=107718
[fdo#108511]: https://bugs.freedesktop.org/show_bug.cgi?id=108511
[fdo#109720]: https://bugs.freedesktop.org/show_bug.cgi?id=109720
[k.org#202973]: https://bugzilla.kernel.org/show_bug.cgi?id=202973
Participating hosts (49 -> 40)
------------------------------
Additional (1): fi-icl-u2
Missing (10): fi-kbl-soraka fi-bxt-dsi fi-hsw-4200u fi-hsw-peppy fi-byt-squawks fi-bsw-cyan fi-ctg-p8600 fi-gdg-551 fi-pnv-d510 fi-byt-clapper
Build changes
-------------
* Linux: CI_DRM_5922 -> Patchwork_12776
CI_DRM_5922: 849ac6dbff7f5073c3181c5eba07936fe3f576ec @ git://anongit.freedesktop.org/gfx-ci/linux
IGT_4944: 9b74b8226e8c108db091bd3b1d105a71dc0fb861 @ git://anongit.freedesktop.org/xorg/app/intel-gpu-tools
Patchwork_12776: b242a6ad7aac6fbe4a9781fe47f183533be9f4c2 @ git://anongit.freedesktop.org/gfx-ci/linux
== Linux commits ==
b242a6ad7aac drm/i915: Enable advance gamma mode
bdc37237baa1 drm: Add Client Cap for advance gamma mode
b369c50ff3bc drm/i915: Attach gamma mode property
cf96572ff8b3 drm/i915/icl: Add support for multi segmented gamma mode
d5c82ca71cfe drm/i915/icl: Add register definitions for Multi Segmented gamma
938ac10de262 drm/i915: Define color lut range structure
d60e638b9241 drm: Add gamma mode property
== Logs ==
For more details see: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_12776/
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
^ permalink raw reply [flat|nested] 24+ messages in thread
* Re: [v3 6/7] drm: Add Client Cap for advance gamma mode
2019-04-12 10:21 ` [v3 6/7] drm: Add Client Cap for advance gamma mode Uma Shankar
@ 2019-04-15 10:57 ` Lankhorst, Maarten
2019-04-15 12:43 ` [Intel-gfx] " Ville Syrjälä
2019-04-15 13:56 ` Sharma, Shashank
0 siblings, 2 replies; 24+ messages in thread
From: Lankhorst, Maarten @ 2019-04-15 10:57 UTC (permalink / raw)
To: Shankar, Uma, intel-gfx, dri-devel
Cc: Syrjala, Ville, sam, seanpaul, dcastagna
[-- Attachment #1.1: Type: text/plain, Size: 4411 bytes --]
fre 2019-04-12 klockan 15:51 +0530 skrev Uma Shankar:
> Introduced a client cap for advance cap mode
> capability. Userspace should set this to get
> to be able to use the new gamma_mode property.
>
> If this is not set, driver will work in legacy
> mode.
>
> Suggested-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
> Signed-off-by: Uma Shankar <uma.shankar@intel.com>
Nack, this doesn't seem like a sensible idea. We already guard it
behind the gamma mode property. Userspace shouldn't set the gamma mode
to a value it doesn't understand.
~Maarten
> drivers/gpu/drm/drm_atomic_uapi.c | 3 +++
> drivers/gpu/drm/drm_ioctl.c | 5 +++++
> include/drm/drm_atomic.h | 1 +
> include/drm/drm_crtc.h | 7 +++++++
> include/drm/drm_file.h | 8 ++++++++
> include/uapi/drm/drm.h | 2 ++
> 6 files changed, 26 insertions(+)
>
> diff --git a/drivers/gpu/drm/drm_atomic_uapi.c
> b/drivers/gpu/drm/drm_atomic_uapi.c
> index d85e0c9..590c87a 100644
> --- a/drivers/gpu/drm/drm_atomic_uapi.c
> +++ b/drivers/gpu/drm/drm_atomic_uapi.c
> @@ -974,6 +974,8 @@ int drm_atomic_set_property(struct
> drm_atomic_state *state,
> break;
> }
>
> + crtc_state->advance_gamma_mode_active =
> + state-
> >advance_gamma_mode_active;
> ret = drm_atomic_crtc_set_property(crtc,
> crtc_state, prop, prop_value);
> break;
> @@ -1297,6 +1299,7 @@ int drm_mode_atomic_ioctl(struct drm_device
> *dev,
> drm_modeset_acquire_init(&ctx,
> DRM_MODESET_ACQUIRE_INTERRUPTIBLE);
> state->acquire_ctx = &ctx;
> state->allow_modeset = !!(arg->flags &
> DRM_MODE_ATOMIC_ALLOW_MODESET);
> + state->advance_gamma_mode_active = file_priv-
> >advance_gamma_mode_active;
>
> retry:
> copied_objs = 0;
> diff --git a/drivers/gpu/drm/drm_ioctl.c
> b/drivers/gpu/drm/drm_ioctl.c
> index d337f16..e593a4c 100644
> --- a/drivers/gpu/drm/drm_ioctl.c
> +++ b/drivers/gpu/drm/drm_ioctl.c
> @@ -348,6 +348,11 @@ static int drm_getcap(struct drm_device *dev,
> void *data, struct drm_file *file_
> return -EINVAL;
> file_priv->writeback_connectors = req->value;
> break;
> + case DRM_CLIENT_CAP_ADVANCE_GAMMA_MODES:
> + if (req->value > 1)
> + return -EINVAL;
> + file_priv->advance_gamma_mode_active = req->value;
> + break;
> default:
> return -EINVAL;
> }
> diff --git a/include/drm/drm_atomic.h b/include/drm/drm_atomic.h
> index 824a5ed..02c1a68 100644
> --- a/include/drm/drm_atomic.h
> +++ b/include/drm/drm_atomic.h
> @@ -338,6 +338,7 @@ struct drm_atomic_state {
> * states.
> */
> bool duplicated : 1;
> + bool advance_gamma_mode_active : 1;
> struct __drm_planes_state *planes;
> struct __drm_crtcs_state *crtcs;
> int num_connector;
> diff --git a/include/drm/drm_crtc.h b/include/drm/drm_crtc.h
> index f789359..f11dc25 100644
> --- a/include/drm/drm_crtc.h
> +++ b/include/drm/drm_crtc.h
> @@ -170,6 +170,11 @@ struct drm_crtc_state {
> bool color_mgmt_changed : 1;
>
> /**
> + * This is to indicate advance gamma mode support
> + */
> + bool advance_gamma_mode_active : 1;
> +
> + /**
> * @no_vblank:
> *
> * Reflects the ability of a CRTC to send VBLANK events.
> This state
> @@ -952,6 +957,8 @@ struct drm_crtc {
> */
> bool enabled;
>
> + bool advance_gamma_mode_active : 1;
> +
> /**
> * @mode:
> *
> diff --git a/include/drm/drm_file.h b/include/drm/drm_file.h
> index 6710b61..b5aa24e 100644
> --- a/include/drm/drm_file.h
> +++ b/include/drm/drm_file.h
> @@ -201,6 +201,14 @@ struct drm_file {
> bool writeback_connectors;
>
> /**
> + * This is to enable advance gamma modes using
> + * gamma_mode property
> + *
> + * True if client understands advance gamma
> + */
> + bool advance_gamma_mode_active : 1;
> +
> + /**
> * @is_master:
> *
> * This client is the creator of @master. Protected by
> struct
> diff --git a/include/uapi/drm/drm.h b/include/uapi/drm/drm.h
> index 236b01a..abef966 100644
> --- a/include/uapi/drm/drm.h
> +++ b/include/uapi/drm/drm.h
> @@ -696,6 +696,8 @@ struct drm_get_cap {
> */
> #define DRM_CLIENT_CAP_WRITEBACK_CONNECTORS 5
>
> +#define DRM_CLIENT_CAP_ADVANCE_GAMMA_MODES 6
> +
> /** DRM_IOCTL_SET_CLIENT_CAP ioctl argument type */
> struct drm_set_client_cap {
> __u64 capability;
[-- Attachment #1.2: smime.p7s --]
[-- Type: application/x-pkcs7-signature, Size: 3282 bytes --]
[-- Attachment #2: Type: text/plain, Size: 159 bytes --]
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
^ permalink raw reply [flat|nested] 24+ messages in thread
* Re: [Intel-gfx] [v3 6/7] drm: Add Client Cap for advance gamma mode
2019-04-15 10:57 ` Lankhorst, Maarten
@ 2019-04-15 12:43 ` Ville Syrjälä
2019-04-16 8:54 ` Lankhorst, Maarten
2019-04-15 13:56 ` Sharma, Shashank
1 sibling, 1 reply; 24+ messages in thread
From: Ville Syrjälä @ 2019-04-15 12:43 UTC (permalink / raw)
To: Lankhorst, Maarten
Cc: Syrjala, Ville, intel-gfx, dri-devel, Shankar, Uma, seanpaul,
dcastagna, sam
On Mon, Apr 15, 2019 at 10:57:52AM +0000, Lankhorst, Maarten wrote:
> fre 2019-04-12 klockan 15:51 +0530 skrev Uma Shankar:
> > Introduced a client cap for advance cap mode
> > capability. Userspace should set this to get
> > to be able to use the new gamma_mode property.
> >
> > If this is not set, driver will work in legacy
> > mode.
> >
> > Suggested-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
> > Signed-off-by: Uma Shankar <uma.shankar@intel.com>
>
> Nack, this doesn't seem like a sensible idea. We already guard it
> behind the gamma mode property. Userspace shouldn't set the gamma mode
> to a value it doesn't understand.
Old userspace wouldn't know what a gamma_mode prop is. While a client
cap isn't an entirely satisfactory solution I can't think of a better
way at the moment.
Well, maybe the old "hey kernel, please reset all my props to some
sane default" idea could be another way to deal with this sort of stuff.
>
> ~Maarten
>
> > drivers/gpu/drm/drm_atomic_uapi.c | 3 +++
> > drivers/gpu/drm/drm_ioctl.c | 5 +++++
> > include/drm/drm_atomic.h | 1 +
> > include/drm/drm_crtc.h | 7 +++++++
> > include/drm/drm_file.h | 8 ++++++++
> > include/uapi/drm/drm.h | 2 ++
> > 6 files changed, 26 insertions(+)
> >
> > diff --git a/drivers/gpu/drm/drm_atomic_uapi.c
> > b/drivers/gpu/drm/drm_atomic_uapi.c
> > index d85e0c9..590c87a 100644
> > --- a/drivers/gpu/drm/drm_atomic_uapi.c
> > +++ b/drivers/gpu/drm/drm_atomic_uapi.c
> > @@ -974,6 +974,8 @@ int drm_atomic_set_property(struct
> > drm_atomic_state *state,
> > break;
> > }
> >
> > + crtc_state->advance_gamma_mode_active =
> > + state-
> > >advance_gamma_mode_active;
> > ret = drm_atomic_crtc_set_property(crtc,
> > crtc_state, prop, prop_value);
> > break;
> > @@ -1297,6 +1299,7 @@ int drm_mode_atomic_ioctl(struct drm_device
> > *dev,
> > drm_modeset_acquire_init(&ctx,
> > DRM_MODESET_ACQUIRE_INTERRUPTIBLE);
> > state->acquire_ctx = &ctx;
> > state->allow_modeset = !!(arg->flags &
> > DRM_MODE_ATOMIC_ALLOW_MODESET);
> > + state->advance_gamma_mode_active = file_priv-
> > >advance_gamma_mode_active;
> >
> > retry:
> > copied_objs = 0;
> > diff --git a/drivers/gpu/drm/drm_ioctl.c
> > b/drivers/gpu/drm/drm_ioctl.c
> > index d337f16..e593a4c 100644
> > --- a/drivers/gpu/drm/drm_ioctl.c
> > +++ b/drivers/gpu/drm/drm_ioctl.c
> > @@ -348,6 +348,11 @@ static int drm_getcap(struct drm_device *dev,
> > void *data, struct drm_file *file_
> > return -EINVAL;
> > file_priv->writeback_connectors = req->value;
> > break;
> > + case DRM_CLIENT_CAP_ADVANCE_GAMMA_MODES:
> > + if (req->value > 1)
> > + return -EINVAL;
> > + file_priv->advance_gamma_mode_active = req->value;
> > + break;
> > default:
> > return -EINVAL;
> > }
> > diff --git a/include/drm/drm_atomic.h b/include/drm/drm_atomic.h
> > index 824a5ed..02c1a68 100644
> > --- a/include/drm/drm_atomic.h
> > +++ b/include/drm/drm_atomic.h
> > @@ -338,6 +338,7 @@ struct drm_atomic_state {
> > * states.
> > */
> > bool duplicated : 1;
> > + bool advance_gamma_mode_active : 1;
> > struct __drm_planes_state *planes;
> > struct __drm_crtcs_state *crtcs;
> > int num_connector;
> > diff --git a/include/drm/drm_crtc.h b/include/drm/drm_crtc.h
> > index f789359..f11dc25 100644
> > --- a/include/drm/drm_crtc.h
> > +++ b/include/drm/drm_crtc.h
> > @@ -170,6 +170,11 @@ struct drm_crtc_state {
> > bool color_mgmt_changed : 1;
> >
> > /**
> > + * This is to indicate advance gamma mode support
> > + */
> > + bool advance_gamma_mode_active : 1;
> > +
> > + /**
> > * @no_vblank:
> > *
> > * Reflects the ability of a CRTC to send VBLANK events.
> > This state
> > @@ -952,6 +957,8 @@ struct drm_crtc {
> > */
> > bool enabled;
> >
> > + bool advance_gamma_mode_active : 1;
> > +
> > /**
> > * @mode:
> > *
> > diff --git a/include/drm/drm_file.h b/include/drm/drm_file.h
> > index 6710b61..b5aa24e 100644
> > --- a/include/drm/drm_file.h
> > +++ b/include/drm/drm_file.h
> > @@ -201,6 +201,14 @@ struct drm_file {
> > bool writeback_connectors;
> >
> > /**
> > + * This is to enable advance gamma modes using
> > + * gamma_mode property
> > + *
> > + * True if client understands advance gamma
> > + */
> > + bool advance_gamma_mode_active : 1;
> > +
> > + /**
> > * @is_master:
> > *
> > * This client is the creator of @master. Protected by
> > struct
> > diff --git a/include/uapi/drm/drm.h b/include/uapi/drm/drm.h
> > index 236b01a..abef966 100644
> > --- a/include/uapi/drm/drm.h
> > +++ b/include/uapi/drm/drm.h
> > @@ -696,6 +696,8 @@ struct drm_get_cap {
> > */
> > #define DRM_CLIENT_CAP_WRITEBACK_CONNECTORS 5
> >
> > +#define DRM_CLIENT_CAP_ADVANCE_GAMMA_MODES 6
> > +
> > /** DRM_IOCTL_SET_CLIENT_CAP ioctl argument type */
> > struct drm_set_client_cap {
> > __u64 capability;
> _______________________________________________
> Intel-gfx mailing list
> Intel-gfx@lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/intel-gfx
--
Ville Syrjälä
Intel
_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel
^ permalink raw reply [flat|nested] 24+ messages in thread
* Re: [v3 6/7] drm: Add Client Cap for advance gamma mode
2019-04-15 10:57 ` Lankhorst, Maarten
2019-04-15 12:43 ` [Intel-gfx] " Ville Syrjälä
@ 2019-04-15 13:56 ` Sharma, Shashank
2019-04-15 14:12 ` Lankhorst, Maarten
1 sibling, 1 reply; 24+ messages in thread
From: Sharma, Shashank @ 2019-04-15 13:56 UTC (permalink / raw)
To: Lankhorst, Maarten, Shankar, Uma, intel-gfx, dri-devel
Cc: Syrjala, Ville, sam, seanpaul, dcastagna
> -----Original Message-----
> From: Lankhorst, Maarten
> Sent: Monday, April 15, 2019 4:28 PM
> To: Shankar, Uma <uma.shankar@intel.com>; intel-gfx@lists.freedesktop.org; dri-
> devel@lists.freedesktop.org
> Cc: Syrjala, Ville <ville.syrjala@intel.com>; emil.l.velikov@gmail.com;
> sam@ravnborg.org; Roper, Matthew D <matthew.d.roper@intel.com>;
> seanpaul@chromium.org; brian.starkey@arm.com; dcastagna@chromium.org;
> Sharma, Shashank <shashank.sharma@intel.com>
> Subject: Re: [v3 6/7] drm: Add Client Cap for advance gamma mode
>
> fre 2019-04-12 klockan 15:51 +0530 skrev Uma Shankar:
> > Introduced a client cap for advance cap mode
> > capability. Userspace should set this to get
> > to be able to use the new gamma_mode property.
> >
> > If this is not set, driver will work in legacy
> > mode.
> >
> > Suggested-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
> > Signed-off-by: Uma Shankar <uma.shankar@intel.com>
>
> Nack, this doesn't seem like a sensible idea. We already guard it
> behind the gamma mode property. Userspace shouldn't set the gamma mode
> to a value it doesn't understand.
>
> ~Maarten
Hey Maarten,
In that case, what do you suggest should be the right way to do this ?
@Ville, any comments here ?
Regards
Shashank
>
> > drivers/gpu/drm/drm_atomic_uapi.c | 3 +++
> > drivers/gpu/drm/drm_ioctl.c | 5 +++++
> > include/drm/drm_atomic.h | 1 +
> > include/drm/drm_crtc.h | 7 +++++++
> > include/drm/drm_file.h | 8 ++++++++
> > include/uapi/drm/drm.h | 2 ++
> > 6 files changed, 26 insertions(+)
> >
> > diff --git a/drivers/gpu/drm/drm_atomic_uapi.c
> > b/drivers/gpu/drm/drm_atomic_uapi.c
> > index d85e0c9..590c87a 100644
> > --- a/drivers/gpu/drm/drm_atomic_uapi.c
> > +++ b/drivers/gpu/drm/drm_atomic_uapi.c
> > @@ -974,6 +974,8 @@ int drm_atomic_set_property(struct
> > drm_atomic_state *state,
> > break;
> > }
> >
> > + crtc_state->advance_gamma_mode_active =
> > + state-
> > >advance_gamma_mode_active;
> > ret = drm_atomic_crtc_set_property(crtc,
> > crtc_state, prop, prop_value);
> > break;
> > @@ -1297,6 +1299,7 @@ int drm_mode_atomic_ioctl(struct drm_device
> > *dev,
> > drm_modeset_acquire_init(&ctx,
> > DRM_MODESET_ACQUIRE_INTERRUPTIBLE);
> > state->acquire_ctx = &ctx;
> > state->allow_modeset = !!(arg->flags &
> > DRM_MODE_ATOMIC_ALLOW_MODESET);
> > + state->advance_gamma_mode_active = file_priv-
> > >advance_gamma_mode_active;
> >
> > retry:
> > copied_objs = 0;
> > diff --git a/drivers/gpu/drm/drm_ioctl.c
> > b/drivers/gpu/drm/drm_ioctl.c
> > index d337f16..e593a4c 100644
> > --- a/drivers/gpu/drm/drm_ioctl.c
> > +++ b/drivers/gpu/drm/drm_ioctl.c
> > @@ -348,6 +348,11 @@ static int drm_getcap(struct drm_device *dev,
> > void *data, struct drm_file *file_
> > return -EINVAL;
> > file_priv->writeback_connectors = req->value;
> > break;
> > + case DRM_CLIENT_CAP_ADVANCE_GAMMA_MODES:
> > + if (req->value > 1)
> > + return -EINVAL;
> > + file_priv->advance_gamma_mode_active = req->value;
> > + break;
> > default:
> > return -EINVAL;
> > }
> > diff --git a/include/drm/drm_atomic.h b/include/drm/drm_atomic.h
> > index 824a5ed..02c1a68 100644
> > --- a/include/drm/drm_atomic.h
> > +++ b/include/drm/drm_atomic.h
> > @@ -338,6 +338,7 @@ struct drm_atomic_state {
> > * states.
> > */
> > bool duplicated : 1;
> > + bool advance_gamma_mode_active : 1;
> > struct __drm_planes_state *planes;
> > struct __drm_crtcs_state *crtcs;
> > int num_connector;
> > diff --git a/include/drm/drm_crtc.h b/include/drm/drm_crtc.h
> > index f789359..f11dc25 100644
> > --- a/include/drm/drm_crtc.h
> > +++ b/include/drm/drm_crtc.h
> > @@ -170,6 +170,11 @@ struct drm_crtc_state {
> > bool color_mgmt_changed : 1;
> >
> > /**
> > + * This is to indicate advance gamma mode support
> > + */
> > + bool advance_gamma_mode_active : 1;
> > +
> > + /**
> > * @no_vblank:
> > *
> > * Reflects the ability of a CRTC to send VBLANK events.
> > This state
> > @@ -952,6 +957,8 @@ struct drm_crtc {
> > */
> > bool enabled;
> >
> > + bool advance_gamma_mode_active : 1;
> > +
> > /**
> > * @mode:
> > *
> > diff --git a/include/drm/drm_file.h b/include/drm/drm_file.h
> > index 6710b61..b5aa24e 100644
> > --- a/include/drm/drm_file.h
> > +++ b/include/drm/drm_file.h
> > @@ -201,6 +201,14 @@ struct drm_file {
> > bool writeback_connectors;
> >
> > /**
> > + * This is to enable advance gamma modes using
> > + * gamma_mode property
> > + *
> > + * True if client understands advance gamma
> > + */
> > + bool advance_gamma_mode_active : 1;
> > +
> > + /**
> > * @is_master:
> > *
> > * This client is the creator of @master. Protected by
> > struct
> > diff --git a/include/uapi/drm/drm.h b/include/uapi/drm/drm.h
> > index 236b01a..abef966 100644
> > --- a/include/uapi/drm/drm.h
> > +++ b/include/uapi/drm/drm.h
> > @@ -696,6 +696,8 @@ struct drm_get_cap {
> > */
> > #define DRM_CLIENT_CAP_WRITEBACK_CONNECTORS 5
> >
> > +#define DRM_CLIENT_CAP_ADVANCE_GAMMA_MODES 6
> > +
> > /** DRM_IOCTL_SET_CLIENT_CAP ioctl argument type */
> > struct drm_set_client_cap {
> > __u64 capability;
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
^ permalink raw reply [flat|nested] 24+ messages in thread
* Re: [v3 6/7] drm: Add Client Cap for advance gamma mode
2019-04-15 13:56 ` Sharma, Shashank
@ 2019-04-15 14:12 ` Lankhorst, Maarten
2019-04-15 14:29 ` Sharma, Shashank
2019-04-15 19:20 ` Daniel Vetter
0 siblings, 2 replies; 24+ messages in thread
From: Lankhorst, Maarten @ 2019-04-15 14:12 UTC (permalink / raw)
To: Shankar, Uma, Sharma, Shashank, intel-gfx, dri-devel
Cc: Syrjala, Ville, sam, emil.l.velikov, seanpaul, dcastagna
[-- Attachment #1.1: Type: text/plain, Size: 1594 bytes --]
mån 2019-04-15 klockan 19:26 +0530 skrev Sharma, Shashank:
> > -----Original Message-----
> > From: Lankhorst, Maarten
> > Sent: Monday, April 15, 2019 4:28 PM
> > To: Shankar, Uma <uma.shankar@intel.com>; intel-gfx@lists.freedeskt
> > op.org; dri-
> > devel@lists.freedesktop.org
> > Cc: Syrjala, Ville <ville.syrjala@intel.com>; emil.l.velikov@gmail.
> > com;
> > sam@ravnborg.org; Roper, Matthew D <matthew.d.roper@intel.com>;
> > seanpaul@chromium.org; brian.starkey@arm.com; dcastagna@chromium.or
> > g;
> > Sharma, Shashank <shashank.sharma@intel.com>
> > Subject: Re: [v3 6/7] drm: Add Client Cap for advance gamma mode
> >
> > fre 2019-04-12 klockan 15:51 +0530 skrev Uma Shankar:
> > > Introduced a client cap for advance cap mode
> > > capability. Userspace should set this to get
> > > to be able to use the new gamma_mode property.
> > >
> > > If this is not set, driver will work in legacy
> > > mode.
> > >
> > > Suggested-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
> > > Signed-off-by: Uma Shankar <uma.shankar@intel.com>
> >
> > Nack, this doesn't seem like a sensible idea. We already guard it
> > behind the gamma mode property. Userspace shouldn't set the gamma
> > mode
> > to a value it doesn't understand.
> >
> > ~Maarten
>
> Hey Maarten,
> In that case, what do you suggest should be the right way to do this
> ?
>
> @Ville, any comments here ?
>
I would say drop this patch, and just enable segmented gamma
unconditionally, it's not the first property that can cause trouble
when not understood.
~Maarten
[-- Attachment #1.2: smime.p7s --]
[-- Type: application/x-pkcs7-signature, Size: 3282 bytes --]
[-- Attachment #2: Type: text/plain, Size: 159 bytes --]
_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel
^ permalink raw reply [flat|nested] 24+ messages in thread
* Re: [v3 6/7] drm: Add Client Cap for advance gamma mode
2019-04-15 14:12 ` Lankhorst, Maarten
@ 2019-04-15 14:29 ` Sharma, Shashank
2019-04-15 19:20 ` Daniel Vetter
1 sibling, 0 replies; 24+ messages in thread
From: Sharma, Shashank @ 2019-04-15 14:29 UTC (permalink / raw)
To: Lankhorst, Maarten, Shankar, Uma, intel-gfx, dri-devel
Cc: Syrjala, Ville, sam, seanpaul, dcastagna
On 4/15/2019 7:42 PM, Lankhorst, Maarten wrote:
> mån 2019-04-15 klockan 19:26 +0530 skrev Sharma, Shashank:
>>> -----Original Message-----
>>> From: Lankhorst, Maarten
>>> Sent: Monday, April 15, 2019 4:28 PM
>>> To: Shankar, Uma <uma.shankar@intel.com>; intel-gfx@lists.freedeskt
>>> op.org; dri-
>>> devel@lists.freedesktop.org
>>> Cc: Syrjala, Ville <ville.syrjala@intel.com>; emil.l.velikov@gmail.
>>> com;
>>> sam@ravnborg.org; Roper, Matthew D <matthew.d.roper@intel.com>;
>>> seanpaul@chromium.org; brian.starkey@arm.com; dcastagna@chromium.or
>>> g;
>>> Sharma, Shashank <shashank.sharma@intel.com>
>>> Subject: Re: [v3 6/7] drm: Add Client Cap for advance gamma mode
>>>
>>> fre 2019-04-12 klockan 15:51 +0530 skrev Uma Shankar:
>>>> Introduced a client cap for advance cap mode
>>>> capability. Userspace should set this to get
>>>> to be able to use the new gamma_mode property.
>>>>
>>>> If this is not set, driver will work in legacy
>>>> mode.
>>>>
>>>> Suggested-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
>>>> Signed-off-by: Uma Shankar <uma.shankar@intel.com>
>>> Nack, this doesn't seem like a sensible idea. We already guard it
>>> behind the gamma mode property. Userspace shouldn't set the gamma
>>> mode
>>> to a value it doesn't understand.
>>>
>>> ~Maarten
>> Hey Maarten,
>> In that case, what do you suggest should be the right way to do this
>> ?
>>
>> @Ville, any comments here ?
>>
> I would say drop this patch, and just enable segmented gamma
> unconditionally, it's not the first property that can cause trouble
> when not understood.
Honestly, I kindof like this simple approach suggestion, which makes the
property focused, and easy to use. But one of the benefits I see for
this new gamma mode property was, that we can handle other advanced
gamma modes like 10/12/split and multi-segmented gamma modes too, using
this. So even though the single property is easy to use, but if we try
to cover each of the gamma modes per single property, it might get
confusing to see 4 different gamma mode properties, and how to set one
of this.
Another problem I see is, the precision and no of entries in the LUT,
for multi-segment gamma is different than traditional gamma, and we
might break some (most ?) of the old userspaces. Do you think is there
any way in which we can avoid this ?
- Shashank
> ~Maarten
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
^ permalink raw reply [flat|nested] 24+ messages in thread
* Re: [v3 6/7] drm: Add Client Cap for advance gamma mode
2019-04-15 14:12 ` Lankhorst, Maarten
2019-04-15 14:29 ` Sharma, Shashank
@ 2019-04-15 19:20 ` Daniel Vetter
2019-04-16 15:06 ` Ville Syrjälä
1 sibling, 1 reply; 24+ messages in thread
From: Daniel Vetter @ 2019-04-15 19:20 UTC (permalink / raw)
To: Lankhorst, Maarten
Cc: Syrjala, Ville, intel-gfx, dri-devel, seanpaul, dcastagna, sam
On Mon, Apr 15, 2019 at 4:14 PM Lankhorst, Maarten
<maarten.lankhorst@intel.com> wrote:
>
> mån 2019-04-15 klockan 19:26 +0530 skrev Sharma, Shashank:
> > > -----Original Message-----
> > > From: Lankhorst, Maarten
> > > Sent: Monday, April 15, 2019 4:28 PM
> > > To: Shankar, Uma <uma.shankar@intel.com>; intel-gfx@lists.freedeskt
> > > op.org; dri-
> > > devel@lists.freedesktop.org
> > > Cc: Syrjala, Ville <ville.syrjala@intel.com>; emil.l.velikov@gmail.
> > > com;
> > > sam@ravnborg.org; Roper, Matthew D <matthew.d.roper@intel.com>;
> > > seanpaul@chromium.org; brian.starkey@arm.com; dcastagna@chromium.or
> > > g;
> > > Sharma, Shashank <shashank.sharma@intel.com>
> > > Subject: Re: [v3 6/7] drm: Add Client Cap for advance gamma mode
> > >
> > > fre 2019-04-12 klockan 15:51 +0530 skrev Uma Shankar:
> > > > Introduced a client cap for advance cap mode
> > > > capability. Userspace should set this to get
> > > > to be able to use the new gamma_mode property.
> > > >
> > > > If this is not set, driver will work in legacy
> > > > mode.
> > > >
> > > > Suggested-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
> > > > Signed-off-by: Uma Shankar <uma.shankar@intel.com>
> > >
> > > Nack, this doesn't seem like a sensible idea. We already guard it
> > > behind the gamma mode property. Userspace shouldn't set the gamma
> > > mode
> > > to a value it doesn't understand.
> > >
> > > ~Maarten
> >
> > Hey Maarten,
> > In that case, what do you suggest should be the right way to do this
> > ?
> >
> > @Ville, any comments here ?
> >
> I would say drop this patch, and just enable segmented gamma
> unconditionally, it's not the first property that can cause trouble
> when not understood.
Yeah, thus far we went with "new properties should have the old
behaviour as default, no cap/flag needed". If you mix old&new
userspace and stuff starts looking funny, that's not a regression imo.
Also, it's a very uncommon use-case.
Wrt reset to default: fbdev emulation should do that for anything
that's too fancy, which is generally enough for the "developer of
compositors" use case. That part might be missing in the gamma/ctm
support in general I think.
-Daniel
--
Daniel Vetter
Software Engineer, Intel Corporation
+41 (0) 79 365 57 48 - http://blog.ffwll.ch
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
^ permalink raw reply [flat|nested] 24+ messages in thread
* Re: [v3 1/7] drm: Add gamma mode property
2019-04-12 10:20 ` [v3 1/7] drm: Add gamma mode property Uma Shankar
@ 2019-04-16 7:28 ` Daniel Vetter
0 siblings, 0 replies; 24+ messages in thread
From: Daniel Vetter @ 2019-04-16 7:28 UTC (permalink / raw)
To: Uma Shankar
Cc: dcastagna, intel-gfx, dri-devel, seanpaul, ville.syrjala, sam,
maarten.lankhorst
On Fri, Apr 12, 2019 at 03:50:57PM +0530, Uma Shankar wrote:
> From: Ville Syrjälä <ville.syrjala@linux.intel.com>
>
> Add a gamma mode property to enable various kind of
> gamma modes supported by platforms like: Interpolated, Split,
> Multi Segmented etc. Userspace can get this property and
> should be able to get the platform capabilties wrt various
> gamma modes possible and the possible ranges.
>
> It can select one of the modes exposed as blob_id as an
> enum and set the respective mode.
>
> It can then create the LUT and send it to driver using
> already available GAMMA_LUT property as blob.
>
> v2: Addressed Sam Ravnborg's review comments. Implemented
> gamma mode with just one property and renamed the current
> one to GAMMA_MODE property as recommended by Ville.
>
> Signed-off-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
> Signed-off-by: Uma Shankar <uma.shankar@intel.com>
Please also extend the CTM property docs, see
https://dri.freedesktop.org/docs/drm/gpu/drm-kms.html#color-management-properties
And especially how GAMMA_MODE interacts with everything else we have
already. I think the current comments don't really explain well how this
is supposed to be used.
Also, since this is quite a complicated data structure, can't we do at
least some basic validation in the core code?
-Daniel
> ---
> drivers/gpu/drm/drm_atomic_uapi.c | 5 +++
> drivers/gpu/drm/drm_color_mgmt.c | 77 +++++++++++++++++++++++++++++++++++++++
> include/drm/drm_color_mgmt.h | 8 ++++
> include/drm/drm_crtc.h | 7 ++++
> include/drm/drm_mode_config.h | 6 +++
> include/uapi/drm/drm_mode.h | 38 +++++++++++++++++++
> 6 files changed, 141 insertions(+)
>
> diff --git a/drivers/gpu/drm/drm_atomic_uapi.c b/drivers/gpu/drm/drm_atomic_uapi.c
> index ea797d4..d85e0c9 100644
> --- a/drivers/gpu/drm/drm_atomic_uapi.c
> +++ b/drivers/gpu/drm/drm_atomic_uapi.c
> @@ -459,6 +459,9 @@ static int drm_atomic_crtc_set_property(struct drm_crtc *crtc,
> &replaced);
> state->color_mgmt_changed |= replaced;
> return ret;
> + } else if (property == config->gamma_mode_property) {
> + state->gamma_mode = val;
> + state->color_mgmt_changed |= replaced;
> } else if (property == config->prop_out_fence_ptr) {
> s32 __user *fence_ptr = u64_to_user_ptr(val);
>
> @@ -495,6 +498,8 @@ static int drm_atomic_crtc_set_property(struct drm_crtc *crtc,
> *val = (state->mode_blob) ? state->mode_blob->base.id : 0;
> else if (property == config->prop_vrr_enabled)
> *val = state->vrr_enabled;
> + else if (property == config->gamma_mode_property)
> + *val = state->gamma_mode;
> else if (property == config->degamma_lut_property)
> *val = (state->degamma_lut) ? state->degamma_lut->base.id : 0;
> else if (property == config->ctm_property)
> diff --git a/drivers/gpu/drm/drm_color_mgmt.c b/drivers/gpu/drm/drm_color_mgmt.c
> index d5d34d0..4d6792d 100644
> --- a/drivers/gpu/drm/drm_color_mgmt.c
> +++ b/drivers/gpu/drm/drm_color_mgmt.c
> @@ -176,6 +176,83 @@ void drm_crtc_enable_color_mgmt(struct drm_crtc *crtc,
> }
> EXPORT_SYMBOL(drm_crtc_enable_color_mgmt);
>
> +void drm_crtc_attach_gamma_mode_property(struct drm_crtc *crtc)
> +{
> + struct drm_device *dev = crtc->dev;
> + struct drm_mode_config *config = &dev->mode_config;
> +
> + if (!config->gamma_mode_property)
> + return;
> +
> + drm_object_attach_property(&crtc->base,
> + config->gamma_mode_property, 0);
> +}
> +EXPORT_SYMBOL(drm_crtc_attach_gamma_mode_property);
> +
> +int drm_color_create_gamma_mode_property(struct drm_device *dev,
> + int num_values)
> +{
> + struct drm_mode_config *config = &dev->mode_config;
> + struct drm_property *prop;
> +
> + prop = drm_property_create(dev,
> + DRM_MODE_PROP_ENUM,
> + "GAMMA_MODE", num_values);
> + if (!prop)
> + return -ENOMEM;
> +
> + config->gamma_mode_property = prop;
> +
> + return 0;
> +}
> +EXPORT_SYMBOL(drm_color_create_gamma_mode_property);
> +
> +int drm_color_add_gamma_mode_range(struct drm_device *dev,
> + const char *name,
> + const struct drm_color_lut_range *ranges,
> + size_t length)
> +{
> + struct drm_mode_config *config = &dev->mode_config;
> + struct drm_property_blob *blob;
> + struct drm_property *prop;
> + int num_ranges = length / sizeof(ranges[0]);
> + int i, ret, num_types_0;
> +
> + if (WARN_ON(length == 0 || length % sizeof(ranges[0]) != 0))
> + return -EINVAL;
> +
> + num_types_0 = hweight8(ranges[0].flags & (DRM_MODE_LUT_GAMMA |
> + DRM_MODE_LUT_DEGAMMA));
> + if (num_types_0 == 0)
> + return -EINVAL;
> +
> + for (i = 1; i < num_ranges; i++) {
> + int num_types = hweight8(ranges[i].flags & (DRM_MODE_LUT_GAMMA |
> + DRM_MODE_LUT_DEGAMMA));
> +
> + /* either all ranges have DEGAMMA|GAMMA or none have it */
> + if (num_types_0 != num_types)
> + return -EINVAL;
> + }
> +
> + prop = config->gamma_mode_property;
> + if (!prop)
> + return -EINVAL;
> +
> + blob = drm_property_create_blob(dev, length, ranges);
> + if (IS_ERR(blob))
> + return PTR_ERR(blob);
> +
> + ret = drm_property_add_enum(prop, blob->base.id, name);
> + if (ret) {
> + drm_property_blob_put(blob);
> + return ret;
> + }
> +
> + return 0;
> +}
> +EXPORT_SYMBOL(drm_color_add_gamma_mode_range);
> +
> /**
> * drm_mode_crtc_set_gamma_size - set the gamma table size
> * @crtc: CRTC to set the gamma table size for
> diff --git a/include/drm/drm_color_mgmt.h b/include/drm/drm_color_mgmt.h
> index d1c662d..f18e9b8 100644
> --- a/include/drm/drm_color_mgmt.h
> +++ b/include/drm/drm_color_mgmt.h
> @@ -51,6 +51,14 @@ static inline int drm_color_lut_size(const struct drm_property_blob *blob)
> return blob->length / sizeof(struct drm_color_lut);
> }
>
> +int drm_color_create_gamma_mode_property(struct drm_device *dev,
> + int num_values);
> +void drm_crtc_attach_gamma_mode_property(struct drm_crtc *crtc);
> +int drm_color_add_gamma_mode_range(struct drm_device *dev,
> + const char *name,
> + const struct drm_color_lut_range *ranges,
> + size_t length);
> +
> enum drm_color_encoding {
> DRM_COLOR_YCBCR_BT601,
> DRM_COLOR_YCBCR_BT709,
> diff --git a/include/drm/drm_crtc.h b/include/drm/drm_crtc.h
> index 58ad983..f2e60bd 100644
> --- a/include/drm/drm_crtc.h
> +++ b/include/drm/drm_crtc.h
> @@ -249,6 +249,13 @@ struct drm_crtc_state {
> struct drm_property_blob *mode_blob;
>
> /**
> + * @gamma_mode: This is a blob_id and exposes the platform capabilties
> + * wrt to various gamma modes and the respective lut ranges. This also
> + * helps user select a gamma mode amongst the supported ones.
> + */
> + u32 gamma_mode;
> +
> + /**
> * @degamma_lut:
> *
> * Lookup table for converting framebuffer pixel data before apply the
> diff --git a/include/drm/drm_mode_config.h b/include/drm/drm_mode_config.h
> index 7f60e8e..8f961c5b 100644
> --- a/include/drm/drm_mode_config.h
> +++ b/include/drm/drm_mode_config.h
> @@ -761,6 +761,12 @@ struct drm_mode_config {
> */
> struct drm_property *content_type_property;
> /**
> + * @gamma_mode_property: Optional CRTC property to enumerate and
> + * select the mode of the crtc gamma/degmama LUTs. This also exposes
> + * the lut ranges of the various supported gamma modes to userspace.
> + */
> + struct drm_property *gamma_mode_property;
> + /**
> * @degamma_lut_property: Optional CRTC property to set the LUT used to
> * convert the framebuffer's colors to linear gamma.
> */
> diff --git a/include/uapi/drm/drm_mode.h b/include/uapi/drm/drm_mode.h
> index 83cd163..e70b7f8 100644
> --- a/include/uapi/drm/drm_mode.h
> +++ b/include/uapi/drm/drm_mode.h
> @@ -630,6 +630,44 @@ struct drm_color_lut {
> __u16 reserved;
> };
>
> +/*
> + * DRM_MODE_LUT_GAMMA|DRM_MODE_LUT_DEGAMMA is legal and means the LUT
> + * can be used for either purpose, but not simultaneously. To expose
> + * modes that support gamma and degamma simultaneously the gamma mode
> + * must declare distinct DRM_MODE_LUT_GAMMA and DRM_MODE_LUT_DEGAMMA
> + * ranges.
> + */
> +/* LUT is for gamma (after CTM) */
> +#define DRM_MODE_LUT_GAMMA BIT(0)
> +/* LUT is for degamma (before CTM) */
> +#define DRM_MODE_LUT_DEGAMMA BIT(1)
> +/* linearly interpolate between the points */
> +#define DRM_MODE_LUT_INTERPOLATE BIT(2)
> +/*
> + * the last value of the previous range is the
> + * first value of the current range.
> + */
> +#define DRM_MODE_LUT_REUSE_LAST BIT(3)
> +/* the curve must be non-decreasing */
> +#define DRM_MODE_LUT_NON_DECREASING BIT(4)
> +/* the curve is reflected across origin for negative inputs */
> +#define DRM_MODE_LUT_REFLECT_NEGATIVE BIT(5)
> +/* the same curve (red) is used for blue and green channels as well */
> +#define DRM_MODE_LUT_SINGLE_CHANNEL BIT(6)
> +
> +struct drm_color_lut_range {
> + /* DRM_MODE_LUT_* */
> + __u32 flags;
> + /* number of points on the curve */
> + __u16 count;
> + /* input/output bits per component */
> + __u8 input_bpc, output_bpc;
> + /* input start/end values */
> + __s32 start, end;
> + /* output min/max values */
> + __s32 min, max;
> +};
> +
> #define DRM_MODE_PAGE_FLIP_EVENT 0x01
> #define DRM_MODE_PAGE_FLIP_ASYNC 0x02
> #define DRM_MODE_PAGE_FLIP_TARGET_ABSOLUTE 0x4
> --
> 1.9.1
>
> _______________________________________________
> Intel-gfx mailing list
> Intel-gfx@lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/intel-gfx
--
Daniel Vetter
Software Engineer, Intel Corporation
http://blog.ffwll.ch
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
^ permalink raw reply [flat|nested] 24+ messages in thread
* Re: [v3 6/7] drm: Add Client Cap for advance gamma mode
2019-04-15 12:43 ` [Intel-gfx] " Ville Syrjälä
@ 2019-04-16 8:54 ` Lankhorst, Maarten
0 siblings, 0 replies; 24+ messages in thread
From: Lankhorst, Maarten @ 2019-04-16 8:54 UTC (permalink / raw)
To: ville.syrjala
Cc: Syrjala, Ville, intel-gfx, dri-devel, seanpaul, dcastagna, sam
[-- Attachment #1.1: Type: text/plain, Size: 1508 bytes --]
mån 2019-04-15 klockan 15:43 +0300 skrev Ville Syrjälä:
> On Mon, Apr 15, 2019 at 10:57:52AM +0000, Lankhorst, Maarten wrote:
> > fre 2019-04-12 klockan 15:51 +0530 skrev Uma Shankar:
> > > Introduced a client cap for advance cap mode
> > > capability. Userspace should set this to get
> > > to be able to use the new gamma_mode property.
> > >
> > > If this is not set, driver will work in legacy
> > > mode.
> > >
> > > Suggested-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
> > > Signed-off-by: Uma Shankar <uma.shankar@intel.com>
> >
> > Nack, this doesn't seem like a sensible idea. We already guard it
> > behind the gamma mode property. Userspace shouldn't set the gamma
> > mode
> > to a value it doesn't understand.
>
> Old userspace wouldn't know what a gamma_mode prop is. While a client
> cap isn't an entirely satisfactory solution I can't think of a better
> way at the moment.
>
> Well, maybe the old "hey kernel, please reset all my props to some
> sane default" idea could be another way to deal with this sort of
> stuff.
Yes, but this approach wouldn't work, lot of other properties that can
cause problems when not reset, like plane alpha and blend mode. I don't
see why gamma is special in that case.
If userspace did set it to some special value, then presumably it knows
how to reset it too. It would be different if the split gamma mode was
the default. Then I would understand this, but right now this would
even break s/r.
~Maarten
[-- Attachment #1.2: smime.p7s --]
[-- Type: application/x-pkcs7-signature, Size: 3282 bytes --]
[-- Attachment #2: Type: text/plain, Size: 159 bytes --]
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
^ permalink raw reply [flat|nested] 24+ messages in thread
* Re: [v3 6/7] drm: Add Client Cap for advance gamma mode
2019-04-15 19:20 ` Daniel Vetter
@ 2019-04-16 15:06 ` Ville Syrjälä
0 siblings, 0 replies; 24+ messages in thread
From: Ville Syrjälä @ 2019-04-16 15:06 UTC (permalink / raw)
To: Daniel Vetter
Cc: dcastagna, intel-gfx, dri-devel, seanpaul, Syrjala, Ville, sam,
Lankhorst, Maarten
On Mon, Apr 15, 2019 at 09:20:55PM +0200, Daniel Vetter wrote:
> On Mon, Apr 15, 2019 at 4:14 PM Lankhorst, Maarten
> <maarten.lankhorst@intel.com> wrote:
> >
> > mån 2019-04-15 klockan 19:26 +0530 skrev Sharma, Shashank:
> > > > -----Original Message-----
> > > > From: Lankhorst, Maarten
> > > > Sent: Monday, April 15, 2019 4:28 PM
> > > > To: Shankar, Uma <uma.shankar@intel.com>; intel-gfx@lists.freedeskt
> > > > op.org; dri-
> > > > devel@lists.freedesktop.org
> > > > Cc: Syrjala, Ville <ville.syrjala@intel.com>; emil.l.velikov@gmail.
> > > > com;
> > > > sam@ravnborg.org; Roper, Matthew D <matthew.d.roper@intel.com>;
> > > > seanpaul@chromium.org; brian.starkey@arm.com; dcastagna@chromium.or
> > > > g;
> > > > Sharma, Shashank <shashank.sharma@intel.com>
> > > > Subject: Re: [v3 6/7] drm: Add Client Cap for advance gamma mode
> > > >
> > > > fre 2019-04-12 klockan 15:51 +0530 skrev Uma Shankar:
> > > > > Introduced a client cap for advance cap mode
> > > > > capability. Userspace should set this to get
> > > > > to be able to use the new gamma_mode property.
> > > > >
> > > > > If this is not set, driver will work in legacy
> > > > > mode.
> > > > >
> > > > > Suggested-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
> > > > > Signed-off-by: Uma Shankar <uma.shankar@intel.com>
> > > >
> > > > Nack, this doesn't seem like a sensible idea. We already guard it
> > > > behind the gamma mode property. Userspace shouldn't set the gamma
> > > > mode
> > > > to a value it doesn't understand.
> > > >
> > > > ~Maarten
> > >
> > > Hey Maarten,
> > > In that case, what do you suggest should be the right way to do this
> > > ?
> > >
> > > @Ville, any comments here ?
> > >
> > I would say drop this patch, and just enable segmented gamma
> > unconditionally, it's not the first property that can cause trouble
> > when not understood.
>
> Yeah, thus far we went with "new properties should have the old
> behaviour as default, no cap/flag needed".
That's a given. But it doesn't help with the mixed userspace when one of
them doesn't know how to reset it back to default.
> If you mix old&new
> userspace and stuff starts looking funny, that's not a regression imo.
> Also, it's a very uncommon use-case.
I was mainly thinking of eg. wayland vs. X type of cases where the user
might want to switch. If the wayland compositor uses the new fancy gamma
thing but the xserver doesn't then this won't work anymore. But I guess
easier solution is to just add the relevant knowledge to the ddx. Which
should probably be a hard requirement for landing this.
>
> Wrt reset to default: fbdev emulation should do that for anything
> that's too fancy, which is generally enough for the "developer of
> compositors" use case. That part might be missing in the gamma/ctm
> support in general I think.
Probably. IIRC even running X at 8bpp and then swithcing back to
fbcon leaves you with a gargbled palette.
--
Ville Syrjälä
Intel
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
^ permalink raw reply [flat|nested] 24+ messages in thread
* Re: [v3 0/7] Add Multi Segment Gamma Support
2019-04-12 10:20 [v3 0/7] Add Multi Segment Gamma Support Uma Shankar
` (8 preceding siblings ...)
2019-04-12 12:50 ` ✗ Fi.CI.BAT: failure " Patchwork
@ 2019-04-17 7:28 ` Daniel Vetter
2019-04-17 11:57 ` Ville Syrjälä
9 siblings, 1 reply; 24+ messages in thread
From: Daniel Vetter @ 2019-04-17 7:28 UTC (permalink / raw)
To: Uma Shankar
Cc: dcastagna, intel-gfx, emil.l.velikov, dri-devel, seanpaul,
ville.syrjala, sam, maarten.lankhorst
On Fri, Apr 12, 2019 at 03:50:56PM +0530, Uma Shankar wrote:
> This series adds support for programmable gamma modes and
> exposes a property interface for the same. Also added,
> support for multi segment gamma mode introduced in ICL+
>
> It creates GAMMA_MODE property interface. This is an enum
> property with values as blob_id's and exposes
> the various gamma modes supported and the lut ranges Getting the
> blob id in userspace, user can get the mode supported and
> also the range of gamma mode supported with number of lut
> coefficients. It can then set one of the modes using this
> enum property.
>
> Lut values will be sent through already available GAMMA_LUT
> blob property.
>
> It also introduces a CLIENT CAP for advanced GAMMA_MODE.
> This is for user to set the and use advance gamma mode and older
> userspace can continue using the legacy paths.
>
> v2: Used Ville's design and approach to define the interfaces.
> Addressed Matt Roper's review feedback and re-ordered the
> patches.
>
> v3: Converged to 1 property interface and introduced a Client cap
> as suggested by Ville. Fixed review comments received.
>
> Uma Shankar (5):
> drm/i915/icl: Add register definitions for Multi Segmented gamma
> drm/i915/icl: Add support for multi segmented gamma mode
> drm/i915: Attach gamma mode property
> drm: Add Client Cap for advance gamma mode
> drm/i915: Enable advance gamma mode
>
> Ville Syrjälä (2):
> drm: Add gamma mode property
> drm/i915: Define color lut range structure
Bunch of higher level comments after some internal discussions:
- we need the userspace for this, can't design new uapi without involving
the compositor folks for hdr.
- single property doesn't work: Once userspace has set it, the old blob
property with the list of all options is gone. We need one read-only
property for the list of options, plus a 2nd property that userspace can
set. This is a general rule for more complex properties, where the usual
property metadata isn't enough to describe the possible options.
- no caps for properties. Yes that gives us a theoretical problem, no in
practice it doesn't matter, since people don't even care enough to make
e.g. fbdev resetting work today for everything. Long form discussion,
see here:
https://blog.ffwll.ch/2016/01/vt-switching-with-atomic-modeset.html
Nothing happened in this area ever since I typed this up, so I guess
it's really not a real-world concern.
- Simplest path forward would be if we accept different LUT sizes than the
one advertised (we already do that for legacy gamma, and this is
officially what we had in mind too), and the kernel automatically picks
the best lut configuration. Will be somewhat awkard for the
multi-segment lut, but would decouple the uapi discussion a bit.
- Frankly the uapi proposed looks like fake generic - it tries to model
all possibilities in a generic way, when really userspace needs to have
special code for special pipelines. To me this feels like the pixel
modifier discussion all over, where we had multi-year discussions on
trying to describe everything in generic terms or just have fairly
opaque enumeration of special cases. Both approaches have been tried.
For this I'm leaning towards the opaque color pipeline description for
the more fancy stuff.
Either way, settling on the right uapi will take some time, and will
need a pile of people to be involved.
Cheers, Daniel
>
> drivers/gpu/drm/drm_atomic_uapi.c | 8 +
> drivers/gpu/drm/drm_color_mgmt.c | 77 ++++
> drivers/gpu/drm/drm_ioctl.c | 5 +
> drivers/gpu/drm/i915/i915_reg.h | 17 +
> drivers/gpu/drm/i915/intel_color.c | 735 ++++++++++++++++++++++++++++++++++-
> drivers/gpu/drm/i915/intel_display.c | 3 +
> include/drm/drm_atomic.h | 1 +
> include/drm/drm_color_mgmt.h | 8 +
> include/drm/drm_crtc.h | 17 +
> include/drm/drm_file.h | 8 +
> include/drm/drm_mode_config.h | 6 +
> include/uapi/drm/drm.h | 2 +
> include/uapi/drm/drm_mode.h | 38 ++
> 13 files changed, 918 insertions(+), 7 deletions(-)
>
> --
> 1.9.1
>
> _______________________________________________
> dri-devel mailing list
> dri-devel@lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/dri-devel
--
Daniel Vetter
Software Engineer, Intel Corporation
http://blog.ffwll.ch
_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel
^ permalink raw reply [flat|nested] 24+ messages in thread
* Re: [v3 0/7] Add Multi Segment Gamma Support
2019-04-17 7:28 ` [v3 0/7] Add Multi Segment Gamma Support Daniel Vetter
@ 2019-04-17 11:57 ` Ville Syrjälä
2019-04-18 7:13 ` Daniel Vetter
0 siblings, 1 reply; 24+ messages in thread
From: Ville Syrjälä @ 2019-04-17 11:57 UTC (permalink / raw)
To: Daniel Vetter
Cc: dcastagna, intel-gfx, dri-devel, seanpaul, sam, maarten.lankhorst
On Wed, Apr 17, 2019 at 09:28:19AM +0200, Daniel Vetter wrote:
> On Fri, Apr 12, 2019 at 03:50:56PM +0530, Uma Shankar wrote:
> > This series adds support for programmable gamma modes and
> > exposes a property interface for the same. Also added,
> > support for multi segment gamma mode introduced in ICL+
> >
> > It creates GAMMA_MODE property interface. This is an enum
> > property with values as blob_id's and exposes
> > the various gamma modes supported and the lut ranges Getting the
> > blob id in userspace, user can get the mode supported and
> > also the range of gamma mode supported with number of lut
> > coefficients. It can then set one of the modes using this
> > enum property.
> >
> > Lut values will be sent through already available GAMMA_LUT
> > blob property.
> >
> > It also introduces a CLIENT CAP for advanced GAMMA_MODE.
> > This is for user to set the and use advance gamma mode and older
> > userspace can continue using the legacy paths.
> >
> > v2: Used Ville's design and approach to define the interfaces.
> > Addressed Matt Roper's review feedback and re-ordered the
> > patches.
> >
> > v3: Converged to 1 property interface and introduced a Client cap
> > as suggested by Ville. Fixed review comments received.
> >
> > Uma Shankar (5):
> > drm/i915/icl: Add register definitions for Multi Segmented gamma
> > drm/i915/icl: Add support for multi segmented gamma mode
> > drm/i915: Attach gamma mode property
> > drm: Add Client Cap for advance gamma mode
> > drm/i915: Enable advance gamma mode
> >
> > Ville Syrjälä (2):
> > drm: Add gamma mode property
> > drm/i915: Define color lut range structure
>
> Bunch of higher level comments after some internal discussions:
>
> - we need the userspace for this, can't design new uapi without involving
> the compositor folks for hdr.
>
> - single property doesn't work: Once userspace has set it, the old blob
> property with the list of all options is gone. We need one read-only
> property for the list of options, plus a 2nd property that userspace can
> set. This is a general rule for more complex properties, where the usual
> property metadata isn't enough to describe the possible options.
I guess no one understood my blob_enum idea? It's an enum where each
possible value is a blob. The only thing that changes is the current
value (which can only point to one of the enumerated blobs).
>
> - no caps for properties. Yes that gives us a theoretical problem, no in
> practice it doesn't matter, since people don't even care enough to make
> e.g. fbdev resetting work today for everything. Long form discussion,
> see here:
>
> https://blog.ffwll.ch/2016/01/vt-switching-with-atomic-modeset.html
>
> Nothing happened in this area ever since I typed this up, so I guess
> it's really not a real-world concern.
>
> - Simplest path forward would be if we accept different LUT sizes than the
> one advertised (we already do that for legacy gamma, and this is
> officially what we had in mind too), and the kernel automatically picks
> the best lut configuration. Will be somewhat awkard for the
> multi-segment lut, but would decouple the uapi discussion a bit.
It'll be ridiculously wasteful. IIRC we need a LUT with 32768 entries,
and then ~98% of those gets thrown away and never programmed to the
hardware.
>
> - Frankly the uapi proposed looks like fake generic - it tries to model
> all possibilities in a generic way, when really userspace needs to have
> special code for special pipelines.
I think it can be used pretty easily. Userspace just has to decide
whether it wants a straight up LUT or whether an interpolated curve
is enough, and how much precision it needs. For x11 the logic would
be simple enough: 1. look for straight up LUT with num_entries >= 1<<bpc,
if that isn't found fall back to an interpolated curve with >= 1<<bpc
precision, and finally just fall back to whatever gives the best
results I suppose.
> To me this feels like the pixel
> modifier discussion all over, where we had multi-year discussions on
> trying to describe everything in generic terms or just have fairly
> opaque enumeration of special cases. Both approaches have been tried.
> For this I'm leaning towards the opaque color pipeline description for
> the more fancy stuff.
>
> Either way, settling on the right uapi will take some time, and will
> need a pile of people to be involved.
>
> Cheers, Daniel
>
> >
> > drivers/gpu/drm/drm_atomic_uapi.c | 8 +
> > drivers/gpu/drm/drm_color_mgmt.c | 77 ++++
> > drivers/gpu/drm/drm_ioctl.c | 5 +
> > drivers/gpu/drm/i915/i915_reg.h | 17 +
> > drivers/gpu/drm/i915/intel_color.c | 735 ++++++++++++++++++++++++++++++++++-
> > drivers/gpu/drm/i915/intel_display.c | 3 +
> > include/drm/drm_atomic.h | 1 +
> > include/drm/drm_color_mgmt.h | 8 +
> > include/drm/drm_crtc.h | 17 +
> > include/drm/drm_file.h | 8 +
> > include/drm/drm_mode_config.h | 6 +
> > include/uapi/drm/drm.h | 2 +
> > include/uapi/drm/drm_mode.h | 38 ++
> > 13 files changed, 918 insertions(+), 7 deletions(-)
> >
> > --
> > 1.9.1
> >
> > _______________________________________________
> > dri-devel mailing list
> > dri-devel@lists.freedesktop.org
> > https://lists.freedesktop.org/mailman/listinfo/dri-devel
>
> --
> Daniel Vetter
> Software Engineer, Intel Corporation
> http://blog.ffwll.ch
--
Ville Syrjälä
Intel
---------------------------------------------------------------------
Intel Finland Oy
Registered Address: PL 281, 00181 Helsinki
Business Identity Code: 0357606 - 4
Domiciled in Helsinki
This e-mail and any attachments may contain confidential material for
the sole use of the intended recipient(s). Any review or distribution
by others is strictly prohibited. If you are not the intended
recipient, please contact the sender and delete all copies.
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
^ permalink raw reply [flat|nested] 24+ messages in thread
* Re: [v3 0/7] Add Multi Segment Gamma Support
2019-04-17 11:57 ` Ville Syrjälä
@ 2019-04-18 7:13 ` Daniel Vetter
2019-04-18 13:11 ` Ville Syrjälä
0 siblings, 1 reply; 24+ messages in thread
From: Daniel Vetter @ 2019-04-18 7:13 UTC (permalink / raw)
To: Ville Syrjälä
Cc: Daniele Castagna, intel-gfx, Emil Velikov, dri-devel,
Uma Shankar, Sean Paul, Sam Ravnborg, Lankhorst, Maarten
On Wed, Apr 17, 2019 at 02:57:31PM +0300, Ville Syrjälä wrote:
> On Wed, Apr 17, 2019 at 09:28:19AM +0200, Daniel Vetter wrote:
> > On Fri, Apr 12, 2019 at 03:50:56PM +0530, Uma Shankar wrote:
> > > This series adds support for programmable gamma modes and
> > > exposes a property interface for the same. Also added,
> > > support for multi segment gamma mode introduced in ICL+
> > >
> > > It creates GAMMA_MODE property interface. This is an enum
> > > property with values as blob_id's and exposes
> > > the various gamma modes supported and the lut ranges Getting the
> > > blob id in userspace, user can get the mode supported and
> > > also the range of gamma mode supported with number of lut
> > > coefficients. It can then set one of the modes using this
> > > enum property.
> > >
> > > Lut values will be sent through already available GAMMA_LUT
> > > blob property.
> > >
> > > It also introduces a CLIENT CAP for advanced GAMMA_MODE.
> > > This is for user to set the and use advance gamma mode and older
> > > userspace can continue using the legacy paths.
> > >
> > > v2: Used Ville's design and approach to define the interfaces.
> > > Addressed Matt Roper's review feedback and re-ordered the
> > > patches.
> > >
> > > v3: Converged to 1 property interface and introduced a Client cap
> > > as suggested by Ville. Fixed review comments received.
> > >
> > > Uma Shankar (5):
> > > drm/i915/icl: Add register definitions for Multi Segmented gamma
> > > drm/i915/icl: Add support for multi segmented gamma mode
> > > drm/i915: Attach gamma mode property
> > > drm: Add Client Cap for advance gamma mode
> > > drm/i915: Enable advance gamma mode
> > >
> > > Ville Syrjälä (2):
> > > drm: Add gamma mode property
> > > drm/i915: Define color lut range structure
> >
> > Bunch of higher level comments after some internal discussions:
> >
> > - we need the userspace for this, can't design new uapi without involving
> > the compositor folks for hdr.
> >
> > - single property doesn't work: Once userspace has set it, the old blob
> > property with the list of all options is gone. We need one read-only
> > property for the list of options, plus a 2nd property that userspace can
> > set. This is a general rule for more complex properties, where the usual
> > property metadata isn't enough to describe the possible options.
>
> I guess no one understood my blob_enum idea? It's an enum where each
> possible value is a blob. The only thing that changes is the current
> value (which can only point to one of the enumerated blobs).
Uh yes that's not clear at all, and if we do go with this, I guess we
should have a pile of core code to make sure it validates and is
consistent.
>> > - no caps for properties. Yes that gives us a theoretical problem, no in
> > practice it doesn't matter, since people don't even care enough to make
> > e.g. fbdev resetting work today for everything. Long form discussion,
> > see here:
> >
> > https://blog.ffwll.ch/2016/01/vt-switching-with-atomic-modeset.html
> >
> > Nothing happened in this area ever since I typed this up, so I guess
> > it's really not a real-world concern.
> >
> > - Simplest path forward would be if we accept different LUT sizes than the
> > one advertised (we already do that for legacy gamma, and this is
> > officially what we had in mind too), and the kernel automatically picks
> > the best lut configuration. Will be somewhat awkard for the
> > multi-segment lut, but would decouple the uapi discussion a bit.
>
> It'll be ridiculously wasteful. IIRC we need a LUT with 32768 entries,
> and then ~98% of those gets thrown away and never programmed to the
> hardware.
Yeah it's a few MB, not that awesome really ...
> > - Frankly the uapi proposed looks like fake generic - it tries to model
> > all possibilities in a generic way, when really userspace needs to have
> > special code for special pipelines.
>
> I think it can be used pretty easily. Userspace just has to decide
> whether it wants a straight up LUT or whether an interpolated curve
> is enough, and how much precision it needs. For x11 the logic would
> be simple enough: 1. look for straight up LUT with num_entries >= 1<<bpc,
> if that isn't found fall back to an interpolated curve with >= 1<<bpc
> precision, and finally just fall back to whatever gives the best
> results I suppose.
Hm, there's also a bunch more defines about mirroring and non-negative and
other stuff that I have no idea how userspace should use it. I do think
some "here's the possible configs for color management" thing is needed,
I'm not sure userspace can do much with all the details provided in the
current series.
-Daniel
-Daniel
> > To me this feels like the pixel
> > modifier discussion all over, where we had multi-year discussions on
> > trying to describe everything in generic terms or just have fairly
> > opaque enumeration of special cases. Both approaches have been tried.
> > For this I'm leaning towards the opaque color pipeline description for
> > the more fancy stuff.
> >
> > Either way, settling on the right uapi will take some time, and will
> > need a pile of people to be involved.
> >
> > Cheers, Daniel
> >
> > >
> > > drivers/gpu/drm/drm_atomic_uapi.c | 8 +
> > > drivers/gpu/drm/drm_color_mgmt.c | 77 ++++
> > > drivers/gpu/drm/drm_ioctl.c | 5 +
> > > drivers/gpu/drm/i915/i915_reg.h | 17 +
> > > drivers/gpu/drm/i915/intel_color.c | 735 ++++++++++++++++++++++++++++++++++-
> > > drivers/gpu/drm/i915/intel_display.c | 3 +
> > > include/drm/drm_atomic.h | 1 +
> > > include/drm/drm_color_mgmt.h | 8 +
> > > include/drm/drm_crtc.h | 17 +
> > > include/drm/drm_file.h | 8 +
> > > include/drm/drm_mode_config.h | 6 +
> > > include/uapi/drm/drm.h | 2 +
> > > include/uapi/drm/drm_mode.h | 38 ++
> > > 13 files changed, 918 insertions(+), 7 deletions(-)
> > >
> > > --
> > > 1.9.1
> > >
> > > _______________________________________________
> > > dri-devel mailing list
> > > dri-devel@lists.freedesktop.org
> > > https://lists.freedesktop.org/mailman/listinfo/dri-devel
> >
> > --
> > Daniel Vetter
> > Software Engineer, Intel Corporation
> > http://blog.ffwll.ch
>
> --
> Ville Syrjälä
> Intel
> ---------------------------------------------------------------------
> Intel Finland Oy
> Registered Address: PL 281, 00181 Helsinki
> Business Identity Code: 0357606 - 4
> Domiciled in Helsinki
>
> This e-mail and any attachments may contain confidential material for
> the sole use of the intended recipient(s). Any review or distribution
> by others is strictly prohibited. If you are not the intended
> recipient, please contact the sender and delete all copies.
>
--
Daniel Vetter
Software Engineer, Intel Corporation
http://blog.ffwll.ch
_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel
^ permalink raw reply [flat|nested] 24+ messages in thread
* Re: [v3 0/7] Add Multi Segment Gamma Support
2019-04-18 7:13 ` Daniel Vetter
@ 2019-04-18 13:11 ` Ville Syrjälä
2019-04-23 6:52 ` Daniel Vetter
0 siblings, 1 reply; 24+ messages in thread
From: Ville Syrjälä @ 2019-04-18 13:11 UTC (permalink / raw)
To: Daniel Vetter
Cc: Daniele Castagna, intel-gfx, dri-devel, Sean Paul, Sam Ravnborg,
Lankhorst, Maarten
On Thu, Apr 18, 2019 at 09:13:04AM +0200, Daniel Vetter wrote:
> On Wed, Apr 17, 2019 at 02:57:31PM +0300, Ville Syrjälä wrote:
> > On Wed, Apr 17, 2019 at 09:28:19AM +0200, Daniel Vetter wrote:
> > > On Fri, Apr 12, 2019 at 03:50:56PM +0530, Uma Shankar wrote:
> > > > This series adds support for programmable gamma modes and
> > > > exposes a property interface for the same. Also added,
> > > > support for multi segment gamma mode introduced in ICL+
> > > >
> > > > It creates GAMMA_MODE property interface. This is an enum
> > > > property with values as blob_id's and exposes
> > > > the various gamma modes supported and the lut ranges Getting the
> > > > blob id in userspace, user can get the mode supported and
> > > > also the range of gamma mode supported with number of lut
> > > > coefficients. It can then set one of the modes using this
> > > > enum property.
> > > >
> > > > Lut values will be sent through already available GAMMA_LUT
> > > > blob property.
> > > >
> > > > It also introduces a CLIENT CAP for advanced GAMMA_MODE.
> > > > This is for user to set the and use advance gamma mode and older
> > > > userspace can continue using the legacy paths.
> > > >
> > > > v2: Used Ville's design and approach to define the interfaces.
> > > > Addressed Matt Roper's review feedback and re-ordered the
> > > > patches.
> > > >
> > > > v3: Converged to 1 property interface and introduced a Client cap
> > > > as suggested by Ville. Fixed review comments received.
> > > >
> > > > Uma Shankar (5):
> > > > drm/i915/icl: Add register definitions for Multi Segmented gamma
> > > > drm/i915/icl: Add support for multi segmented gamma mode
> > > > drm/i915: Attach gamma mode property
> > > > drm: Add Client Cap for advance gamma mode
> > > > drm/i915: Enable advance gamma mode
> > > >
> > > > Ville Syrjälä (2):
> > > > drm: Add gamma mode property
> > > > drm/i915: Define color lut range structure
> > >
> > > Bunch of higher level comments after some internal discussions:
> > >
> > > - we need the userspace for this, can't design new uapi without involving
> > > the compositor folks for hdr.
> > >
> > > - single property doesn't work: Once userspace has set it, the old blob
> > > property with the list of all options is gone. We need one read-only
> > > property for the list of options, plus a 2nd property that userspace can
> > > set. This is a general rule for more complex properties, where the usual
> > > property metadata isn't enough to describe the possible options.
> >
> > I guess no one understood my blob_enum idea? It's an enum where each
> > possible value is a blob. The only thing that changes is the current
> > value (which can only point to one of the enumerated blobs).
>
> Uh yes that's not clear at all, and if we do go with this, I guess we
> should have a pile of core code to make sure it validates and is
> consistent.
>
> >> > - no caps for properties. Yes that gives us a theoretical problem, no in
> > > practice it doesn't matter, since people don't even care enough to make
> > > e.g. fbdev resetting work today for everything. Long form discussion,
> > > see here:
> > >
> > > https://blog.ffwll.ch/2016/01/vt-switching-with-atomic-modeset.html
> > >
> > > Nothing happened in this area ever since I typed this up, so I guess
> > > it's really not a real-world concern.
> > >
> > > - Simplest path forward would be if we accept different LUT sizes than the
> > > one advertised (we already do that for legacy gamma, and this is
> > > officially what we had in mind too), and the kernel automatically picks
> > > the best lut configuration. Will be somewhat awkard for the
> > > multi-segment lut, but would decouple the uapi discussion a bit.
> >
> > It'll be ridiculously wasteful. IIRC we need a LUT with 32768 entries,
> > and then ~98% of those gets thrown away and never programmed to the
> > hardware.
>
> Yeah it's a few MB, not that awesome really ...
>
> > > - Frankly the uapi proposed looks like fake generic - it tries to model
> > > all possibilities in a generic way, when really userspace needs to have
> > > special code for special pipelines.
> >
> > I think it can be used pretty easily. Userspace just has to decide
> > whether it wants a straight up LUT or whether an interpolated curve
> > is enough, and how much precision it needs. For x11 the logic would
> > be simple enough: 1. look for straight up LUT with num_entries >= 1<<bpc,
> > if that isn't found fall back to an interpolated curve with >= 1<<bpc
> > precision, and finally just fall back to whatever gives the best
> > results I suppose.
>
> Hm, there's also a bunch more defines about mirroring and non-negative and
> other stuff that I have no idea how userspace should use it. I do think
> some "here's the possible configs for color management" thing is needed,
> I'm not sure userspace can do much with all the details provided in the
> current series.
The negative values would represent out of gamut colors, which
can definitely happen with xvYCC. Looks like the v4l folks
also considered this in their transfer func docs [1], which
are specifying the formulas extended for <0.0 values.
Also based on my chat with Ilia on irc at some point I got the
impression that nv hardware may have gamma LUTs which support
negative values without this mirroring trick. Hence I wanted to
make all the numbers signed rather than unsigned.
We could of course leave a bunch of these advanced things
undefined until an actual use case comes around. But I wanted to
include it all in my initial proposal so that we could be more
confident that we're not painting ourselves in a corner that
would require yet another uapi to escape.
[1] https://www.linuxtv.org/downloads/v4l-dvb-apis-old/ch02s06.html
--
Ville Syrjälä
Intel
---------------------------------------------------------------------
Intel Finland Oy
Registered Address: PL 281, 00181 Helsinki
Business Identity Code: 0357606 - 4
Domiciled in Helsinki
This e-mail and any attachments may contain confidential material for
the sole use of the intended recipient(s). Any review or distribution
by others is strictly prohibited. If you are not the intended
recipient, please contact the sender and delete all copies.
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
^ permalink raw reply [flat|nested] 24+ messages in thread
* Re: [v3 0/7] Add Multi Segment Gamma Support
2019-04-18 13:11 ` Ville Syrjälä
@ 2019-04-23 6:52 ` Daniel Vetter
0 siblings, 0 replies; 24+ messages in thread
From: Daniel Vetter @ 2019-04-23 6:52 UTC (permalink / raw)
To: Ville Syrjälä
Cc: Daniele Castagna, intel-gfx, Emil Velikov, dri-devel,
Uma Shankar, Sean Paul, Sam Ravnborg, Lankhorst, Maarten
On Thu, Apr 18, 2019 at 04:11:58PM +0300, Ville Syrjälä wrote:
> On Thu, Apr 18, 2019 at 09:13:04AM +0200, Daniel Vetter wrote:
> > On Wed, Apr 17, 2019 at 02:57:31PM +0300, Ville Syrjälä wrote:
> > > On Wed, Apr 17, 2019 at 09:28:19AM +0200, Daniel Vetter wrote:
> > > > On Fri, Apr 12, 2019 at 03:50:56PM +0530, Uma Shankar wrote:
> > > > > This series adds support for programmable gamma modes and
> > > > > exposes a property interface for the same. Also added,
> > > > > support for multi segment gamma mode introduced in ICL+
> > > > >
> > > > > It creates GAMMA_MODE property interface. This is an enum
> > > > > property with values as blob_id's and exposes
> > > > > the various gamma modes supported and the lut ranges Getting the
> > > > > blob id in userspace, user can get the mode supported and
> > > > > also the range of gamma mode supported with number of lut
> > > > > coefficients. It can then set one of the modes using this
> > > > > enum property.
> > > > >
> > > > > Lut values will be sent through already available GAMMA_LUT
> > > > > blob property.
> > > > >
> > > > > It also introduces a CLIENT CAP for advanced GAMMA_MODE.
> > > > > This is for user to set the and use advance gamma mode and older
> > > > > userspace can continue using the legacy paths.
> > > > >
> > > > > v2: Used Ville's design and approach to define the interfaces.
> > > > > Addressed Matt Roper's review feedback and re-ordered the
> > > > > patches.
> > > > >
> > > > > v3: Converged to 1 property interface and introduced a Client cap
> > > > > as suggested by Ville. Fixed review comments received.
> > > > >
> > > > > Uma Shankar (5):
> > > > > drm/i915/icl: Add register definitions for Multi Segmented gamma
> > > > > drm/i915/icl: Add support for multi segmented gamma mode
> > > > > drm/i915: Attach gamma mode property
> > > > > drm: Add Client Cap for advance gamma mode
> > > > > drm/i915: Enable advance gamma mode
> > > > >
> > > > > Ville Syrjälä (2):
> > > > > drm: Add gamma mode property
> > > > > drm/i915: Define color lut range structure
> > > >
> > > > Bunch of higher level comments after some internal discussions:
> > > >
> > > > - we need the userspace for this, can't design new uapi without involving
> > > > the compositor folks for hdr.
> > > >
> > > > - single property doesn't work: Once userspace has set it, the old blob
> > > > property with the list of all options is gone. We need one read-only
> > > > property for the list of options, plus a 2nd property that userspace can
> > > > set. This is a general rule for more complex properties, where the usual
> > > > property metadata isn't enough to describe the possible options.
> > >
> > > I guess no one understood my blob_enum idea? It's an enum where each
> > > possible value is a blob. The only thing that changes is the current
> > > value (which can only point to one of the enumerated blobs).
> >
> > Uh yes that's not clear at all, and if we do go with this, I guess we
> > should have a pile of core code to make sure it validates and is
> > consistent.
> >
> > >> > - no caps for properties. Yes that gives us a theoretical problem, no in
> > > > practice it doesn't matter, since people don't even care enough to make
> > > > e.g. fbdev resetting work today for everything. Long form discussion,
> > > > see here:
> > > >
> > > > https://blog.ffwll.ch/2016/01/vt-switching-with-atomic-modeset.html
> > > >
> > > > Nothing happened in this area ever since I typed this up, so I guess
> > > > it's really not a real-world concern.
> > > >
> > > > - Simplest path forward would be if we accept different LUT sizes than the
> > > > one advertised (we already do that for legacy gamma, and this is
> > > > officially what we had in mind too), and the kernel automatically picks
> > > > the best lut configuration. Will be somewhat awkard for the
> > > > multi-segment lut, but would decouple the uapi discussion a bit.
> > >
> > > It'll be ridiculously wasteful. IIRC we need a LUT with 32768 entries,
> > > and then ~98% of those gets thrown away and never programmed to the
> > > hardware.
> >
> > Yeah it's a few MB, not that awesome really ...
> >
> > > > - Frankly the uapi proposed looks like fake generic - it tries to model
> > > > all possibilities in a generic way, when really userspace needs to have
> > > > special code for special pipelines.
> > >
> > > I think it can be used pretty easily. Userspace just has to decide
> > > whether it wants a straight up LUT or whether an interpolated curve
> > > is enough, and how much precision it needs. For x11 the logic would
> > > be simple enough: 1. look for straight up LUT with num_entries >= 1<<bpc,
> > > if that isn't found fall back to an interpolated curve with >= 1<<bpc
> > > precision, and finally just fall back to whatever gives the best
> > > results I suppose.
> >
> > Hm, there's also a bunch more defines about mirroring and non-negative and
> > other stuff that I have no idea how userspace should use it. I do think
> > some "here's the possible configs for color management" thing is needed,
> > I'm not sure userspace can do much with all the details provided in the
> > current series.
>
> The negative values would represent out of gamut colors, which
> can definitely happen with xvYCC. Looks like the v4l folks
> also considered this in their transfer func docs [1], which
> are specifying the formulas extended for <0.0 values.
>
> Also based on my chat with Ilia on irc at some point I got the
> impression that nv hardware may have gamma LUTs which support
> negative values without this mirroring trick. Hence I wanted to
> make all the numbers signed rather than unsigned.
>
> We could of course leave a bunch of these advanced things
> undefined until an actual use case comes around. But I wanted to
> include it all in my initial proposal so that we could be more
> confident that we're not painting ourselves in a corner that
> would require yet another uapi to escape.
Hm good point. I just feel like with just one driver (at least in kms) it
might be a bit early to go all in on color manager v2.0. From a kms
ecosystem pov we're still pretty busy rolling out the first one. But if we
need it now, I guess we'll need it now ...
> [1] https://www.linuxtv.org/downloads/v4l-dvb-apis-old/ch02s06.html
>
> --
> Ville Syrjälä
> Intel
> ---------------------------------------------------------------------
> Intel Finland Oy
> Registered Address: PL 281, 00181 Helsinki
> Business Identity Code: 0357606 - 4
> Domiciled in Helsinki
>
> This e-mail and any attachments may contain confidential material for
> the sole use of the intended recipient(s). Any review or distribution
> by others is strictly prohibited. If you are not the intended
> recipient, please contact the sender and delete all copies.
Wrong mail address :-)
-Daniel
--
Daniel Vetter
Software Engineer, Intel Corporation
http://blog.ffwll.ch
_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel
^ permalink raw reply [flat|nested] 24+ messages in thread
end of thread, other threads:[~2019-04-23 6:52 UTC | newest]
Thread overview: 24+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2019-04-12 10:20 [v3 0/7] Add Multi Segment Gamma Support Uma Shankar
2019-04-12 10:20 ` [v3 1/7] drm: Add gamma mode property Uma Shankar
2019-04-16 7:28 ` Daniel Vetter
2019-04-12 10:20 ` [v3 2/7] drm/i915: Define color lut range structure Uma Shankar
2019-04-12 10:20 ` [v3 3/7] drm/i915/icl: Add register definitions for Multi Segmented gamma Uma Shankar
2019-04-12 10:21 ` [v3 4/7] drm/i915/icl: Add support for multi segmented gamma mode Uma Shankar
2019-04-12 10:21 ` [v3 5/7] drm/i915: Attach gamma mode property Uma Shankar
2019-04-12 10:21 ` [v3 6/7] drm: Add Client Cap for advance gamma mode Uma Shankar
2019-04-15 10:57 ` Lankhorst, Maarten
2019-04-15 12:43 ` [Intel-gfx] " Ville Syrjälä
2019-04-16 8:54 ` Lankhorst, Maarten
2019-04-15 13:56 ` Sharma, Shashank
2019-04-15 14:12 ` Lankhorst, Maarten
2019-04-15 14:29 ` Sharma, Shashank
2019-04-15 19:20 ` Daniel Vetter
2019-04-16 15:06 ` Ville Syrjälä
2019-04-12 10:21 ` [v3 7/7] drm/i915: Enable " Uma Shankar
2019-04-12 10:39 ` ✗ Fi.CI.CHECKPATCH: warning for Add Multi Segment Gamma Support (rev3) Patchwork
2019-04-12 12:50 ` ✗ Fi.CI.BAT: failure " Patchwork
2019-04-17 7:28 ` [v3 0/7] Add Multi Segment Gamma Support Daniel Vetter
2019-04-17 11:57 ` Ville Syrjälä
2019-04-18 7:13 ` Daniel Vetter
2019-04-18 13:11 ` Ville Syrjälä
2019-04-23 6:52 ` Daniel Vetter
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.