Re: [Intel-gfx] [PATCH] drm/i915: Allow disabling the destination colorkey for overlay

[Ville Syrjälä]

On Thu, Apr 02, 2015 at 10:35:08AM +0100, Chris Wilson wrote:
> Sometimes userspace wants a true overlay that is never clipped. In such
> cases, we need to disable the destination colorkey. However, it is
> currently unconditionally enabled in the overlay with no means of
> disabling. So rectify that by always default to on, and extending the
> UPDATE_ATTR ioctl to support explicit disabling of the colorkey.
> 
> This is contrast to the spite code which requires explicit enabling of
> either the destination or source colorkey. Handling source colorkey is
> still todo for the overlay. (Of course it may be worth migrating overlay
> to sprite before then.)
> 
> Signed-off-by: Chris Wilson <ch...@chris-wilson.co.uk>

Looks all right to me.

Reviewed-by: Ville Syrjälä <ville.syrj...@linux.intel.com>

> ---
> The recent discussion over colorkey reminded me about this feature
> discrepancy in the overlay. 2013!
> ---
>  drivers/gpu/drm/i915/intel_overlay.c | 30 +++++++++++++++++++-----------
>  include/uapi/drm/i915_drm.h          |  1 +
>  2 files changed, 20 insertions(+), 11 deletions(-)
> 
> diff --git a/drivers/gpu/drm/i915/intel_overlay.c 
> b/drivers/gpu/drm/i915/intel_overlay.c
> index 36400bbbf715..936cf160bb7d 100644
> --- a/drivers/gpu/drm/i915/intel_overlay.c
> +++ b/drivers/gpu/drm/i915/intel_overlay.c
> @@ -175,7 +175,8 @@ struct intel_overlay {
>       bool active;
>       bool pfit_active;
>       u32 pfit_vscale_ratio; /* shifted-point number, (1<<12) == 1.0 */
> -     u32 color_key;
> +     u32 color_key:24;
> +     u32 color_key_enabled:1;
>       u32 brightness, contrast, saturation;
>       u32 old_xscale, old_yscale;
>       /* register access */
> @@ -630,31 +631,36 @@ static void update_colorkey(struct intel_overlay 
> *overlay,
>                           struct overlay_registers __iomem *regs)
>  {
>       u32 key = overlay->color_key;
> +     u32 flags;
> +
> +     flags = 0;
> +     if (overlay->color_key_enabled)
> +             flags |= DST_KEY_ENABLE;
>  
>       switch (overlay->crtc->base.primary->fb->bits_per_pixel) {
>       case 8:
> -             iowrite32(0, &regs->DCLRKV);
> -             iowrite32(CLK_RGB8I_MASK | DST_KEY_ENABLE, &regs->DCLRKM);
> +             key = 0;
> +             flags |= CLK_RGB8I_MASK;
>               break;
>  
>       case 16:
>               if (overlay->crtc->base.primary->fb->depth == 15) {
> -                     iowrite32(RGB15_TO_COLORKEY(key), &regs->DCLRKV);
> -                     iowrite32(CLK_RGB15_MASK | DST_KEY_ENABLE,
> -                               &regs->DCLRKM);
> +                     key = RGB15_TO_COLORKEY(key);
> +                     flags |= CLK_RGB15_MASK;
>               } else {
> -                     iowrite32(RGB16_TO_COLORKEY(key), &regs->DCLRKV);
> -                     iowrite32(CLK_RGB16_MASK | DST_KEY_ENABLE,
> -                               &regs->DCLRKM);
> +                     key = RGB16_TO_COLORKEY(key);
> +                     flags |= CLK_RGB16_MASK;
>               }
>               break;
>  
>       case 24:
>       case 32:
> -             iowrite32(key, &regs->DCLRKV);
> -             iowrite32(CLK_RGB24_MASK | DST_KEY_ENABLE, &regs->DCLRKM);
> +             flags |= CLK_RGB24_MASK;
>               break;
>       }
> +
> +     iowrite32(key, &regs->DCLRKV);
> +     iowrite32(flags, &regs->DCLRKM);
>  }
>  
>  static u32 overlay_cmd_reg(struct put_image_params *params)
> @@ -1331,6 +1337,7 @@ int intel_overlay_attrs(struct drm_device *dev, void 
> *data,
>                       I915_WRITE(OGAMC5, attrs->gamma5);
>               }
>       }
> +     overlay->color_key_enabled = (attrs->flags & 
> I915_OVERLAY_DISABLE_DEST_COLORKEY) == 0;
>  
>       ret = 0;
>  out_unlock:
> @@ -1397,6 +1404,7 @@ void intel_setup_overlay(struct drm_device *dev)
>  
>       /* init all values */
>       overlay->color_key = 0x0101fe;
> +     overlay->color_key_enabled = true;
>       overlay->brightness = -19;
>       overlay->contrast = 75;
>       overlay->saturation = 146;
> diff --git a/include/uapi/drm/i915_drm.h b/include/uapi/drm/i915_drm.h
> index 006f026f1ce0..c2e679be8903 100644
> --- a/include/uapi/drm/i915_drm.h
> +++ b/include/uapi/drm/i915_drm.h
> @@ -998,6 +998,7 @@ struct drm_intel_overlay_put_image {
>  /* flags */
>  #define I915_OVERLAY_UPDATE_ATTRS    (1<<0)
>  #define I915_OVERLAY_UPDATE_GAMMA    (1<<1)
> +#define I915_OVERLAY_DISABLE_DEST_COLORKEY   (1<<2)
>  struct drm_intel_overlay_attrs {
>       __u32 flags;
>       __u32 color_key;
> -- 
> 2.1.4
> 
> _______________________________________________
> Intel-gfx mailing list
> Intel-gfx@lists.freedesktop.org
> http://lists.freedesktop.org/mailman/listinfo/intel-gfx

-- 
Ville Syrjälä
Intel OTC
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx