Re: [PATCH] drm/i915: Add support for CHV pipe B sprite CSC

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



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





[Index of Archives]     [Linux USB Devel]     [Linux Audio Users]     [Yosemite News]     [Linux Kernel]     [Linux SCSI]
  Powered by Linux