From patchwork Thu Jul 21 19:17:34 2016 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 8bit X-Patchwork-Submitter: =?utf-8?b?VmlsbGUgU3lyasOkbMOk?= X-Patchwork-Id: 9242363 Return-Path: Received: from mail.wl.linuxfoundation.org (pdx-wl-mail.web.codeaurora.org [172.30.200.125]) by pdx-korg-patchwork.web.codeaurora.org (Postfix) with ESMTP id 9BCA860574 for ; Thu, 21 Jul 2016 19:17:44 +0000 (UTC) Received: from mail.wl.linuxfoundation.org (localhost [127.0.0.1]) by mail.wl.linuxfoundation.org (Postfix) with ESMTP id 86DCD27A98 for ; Thu, 21 Jul 2016 19:17:44 +0000 (UTC) Received: by mail.wl.linuxfoundation.org (Postfix, from userid 486) id 78C5D27DE0; Thu, 21 Jul 2016 19:17:44 +0000 (UTC) X-Spam-Checker-Version: SpamAssassin 3.3.1 (2010-03-16) on pdx-wl-mail.web.codeaurora.org X-Spam-Level: X-Spam-Status: No, score=-4.2 required=2.0 tests=BAYES_00, RCVD_IN_DNSWL_MED autolearn=ham version=3.3.1 Received: from gabe.freedesktop.org (gabe.freedesktop.org [131.252.210.177]) by mail.wl.linuxfoundation.org (Postfix) with ESMTP id B85DF27A98 for ; Thu, 21 Jul 2016 19:17:43 +0000 (UTC) Received: from gabe.freedesktop.org (localhost [127.0.0.1]) by gabe.freedesktop.org (Postfix) with ESMTP id 438E06E19C; Thu, 21 Jul 2016 19:17:41 +0000 (UTC) X-Original-To: intel-gfx@lists.freedesktop.org Delivered-To: intel-gfx@lists.freedesktop.org Received: from mga11.intel.com (mga11.intel.com [192.55.52.93]) by gabe.freedesktop.org (Postfix) with ESMTP id 4301D6EA2F for ; Thu, 21 Jul 2016 19:17:38 +0000 (UTC) Received: from fmsmga001.fm.intel.com ([10.253.24.23]) by fmsmga102.fm.intel.com with ESMTP; 21 Jul 2016 12:17:37 -0700 X-ExtLoop1: 1 X-IronPort-AV: E=Sophos; i="5.28,400,1464678000"; d="scan'208"; a="1011461224" Received: from stinkbox.fi.intel.com (HELO stinkbox) ([10.237.72.174]) by fmsmga001.fm.intel.com with SMTP; 21 Jul 2016 12:17:35 -0700 Received: by stinkbox (sSMTP sendmail emulation); Thu, 21 Jul 2016 22:17:34 +0300 From: ville.syrjala@linux.intel.com To: intel-gfx@lists.freedesktop.org Date: Thu, 21 Jul 2016 22:17:34 +0300 Message-Id: <1469128654-5881-1-git-send-email-ville.syrjala@linux.intel.com> X-Mailer: git-send-email 2.7.4 MIME-Version: 1.0 Cc: "Tang, Jun" , stable@vger.kernel.org Subject: [Intel-gfx] [PATCH] drm/i915: Correctly handle limited range YCbCr data on VLV/CHV X-BeenThere: intel-gfx@lists.freedesktop.org X-Mailman-Version: 2.1.18 Precedence: list List-Id: Intel graphics driver community testing & development List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Errors-To: intel-gfx-bounces@lists.freedesktop.org Sender: "Intel-gfx" X-Virus-Scanned: ClamAV using ClamSMTP From: Ville Syrjälä Turns out the VLV/CHV fixed function sprite CSC expects full range data as input. We've been feeding it limited range data to it all along. To expand the data out to full range we'll use the color correction registers (brightness, contrast, and saturation). On CHV pipe B we were actually doing the right thing already because we progammed the custom CSC matrix to do expect limited range input. Now that well pre-expand the data out with the color correction unit, we need to change the CSC matrix to operate with full range input instead. This should make the sprite output of the other pipes match the sprite output of pipe B reasonably well. Looking at the resulting pipe CRCs, there can be a slight difference in the output, but as I don't know the formula used by the fixed function CSC of the other pipes, I don't think it's worth the effort to try to match the output exactly. It might not even be possible due to difference in internal precision etc. One slight caveat here is that the color correction registers are single bufferred, so we should really be updating them during vblank, but we still don't have a mechanism for that, so just toss in another FIXME. Cc: "Tang, Jun" Reported-by: "Tang, Jun" Cc: stable@vger.kernel.org Fixes: 7f1f3851feb0 ("drm/i915: sprite support for ValleyView v4") Signed-off-by: Ville Syrjälä --- drivers/gpu/drm/i915/i915_reg.h | 10 ++++++ drivers/gpu/drm/i915/intel_sprite.c | 62 ++++++++++++++++++++++++++++--------- 2 files changed, 57 insertions(+), 15 deletions(-) diff --git a/drivers/gpu/drm/i915/i915_reg.h b/drivers/gpu/drm/i915/i915_reg.h index 8bfde75789f6..25223b1d9775 100644 --- a/drivers/gpu/drm/i915/i915_reg.h +++ b/drivers/gpu/drm/i915/i915_reg.h @@ -5385,6 +5385,12 @@ enum { #define _SPATILEOFF (VLV_DISPLAY_BASE + 0x721a4) #define _SPACONSTALPHA (VLV_DISPLAY_BASE + 0x721a8) #define SP_CONST_ALPHA_ENABLE (1<<31) +#define _SPACLRC0 (VLV_DISPLAY_BASE + 0x721d0) +#define SP_CONTRAST(x) ((x) << 18) /* u3.6 */ +#define SP_BRIGHTNESS(x) ((x) & 0xff) /* s8 */ +#define _SPACLRC1 (VLV_DISPLAY_BASE + 0x721d4) +#define SP_SH_SIN(x) (((x) & 0x7ff) << 16) /* s4.7 */ +#define SP_SH_COS(x) (x) /* u3.7 */ #define _SPAGAMC (VLV_DISPLAY_BASE + 0x721f4) #define _SPBCNTR (VLV_DISPLAY_BASE + 0x72280) @@ -5398,6 +5404,8 @@ enum { #define _SPBKEYMAXVAL (VLV_DISPLAY_BASE + 0x722a0) #define _SPBTILEOFF (VLV_DISPLAY_BASE + 0x722a4) #define _SPBCONSTALPHA (VLV_DISPLAY_BASE + 0x722a8) +#define _SPBCLRC0 (VLV_DISPLAY_BASE + 0x722d0) +#define _SPBCLRC1 (VLV_DISPLAY_BASE + 0x722d4) #define _SPBGAMC (VLV_DISPLAY_BASE + 0x722f4) #define SPCNTR(pipe, plane) _MMIO_PIPE((pipe) * 2 + (plane), _SPACNTR, _SPBCNTR) @@ -5411,6 +5419,8 @@ enum { #define SPKEYMAXVAL(pipe, plane) _MMIO_PIPE((pipe) * 2 + (plane), _SPAKEYMAXVAL, _SPBKEYMAXVAL) #define SPTILEOFF(pipe, plane) _MMIO_PIPE((pipe) * 2 + (plane), _SPATILEOFF, _SPBTILEOFF) #define SPCONSTALPHA(pipe, plane) _MMIO_PIPE((pipe) * 2 + (plane), _SPACONSTALPHA, _SPBCONSTALPHA) +#define SPCLRC0(pipe, plane) _MMIO_PIPE((pipe) * 2 + (plane), _SPACLRC0, _SPBCLRC0) +#define SPCLRC1(pipe, plane) _MMIO_PIPE((pipe) * 2 + (plane), _SPACLRC1, _SPBCLRC1) #define SPGAMC(pipe, plane) _MMIO_PIPE((pipe) * 2 + (plane), _SPAGAMC, _SPBGAMC) /* diff --git a/drivers/gpu/drm/i915/intel_sprite.c b/drivers/gpu/drm/i915/intel_sprite.c index 0de935ad01c2..7bea7859df8e 100644 --- a/drivers/gpu/drm/i915/intel_sprite.c +++ b/drivers/gpu/drm/i915/intel_sprite.c @@ -325,28 +325,28 @@ chv_update_csc(struct intel_plane *intel_plane, uint32_t format) return; /* - * BT.601 limited range YCbCr -> full range RGB + * BT.601 full range YCbCr -> full range RGB * - * |r| | 6537 4769 0| |cr | - * |g| = |-3330 4769 -1605| x |y-64| - * |b| | 0 4769 8263| |cb | + * |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, so no - * need for any offset. For Y we need to remove the offset. + * Cb and Cr apparently come in as signed already, + * and we get full range data in on account of CLRC0/1 */ - I915_WRITE(SPCSCYGOFF(plane), SPCSC_OOFF(0) | SPCSC_IOFF(-64)); + I915_WRITE(SPCSCYGOFF(plane), SPCSC_OOFF(0) | SPCSC_IOFF(0)); I915_WRITE(SPCSCCBOFF(plane), SPCSC_OOFF(0) | SPCSC_IOFF(0)); I915_WRITE(SPCSCCROFF(plane), SPCSC_OOFF(0) | SPCSC_IOFF(0)); - I915_WRITE(SPCSCC01(plane), SPCSC_C1(4769) | SPCSC_C0(6537)); - I915_WRITE(SPCSCC23(plane), SPCSC_C1(-3330) | SPCSC_C0(0)); - I915_WRITE(SPCSCC45(plane), SPCSC_C1(-1605) | SPCSC_C0(4769)); - I915_WRITE(SPCSCC67(plane), SPCSC_C1(4769) | SPCSC_C0(0)); - I915_WRITE(SPCSCC8(plane), SPCSC_C0(8263)); + I915_WRITE(SPCSCC01(plane), SPCSC_C1(4096) | SPCSC_C0(5743)); + I915_WRITE(SPCSCC23(plane), SPCSC_C1(-2925) | SPCSC_C0(0)); + I915_WRITE(SPCSCC45(plane), SPCSC_C1(-1410) | SPCSC_C0(4096)); + I915_WRITE(SPCSCC67(plane), SPCSC_C1(4096) | SPCSC_C0(0)); + I915_WRITE(SPCSCC8(plane), SPCSC_C0(7258)); - I915_WRITE(SPCSCYGICLAMP(plane), SPCSC_IMAX(940) | SPCSC_IMIN(64)); - I915_WRITE(SPCSCCBICLAMP(plane), SPCSC_IMAX(448) | SPCSC_IMIN(-448)); - I915_WRITE(SPCSCCRICLAMP(plane), SPCSC_IMAX(448) | SPCSC_IMIN(-448)); + I915_WRITE(SPCSCYGICLAMP(plane), SPCSC_IMAX(1023) | SPCSC_IMIN(0)); + I915_WRITE(SPCSCCBICLAMP(plane), SPCSC_IMAX(512) | SPCSC_IMIN(-512)); + I915_WRITE(SPCSCCRICLAMP(plane), SPCSC_IMAX(512) | SPCSC_IMIN(-512)); I915_WRITE(SPCSCYGOCLAMP(plane), SPCSC_OMAX(1023) | SPCSC_OMIN(0)); I915_WRITE(SPCSCCBOCLAMP(plane), SPCSC_OMAX(1023) | SPCSC_OMIN(0)); @@ -354,6 +354,36 @@ chv_update_csc(struct intel_plane *intel_plane, uint32_t format) } static void +vlv_update_clrc(struct intel_plane *intel_plane, uint32_t format) +{ + struct drm_i915_private *dev_priv = to_i915(intel_plane->base.dev); + enum pipe pipe = intel_plane->pipe; + int plane = intel_plane->plane; + int con, bri, sh_sin, sh_cos; + + if (format_is_yuv(format)) { + /* + * expand limited range to full range. + * contrast is applied first, then brightness + */ + con = ((255 << 7) / 219 + 1) >> 1; + bri = -((16 << 1) * 255 / 219 + 1) >> 1; + sh_sin = 0; + sh_cos = (((128 << 8) / 112) + 1) >> 1; + } else { + /* pass-through everything */ + con = 1 << 6; + bri = 0; + sh_sin = 0; + sh_cos = 1 << 7; + } + + /* FIXME these register are single buffered :( */ + I915_WRITE(SPCLRC0(pipe, plane), SP_CONTRAST(con) | SP_BRIGHTNESS(bri)); + I915_WRITE(SPCLRC1(pipe, plane), SP_SH_SIN(sh_sin) | SP_SH_COS(sh_cos)); +} + +static void vlv_update_plane(struct drm_plane *dplane, const struct intel_crtc_state *crtc_state, const struct intel_plane_state *plane_state) @@ -461,6 +491,8 @@ vlv_update_plane(struct drm_plane *dplane, if (key->flags & I915_SET_COLORKEY_SOURCE) sprctl |= SP_SOURCE_KEY; + vlv_update_clrc(intel_plane, fb->pixel_format); + if (IS_CHERRYVIEW(dev) && pipe == PIPE_B) chv_update_csc(intel_plane, fb->pixel_format);