Re: [PATCH v2 4/5] drm/exynos: mixer: do blending setup in mixer_cfg_layer()

[Inki Dae <inki.dae@samsung.com>]

2015년 11월 23일 01:09에 Tobias Jakobi 이(가) 쓴 글:
> This updates the blending setup when the layer configuration
> changes (triggered by mixer_win_{commit,disable}).
> 
> To avoid unnecesary reconfigurations we cache the layer
> state in the mixer context.
> 
> Extra care has to be taken for the layer that is currently
> being enabled/disabled.
> 
> Signed-off-by: Tobias Jakobi <tjakobi@math.uni-bielefeld.de>
> ---
>  drivers/gpu/drm/exynos/exynos_mixer.c | 41 +++++++++++++++++++++++++++++++++--
>  1 file changed, 39 insertions(+), 2 deletions(-)
> 
> diff --git a/drivers/gpu/drm/exynos/exynos_mixer.c b/drivers/gpu/drm/exynos/exynos_mixer.c
> index ec9659e..1c24fb5 100644
> --- a/drivers/gpu/drm/exynos/exynos_mixer.c
> +++ b/drivers/gpu/drm/exynos/exynos_mixer.c
> @@ -99,6 +99,7 @@ struct mixer_context {
>  	struct exynos_drm_plane	planes[MIXER_WIN_NR];
>  	const struct layer_cfg	*layer_cfg;
>  	unsigned int		num_layer;
> +	u32			layer_state;
>  	int			pipe;
>  	unsigned long		flags;
>  	bool			interlace;
> @@ -189,6 +190,27 @@ static inline bool is_alpha_format(const struct mixer_context* ctx, unsigned int
>  	}
>  }
>  
> +static inline u32 get_layer_state(const struct mixer_context *ctx,
> +	unsigned int win, bool enable)
> +{
> +	u32 enable_state, alpha_state;
> +
> +	enable_state = ctx->layer_state & 0xffff;
> +	alpha_state = ctx->layer_state >> 16;
> +
> +	if (enable)
> +		enable_state |= (1 << win);
> +	else
> +		enable_state &= ~(1 << win);
> +
> +	if (enable && is_alpha_format(ctx, win))
> +		alpha_state |= (1 << win);
> +	else
> +		alpha_state &= ~(1 << win);
> +
> +	return ((alpha_state << 16) | enable_state);
> +}
> +
>  static inline u32 vp_reg_read(struct mixer_resources *res, u32 reg_id)
>  {
>  	return readl(res->vp_regs + reg_id);
> @@ -370,8 +392,9 @@ static void mixer_general_layer(struct mixer_context *ctx,
>  {
>  	u32 val;
>  	struct mixer_resources *res = &ctx->mixer_res;
> +	const u32 alpha_state = ctx->layer_state >> 16;
>  
> -	if (is_alpha_format(ctx, cfg->index)) {
> +	if (alpha_state & (1 << cfg->index)) {
>  		val  = MXR_GRP_CFG_COLOR_KEY_DISABLE; /* no blank key */
>  		val |= MXR_GRP_CFG_BLEND_PRE_MUL;
>  		val |= MXR_GRP_CFG_PIXEL_BLEND_EN; /* blending based on pixel alpha */
> @@ -397,10 +420,11 @@ static void mixer_general_layer(struct mixer_context *ctx,
>  	}
>  }
>  
> -static void mixer_layer_blending(struct mixer_context *ctx, unsigned int enable_state)
> +static void mixer_layer_blending(struct mixer_context *ctx)
>  {
>  	unsigned int i, index;
>  	bool bottom_layer = false;
> +	const u32 enable_state = ctx->layer_state & 0xffff;
>  
>  	for (i = 0; i < ctx->num_layer; ++i) {
>  		index = ctx->layer_cfg[i].index;
> @@ -503,8 +527,19 @@ static void mixer_cfg_layer(struct mixer_context *ctx, unsigned int win,
>  				bool enable)
>  {
>  	struct mixer_resources *res = &ctx->mixer_res;
> +	u32 new_layer_state;
>  	u32 val = enable ? ~0 : 0;
>  
> +	new_layer_state = get_layer_state(ctx, win, enable);
> +	if (new_layer_state == ctx->layer_state)
> +		return;
> +
> +	/*
> +	 * Update the layer state so that mixer_layer_blending()
> +	 * below can use it.
> +	 */
> +	ctx->layer_state = new_layer_state;

It may be trivial but I think it'd be better to move above line to most bottom of this function.

> +
>  	switch (win) {
>  	case 0:
>  		mixer_reg_writemask(res, MXR_CFG, val, MXR_CFG_GRP0_ENABLE);
> @@ -520,6 +555,8 @@ static void mixer_cfg_layer(struct mixer_context *ctx, unsigned int win,
>  		}
>  		break;
>  	}
> +
> +	mixer_layer_blending(ctx);

Here.

Thanks,
Inki Dae

>  }
>  
>  static void mixer_run(struct mixer_context *ctx)
>