Re: [PATCH 3/9] drm: Introduce DRM_MODE_COLOR()

[Matt Roper <matthew.d.roper@intel.com>]

On Mon, Jan 18, 2016 at 08:45:37PM +0530, Vandita Kulkarni wrote:
> From: Damien Lespiau <damien.lespiau@intel.com>
> 
> In the hope of expressing colors in the KMS API in a consitant want,
> let's introduce a ARGB 16161616 color and a few convinience macros
> around it.
> 
> Signed-off-by: Damien Lespiau <damien.lespiau@intel.com>

This is somewhat similar to some of the background color prep work I did
here:

        http://patchwork.freedesktop.org/patch/62679/

I purposely wrapped the value into a struct to prevent drivers from
trying to do their own bitwise operations on color values without using
the provided helpers (which could easily lead to bugs if they mistakenly
forget that the color values aren't in their preferred format already).

Since you're going on to make properties that take values in this
format, the drm_property_create_rgba() and such that I have in that
patch (and the corresponding libdrm helpers that are also on the mailing
list) might be useful here as well.


Matt

> ---
>  drivers/gpu/drm/i915/intel_sprite.c |  1 -
>  include/uapi/drm/drm_mode.h         | 34 ++++++++++++++++++++++++++++++++++
>  2 files changed, 34 insertions(+), 1 deletion(-)
> 
> diff --git a/drivers/gpu/drm/i915/intel_sprite.c b/drivers/gpu/drm/i915/intel_sprite.c
> index b7acfdf..9538658 100644
> --- a/drivers/gpu/drm/i915/intel_sprite.c
> +++ b/drivers/gpu/drm/i915/intel_sprite.c
> @@ -210,7 +210,6 @@ skl_update_plane(struct drm_plane *drm_plane,
>  		PLANE_CTL_PIPE_GAMMA_ENABLE |
>  		PLANE_CTL_PIPE_CSC_ENABLE;
>  
> -	plane_ctl |= skl_plane_ctl_format(fb->pixel_format);
>  	plane_ctl |= skl_plane_ctl_format(fb->pixel_format,
>  					plane_state->premultiplied_alpha,
>  					plane_state->drop_alpha);

This hunk should be squashed into the previous patch.


> diff --git a/include/uapi/drm/drm_mode.h b/include/uapi/drm/drm_mode.h
> index 50adb46..12687bc 100644
> --- a/include/uapi/drm/drm_mode.h
> +++ b/include/uapi/drm/drm_mode.h
> @@ -295,6 +295,40 @@ struct drm_mode_get_connector {
>   */
>  #define DRM_MODE_PROP_ATOMIC        0x80000000
>  
> +/* Color for the KMS API, ARGB (msb -> lsb) 16bits per component. */
> +#define DRM_MODE_COLOR(a, r, b, g)	\
> +	(((__u64)(a) << 48) | ((__u64)(r) << 32) | \
> +	 ((__u64)(g) << 16) | (__u64)(b))
> +
> +/* Extract full precision, 8 bits, 10 bits and 12 bits components. */
> +#define DRM_MODE_COLOR_ALPHA(color)	(((color) >> 48) & 0xffff)
> +#define DRM_MODE_COLOR_RED(color)	(((color) >> 32) & 0xffff)
> +#define DRM_MODE_COLOR_BLUE(color)	(((color) >> 16) & 0xffff)
> +#define DRM_MODE_COLOR_GREEN(color)	((color) & 0xffff)
> +#define DRM_MODE_COLOR_ALPHA_8(color)	(((color) >> (48 + 8)) & 0xff)
> +#define DRM_MODE_COLOR_RED_8(color)	(((color) >> (32 + 8)) & 0xff)
> +#define DRM_MODE_COLOR_BLUE_8(color)	(((color) >> (16 + 8)) & 0xff)
> +#define DRM_MODE_COLOR_GREEN_8(color)	(((color) >>  8) & 0xff)
> +#define DRM_MODE_COLOR_ALPHA_10(color)	(((color) >> (48 + 6)) & 0x3ff)
> +#define DRM_MODE_COLOR_RED_10(color)	(((color) >> (32 + 6)) & 0x3ff)
> +#define DRM_MODE_COLOR_BLUE_10(color)	(((color) >> (16 + 6)) & 0x3ff)
> +#define DRM_MODE_COLOR_GREEN_10(color)	(((color) >>  6) & 0x3ff)
> +#define DRM_MODE_COLOR_ALPHA_12(color)	(((color) >> (48 + 4)) & 0xfff)
> +#define DRM_MODE_COLOR_RED_12(color)	(((color) >> (32 + 4)) & 0xfff)
> +#define DRM_MODE_COLOR_BLUE_12(color)	(((color) >> (16 + 4)) & 0xfff)
> +#define DRM_MODE_COLOR_GREEN_12(color)	(((color) >>  4) & 0xfff)
> +
> +/* Handy macros to convert a DRM_MODE_COLOR() into common precisions */
> +#define DRM_MODE_COLOR_TO_ARGB_8888(color)	 \
> +	((DRM_MODE_COLOR_ALPHA_8(color) << 24) | \
> +	 (DRM_MODE_COLOR_RED_8(color)   << 16) | \
> +	 (DRM_MODE_COLOR_GREEN_8(color) << 8)  | \
> +	 DRM_MODE_COLOR_BLUE_8(color))
> +#define DRM_MODE_COLOR_TO_RGB_101010(color)	  \
> +	 ((DRM_MODE_COLOR_RED_10(color)  << 20) | \
> +	 (DRM_MODE_COLOR_GREEN_10(color) << 10) | \
> +	 DRM_MODE_COLOR_BLUE_10(color))
> +
>  struct drm_mode_property_enum {
>  	__u64 value;
>  	char name[DRM_PROP_NAME_LEN];
> -- 
> 1.9.1
> 
> _______________________________________________
> Intel-gfx mailing list
> Intel-gfx@lists.freedesktop.org
> http://lists.freedesktop.org/mailman/listinfo/intel-gfx

-- 
Matt Roper
Graphics Software Engineer
IoTG Platform Enabling & Development
Intel Corporation
(916) 356-2795
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx