[Intel-gfx] [PATCH 3/5] drm/i915: Add support for the YCbCr COLOR_ENCODING property
Sharma, Shashank
shashank.sharma at intel.com
Thu Jun 15 13:22:53 UTC 2017
Regards
Shashank
On 6/9/2017 2:03 AM, ville.syrjala at linux.intel.com wrote:
> From: Ville Syrjälä <ville.syrjala at linux.intel.com>
>
> Add support for the COLOR_ENCODING plane property which selects
> the matrix coefficients used for the YCbCr->RGB conversion. Our
> hardware can generally handle BT.601 and BT.709.
>
> CHV pipe B sprites have a fully programmable matrix, so in theory
> we could handle anything, but it doesn't seem all that useful to
> expose anything beyond BT.601 and BT.709 at this time.
>
> GLK can supposedly do BT.2020, but let's leave enabling that for
> the future as well.
>
> Cc: Jyri Sarha <jsarha at ti.com>
> Signed-off-by: Ville Syrjälä <ville.syrjala at linux.intel.com>
> ---
> drivers/gpu/drm/i915/i915_reg.h | 3 ++
> drivers/gpu/drm/i915/intel_display.c | 19 +++++++++--
> drivers/gpu/drm/i915/intel_sprite.c | 61 +++++++++++++++++++++++++++---------
> 3 files changed, 66 insertions(+), 17 deletions(-)
>
> diff --git a/drivers/gpu/drm/i915/i915_reg.h b/drivers/gpu/drm/i915/i915_reg.h
> index f4f51e7a3e83..bac3ec378b6e 100644
> --- a/drivers/gpu/drm/i915/i915_reg.h
> +++ b/drivers/gpu/drm/i915/i915_reg.h
> @@ -5612,6 +5612,7 @@ enum {
> #define DVS_PIPE_CSC_ENABLE (1<<24)
> #define DVS_SOURCE_KEY (1<<22)
> #define DVS_RGB_ORDER_XBGR (1<<20)
> +#define DVS_YUV_CSC_FORMAT_BT709 (1<<18)
> #define DVS_YUV_BYTE_ORDER_MASK (3<<16)
> #define DVS_YUV_ORDER_YUYV (0<<16)
> #define DVS_YUV_ORDER_UYVY (1<<16)
> @@ -5758,6 +5759,7 @@ enum {
> #define SP_FORMAT_RGBA8888 (0xf<<26)
> #define SP_ALPHA_PREMULTIPLY (1<<23) /* CHV pipe B */
> #define SP_SOURCE_KEY (1<<22)
> +#define SP_YUV_CSC_FORMAT_BT709 (1<<18)
> #define SP_YUV_BYTE_ORDER_MASK (3<<16)
> #define SP_YUV_ORDER_YUYV (0<<16)
> #define SP_YUV_ORDER_UYVY (1<<16)
> @@ -5876,6 +5878,7 @@ enum {
> #define PLANE_CTL_KEY_ENABLE_DESTINATION ( 2 << 21)
> #define PLANE_CTL_ORDER_BGRX (0 << 20)
> #define PLANE_CTL_ORDER_RGBX (1 << 20)
> +#define PLANE_CTL_YUV_CSC_FORMAT_BT709 (1 << 18)
Should we call it PLANE_CTL_CSC_YUV_FORMAT_BT709 (or similar) so that it
would be clear that source is BT709 format.
Right now, it feels like CSC(destination) format is BT709.
> #define PLANE_CTL_YUV422_ORDER_MASK (0x3 << 16)
> #define PLANE_CTL_YUV422_YUYV ( 0 << 16)
> #define PLANE_CTL_YUV422_UYVY ( 1 << 16)
> diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c
> index e2aaaf60a969..810b53b0128c 100644
> --- a/drivers/gpu/drm/i915/intel_display.c
> +++ b/drivers/gpu/drm/i915/intel_display.c
> @@ -3321,6 +3321,9 @@ u32 skl_plane_ctl(const struct intel_crtc_state *crtc_state,
> PLANE_CTL_PIPE_GAMMA_ENABLE |
> PLANE_CTL_PIPE_CSC_ENABLE |
> PLANE_CTL_PLANE_GAMMA_DISABLE;
> +
> + if (plane_state->base.color_encoding == DRM_COLOR_YCBCR_BT709)
> + plane_ctl |= PLANE_CTL_YUV_CSC_FORMAT_BT709;
> }
>
> plane_ctl |= skl_plane_ctl_format(fb->format->format);
> @@ -3344,8 +3347,12 @@ u32 glk_color_ctl(const struct intel_plane_state *plane_state)
> PLANE_COLOR_PIPE_CSC_ENABLE |
> PLANE_COLOR_PLANE_GAMMA_DISABLE;
>
> - if (intel_format_is_yuv(fb->format->format))
> - color_ctl |= PLANE_COLOR_CSC_MODE_YUV601_TO_RGB709;
> + if (intel_format_is_yuv(fb->format->format)) {
> + if (plane_state->base.color_encoding == DRM_COLOR_YCBCR_BT709)
> + color_ctl |= PLANE_COLOR_CSC_MODE_YUV709_TO_RGB709;
> + else
> + color_ctl |= PLANE_COLOR_CSC_MODE_YUV601_TO_RGB709;
> + }
>
> return color_ctl;
> }
> @@ -13819,6 +13826,14 @@ intel_primary_plane_create(struct drm_i915_private *dev_priv, enum pipe pipe)
> DRM_MODE_ROTATE_0,
> supported_rotations);
>
> + if (INTEL_GEN(dev_priv) >= 9)
I am seeing few if (INTEL_GEN(dev_priv) >= 9) checks above, would it
make sense to reuse any of those, instead of adding
a new one ?
- Shashank
> + drm_plane_create_color_properties(&primary->base,
> + BIT(DRM_COLOR_YCBCR_BT601) |
> + BIT(DRM_COLOR_YCBCR_BT709),
> + BIT(DRM_COLOR_YCBCR_LIMITED_RANGE),
> + DRM_COLOR_YCBCR_BT601,
> + DRM_COLOR_YCBCR_LIMITED_RANGE);
> +
> drm_plane_helper_add(&primary->base, &intel_plane_helper_funcs);
>
> return primary;
> diff --git a/drivers/gpu/drm/i915/intel_sprite.c b/drivers/gpu/drm/i915/intel_sprite.c
> index cc07157f2eb6..c21f43d64b00 100644
> --- a/drivers/gpu/drm/i915/intel_sprite.c
> +++ b/drivers/gpu/drm/i915/intel_sprite.c
> @@ -328,30 +328,45 @@ chv_update_csc(const struct intel_plane_state *plane_state)
> struct drm_i915_private *dev_priv = to_i915(plane->base.dev);
> const struct drm_framebuffer *fb = plane_state->base.fb;
> enum plane_id plane_id = plane->id;
> + /*
> + * |r| | c0 c1 c2 | |cr|
> + * |g| = | c3 c4 c5 | x |y |
> + * |b| | c6 c7 c8 | |cb|
> + *
> + * Coefficients are s3.12.
> + *
> + * Cb and Cr apparently come in as signed already, and
> + * we always get full range data in on account of CLRC0/1.
> + */
> + static const s16 csc_matrix[][9] = {
> + /* BT.601 full range YCbCr -> full range RGB */
> + [DRM_COLOR_YCBCR_BT601] = {
> + 5743, 4096, 0,
> + -2925, 4096, -1410,
> + 0, 4096, 7258,
> + },
> + /* BT.709 full range YCbCr -> full range RGB */
> + [DRM_COLOR_YCBCR_BT709] = {
> + 6450, 4096, 0,
> + -1917, 4096, -767,
> + 0, 4096, 7601,
> + },
> + };
> + const s16 *csc = csc_matrix[plane_state->base.color_encoding];
>
> /* Seems RGB data bypasses the CSC always */
> if (!intel_format_is_yuv(fb->format->format))
> return;
>
> - /*
> - * BT.601 full range YCbCr -> full range RGB
> - *
> - * |r| | 5743 4096 0| |cr|
> - * |g| = |-2925 4096 -1410| x |y |
> - * |b| | 0 4096 7258| |cb|
> - *
> - * Cb and Cr apparently come in as signed already,
> - * and we get full range data in on account of CLRC0/1
> - */
> I915_WRITE_FW(SPCSCYGOFF(plane_id), SPCSC_OOFF(0) | SPCSC_IOFF(0));
> I915_WRITE_FW(SPCSCCBOFF(plane_id), SPCSC_OOFF(0) | SPCSC_IOFF(0));
> I915_WRITE_FW(SPCSCCROFF(plane_id), SPCSC_OOFF(0) | SPCSC_IOFF(0));
>
> - I915_WRITE_FW(SPCSCC01(plane_id), SPCSC_C1(4096) | SPCSC_C0(5743));
> - I915_WRITE_FW(SPCSCC23(plane_id), SPCSC_C1(-2925) | SPCSC_C0(0));
> - I915_WRITE_FW(SPCSCC45(plane_id), SPCSC_C1(-1410) | SPCSC_C0(4096));
> - I915_WRITE_FW(SPCSCC67(plane_id), SPCSC_C1(4096) | SPCSC_C0(0));
> - I915_WRITE_FW(SPCSCC8(plane_id), SPCSC_C0(7258));
> + I915_WRITE_FW(SPCSCC01(plane_id), SPCSC_C1(csc[1]) | SPCSC_C0(csc[0]));
> + I915_WRITE_FW(SPCSCC23(plane_id), SPCSC_C1(csc[3]) | SPCSC_C0(csc[2]));
> + I915_WRITE_FW(SPCSCC45(plane_id), SPCSC_C1(csc[5]) | SPCSC_C0(csc[4]));
> + I915_WRITE_FW(SPCSCC67(plane_id), SPCSC_C1(csc[7]) | SPCSC_C0(csc[6]));
> + I915_WRITE_FW(SPCSCC8(plane_id), SPCSC_C0(csc[8]));
>
> I915_WRITE_FW(SPCSCYGICLAMP(plane_id), SPCSC_IMAX(1023) | SPCSC_IMIN(0));
> I915_WRITE_FW(SPCSCCBICLAMP(plane_id), SPCSC_IMAX(512) | SPCSC_IMIN(-512));
> @@ -445,6 +460,9 @@ static u32 vlv_sprite_ctl(const struct intel_crtc_state *crtc_state,
> return 0;
> }
>
> + if (plane_state->base.color_encoding == DRM_COLOR_YCBCR_BT709)
> + sprctl |= SP_YUV_CSC_FORMAT_BT709;
> +
> if (fb->modifier == I915_FORMAT_MOD_X_TILED)
> sprctl |= SP_TILED;
>
> @@ -578,6 +596,9 @@ static u32 ivb_sprite_ctl(const struct intel_crtc_state *crtc_state,
> return 0;
> }
>
> + if (plane_state->base.color_encoding == DRM_COLOR_YCBCR_BT709)
> + sprctl |= SPRITE_YUV_CSC_FORMAT_BT709;
> +
> if (fb->modifier == I915_FORMAT_MOD_X_TILED)
> sprctl |= SPRITE_TILED;
>
> @@ -715,6 +736,9 @@ static u32 g4x_sprite_ctl(const struct intel_crtc_state *crtc_state,
> return 0;
> }
>
> + if (plane_state->base.color_encoding == DRM_COLOR_YCBCR_BT709)
> + dvscntr |= DVS_YUV_CSC_FORMAT_BT709;
> +
> if (fb->modifier == I915_FORMAT_MOD_X_TILED)
> dvscntr |= DVS_TILED;
>
> @@ -1221,6 +1245,13 @@ intel_sprite_plane_create(struct drm_i915_private *dev_priv,
> DRM_MODE_ROTATE_0,
> supported_rotations);
>
> + drm_plane_create_color_properties(&intel_plane->base,
> + BIT(DRM_COLOR_YCBCR_BT601) |
> + BIT(DRM_COLOR_YCBCR_BT709),
> + BIT(DRM_COLOR_YCBCR_LIMITED_RANGE),
> + DRM_COLOR_YCBCR_BT601,
> + DRM_COLOR_YCBCR_LIMITED_RANGE);
> +
> drm_plane_helper_add(&intel_plane->base, &intel_plane_helper_funcs);
>
> return intel_plane;
More information about the Intel-gfx
mailing list