diff mbox series

[RFC,v5,1/8] drm: Add Enhanced Gamma LUT precision structure

Message ID 1537085731-6355-2-git-send-email-uma.shankar@intel.com (mailing list archive)
State New, archived
Headers show
Series Add Plane Color Properties | expand

Commit Message

Shankar, Uma Sept. 16, 2018, 8:15 a.m. UTC 
Existing LUT precision structure is having only 16 bit
precision. This is not enough for upcoming enhanced hardwares
and advance usecases like HDR processing. Hence added a new
structure with 32 bit precision values. Also added the code,
for extracting the same from values passed from userspace.

v4: Rebase

v5: Relocated the helper function to drm_color_mgmt.c. Declared
the same in a header file (Alexandru Gheorghe)

Signed-off-by: Uma Shankar <uma.shankar@intel.com>
Reviewed-by: Alexandru Gheorghe <alexandru-cosmin.gheorghe@arm.com>
---
 drivers/gpu/drm/drm_color_mgmt.c | 19 +++++++++++++++++++
 include/drm/drm_color_mgmt.h     |  1 +
 include/uapi/drm/drm_mode.h      | 15 +++++++++++++++
 3 files changed, 35 insertions(+)

Comments

Matt Roper Nov. 5, 2018, 6:17 p.m. UTC | #1 
On Sun, Sep 16, 2018 at 01:45:24PM +0530, Uma Shankar wrote:
> Existing LUT precision structure is having only 16 bit
> precision. This is not enough for upcoming enhanced hardwares
> and advance usecases like HDR processing. Hence added a new
> structure with 32 bit precision values. Also added the code,
> for extracting the same from values passed from userspace.
> 
> v4: Rebase
> 
> v5: Relocated the helper function to drm_color_mgmt.c. Declared
> the same in a header file (Alexandru Gheorghe)
> 
> Signed-off-by: Uma Shankar <uma.shankar@intel.com>
> Reviewed-by: Alexandru Gheorghe <alexandru-cosmin.gheorghe@arm.com>
> ---
>  drivers/gpu/drm/drm_color_mgmt.c | 19 +++++++++++++++++++
>  include/drm/drm_color_mgmt.h     |  1 +
>  include/uapi/drm/drm_mode.h      | 15 +++++++++++++++
>  3 files changed, 35 insertions(+)
> 
> diff --git a/drivers/gpu/drm/drm_color_mgmt.c b/drivers/gpu/drm/drm_color_mgmt.c
> index b97e2de..1bdcc1a 100644
> --- a/drivers/gpu/drm/drm_color_mgmt.c
> +++ b/drivers/gpu/drm/drm_color_mgmt.c
> @@ -128,6 +128,25 @@ uint32_t drm_color_lut_extract(uint32_t user_input, uint32_t bit_precision)
>  }
>  EXPORT_SYMBOL(drm_color_lut_extract);
>  
> +/*
> + * Added to accommodate enhanced LUT precision.
> + * Max LUT precision is 32 bits.
> + */
> +uint32_t drm_color_lut_extract_ext(uint32_t user_input, uint32_t bit_precision)
> +{
> +	uint32_t val = user_input;
> +	uint32_t max = 0xffffffff >> (32 - bit_precision);
> +
> +	/* Round only if we're not using full precision. */
> +	if (bit_precision < 32) {
> +		val += 1UL << (32 - bit_precision - 1);

Does val need to be a 64-bit type to prevent this from overflowing?


Matt

> +		val >>= 32 - bit_precision;
> +	}
> +
> +	return clamp_val(val, 0, max);
> +}
> +EXPORT_SYMBOL(drm_color_lut_extract_ext);
> +
>  /**
>   * drm_crtc_enable_color_mgmt - enable color management properties
>   * @crtc: DRM CRTC
> diff --git a/include/drm/drm_color_mgmt.h b/include/drm/drm_color_mgmt.h
> index 44f04233..78b5a37 100644
> --- a/include/drm/drm_color_mgmt.h
> +++ b/include/drm/drm_color_mgmt.h
> @@ -29,6 +29,7 @@
>  struct drm_plane;
>  
>  uint32_t drm_color_lut_extract(uint32_t user_input, uint32_t bit_precision);
> +uint32_t drm_color_lut_extract_ext(uint32_t user_input, uint32_t bit_precision);
>  
>  void drm_crtc_enable_color_mgmt(struct drm_crtc *crtc,
>  				uint degamma_lut_size,
> diff --git a/include/uapi/drm/drm_mode.h b/include/uapi/drm/drm_mode.h
> index 8d67243..874407b 100644
> --- a/include/uapi/drm/drm_mode.h
> +++ b/include/uapi/drm/drm_mode.h
> @@ -629,6 +629,21 @@ struct drm_color_lut {
>  	__u16 reserved;
>  };
>  
> +/*
> + * Creating 32 bit palette entries for better data
> + * precision. This will be required for HDR and
> + * similar color processing usecases.
> + */
> +struct drm_color_lut_ext {
> +	/*
> +	 * Data is U0.32 fixed point format.
> +	 */
> +	__u32 red;
> +	__u32 green;
> +	__u32 blue;
> +	__u32 reserved;
> +};
> +
>  #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 Nov. 6, 2018, 9:03 a.m. UTC | #2 
On Sun, Sep 16, 2018 at 01:45:24PM +0530, Uma Shankar wrote:
> Existing LUT precision structure is having only 16 bit
> precision. This is not enough for upcoming enhanced hardwares
> and advance usecases like HDR processing. Hence added a new
> structure with 32 bit precision values. Also added the code,
> for extracting the same from values passed from userspace.
> 
> v4: Rebase
> 
> v5: Relocated the helper function to drm_color_mgmt.c. Declared
> the same in a header file (Alexandru Gheorghe)
> 
> Signed-off-by: Uma Shankar <uma.shankar@intel.com>
> Reviewed-by: Alexandru Gheorghe <alexandru-cosmin.gheorghe@arm.com>

Why is this not enough? If it's just about rounding, then I've already
made a comment internally how that should be handled, so that we don't
have rounding artifacts when upgrading a 16bit entry to e.g. 20 bits.

Creating new uapi is generally really hard, and needs a bunch more thought
than this here. If possible it's much better (or at least, much easier,
and much less effort) if this can be avoided.
-Daniel

> ---
>  drivers/gpu/drm/drm_color_mgmt.c | 19 +++++++++++++++++++
>  include/drm/drm_color_mgmt.h     |  1 +
>  include/uapi/drm/drm_mode.h      | 15 +++++++++++++++
>  3 files changed, 35 insertions(+)
> 
> diff --git a/drivers/gpu/drm/drm_color_mgmt.c b/drivers/gpu/drm/drm_color_mgmt.c
> index b97e2de..1bdcc1a 100644
> --- a/drivers/gpu/drm/drm_color_mgmt.c
> +++ b/drivers/gpu/drm/drm_color_mgmt.c
> @@ -128,6 +128,25 @@ uint32_t drm_color_lut_extract(uint32_t user_input, uint32_t bit_precision)
>  }
>  EXPORT_SYMBOL(drm_color_lut_extract);
>  
> +/*
> + * Added to accommodate enhanced LUT precision.
> + * Max LUT precision is 32 bits.
> + */
> +uint32_t drm_color_lut_extract_ext(uint32_t user_input, uint32_t bit_precision)
> +{
> +	uint32_t val = user_input;
> +	uint32_t max = 0xffffffff >> (32 - bit_precision);
> +
> +	/* Round only if we're not using full precision. */
> +	if (bit_precision < 32) {
> +		val += 1UL << (32 - bit_precision - 1);
> +		val >>= 32 - bit_precision;
> +	}
> +
> +	return clamp_val(val, 0, max);
> +}
> +EXPORT_SYMBOL(drm_color_lut_extract_ext);
> +
>  /**
>   * drm_crtc_enable_color_mgmt - enable color management properties
>   * @crtc: DRM CRTC
> diff --git a/include/drm/drm_color_mgmt.h b/include/drm/drm_color_mgmt.h
> index 44f04233..78b5a37 100644
> --- a/include/drm/drm_color_mgmt.h
> +++ b/include/drm/drm_color_mgmt.h
> @@ -29,6 +29,7 @@
>  struct drm_plane;
>  
>  uint32_t drm_color_lut_extract(uint32_t user_input, uint32_t bit_precision);
> +uint32_t drm_color_lut_extract_ext(uint32_t user_input, uint32_t bit_precision);
>  
>  void drm_crtc_enable_color_mgmt(struct drm_crtc *crtc,
>  				uint degamma_lut_size,
> diff --git a/include/uapi/drm/drm_mode.h b/include/uapi/drm/drm_mode.h
> index 8d67243..874407b 100644
> --- a/include/uapi/drm/drm_mode.h
> +++ b/include/uapi/drm/drm_mode.h
> @@ -629,6 +629,21 @@ struct drm_color_lut {
>  	__u16 reserved;
>  };
>  
> +/*
> + * Creating 32 bit palette entries for better data
> + * precision. This will be required for HDR and
> + * similar color processing usecases.
> + */
> +struct drm_color_lut_ext {
> +	/*
> +	 * Data is U0.32 fixed point format.
> +	 */
> +	__u32 red;
> +	__u32 green;
> +	__u32 blue;
> +	__u32 reserved;
> +};
> +
>  #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
diff mbox series

Patch

diff --git a/drivers/gpu/drm/drm_color_mgmt.c b/drivers/gpu/drm/drm_color_mgmt.c
index b97e2de..1bdcc1a 100644
--- a/drivers/gpu/drm/drm_color_mgmt.c
+++ b/drivers/gpu/drm/drm_color_mgmt.c
@@ -128,6 +128,25 @@  uint32_t drm_color_lut_extract(uint32_t user_input, uint32_t bit_precision)
 }
 EXPORT_SYMBOL(drm_color_lut_extract);
 
+/*
+ * Added to accommodate enhanced LUT precision.
+ * Max LUT precision is 32 bits.
+ */
+uint32_t drm_color_lut_extract_ext(uint32_t user_input, uint32_t bit_precision)
+{
+	uint32_t val = user_input;
+	uint32_t max = 0xffffffff >> (32 - bit_precision);
+
+	/* Round only if we're not using full precision. */
+	if (bit_precision < 32) {
+		val += 1UL << (32 - bit_precision - 1);
+		val >>= 32 - bit_precision;
+	}
+
+	return clamp_val(val, 0, max);
+}
+EXPORT_SYMBOL(drm_color_lut_extract_ext);
+
 /**
  * drm_crtc_enable_color_mgmt - enable color management properties
  * @crtc: DRM CRTC
diff --git a/include/drm/drm_color_mgmt.h b/include/drm/drm_color_mgmt.h
index 44f04233..78b5a37 100644
--- a/include/drm/drm_color_mgmt.h
+++ b/include/drm/drm_color_mgmt.h
@@ -29,6 +29,7 @@ 
 struct drm_plane;
 
 uint32_t drm_color_lut_extract(uint32_t user_input, uint32_t bit_precision);
+uint32_t drm_color_lut_extract_ext(uint32_t user_input, uint32_t bit_precision);
 
 void drm_crtc_enable_color_mgmt(struct drm_crtc *crtc,
 				uint degamma_lut_size,
diff --git a/include/uapi/drm/drm_mode.h b/include/uapi/drm/drm_mode.h
index 8d67243..874407b 100644
--- a/include/uapi/drm/drm_mode.h
+++ b/include/uapi/drm/drm_mode.h
@@ -629,6 +629,21 @@  struct drm_color_lut {
 	__u16 reserved;
 };
 
+/*
+ * Creating 32 bit palette entries for better data
+ * precision. This will be required for HDR and
+ * similar color processing usecases.
+ */
+struct drm_color_lut_ext {
+	/*
+	 * Data is U0.32 fixed point format.
+	 */
+	__u32 red;
+	__u32 green;
+	__u32 blue;
+	__u32 reserved;
+};
+
 #define DRM_MODE_PAGE_FLIP_EVENT 0x01
 #define DRM_MODE_PAGE_FLIP_ASYNC 0x02
 #define DRM_MODE_PAGE_FLIP_TARGET_ABSOLUTE 0x4