From mboxrd@z Thu Jan 1 00:00:00 1970 From: Paulo Zanoni Subject: Re: [PATCH] drm/i915: Implement pipe CSC based limited range RGB output Date: Thu, 14 Feb 2013 17:30:12 -0200 Message-ID: References: <1358529098-31596-1-git-send-email-ville.syrjala@linux.intel.com> Mime-Version: 1.0 Content-Type: text/plain; charset="iso-8859-1" Content-Transfer-Encoding: quoted-printable Return-path: Received: from mail-oa0-f49.google.com (mail-oa0-f49.google.com [209.85.219.49]) by gabe.freedesktop.org (Postfix) with ESMTP id 06473E5C3C for ; Thu, 14 Feb 2013 11:30:12 -0800 (PST) Received: by mail-oa0-f49.google.com with SMTP id j6so2901688oag.22 for ; Thu, 14 Feb 2013 11:30:12 -0800 (PST) In-Reply-To: <1358529098-31596-1-git-send-email-ville.syrjala@linux.intel.com> List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Sender: intel-gfx-bounces+gcfxdi-intel-gfx=m.gmane.org@lists.freedesktop.org Errors-To: intel-gfx-bounces+gcfxdi-intel-gfx=m.gmane.org@lists.freedesktop.org To: ville.syrjala@linux.intel.com Cc: intel-gfx@lists.freedesktop.org List-Id: intel-gfx@lists.freedesktop.org Hi 2013/1/18 : > From: Ville Syrj=E4l=E4 > > HSW no longer has the PIPECONF bit for limited range RGB output. > Instead the pipe CSC unit must be used to perform that task. > > The CSC pre offset are set to 0, since the incoming data is full > [0:255] range RGB, the coefficients are programmed to compress the > data into [0:219] range, and then we use either the CSC_MODE black > screen offset bit, or the CSC post offsets to shift the data to > the correct [16:235] range. > > Also have to change the confiuration of all planes so that the > data is sent through the pipe CSC unit. For simplicity send the > plane data through the pipe CSC unit always, and in case full > range output is requested, the pipe CSC unit is set up with an > identity transform to pass the plane data through unchanged. > > I've been told by some hardware people that the use of the pipe > CSC unit shouldn't result in any measurable increase in power > consumption numbers. > > Signed-off-by: Ville Syrj=E4l=E4 Reviewed-by: Paulo Zanoni > --- > Note that I haven't actually tested this on HSW. I did test the earlier > prototype version on ILK and IVB. The pipe CSC unit on ILK isn't programm= ed > in quite the same as on HSW, but the IVB unit _should_ be identical to HS= W. > > The main risk involves the coefficient registers. If the channel mapping > changed for some reason, we could get swapped channels. For some reason > reality and documenation didn't seem to agree how the channels are mapped > even on ILK and IVB. So I'd like someone to try this out on HSW to make > sure the output still looks correct. > > drivers/gpu/drm/i915/i915_reg.h | 52 ++++++++++++++++++++++++- > drivers/gpu/drm/i915/intel_display.c | 71 ++++++++++++++++++++++++++++= +++++- > drivers/gpu/drm/i915/intel_sprite.c | 3 + > 3 files changed, 124 insertions(+), 2 deletions(-) > > diff --git a/drivers/gpu/drm/i915/i915_reg.h b/drivers/gpu/drm/i915/i915_= reg.h > index a2550c5..63ebda8 100644 > --- a/drivers/gpu/drm/i915/i915_reg.h > +++ b/drivers/gpu/drm/i915/i915_reg.h > @@ -2944,6 +2944,7 @@ > #define CURSOR_ENABLE 0x80000000 > #define CURSOR_GAMMA_ENABLE 0x40000000 > #define CURSOR_STRIDE_MASK 0x30000000 > +#define CURSOR_PIPE_CSC_ENABLE (1<<24) > #define CURSOR_FORMAT_SHIFT 24 > #define CURSOR_FORMAT_MASK (0x07 << CURSOR_FORMAT_SHIFT) > #define CURSOR_FORMAT_2C (0x00 << CURSOR_FORMAT_SHIFT) > @@ -3005,6 +3006,7 @@ > #define DISPPLANE_RGBA888 (0xf<<26) > #define DISPPLANE_STEREO_ENABLE (1<<25) > #define DISPPLANE_STEREO_DISABLE 0 > +#define DISPPLANE_PIPE_CSC_ENABLE (1<<24) > #define DISPPLANE_SEL_PIPE_SHIFT 24 > #define DISPPLANE_SEL_PIPE_MASK (3< #define DISPPLANE_SEL_PIPE_A 0 > @@ -3093,6 +3095,7 @@ > #define DVS_FORMAT_RGBX101010 (1<<25) > #define DVS_FORMAT_RGBX888 (2<<25) > #define DVS_FORMAT_RGBX161616 (3<<25) > +#define DVS_PIPE_CSC_ENABLE (1<<24) > #define DVS_SOURCE_KEY (1<<22) > #define DVS_RGB_ORDER_XBGR (1<<20) > #define DVS_YUV_BYTE_ORDER_MASK (3<<16) > @@ -3160,7 +3163,7 @@ > #define SPRITE_FORMAT_RGBX161616 (3<<25) > #define SPRITE_FORMAT_YUV444 (4<<25) > #define SPRITE_FORMAT_XR_BGR101010 (5<<25) /* Extended range */ > -#define SPRITE_CSC_ENABLE (1<<24) > +#define SPRITE_PIPE_CSC_ENABLE (1<<24) > #define SPRITE_SOURCE_KEY (1<<22) > #define SPRITE_RGB_ORDER_RGBX (1<<20) /* only for 888 a= nd 161616 */ > #define SPRITE_YUV_TO_RGB_CSC_DISABLE (1<<19) > @@ -4635,4 +4638,51 @@ > #define WM_DBG_DISALLOW_MAXFIFO (1<<1) > #define WM_DBG_DISALLOW_SPRITE (1<<2) > > +/* pipe CSC */ > +#define _PIPE_A_CSC_COEFF_RY_GY 0x49010 > +#define _PIPE_A_CSC_COEFF_BY 0x49014 > +#define _PIPE_A_CSC_COEFF_RU_GU 0x49018 > +#define _PIPE_A_CSC_COEFF_BU 0x4901c > +#define _PIPE_A_CSC_COEFF_RV_GV 0x49020 > +#define _PIPE_A_CSC_COEFF_BV 0x49024 > +#define _PIPE_A_CSC_MODE 0x49028 > +#define _PIPE_A_CSC_PREOFF_HI 0x49030 > +#define _PIPE_A_CSC_PREOFF_ME 0x49034 > +#define _PIPE_A_CSC_PREOFF_LO 0x49038 > +#define _PIPE_A_CSC_POSTOFF_HI 0x49040 > +#define _PIPE_A_CSC_POSTOFF_ME 0x49044 > +#define _PIPE_A_CSC_POSTOFF_LO 0x49048 > + > +#define _PIPE_B_CSC_COEFF_RY_GY 0x49110 > +#define _PIPE_B_CSC_COEFF_BY 0x49114 > +#define _PIPE_B_CSC_COEFF_RU_GU 0x49118 > +#define _PIPE_B_CSC_COEFF_BU 0x4911c > +#define _PIPE_B_CSC_COEFF_RV_GV 0x49120 > +#define _PIPE_B_CSC_COEFF_BV 0x49124 > +#define _PIPE_B_CSC_MODE 0x49128 > +#define _PIPE_B_CSC_PREOFF_HI 0x49130 > +#define _PIPE_B_CSC_PREOFF_ME 0x49134 > +#define _PIPE_B_CSC_PREOFF_LO 0x49138 > +#define _PIPE_B_CSC_POSTOFF_HI 0x49140 > +#define _PIPE_B_CSC_POSTOFF_ME 0x49144 > +#define _PIPE_B_CSC_POSTOFF_LO 0x49148 > + > +#define CSC_BLACK_SCREEN_OFFSET (1 << 2) > +#define CSC_POSITION_BEFORE_GAMMA (1 << 1) > +#define CSC_MODE_YUV_TO_RGB (1 << 0) > + > +#define PIPE_CSC_COEFF_RY_GY(pipe) _PIPE(pipe, _PIPE_A_CSC_COEFF_RY_GY, = _PIPE_B_CSC_COEFF_RY_GY) > +#define PIPE_CSC_COEFF_BY(pipe) _PIPE(pipe, _PIPE_A_CSC_COEFF_BY, _PIPE_= B_CSC_COEFF_BY) > +#define PIPE_CSC_COEFF_RU_GU(pipe) _PIPE(pipe, _PIPE_A_CSC_COEFF_RU_GU, = _PIPE_B_CSC_COEFF_RU_GU) > +#define PIPE_CSC_COEFF_BU(pipe) _PIPE(pipe, _PIPE_A_CSC_COEFF_BU, _PIPE_= B_CSC_COEFF_BU) > +#define PIPE_CSC_COEFF_RV_GV(pipe) _PIPE(pipe, _PIPE_A_CSC_COEFF_RV_GV, = _PIPE_B_CSC_COEFF_RV_GV) > +#define PIPE_CSC_COEFF_BV(pipe) _PIPE(pipe, _PIPE_A_CSC_COEFF_BV, _PIPE_= B_CSC_COEFF_BV) > +#define PIPE_CSC_MODE(pipe) _PIPE(pipe, _PIPE_A_CSC_MODE, _PIPE_B_CSC_MO= DE) > +#define PIPE_CSC_PREOFF_HI(pipe) _PIPE(pipe, _PIPE_A_CSC_PREOFF_HI, _PIP= E_B_CSC_PREOFF_HI) > +#define PIPE_CSC_PREOFF_ME(pipe) _PIPE(pipe, _PIPE_A_CSC_PREOFF_ME, _PIP= E_B_CSC_PREOFF_ME) > +#define PIPE_CSC_PREOFF_LO(pipe) _PIPE(pipe, _PIPE_A_CSC_PREOFF_LO, _PIP= E_B_CSC_PREOFF_LO) > +#define PIPE_CSC_POSTOFF_HI(pipe) _PIPE(pipe, _PIPE_A_CSC_POSTOFF_HI, _P= IPE_B_CSC_POSTOFF_HI) > +#define PIPE_CSC_POSTOFF_ME(pipe) _PIPE(pipe, _PIPE_A_CSC_POSTOFF_ME, _P= IPE_B_CSC_POSTOFF_ME) > +#define PIPE_CSC_POSTOFF_LO(pipe) _PIPE(pipe, _PIPE_A_CSC_POSTOFF_LO, _P= IPE_B_CSC_POSTOFF_LO) > + > #endif /* _I915_REG_H_ */ > diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/= intel_display.c > index f48f698..f22b0a8 100644 > --- a/drivers/gpu/drm/i915/intel_display.c > +++ b/drivers/gpu/drm/i915/intel_display.c > @@ -5106,6 +5106,71 @@ static void ironlake_set_pipeconf(struct drm_crtc = *crtc, > POSTING_READ(PIPECONF(pipe)); > } > > +/* > + * Set up the pipe CSC unit. > + * > + * Currently only full range RGB to limited range RGB conversion > + * is supported, but eventually this should handle various > + * RGB<->YCbCr scenarios as well. > + */ > +static void intel_set_pipe_csc(struct drm_crtc *crtc, > + const struct drm_display_mode *adjusted_mo= de) > +{ > + struct drm_device *dev =3D crtc->dev; > + struct drm_i915_private *dev_priv =3D dev->dev_private; > + struct intel_crtc *intel_crtc =3D to_intel_crtc(crtc); > + int pipe =3D intel_crtc->pipe; > + uint16_t coeff =3D 0x7800; /* 1.0 */ > + > + /* > + * TODO: Check what kind of values actually come out of the pipe > + * with these coeff/postoff values and adjust to get the best > + * accuracy. Perhaps we even need to take the bpc value into > + * consideration. > + */ > + > + if (adjusted_mode->private_flags & INTEL_MODE_LIMITED_COLOR_RANGE) > + coeff =3D ((235 - 16) * (1 << 12) / 255) & 0xff8; /* 0.xx= x... */ > + > + /* > + * GY/GU and RY/RU should be the other way around according > + * to BSpec, but reality doesn't agree. Just set them up in > + * a way that results in the correct picture. > + */ > + I915_WRITE(PIPE_CSC_COEFF_RY_GY(pipe), coeff << 16); > + I915_WRITE(PIPE_CSC_COEFF_BY(pipe), 0); > + > + I915_WRITE(PIPE_CSC_COEFF_RU_GU(pipe), coeff); > + I915_WRITE(PIPE_CSC_COEFF_BU(pipe), 0); > + > + I915_WRITE(PIPE_CSC_COEFF_RV_GV(pipe), 0); > + I915_WRITE(PIPE_CSC_COEFF_BV(pipe), coeff << 16); > + > + I915_WRITE(PIPE_CSC_PREOFF_HI(pipe), 0); > + I915_WRITE(PIPE_CSC_PREOFF_ME(pipe), 0); > + I915_WRITE(PIPE_CSC_PREOFF_LO(pipe), 0); > + > + if (INTEL_INFO(dev)->gen > 6) { > + uint16_t postoff =3D 0; > + > + if (adjusted_mode->private_flags & INTEL_MODE_LIMITED_COL= OR_RANGE) > + postoff =3D (16 * (1 << 13) / 255) & 0x1fff; > + > + I915_WRITE(PIPE_CSC_POSTOFF_HI(pipe), postoff); > + I915_WRITE(PIPE_CSC_POSTOFF_ME(pipe), postoff); > + I915_WRITE(PIPE_CSC_POSTOFF_LO(pipe), postoff); > + > + I915_WRITE(PIPE_CSC_MODE(pipe), 0); > + } else { > + uint32_t mode =3D CSC_MODE_YUV_TO_RGB; > + > + if (adjusted_mode->private_flags & INTEL_MODE_LIMITED_COL= OR_RANGE) > + mode |=3D CSC_BLACK_SCREEN_OFFSET; > + > + I915_WRITE(PIPE_CSC_MODE(pipe), mode); > + } > +} > + > static void haswell_set_pipeconf(struct drm_crtc *crtc, > struct drm_display_mode *adjusted_mode, > bool dither) > @@ -5670,8 +5735,10 @@ static int haswell_crtc_mode_set(struct drm_crtc *= crtc, > > haswell_set_pipeconf(crtc, adjusted_mode, dither); > > + intel_set_pipe_csc(crtc, adjusted_mode); > + > /* Set up the display plane register */ > - I915_WRITE(DSPCNTR(plane), DISPPLANE_GAMMA_ENABLE); > + I915_WRITE(DSPCNTR(plane), DISPPLANE_GAMMA_ENABLE | DISPPLANE_PIP= E_CSC_ENABLE); > POSTING_READ(DSPCNTR(plane)); > > ret =3D intel_pipe_set_base(crtc, x, y, fb); > @@ -6069,6 +6136,8 @@ static void ivb_update_cursor(struct drm_crtc *crtc= , u32 base) > cntl &=3D ~(CURSOR_MODE | MCURSOR_GAMMA_ENABLE); > cntl |=3D CURSOR_MODE_DISABLE; > } > + if (IS_HASWELL(dev)) > + cntl |=3D CURSOR_PIPE_CSC_ENABLE; > I915_WRITE(CURCNTR_IVB(pipe), cntl); > > intel_crtc->cursor_visible =3D visible; > diff --git a/drivers/gpu/drm/i915/intel_sprite.c b/drivers/gpu/drm/i915/i= ntel_sprite.c > index d7b060e..9dedf68 100644 > --- a/drivers/gpu/drm/i915/intel_sprite.c > +++ b/drivers/gpu/drm/i915/intel_sprite.c > @@ -89,6 +89,9 @@ ivb_update_plane(struct drm_plane *plane, struct drm_fr= amebuffer *fb, > sprctl |=3D SPRITE_TRICKLE_FEED_DISABLE; > sprctl |=3D SPRITE_ENABLE; > > + if (IS_HASWELL(dev)) > + sprctl |=3D SPRITE_PIPE_CSC_ENABLE; > + > /* Sizes are 0 based */ > src_w--; > src_h--; > -- > 1.7.8.6 > > _______________________________________________ > Intel-gfx mailing list > Intel-gfx@lists.freedesktop.org > http://lists.freedesktop.org/mailman/listinfo/intel-gfx -- = Paulo Zanoni