* [PATCH] drm/i915: Implement pipe CSC based limited range RGB output @ 2013-01-18 17:11 ville.syrjala 2013-01-18 17:37 ` Daniel Vetter ` (2 more replies) 0 siblings, 3 replies; 7+ messages in thread From: ville.syrjala @ 2013-01-18 17:11 UTC (permalink / raw) To: intel-gfx From: Ville Syrjälä <ville.syrjala@linux.intel.com> 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älä <ville.syrjala@linux.intel.com> --- 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 programmed in quite the same as on HSW, but the IVB unit _should_ be identical to HSW. 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<<DISPPLANE_SEL_PIPE_SHIFT) #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 and 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_MODE) +#define PIPE_CSC_PREOFF_HI(pipe) _PIPE(pipe, _PIPE_A_CSC_PREOFF_HI, _PIPE_B_CSC_PREOFF_HI) +#define PIPE_CSC_PREOFF_ME(pipe) _PIPE(pipe, _PIPE_A_CSC_PREOFF_ME, _PIPE_B_CSC_PREOFF_ME) +#define PIPE_CSC_PREOFF_LO(pipe) _PIPE(pipe, _PIPE_A_CSC_PREOFF_LO, _PIPE_B_CSC_PREOFF_LO) +#define PIPE_CSC_POSTOFF_HI(pipe) _PIPE(pipe, _PIPE_A_CSC_POSTOFF_HI, _PIPE_B_CSC_POSTOFF_HI) +#define PIPE_CSC_POSTOFF_ME(pipe) _PIPE(pipe, _PIPE_A_CSC_POSTOFF_ME, _PIPE_B_CSC_POSTOFF_ME) +#define PIPE_CSC_POSTOFF_LO(pipe) _PIPE(pipe, _PIPE_A_CSC_POSTOFF_LO, _PIPE_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_mode) +{ + struct drm_device *dev = crtc->dev; + struct drm_i915_private *dev_priv = dev->dev_private; + struct intel_crtc *intel_crtc = to_intel_crtc(crtc); + int pipe = intel_crtc->pipe; + uint16_t coeff = 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 = ((235 - 16) * (1 << 12) / 255) & 0xff8; /* 0.xxx... */ + + /* + * 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 = 0; + + if (adjusted_mode->private_flags & INTEL_MODE_LIMITED_COLOR_RANGE) + postoff = (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 = CSC_MODE_YUV_TO_RGB; + + if (adjusted_mode->private_flags & INTEL_MODE_LIMITED_COLOR_RANGE) + mode |= 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_PIPE_CSC_ENABLE); POSTING_READ(DSPCNTR(plane)); ret = intel_pipe_set_base(crtc, x, y, fb); @@ -6069,6 +6136,8 @@ static void ivb_update_cursor(struct drm_crtc *crtc, u32 base) cntl &= ~(CURSOR_MODE | MCURSOR_GAMMA_ENABLE); cntl |= CURSOR_MODE_DISABLE; } + if (IS_HASWELL(dev)) + cntl |= CURSOR_PIPE_CSC_ENABLE; I915_WRITE(CURCNTR_IVB(pipe), cntl); intel_crtc->cursor_visible = visible; diff --git a/drivers/gpu/drm/i915/intel_sprite.c b/drivers/gpu/drm/i915/intel_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_framebuffer *fb, sprctl |= SPRITE_TRICKLE_FEED_DISABLE; sprctl |= SPRITE_ENABLE; + if (IS_HASWELL(dev)) + sprctl |= 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 ^ permalink raw reply related [flat|nested] 7+ messages in thread
* Re: [PATCH] drm/i915: Implement pipe CSC based limited range RGB output 2013-01-18 17:11 [PATCH] drm/i915: Implement pipe CSC based limited range RGB output ville.syrjala @ 2013-01-18 17:37 ` Daniel Vetter 2013-01-18 18:08 ` Ville Syrjälä 2013-01-21 14:43 ` Ville Syrjälä 2013-02-08 20:43 ` Ville Syrjälä 2013-02-14 19:30 ` Paulo Zanoni 2 siblings, 2 replies; 7+ messages in thread From: Daniel Vetter @ 2013-01-18 17:37 UTC (permalink / raw) To: ville.syrjala; +Cc: intel-gfx On Fri, Jan 18, 2013 at 6:11 PM, <ville.syrjala@linux.intel.com> wrote: > From: Ville Syrjälä <ville.syrjala@linux.intel.com> > > 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älä <ville.syrjala@linux.intel.com> > --- > 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 programmed > in quite the same as on HSW, but the IVB unit _should_ be identical to HSW. > > 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. Two questions from the clueless: - Since there's some question whether the CSC blows through additional power: Can't we just enable it only when we need it? It looks like updating the broadcast property requires a full modeset anyway, so we don't gain anything by keeping the csc on when not required. - I might just be mislead by the 256/219 values, but: How does this work for non-8bpc pipe configurations? Or does the CSC thing not care and only scale the normalized 0.0 - 1.0 range? Cheers, Daniel > > 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<<DISPPLANE_SEL_PIPE_SHIFT) > #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 and 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_MODE) > +#define PIPE_CSC_PREOFF_HI(pipe) _PIPE(pipe, _PIPE_A_CSC_PREOFF_HI, _PIPE_B_CSC_PREOFF_HI) > +#define PIPE_CSC_PREOFF_ME(pipe) _PIPE(pipe, _PIPE_A_CSC_PREOFF_ME, _PIPE_B_CSC_PREOFF_ME) > +#define PIPE_CSC_PREOFF_LO(pipe) _PIPE(pipe, _PIPE_A_CSC_PREOFF_LO, _PIPE_B_CSC_PREOFF_LO) > +#define PIPE_CSC_POSTOFF_HI(pipe) _PIPE(pipe, _PIPE_A_CSC_POSTOFF_HI, _PIPE_B_CSC_POSTOFF_HI) > +#define PIPE_CSC_POSTOFF_ME(pipe) _PIPE(pipe, _PIPE_A_CSC_POSTOFF_ME, _PIPE_B_CSC_POSTOFF_ME) > +#define PIPE_CSC_POSTOFF_LO(pipe) _PIPE(pipe, _PIPE_A_CSC_POSTOFF_LO, _PIPE_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_mode) > +{ > + struct drm_device *dev = crtc->dev; > + struct drm_i915_private *dev_priv = dev->dev_private; > + struct intel_crtc *intel_crtc = to_intel_crtc(crtc); > + int pipe = intel_crtc->pipe; > + uint16_t coeff = 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 = ((235 - 16) * (1 << 12) / 255) & 0xff8; /* 0.xxx... */ > + > + /* > + * 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 = 0; > + > + if (adjusted_mode->private_flags & INTEL_MODE_LIMITED_COLOR_RANGE) > + postoff = (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 = CSC_MODE_YUV_TO_RGB; > + > + if (adjusted_mode->private_flags & INTEL_MODE_LIMITED_COLOR_RANGE) > + mode |= 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_PIPE_CSC_ENABLE); > POSTING_READ(DSPCNTR(plane)); > > ret = intel_pipe_set_base(crtc, x, y, fb); > @@ -6069,6 +6136,8 @@ static void ivb_update_cursor(struct drm_crtc *crtc, u32 base) > cntl &= ~(CURSOR_MODE | MCURSOR_GAMMA_ENABLE); > cntl |= CURSOR_MODE_DISABLE; > } > + if (IS_HASWELL(dev)) > + cntl |= CURSOR_PIPE_CSC_ENABLE; > I915_WRITE(CURCNTR_IVB(pipe), cntl); > > intel_crtc->cursor_visible = visible; > diff --git a/drivers/gpu/drm/i915/intel_sprite.c b/drivers/gpu/drm/i915/intel_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_framebuffer *fb, > sprctl |= SPRITE_TRICKLE_FEED_DISABLE; > sprctl |= SPRITE_ENABLE; > > + if (IS_HASWELL(dev)) > + sprctl |= 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 -- Daniel Vetter Software Engineer, Intel Corporation +41 (0) 79 365 57 48 - http://blog.ffwll.ch ^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [PATCH] drm/i915: Implement pipe CSC based limited range RGB output 2013-01-18 17:37 ` Daniel Vetter @ 2013-01-18 18:08 ` Ville Syrjälä 2013-01-21 14:43 ` Ville Syrjälä 1 sibling, 0 replies; 7+ messages in thread From: Ville Syrjälä @ 2013-01-18 18:08 UTC (permalink / raw) To: Daniel Vetter; +Cc: intel-gfx On Fri, Jan 18, 2013 at 06:37:09PM +0100, Daniel Vetter wrote: > On Fri, Jan 18, 2013 at 6:11 PM, <ville.syrjala@linux.intel.com> wrote: > > From: Ville Syrjälä <ville.syrjala@linux.intel.com> > > > > 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älä <ville.syrjala@linux.intel.com> > > --- > > 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 programmed > > in quite the same as on HSW, but the IVB unit _should_ be identical to HSW. > > > > 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. > > Two questions from the clueless: > - Since there's some question whether the CSC blows through additional > power: Can't we just enable it only when we need it? It looks like > updating the broadcast property requires a full modeset anyway, so we > don't gain anything by keeping the csc on when not required. We could, but it's a bit more work since we need to look at the adjusted_mode private_flags. So we need to pass that somehow to all plane code. For cursors we could pass adjusted_mode from the mode_set path, and hwmode from the cursor ioctls. And since sprites aren't currently integrated into the mode_set sequence at all, we could just look at hwmode there. So I suppose it's doable. Or we could just set the bit for all planes on the pipe in mode_set, the same way as it's done for DSPCNTR, and then hope all other code uses RMW when changing the CNTR regs. > - I might just be mislead by the 256/219 values, but: How does this > work for non-8bpc pipe configurations? Or does the CSC thing not care > and only scale the normalized 0.0 - 1.0 range? Yeah, it uses normalized numbers. But as stated in the comment I don't really know what the exact arithmetic inside the HW is, so I can't be sure if the values I've calculated are the best ones. We might be slighly over/undershooting the limits. > Cheers, Daniel > > > > > 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<<DISPPLANE_SEL_PIPE_SHIFT) > > #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 and 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_MODE) > > +#define PIPE_CSC_PREOFF_HI(pipe) _PIPE(pipe, _PIPE_A_CSC_PREOFF_HI, _PIPE_B_CSC_PREOFF_HI) > > +#define PIPE_CSC_PREOFF_ME(pipe) _PIPE(pipe, _PIPE_A_CSC_PREOFF_ME, _PIPE_B_CSC_PREOFF_ME) > > +#define PIPE_CSC_PREOFF_LO(pipe) _PIPE(pipe, _PIPE_A_CSC_PREOFF_LO, _PIPE_B_CSC_PREOFF_LO) > > +#define PIPE_CSC_POSTOFF_HI(pipe) _PIPE(pipe, _PIPE_A_CSC_POSTOFF_HI, _PIPE_B_CSC_POSTOFF_HI) > > +#define PIPE_CSC_POSTOFF_ME(pipe) _PIPE(pipe, _PIPE_A_CSC_POSTOFF_ME, _PIPE_B_CSC_POSTOFF_ME) > > +#define PIPE_CSC_POSTOFF_LO(pipe) _PIPE(pipe, _PIPE_A_CSC_POSTOFF_LO, _PIPE_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_mode) > > +{ > > + struct drm_device *dev = crtc->dev; > > + struct drm_i915_private *dev_priv = dev->dev_private; > > + struct intel_crtc *intel_crtc = to_intel_crtc(crtc); > > + int pipe = intel_crtc->pipe; > > + uint16_t coeff = 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 = ((235 - 16) * (1 << 12) / 255) & 0xff8; /* 0.xxx... */ > > + > > + /* > > + * 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 = 0; > > + > > + if (adjusted_mode->private_flags & INTEL_MODE_LIMITED_COLOR_RANGE) > > + postoff = (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 = CSC_MODE_YUV_TO_RGB; > > + > > + if (adjusted_mode->private_flags & INTEL_MODE_LIMITED_COLOR_RANGE) > > + mode |= 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_PIPE_CSC_ENABLE); > > POSTING_READ(DSPCNTR(plane)); > > > > ret = intel_pipe_set_base(crtc, x, y, fb); > > @@ -6069,6 +6136,8 @@ static void ivb_update_cursor(struct drm_crtc *crtc, u32 base) > > cntl &= ~(CURSOR_MODE | MCURSOR_GAMMA_ENABLE); > > cntl |= CURSOR_MODE_DISABLE; > > } > > + if (IS_HASWELL(dev)) > > + cntl |= CURSOR_PIPE_CSC_ENABLE; > > I915_WRITE(CURCNTR_IVB(pipe), cntl); > > > > intel_crtc->cursor_visible = visible; > > diff --git a/drivers/gpu/drm/i915/intel_sprite.c b/drivers/gpu/drm/i915/intel_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_framebuffer *fb, > > sprctl |= SPRITE_TRICKLE_FEED_DISABLE; > > sprctl |= SPRITE_ENABLE; > > > > + if (IS_HASWELL(dev)) > > + sprctl |= 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 > > > > -- > Daniel Vetter > Software Engineer, Intel Corporation > +41 (0) 79 365 57 48 - http://blog.ffwll.ch -- Ville Syrjälä Intel OTC ^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [PATCH] drm/i915: Implement pipe CSC based limited range RGB output 2013-01-18 17:37 ` Daniel Vetter 2013-01-18 18:08 ` Ville Syrjälä @ 2013-01-21 14:43 ` Ville Syrjälä 1 sibling, 0 replies; 7+ messages in thread From: Ville Syrjälä @ 2013-01-21 14:43 UTC (permalink / raw) To: Daniel Vetter; +Cc: intel-gfx On Fri, Jan 18, 2013 at 06:37:09PM +0100, Daniel Vetter wrote: > On Fri, Jan 18, 2013 at 6:11 PM, <ville.syrjala@linux.intel.com> wrote: > > From: Ville Syrjälä <ville.syrjala@linux.intel.com> > > > > 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älä <ville.syrjala@linux.intel.com> > > --- > > 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 programmed > > in quite the same as on HSW, but the IVB unit _should_ be identical to HSW. > > > > 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. > > Two questions from the clueless: > - Since there's some question whether the CSC blows through additional > power: Can't we just enable it only when we need it? It looks like > updating the broadcast property requires a full modeset anyway, so we > don't gain anything by keeping the csc on when not required. BTW I did a few quick tests on my ILK laptop. There is no difference in ACPI power numbers from pipe CSC usage, so it seems any increase in power consumption is so small that ACPI can't measure it. So I'd say there's no point in worrying about it. Well, assuming the hardware didn't change too much for HSW. -- Ville Syrjälä Intel OTC ^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [PATCH] drm/i915: Implement pipe CSC based limited range RGB output 2013-01-18 17:11 [PATCH] drm/i915: Implement pipe CSC based limited range RGB output ville.syrjala 2013-01-18 17:37 ` Daniel Vetter @ 2013-02-08 20:43 ` Ville Syrjälä 2013-02-14 19:30 ` Paulo Zanoni 2 siblings, 0 replies; 7+ messages in thread From: Ville Syrjälä @ 2013-02-08 20:43 UTC (permalink / raw) To: intel-gfx On Fri, Jan 18, 2013 at 07:11:38PM +0200, ville.syrjala@linux.intel.com wrote: > From: Ville Syrjälä <ville.syrjala@linux.intel.com> > > 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älä <ville.syrjala@linux.intel.com> > --- > 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 programmed > in quite the same as on HSW, but the IVB unit _should_ be identical to HSW. > > 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. FYI I just tested this on a HSW box, and all the colors look correct. I also verified that the pipeconf color range bit is well and truly gone (hw won't allow you to even set it). So Daniel, I think this can go in as is. -- Ville Syrjälä Intel OTC ^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [PATCH] drm/i915: Implement pipe CSC based limited range RGB output 2013-01-18 17:11 [PATCH] drm/i915: Implement pipe CSC based limited range RGB output ville.syrjala 2013-01-18 17:37 ` Daniel Vetter 2013-02-08 20:43 ` Ville Syrjälä @ 2013-02-14 19:30 ` Paulo Zanoni 2013-02-15 20:34 ` Daniel Vetter 2 siblings, 1 reply; 7+ messages in thread From: Paulo Zanoni @ 2013-02-14 19:30 UTC (permalink / raw) To: ville.syrjala; +Cc: intel-gfx Hi 2013/1/18 <ville.syrjala@linux.intel.com>: > From: Ville Syrjälä <ville.syrjala@linux.intel.com> > > 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älä <ville.syrjala@linux.intel.com> Reviewed-by: Paulo Zanoni <paulo.r.zanoni@intel.com> > --- > 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 programmed > in quite the same as on HSW, but the IVB unit _should_ be identical to HSW. > > 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<<DISPPLANE_SEL_PIPE_SHIFT) > #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 and 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_MODE) > +#define PIPE_CSC_PREOFF_HI(pipe) _PIPE(pipe, _PIPE_A_CSC_PREOFF_HI, _PIPE_B_CSC_PREOFF_HI) > +#define PIPE_CSC_PREOFF_ME(pipe) _PIPE(pipe, _PIPE_A_CSC_PREOFF_ME, _PIPE_B_CSC_PREOFF_ME) > +#define PIPE_CSC_PREOFF_LO(pipe) _PIPE(pipe, _PIPE_A_CSC_PREOFF_LO, _PIPE_B_CSC_PREOFF_LO) > +#define PIPE_CSC_POSTOFF_HI(pipe) _PIPE(pipe, _PIPE_A_CSC_POSTOFF_HI, _PIPE_B_CSC_POSTOFF_HI) > +#define PIPE_CSC_POSTOFF_ME(pipe) _PIPE(pipe, _PIPE_A_CSC_POSTOFF_ME, _PIPE_B_CSC_POSTOFF_ME) > +#define PIPE_CSC_POSTOFF_LO(pipe) _PIPE(pipe, _PIPE_A_CSC_POSTOFF_LO, _PIPE_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_mode) > +{ > + struct drm_device *dev = crtc->dev; > + struct drm_i915_private *dev_priv = dev->dev_private; > + struct intel_crtc *intel_crtc = to_intel_crtc(crtc); > + int pipe = intel_crtc->pipe; > + uint16_t coeff = 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 = ((235 - 16) * (1 << 12) / 255) & 0xff8; /* 0.xxx... */ > + > + /* > + * 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 = 0; > + > + if (adjusted_mode->private_flags & INTEL_MODE_LIMITED_COLOR_RANGE) > + postoff = (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 = CSC_MODE_YUV_TO_RGB; > + > + if (adjusted_mode->private_flags & INTEL_MODE_LIMITED_COLOR_RANGE) > + mode |= 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_PIPE_CSC_ENABLE); > POSTING_READ(DSPCNTR(plane)); > > ret = intel_pipe_set_base(crtc, x, y, fb); > @@ -6069,6 +6136,8 @@ static void ivb_update_cursor(struct drm_crtc *crtc, u32 base) > cntl &= ~(CURSOR_MODE | MCURSOR_GAMMA_ENABLE); > cntl |= CURSOR_MODE_DISABLE; > } > + if (IS_HASWELL(dev)) > + cntl |= CURSOR_PIPE_CSC_ENABLE; > I915_WRITE(CURCNTR_IVB(pipe), cntl); > > intel_crtc->cursor_visible = visible; > diff --git a/drivers/gpu/drm/i915/intel_sprite.c b/drivers/gpu/drm/i915/intel_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_framebuffer *fb, > sprctl |= SPRITE_TRICKLE_FEED_DISABLE; > sprctl |= SPRITE_ENABLE; > > + if (IS_HASWELL(dev)) > + sprctl |= 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 ^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [PATCH] drm/i915: Implement pipe CSC based limited range RGB output 2013-02-14 19:30 ` Paulo Zanoni @ 2013-02-15 20:34 ` Daniel Vetter 0 siblings, 0 replies; 7+ messages in thread From: Daniel Vetter @ 2013-02-15 20:34 UTC (permalink / raw) To: Paulo Zanoni; +Cc: intel-gfx On Thu, Feb 14, 2013 at 05:30:12PM -0200, Paulo Zanoni wrote: > Hi > > 2013/1/18 <ville.syrjala@linux.intel.com>: > > From: Ville Syrjälä <ville.syrjala@linux.intel.com> > > > > 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älä <ville.syrjala@linux.intel.com> > > Reviewed-by: Paulo Zanoni <paulo.r.zanoni@intel.com> Queued for -next, thanks for the patch. -Daniel -- Daniel Vetter Software Engineer, Intel Corporation +41 (0) 79 365 57 48 - http://blog.ffwll.ch ^ permalink raw reply [flat|nested] 7+ messages in thread
end of thread, other threads:[~2013-02-15 20:32 UTC | newest] Thread overview: 7+ messages (download: mbox.gz / follow: Atom feed) -- links below jump to the message on this page -- 2013-01-18 17:11 [PATCH] drm/i915: Implement pipe CSC based limited range RGB output ville.syrjala 2013-01-18 17:37 ` Daniel Vetter 2013-01-18 18:08 ` Ville Syrjälä 2013-01-21 14:43 ` Ville Syrjälä 2013-02-08 20:43 ` Ville Syrjälä 2013-02-14 19:30 ` Paulo Zanoni 2013-02-15 20:34 ` Daniel Vetter
This is an external index of several public inboxes, see mirroring instructions on how to clone and mirror all data and code used by this external index.