Hello Joonyoung, On 2015-05-22 11:12, Joonyoung Shim wrote: > On 05/06/2015 10:36 PM, Tobias Jakobi wrote: >> This analyses the current layer configuration (which layers >> are enabled, which have alpha-pixelformat, etc.) and setups >> blending accordingly. >> >> We currently disable all kinds of blending for the bottom-most >> layer, since configuration of the mixer background is not >> yet exposed. >> Also blending is only enabled when the layer has a pixelformat >> with alpha attached. >> >> Signed-off-by: Tobias Jakobi <tjakobi at math.uni-bielefeld.de> >> --- >> drivers/gpu/drm/exynos/exynos_mixer.c | 108 >> ++++++++++++++++++++++++++++++++++ >> drivers/gpu/drm/exynos/regs-mixer.h | 1 + >> 2 files changed, 109 insertions(+) >> >> diff --git a/drivers/gpu/drm/exynos/exynos_mixer.c >> b/drivers/gpu/drm/exynos/exynos_mixer.c >> index e4a5e76..5e95ef2 100644 >> --- a/drivers/gpu/drm/exynos/exynos_mixer.c >> +++ b/drivers/gpu/drm/exynos/exynos_mixer.c >> @@ -165,6 +165,18 @@ static const u8 filter_cr_horiz_tap4[] = { >> 70, 59, 48, 37, 27, 19, 11, 5, >> }; >> >> +static inline bool is_alpha_format(const struct mixer_context* ctx, >> unsigned int win) >> +{ >> + switch (ctx->planes[win].pixel_format) { >> + case DRM_FORMAT_ARGB8888: >> + case DRM_FORMAT_ARGB1555: >> + case DRM_FORMAT_ARGB4444: >> + return true; >> + default: >> + return false; >> + } >> +} >> + >> static inline u32 vp_reg_read(struct mixer_resources *res, u32 >> reg_id) >> { >> return readl(res->vp_regs + reg_id); >> @@ -322,6 +334,102 @@ static void mixer_layer_priority(struct >> mixer_context *ctx) >> mixer_reg_write(&ctx->mixer_res, MXR_LAYER_CFG, val); >> } >> >> +/* Configure blending for bottom-most layer. */ >> +static void mixer_bottom_layer(struct mixer_context *ctx, >> + const struct layer_config *cfg) >> +{ >> + u32 val; >> + struct mixer_resources *res = &ctx->mixer_res; >> + >> + if (cfg->index == 2) { >> + val = 0; /* use defaults for video layer */ >> + mixer_reg_write(res, MXR_VIDEO_CFG, val); >> + } else { >> + val = MXR_GRP_CFG_COLOR_KEY_DISABLE; /* no blank key */ >> + >> + /* Don't blend bottom-most layer onto the mixer background. */ >> + mixer_reg_writemask(res, MXR_GRAPHIC_CFG(cfg->index), >> + val, MXR_GRP_CFG_MISC_MASK); >> + } >> +} >> + >> +static void mixer_general_layer(struct mixer_context *ctx, >> + const struct layer_config *cfg) >> +{ >> + u32 val; >> + struct mixer_resources *res = &ctx->mixer_res; >> + >> + if (is_alpha_format(ctx, 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 >> */ >> + >> + /* The video layer never has an alpha pixelformat. */ >> + mixer_reg_writemask(res, MXR_GRAPHIC_CFG(cfg->index), >> + val, MXR_GRP_CFG_MISC_MASK); >> + } else { >> + if (cfg->index == 2) { >> + /* >> + * No blending at the moment since the NV12/NV21 >> pixelformats >> don't >> + * have an alpha channel. However the mixer supports a >> global >> alpha >> + * value for a layer. Once this functionality is >> exposed, we can >> + * support blending of the video layer through this. >> + */ >> + val = 0; >> + mixer_reg_write(res, MXR_VIDEO_CFG, val); >> + } else { >> + val = MXR_GRP_CFG_COLOR_KEY_DISABLE; >> + mixer_reg_writemask(res, MXR_GRAPHIC_CFG(cfg->index), >> + val, MXR_GRP_CFG_MISC_MASK); >> + } >> + } >> +} >> + >> +static void mixer_layer_blending(struct mixer_context *ctx, unsigned >> int enable_state) >> +{ >> + const struct layer_config *cfg; >> + unsigned int i = 0; >> + unsigned int index; >> + >> + /* Find bottom-most enabled layer. */ >> + cfg = NULL; >> + while (i < ctx->num_layer) { >> + index = ctx->layer_config[i].index; >> + ++i; > > Don't use priority? Used in mixer_layer_priority(), see first patch.
> >> + >> + if (enable_state & (1 << index)) { >> + cfg = &ctx->layer_config[i-1]; >> + break; >> + } >> + } >> + >> + /* No enabled layers found, nothing to do. */ >> + if (!cfg) >> + return; >> + >> + mixer_bottom_layer(ctx, cfg); >> + >> + while (1) { >> + /* Find the next layer. */ >> + cfg = NULL; >> + while (i < ctx->num_layer) { >> + index = ctx->layer_config[i].index; >> + ++i; >> + >> + if (enable_state & (1 << index)) { >> + cfg = &ctx->layer_config[i-1]; >> + break; >> + } >> + } >> + >> + /* No more enabled layers found. */ >> + if (!cfg) >> + return; >> + >> + mixer_general_layer(ctx, cfg); >> + } >> +} >> + >> static void mixer_vsync_set_update(struct mixer_context *ctx, bool >> enable) >> { >> struct mixer_resources *res = &ctx->mixer_res; >> diff --git a/drivers/gpu/drm/exynos/regs-mixer.h >> b/drivers/gpu/drm/exynos/regs-mixer.h >> index ac60260..118872e 100644 >> --- a/drivers/gpu/drm/exynos/regs-mixer.h >> +++ b/drivers/gpu/drm/exynos/regs-mixer.h >> @@ -113,6 +113,7 @@ >> #define MXR_GRP_CFG_BLEND_PRE_MUL (1 << 20) >> #define MXR_GRP_CFG_WIN_BLEND_EN (1 << 17) >> #define MXR_GRP_CFG_PIXEL_BLEND_EN (1 << 16) >> +#define MXR_GRP_CFG_MISC_MASK ((3 << 16) | (3 << 20)) >> #define MXR_GRP_CFG_FORMAT_VAL(x) MXR_MASK_VAL(x, 11, 8) >> #define MXR_GRP_CFG_FORMAT_MASK MXR_GRP_CFG_FORMAT_VAL(~0) >> #define MXR_GRP_CFG_ALPHA_VAL(x) MXR_MASK_VAL(x, 7, 0) >> > > I think this patch can be clean more, > > e.g. remove duplicated codes and while (1) ... Can you point out what 'duplicated codes' you mean, and why 'while (1)' is wrong here? With best wishes, Tobias