Hey Inki, Inki Dae wrote: > Hi Tobias, > > 2015년 11월 23일 01:09에 Tobias Jakobi 이(가) 쓴 글: >> First step in allowing a more generic way to setup complex >> blending for the different layers. >> >> Signed-off-by: Tobias Jakobi <tjakobi@xxxxxxxxxxxxxxxxxxxxx> >> --- >> drivers/gpu/drm/exynos/exynos_mixer.c | 84 ++++++++++++++++++++++++++++++----- >> 1 file changed, 73 insertions(+), 11 deletions(-) >> >> diff --git a/drivers/gpu/drm/exynos/exynos_mixer.c b/drivers/gpu/drm/exynos/exynos_mixer.c >> index 7498c6e..2c1cea3 100644 >> --- a/drivers/gpu/drm/exynos/exynos_mixer.c >> +++ b/drivers/gpu/drm/exynos/exynos_mixer.c >> @@ -63,6 +63,11 @@ struct mixer_resources { >> struct clk *mout_mixer; >> }; >> >> +struct layer_cfg { >> + unsigned int index; >> + unsigned int priority; >> +}; >> + >> enum mixer_version_id { >> MXR_VER_0_0_0_16, >> MXR_VER_16_0_33_0, >> @@ -92,6 +97,8 @@ struct mixer_context { >> struct drm_device *drm_dev; >> struct exynos_drm_crtc *crtc; >> struct exynos_drm_plane planes[MIXER_WIN_NR]; >> + const struct layer_cfg *layer_cfg; >> + unsigned int num_layer; >> int pipe; >> unsigned long flags; >> bool interlace; >> @@ -110,6 +117,34 @@ struct mixer_drv_data { >> bool has_sclk; >> }; >> >> +/* >> + * The default layer priorities for non-VP (video processor) >> + * and VP mixer configurations. >> + * >> + * A higher priority means that the layer is at the top of >> + * the layer stack. >> + * Configurations have to be specified with its entries >> + * sorted with increasing priority. >> + * >> + * The default config assumes the following usage scenario: >> + * layer1: OSD [top] >> + * layer0: main framebuffer >> + * video layer: video overlay [bottom] >> + * Note that the video layer is only usable when the >> + * VP is available. >> + */ >> + >> +static const struct layer_cfg nonvp_default_cfg[] = { >> + { .index = 0, .priority = 1 }, /* layer0 */ >> + { .index = 1, .priority = 2 }, /* layer1 */ >> +}; >> + >> +static const struct layer_cfg vp_default_cfg[] = { >> + { .index = 2, .priority = 1 }, /* video layer */ >> + { .index = 0, .priority = 2 }, /* layer0 */ >> + { .index = 1, .priority = 3 }, /* layer1 */ >> +}; >> + >> static const u8 filter_y_horiz_tap8[] = { >> 0, -1, -1, -1, -1, -1, -1, -1, >> -1, -1, -1, -1, -1, 0, 0, 0, >> @@ -268,6 +303,34 @@ static void vp_default_filter(struct mixer_resources *res) >> filter_cr_horiz_tap4, sizeof(filter_cr_horiz_tap4)); >> } >> >> +static void mixer_layer_priority(struct mixer_context *ctx) >> +{ >> + u32 val = 0; >> + unsigned int i, priority; >> + >> + for (i = 0; i < ctx->num_layer; ++i) { >> + priority = ctx->layer_cfg[i].priority; >> + BUG_ON(priority > 15); > > What doesn constant, 15 mean? You need to clarify the meaning > and please, use a macro instead. If I read the specs correctly the layer priority is encoded with just 4-bits in the corresponding register. I'll add a define to make this more clear. > And it'd better to just return in this case so that the layer > configuration can be set with a default value. The point is that these (currently) are the default values that mixer_layer_priority() sets. The BUG_ON() is there to make sure that nobody changes {vp,nonvp}_default_cfg[] later with illegal values. With best wishes, Tobias > I think it'd be enough to print out warning message. > > Thanks, > Inki Dae > >> + >> + switch (ctx->layer_cfg[i].index) { >> + case 0: >> + val |= MXR_LAYER_CFG_GRP0_VAL(priority); >> + break; >> + case 1: >> + val |= MXR_LAYER_CFG_GRP1_VAL(priority); >> + break; >> + case 2: >> + val |= MXR_LAYER_CFG_VP_VAL(priority); >> + break; >> + default: >> + BUG_ON(true); >> + break; >> + } >> + } >> + >> + mixer_reg_write(&ctx->mixer_res, MXR_LAYER_CFG, val); >> +} >> + >> static void mixer_vsync_set_update(struct mixer_context *ctx, bool enable) >> { >> struct mixer_resources *res = &ctx->mixer_res; >> @@ -673,17 +736,7 @@ static void mixer_win_reset(struct mixer_context *ctx) >> mixer_reg_writemask(res, MXR_STATUS, MXR_STATUS_16_BURST, >> MXR_STATUS_BURST_MASK); >> >> - /* setting default layer priority: layer1 > layer0 > video >> - * because typical usage scenario would be >> - * layer1 - OSD >> - * layer0 - framebuffer >> - * video - video overlay >> - */ >> - val = MXR_LAYER_CFG_GRP1_VAL(3); >> - val |= MXR_LAYER_CFG_GRP0_VAL(2); >> - if (ctx->vp_enabled) >> - val |= MXR_LAYER_CFG_VP_VAL(1); >> - mixer_reg_write(res, MXR_LAYER_CFG, val); >> + mixer_layer_priority(ctx); >> >> /* setting background color */ >> mixer_reg_write(res, MXR_BG_COLOR0, 0x008080); >> @@ -1209,6 +1262,15 @@ static int mixer_probe(struct platform_device *pdev) >> ctx->vp_enabled = drv->is_vp_enabled; >> ctx->has_sclk = drv->has_sclk; >> ctx->mxr_ver = drv->version; >> + >> + if (drv->is_vp_enabled) { >> + ctx->layer_cfg = vp_default_cfg; >> + ctx->num_layer = ARRAY_SIZE(vp_default_cfg); >> + } else { >> + ctx->layer_cfg = nonvp_default_cfg; >> + ctx->num_layer = ARRAY_SIZE(nonvp_default_cfg); >> + } >> + >> init_waitqueue_head(&ctx->wait_vsync_queue); >> atomic_set(&ctx->wait_vsync_event, 0); >> >> -- To unsubscribe from this list: send the line "unsubscribe linux-samsung-soc" in the body of a message to majordomo@xxxxxxxxxxxxxxx More majordomo info at http://vger.kernel.org/majordomo-info.html