First of all thanks for the spec
On Mon, Oct 20, 2014 at 9:47 AM, <ville.syrjala@xxxxxxxxxxxxxxx> wrote:
> From: Ville Syrjälä <ville.syrjala@xxxxxxxxxxxxxxx>
>
> CHV has a programmable CSC unit on the pipe B sprites. Program the unit
> appropriately for BT.601 limited range YCbCr to full range RGB color
> conversion. This matches the programming we currently do for sprites
> on the other pipes and on other platforms.
>
> It seems the CSC only works when the input data is YCbCr. For RGB
> pixel formats it doesn't matter what we program into the CSC registers.
> Doesn't make much sense to me especially since the register names give
> the impression that RGB input data would also work. But that's how
> it behaves here.
>
> Signed-off-by: Ville Syrjälä <ville.syrjala@xxxxxxxxxxxxxxx>
> ---
> drivers/gpu/drm/i915/i915_reg.h | 33 +++++++++++++++++
> drivers/gpu/drm/i915/intel_sprite.c | 70 +++++++++++++++++++++++++++++--------
> 2 files changed, 89 insertions(+), 14 deletions(-)
>
> diff --git a/drivers/gpu/drm/i915/i915_reg.h b/drivers/gpu/drm/i915/i915_reg.h
> index 9dee063..05e7fd3 100644
> --- a/drivers/gpu/drm/i915/i915_reg.h
> +++ b/drivers/gpu/drm/i915/i915_reg.h
> @@ -4584,6 +4584,39 @@ enum punit_power_well {
> #define SPCONSTALPHA(pipe, plane) _PIPE(pipe * 2 + plane, _SPACONSTALPHA, _SPBCONSTALPHA)
> #define SPGAMC(pipe, plane) _PIPE(pipe * 2 + plane, _SPAGAMC, _SPBGAMC)
>
> +/*
> + * CHV pipe B sprite CSC
> + *
> + * |cr| |c0 c1 c2| |cr + cr_ioff| |cr_ooff|
> + * |yg| = |c3 c4 c5| x |yg + yg_ioff| + |yg_ooff|
> + * |cb| |c6 c7 c8| |cb + cr_ioff| |cb_ooff|
> + */
> +#define SPCSCYGOFF(sprite) (VLV_DISPLAY_BASE + 0x6d900 + (sprite) * 0x1000)
> +#define SPCSCCBOFF(sprite) (VLV_DISPLAY_BASE + 0x6d904 + (sprite) * 0x1000)
> +#define SPCSCCROFF(sprite) (VLV_DISPLAY_BASE + 0x6d908 + (sprite) * 0x1000)
> +#define SPCSC_OOFF(x) (((x) & 0x7ff) << 16) /* s11 */
> +#define SPCSC_IOFF(x) (((x) & 0x7ff) << 0) /* s11 */
> +
> +#define SPCSCC01(sprite) (VLV_DISPLAY_BASE + 0x6d90c + (sprite) * 0x1000)
> +#define SPCSCC23(sprite) (VLV_DISPLAY_BASE + 0x6d910 + (sprite) * 0x1000)
> +#define SPCSCC45(sprite) (VLV_DISPLAY_BASE + 0x6d914 + (sprite) * 0x1000)
> +#define SPCSCC67(sprite) (VLV_DISPLAY_BASE + 0x6d918 + (sprite) * 0x1000)
> +#define SPCSCC8(sprite) (VLV_DISPLAY_BASE + 0x6d91c + (sprite) * 0x1000)
> +#define SPCSC_C1(x) (((x) & 0x7fff) << 16) /* s3.12 */
> +#define SPCSC_C0(x) (((x) & 0x7fff) << 0) /* s3.12 */
> +
> +#define SPCSCYGICLAMP(sprite) (VLV_DISPLAY_BASE + 0x6d920 + (sprite) * 0x1000)
> +#define SPCSCCBICLAMP(sprite) (VLV_DISPLAY_BASE + 0x6d924 + (sprite) * 0x1000)
> +#define SPCSCCRICLAMP(sprite) (VLV_DISPLAY_BASE + 0x6d928 + (sprite) * 0x1000)
> +#define SPCSC_IMAX(x) (((x) & 0x7ff) << 16) /* s11 */
> +#define SPCSC_IMIN(x) (((x) & 0x7ff) << 0) /* s11 */
> +
> +#define SPCSCYGOCLAMP(sprite) (VLV_DISPLAY_BASE + 0x6d92c + (sprite) * 0x1000)
> +#define SPCSCCBOCLAMP(sprite) (VLV_DISPLAY_BASE + 0x6d930 + (sprite) * 0x1000)
> +#define SPCSCCROCLAMP(sprite) (VLV_DISPLAY_BASE + 0x6d934 + (sprite) * 0x1000)
> +#define SPCSC_OMAX(x) ((x) << 16) /* u10 */
> +#define SPCSC_OMIN(x) ((x) << 0) /* u10 */
> +
> /* Skylake plane registers */
>
> #define _PLANE_CTL_1_A 0x70180
> diff --git a/drivers/gpu/drm/i915/intel_sprite.c b/drivers/gpu/drm/i915/intel_sprite.c
> index a452819..fb6ae5f 100644
> --- a/drivers/gpu/drm/i915/intel_sprite.c
> +++ b/drivers/gpu/drm/i915/intel_sprite.c
> @@ -37,6 +37,20 @@
> #include <drm/i915_drm.h>
> #include "i915_drv.h"
>
> +static bool
> +format_is_yuv(uint32_t format)
> +{
> + switch (format) {
> + case DRM_FORMAT_YUYV:
> + case DRM_FORMAT_UYVY:
> + case DRM_FORMAT_VYUY:
> + case DRM_FORMAT_YVYU:
> + return true;
> + default:
> + return false;
> + }
> +}
> +
> static int usecs_to_scanlines(const struct drm_display_mode *mode, int usecs)
> {
> /* paranoia */
> @@ -320,6 +334,45 @@ skl_get_colorkey(struct drm_plane *drm_plane,
> }
>
> static void
> +chv_update_csc(struct intel_plane *intel_plane, uint32_t format)
> +{
> + struct drm_i915_private *dev_priv = intel_plane->base.dev->dev_private;
> + int plane = intel_plane->plane;
> +
> + /* Seems RGB data bypasses the CSC always */
> + if (!format_is_yuv(format))
> + return;
> +
> + /*
> + * BT.601 limited range YCbCr -> full range RGB
> + *
> + * |r| | 6537 4769 0| |cr |
> + * |g| = |-3330 4769 -1605| x |y-64|
> + * |b| | 0 4769 8263| |cb |
Where did you get this values? the ones at wikipedia didn't match...
> + *
> + * Cb and Cr apparently come in as signed already, so no
> + * need for any offset. For Y we need to remove the offset.
> + */
> + I915_WRITE(SPCSCYGOFF(plane), SPCSC_OOFF(0) | SPCSC_IOFF(-64));
> + 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(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));
where did you get these min/max?
> +
> + I915_WRITE(SPCSCYGOCLAMP(plane), SPCSC_OMAX(1023) | SPCSC_OMIN(0));
> + I915_WRITE(SPCSCCBOCLAMP(plane), SPCSC_OMAX(1023) | SPCSC_OMIN(0));
> + I915_WRITE(SPCSCCROCLAMP(plane), SPCSC_OMAX(1023) | SPCSC_OMIN(0));
and these ones?
> +}
> +
> +static void
> vlv_update_plane(struct drm_plane *dplane, struct drm_crtc *crtc,
> struct drm_framebuffer *fb,
> struct drm_i915_gem_object *obj, int crtc_x, int crtc_y,
> @@ -430,6 +483,9 @@ vlv_update_plane(struct drm_plane *dplane, struct drm_crtc *crtc,
>
> intel_update_primary_plane(intel_crtc);
>
> + if (IS_CHERRYVIEW(dev) && pipe == PIPE_B)
> + chv_update_csc(intel_plane, fb->pixel_format);
> +
> I915_WRITE(SPSTRIDE(pipe, plane), fb->pitches[0]);
> I915_WRITE(SPPOS(pipe, plane), (crtc_y << 16) | crtc_x);
>
> @@ -1004,20 +1060,6 @@ ilk_get_colorkey(struct drm_plane *plane, struct drm_intel_sprite_colorkey *key)
> key->flags = I915_SET_COLORKEY_NONE;
> }
>
> -static bool
> -format_is_yuv(uint32_t format)
> -{
> - switch (format) {
> - case DRM_FORMAT_YUYV:
> - case DRM_FORMAT_UYVY:
> - case DRM_FORMAT_VYUY:
> - case DRM_FORMAT_YVYU:
> - return true;
> - default:
> - return false;
> - }
> -}
> -
> static bool colorkey_enabled(struct intel_plane *intel_plane)
> {
> struct drm_intel_sprite_colorkey key;
> --
> 2.0.4
>
> _______________________________________________
> Intel-gfx mailing list
> Intel-gfx@xxxxxxxxxxxxxxxxxxxxx
> http://lists.freedesktop.org/mailman/listinfo/intel-gfx
With numbers explained the rest is ok and you can use Reviewed-by:
Rodrigo Vivi <rodrigo.vivi@xxxxxxxxx>
--
Rodrigo Vivi
Blog: http://blog.vivi.eng.br
_______________________________________________
Intel-gfx mailing list
Intel-gfx@xxxxxxxxxxxxxxxxxxxxx
http://lists.freedesktop.org/mailman/listinfo/intel-gfx
