| Message ID | 20170608203315.21196-2-ville.syrjala@linux.intel.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
Regards Shashank On 6/9/2017 2:03 AM, ville.syrjala@linux.intel.com wrote: > From: Ville Syrjälä <ville.syrjala@linux.intel.com> > > 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. > > v2: Rebase > > Cc: Jyri Sarha <jsarha@ti.com> > Cc: "Tang, Jun" <jun.tang@intel.com> > Reported-by: "Tang, Jun" <jun.tang@intel.com> > Cc: stable@vger.kernel.org > Fixes: 7f1f3851feb0 ("drm/i915: sprite support for ValleyView v4") > Signed-off-by: Ville Syrjälä <ville.syrjala@linux.intel.com> > --- > drivers/gpu/drm/i915/i915_reg.h | 10 +++++ > drivers/gpu/drm/i915/intel_sprite.c | 74 ++++++++++++++++++++++++++++--------- > 2 files changed, 66 insertions(+), 18 deletions(-) > > diff --git a/drivers/gpu/drm/i915/i915_reg.h b/drivers/gpu/drm/i915/i915_reg.h > index b6d69e289974..ce7c0dc79cf7 100644 > --- a/drivers/gpu/drm/i915/i915_reg.h > +++ b/drivers/gpu/drm/i915/i915_reg.h > @@ -5777,6 +5777,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) > @@ -5790,6 +5796,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 _MMIO_VLV_SPR(pipe, plane_id, reg_a, reg_b) \ > @@ -5806,6 +5814,8 @@ enum { > #define SPKEYMAXVAL(pipe, plane_id) _MMIO_VLV_SPR((pipe), (plane_id), _SPAKEYMAXVAL, _SPBKEYMAXVAL) > #define SPTILEOFF(pipe, plane_id) _MMIO_VLV_SPR((pipe), (plane_id), _SPATILEOFF, _SPBTILEOFF) > #define SPCONSTALPHA(pipe, plane_id) _MMIO_VLV_SPR((pipe), (plane_id), _SPACONSTALPHA, _SPBCONSTALPHA) > +#define SPCLRC0(pipe, plane_id) _MMIO_VLV_SPR((pipe), (plane_id), _SPACLRC0, _SPBCLRC0) > +#define SPCLRC1(pipe, plane_id) _MMIO_VLV_SPR((pipe), (plane_id), _SPACLRC1, _SPBCLRC1) > #define SPGAMC(pipe, plane_id) _MMIO_VLV_SPR((pipe), (plane_id), _SPAGAMC, _SPBGAMC) > > /* > diff --git a/drivers/gpu/drm/i915/intel_sprite.c b/drivers/gpu/drm/i915/intel_sprite.c > index 0c650c2cbca8..2ce3b3c6ffa8 100644 > --- a/drivers/gpu/drm/i915/intel_sprite.c > +++ b/drivers/gpu/drm/i915/intel_sprite.c > @@ -325,44 +325,80 @@ skl_disable_plane(struct intel_plane *plane, struct intel_crtc *crtc) > } > > static void > -chv_update_csc(struct intel_plane *plane, uint32_t format) > +chv_update_csc(const struct intel_plane_state *plane_state) > { > + struct intel_plane *plane = to_intel_plane(plane_state->base.plane); > 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; > > /* Seems RGB data bypasses the CSC always */ > - if (!format_is_yuv(format)) > + if (!format_is_yuv(fb->format->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_FW(SPCSCYGOFF(plane_id), SPCSC_OOFF(0) | SPCSC_IOFF(-64)); > + 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(4769) | SPCSC_C0(6537)); > - I915_WRITE_FW(SPCSCC23(plane_id), SPCSC_C1(-3330) | SPCSC_C0(0)); > - I915_WRITE_FW(SPCSCC45(plane_id), SPCSC_C1(-1605) | SPCSC_C0(4769)); > - I915_WRITE_FW(SPCSCC67(plane_id), SPCSC_C1(4769) | SPCSC_C0(0)); > - I915_WRITE_FW(SPCSCC8(plane_id), SPCSC_C0(8263)); > + 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(SPCSCYGICLAMP(plane_id), SPCSC_IMAX(940) | SPCSC_IMIN(64)); > - I915_WRITE_FW(SPCSCCBICLAMP(plane_id), SPCSC_IMAX(448) | SPCSC_IMIN(-448)); > - I915_WRITE_FW(SPCSCCRICLAMP(plane_id), SPCSC_IMAX(448) | SPCSC_IMIN(-448)); > + I915_WRITE_FW(SPCSCYGICLAMP(plane_id), SPCSC_IMAX(1023) | SPCSC_IMIN(0)); > + I915_WRITE_FW(SPCSCCBICLAMP(plane_id), SPCSC_IMAX(512) | SPCSC_IMIN(-512)); > + I915_WRITE_FW(SPCSCCRICLAMP(plane_id), SPCSC_IMAX(512) | SPCSC_IMIN(-512)); > > I915_WRITE_FW(SPCSCYGOCLAMP(plane_id), SPCSC_OMAX(1023) | SPCSC_OMIN(0)); > I915_WRITE_FW(SPCSCCBOCLAMP(plane_id), SPCSC_OMAX(1023) | SPCSC_OMIN(0)); > I915_WRITE_FW(SPCSCCROCLAMP(plane_id), SPCSC_OMAX(1023) | SPCSC_OMIN(0)); > } > > +static void > +vlv_update_clrc(const struct intel_plane_state *plane_state) > +{ > + struct intel_plane *plane = to_intel_plane(plane_state->base.plane); > + struct drm_i915_private *dev_priv = to_i915(plane->base.dev); > + const struct drm_framebuffer *fb = plane_state->base.fb; > + enum pipe pipe = plane->pipe; > + enum plane_id plane_id = plane->id; > + int con, bri, sh_sin, sh_cos; > + con = contrast bri = brightness for better reading ? > + if (format_is_yuv(fb->format->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; > + } contrast and brightness are mostly used for color / display correction. Would it be better to create a plane level property for the same, and attach into color management framework ? In this way, userspace can also play around. - Shashank > + > + /* FIXME these register are single buffered :( */ > + I915_WRITE_FW(SPCLRC0(pipe, plane_id), > + SP_CONTRAST(con) | SP_BRIGHTNESS(bri)); > + I915_WRITE_FW(SPCLRC1(pipe, plane_id), > + SP_SH_SIN(sh_sin) | SP_SH_COS(sh_cos)); > +} > + > static u32 vlv_sprite_ctl(const struct intel_crtc_state *crtc_state, > const struct intel_plane_state *plane_state) > { > @@ -456,8 +492,10 @@ vlv_update_plane(struct intel_plane *plane, > > spin_lock_irqsave(&dev_priv->uncore.lock, irqflags); > > + vlv_update_clrc(plane_state); > + > if (IS_CHERRYVIEW(dev_priv) && pipe == PIPE_B) > - chv_update_csc(plane, fb->format->format); > + chv_update_csc(plane_state); > > if (key->flags) { > I915_WRITE_FW(SPKEYMINVAL(pipe, plane_id), key->min_value);
On Thu, Jun 15, 2017 at 06:08:43PM +0530, Sharma, Shashank wrote: > Regards > > Shashank > > > On 6/9/2017 2:03 AM, ville.syrjala@linux.intel.com wrote: > > From: Ville Syrjälä <ville.syrjala@linux.intel.com> > > > > 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. > > > > v2: Rebase > > > > Cc: Jyri Sarha <jsarha@ti.com> > > Cc: "Tang, Jun" <jun.tang@intel.com> > > Reported-by: "Tang, Jun" <jun.tang@intel.com> > > Cc: stable@vger.kernel.org > > Fixes: 7f1f3851feb0 ("drm/i915: sprite support for ValleyView v4") > > Signed-off-by: Ville Syrjälä <ville.syrjala@linux.intel.com> > > --- > > drivers/gpu/drm/i915/i915_reg.h | 10 +++++ > > drivers/gpu/drm/i915/intel_sprite.c | 74 ++++++++++++++++++++++++++++--------- > > 2 files changed, 66 insertions(+), 18 deletions(-) > > > > diff --git a/drivers/gpu/drm/i915/i915_reg.h b/drivers/gpu/drm/i915/i915_reg.h > > index b6d69e289974..ce7c0dc79cf7 100644 > > --- a/drivers/gpu/drm/i915/i915_reg.h > > +++ b/drivers/gpu/drm/i915/i915_reg.h > > @@ -5777,6 +5777,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) > > @@ -5790,6 +5796,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 _MMIO_VLV_SPR(pipe, plane_id, reg_a, reg_b) \ > > @@ -5806,6 +5814,8 @@ enum { > > #define SPKEYMAXVAL(pipe, plane_id) _MMIO_VLV_SPR((pipe), (plane_id), _SPAKEYMAXVAL, _SPBKEYMAXVAL) > > #define SPTILEOFF(pipe, plane_id) _MMIO_VLV_SPR((pipe), (plane_id), _SPATILEOFF, _SPBTILEOFF) > > #define SPCONSTALPHA(pipe, plane_id) _MMIO_VLV_SPR((pipe), (plane_id), _SPACONSTALPHA, _SPBCONSTALPHA) > > +#define SPCLRC0(pipe, plane_id) _MMIO_VLV_SPR((pipe), (plane_id), _SPACLRC0, _SPBCLRC0) > > +#define SPCLRC1(pipe, plane_id) _MMIO_VLV_SPR((pipe), (plane_id), _SPACLRC1, _SPBCLRC1) > > #define SPGAMC(pipe, plane_id) _MMIO_VLV_SPR((pipe), (plane_id), _SPAGAMC, _SPBGAMC) > > > > /* > > diff --git a/drivers/gpu/drm/i915/intel_sprite.c b/drivers/gpu/drm/i915/intel_sprite.c > > index 0c650c2cbca8..2ce3b3c6ffa8 100644 > > --- a/drivers/gpu/drm/i915/intel_sprite.c > > +++ b/drivers/gpu/drm/i915/intel_sprite.c > > @@ -325,44 +325,80 @@ skl_disable_plane(struct intel_plane *plane, struct intel_crtc *crtc) > > } > > > > static void > > -chv_update_csc(struct intel_plane *plane, uint32_t format) > > +chv_update_csc(const struct intel_plane_state *plane_state) > > { > > + struct intel_plane *plane = to_intel_plane(plane_state->base.plane); > > 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; > > > > /* Seems RGB data bypasses the CSC always */ > > - if (!format_is_yuv(format)) > > + if (!format_is_yuv(fb->format->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_FW(SPCSCYGOFF(plane_id), SPCSC_OOFF(0) | SPCSC_IOFF(-64)); > > + 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(4769) | SPCSC_C0(6537)); > > - I915_WRITE_FW(SPCSCC23(plane_id), SPCSC_C1(-3330) | SPCSC_C0(0)); > > - I915_WRITE_FW(SPCSCC45(plane_id), SPCSC_C1(-1605) | SPCSC_C0(4769)); > > - I915_WRITE_FW(SPCSCC67(plane_id), SPCSC_C1(4769) | SPCSC_C0(0)); > > - I915_WRITE_FW(SPCSCC8(plane_id), SPCSC_C0(8263)); > > + 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(SPCSCYGICLAMP(plane_id), SPCSC_IMAX(940) | SPCSC_IMIN(64)); > > - I915_WRITE_FW(SPCSCCBICLAMP(plane_id), SPCSC_IMAX(448) | SPCSC_IMIN(-448)); > > - I915_WRITE_FW(SPCSCCRICLAMP(plane_id), SPCSC_IMAX(448) | SPCSC_IMIN(-448)); > > + I915_WRITE_FW(SPCSCYGICLAMP(plane_id), SPCSC_IMAX(1023) | SPCSC_IMIN(0)); > > + I915_WRITE_FW(SPCSCCBICLAMP(plane_id), SPCSC_IMAX(512) | SPCSC_IMIN(-512)); > > + I915_WRITE_FW(SPCSCCRICLAMP(plane_id), SPCSC_IMAX(512) | SPCSC_IMIN(-512)); > > > > I915_WRITE_FW(SPCSCYGOCLAMP(plane_id), SPCSC_OMAX(1023) | SPCSC_OMIN(0)); > > I915_WRITE_FW(SPCSCCBOCLAMP(plane_id), SPCSC_OMAX(1023) | SPCSC_OMIN(0)); > > I915_WRITE_FW(SPCSCCROCLAMP(plane_id), SPCSC_OMAX(1023) | SPCSC_OMIN(0)); > > } > > > > +static void > > +vlv_update_clrc(const struct intel_plane_state *plane_state) > > +{ > > + struct intel_plane *plane = to_intel_plane(plane_state->base.plane); > > + struct drm_i915_private *dev_priv = to_i915(plane->base.dev); > > + const struct drm_framebuffer *fb = plane_state->base.fb; > > + enum pipe pipe = plane->pipe; > > + enum plane_id plane_id = plane->id; > > + int con, bri, sh_sin, sh_cos; > > + > con = contrast bri = brightness for better reading ? Sure, why not. > > + if (format_is_yuv(fb->format->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; > > + } > contrast and brightness are mostly used for color / display correction. > Would it be better to create a > plane level property for the same, and attach into color management > framework ? In this way, userspace > can also play around. I did have such plans long ago, but I'm not sure there's much point in doing that since we would only be able to support these propeerties on a very limited subset of planes on a very limited subset of platforms. > > - Shashank > > + > > + /* FIXME these register are single buffered :( */ > > + I915_WRITE_FW(SPCLRC0(pipe, plane_id), > > + SP_CONTRAST(con) | SP_BRIGHTNESS(bri)); > > + I915_WRITE_FW(SPCLRC1(pipe, plane_id), > > + SP_SH_SIN(sh_sin) | SP_SH_COS(sh_cos)); > > +} > > + > > static u32 vlv_sprite_ctl(const struct intel_crtc_state *crtc_state, > > const struct intel_plane_state *plane_state) > > { > > @@ -456,8 +492,10 @@ vlv_update_plane(struct intel_plane *plane, > > > > spin_lock_irqsave(&dev_priv->uncore.lock, irqflags); > > > > + vlv_update_clrc(plane_state); > > + > > if (IS_CHERRYVIEW(dev_priv) && pipe == PIPE_B) > > - chv_update_csc(plane, fb->format->format); > > + chv_update_csc(plane_state); > > > > if (key->flags) { > > I915_WRITE_FW(SPKEYMINVAL(pipe, plane_id), key->min_value);
diff --git a/drivers/gpu/drm/i915/i915_reg.h b/drivers/gpu/drm/i915/i915_reg.h index b6d69e289974..ce7c0dc79cf7 100644 --- a/drivers/gpu/drm/i915/i915_reg.h +++ b/drivers/gpu/drm/i915/i915_reg.h @@ -5777,6 +5777,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) @@ -5790,6 +5796,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 _MMIO_VLV_SPR(pipe, plane_id, reg_a, reg_b) \ @@ -5806,6 +5814,8 @@ enum { #define SPKEYMAXVAL(pipe, plane_id) _MMIO_VLV_SPR((pipe), (plane_id), _SPAKEYMAXVAL, _SPBKEYMAXVAL) #define SPTILEOFF(pipe, plane_id) _MMIO_VLV_SPR((pipe), (plane_id), _SPATILEOFF, _SPBTILEOFF) #define SPCONSTALPHA(pipe, plane_id) _MMIO_VLV_SPR((pipe), (plane_id), _SPACONSTALPHA, _SPBCONSTALPHA) +#define SPCLRC0(pipe, plane_id) _MMIO_VLV_SPR((pipe), (plane_id), _SPACLRC0, _SPBCLRC0) +#define SPCLRC1(pipe, plane_id) _MMIO_VLV_SPR((pipe), (plane_id), _SPACLRC1, _SPBCLRC1) #define SPGAMC(pipe, plane_id) _MMIO_VLV_SPR((pipe), (plane_id), _SPAGAMC, _SPBGAMC) /* diff --git a/drivers/gpu/drm/i915/intel_sprite.c b/drivers/gpu/drm/i915/intel_sprite.c index 0c650c2cbca8..2ce3b3c6ffa8 100644 --- a/drivers/gpu/drm/i915/intel_sprite.c +++ b/drivers/gpu/drm/i915/intel_sprite.c @@ -325,44 +325,80 @@ skl_disable_plane(struct intel_plane *plane, struct intel_crtc *crtc) } static void -chv_update_csc(struct intel_plane *plane, uint32_t format) +chv_update_csc(const struct intel_plane_state *plane_state) { + struct intel_plane *plane = to_intel_plane(plane_state->base.plane); 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; /* Seems RGB data bypasses the CSC always */ - if (!format_is_yuv(format)) + if (!format_is_yuv(fb->format->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_FW(SPCSCYGOFF(plane_id), SPCSC_OOFF(0) | SPCSC_IOFF(-64)); + 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(4769) | SPCSC_C0(6537)); - I915_WRITE_FW(SPCSCC23(plane_id), SPCSC_C1(-3330) | SPCSC_C0(0)); - I915_WRITE_FW(SPCSCC45(plane_id), SPCSC_C1(-1605) | SPCSC_C0(4769)); - I915_WRITE_FW(SPCSCC67(plane_id), SPCSC_C1(4769) | SPCSC_C0(0)); - I915_WRITE_FW(SPCSCC8(plane_id), SPCSC_C0(8263)); + 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(SPCSCYGICLAMP(plane_id), SPCSC_IMAX(940) | SPCSC_IMIN(64)); - I915_WRITE_FW(SPCSCCBICLAMP(plane_id), SPCSC_IMAX(448) | SPCSC_IMIN(-448)); - I915_WRITE_FW(SPCSCCRICLAMP(plane_id), SPCSC_IMAX(448) | SPCSC_IMIN(-448)); + I915_WRITE_FW(SPCSCYGICLAMP(plane_id), SPCSC_IMAX(1023) | SPCSC_IMIN(0)); + I915_WRITE_FW(SPCSCCBICLAMP(plane_id), SPCSC_IMAX(512) | SPCSC_IMIN(-512)); + I915_WRITE_FW(SPCSCCRICLAMP(plane_id), SPCSC_IMAX(512) | SPCSC_IMIN(-512)); I915_WRITE_FW(SPCSCYGOCLAMP(plane_id), SPCSC_OMAX(1023) | SPCSC_OMIN(0)); I915_WRITE_FW(SPCSCCBOCLAMP(plane_id), SPCSC_OMAX(1023) | SPCSC_OMIN(0)); I915_WRITE_FW(SPCSCCROCLAMP(plane_id), SPCSC_OMAX(1023) | SPCSC_OMIN(0)); } +static void +vlv_update_clrc(const struct intel_plane_state *plane_state) +{ + struct intel_plane *plane = to_intel_plane(plane_state->base.plane); + struct drm_i915_private *dev_priv = to_i915(plane->base.dev); + const struct drm_framebuffer *fb = plane_state->base.fb; + enum pipe pipe = plane->pipe; + enum plane_id plane_id = plane->id; + int con, bri, sh_sin, sh_cos; + + if (format_is_yuv(fb->format->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_FW(SPCLRC0(pipe, plane_id), + SP_CONTRAST(con) | SP_BRIGHTNESS(bri)); + I915_WRITE_FW(SPCLRC1(pipe, plane_id), + SP_SH_SIN(sh_sin) | SP_SH_COS(sh_cos)); +} + static u32 vlv_sprite_ctl(const struct intel_crtc_state *crtc_state, const struct intel_plane_state *plane_state) { @@ -456,8 +492,10 @@ vlv_update_plane(struct intel_plane *plane, spin_lock_irqsave(&dev_priv->uncore.lock, irqflags); + vlv_update_clrc(plane_state); + if (IS_CHERRYVIEW(dev_priv) && pipe == PIPE_B) - chv_update_csc(plane, fb->format->format); + chv_update_csc(plane_state); if (key->flags) { I915_WRITE_FW(SPKEYMINVAL(pipe, plane_id), key->min_value);