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

[Ville Syrjälä <ville.syrjala@xxxxxxxxxxxxxxx>]

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 <chris@xxxxxxxxxxxxxxxxxx>

Looks all right to me.

Reviewed-by: Ville Syrjälä <ville.syrjala@xxxxxxxxxxxxxxx>

> ---
> 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@xxxxxxxxxxxxxxxxxxxxx
> http://lists.freedesktop.org/mailman/listinfo/intel-gfx

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