2015-04-30 Tobias Jakobi liquid.acid@gmx.net:
Hello Gustavo!
Gustavo Padovan wrote:
Hi Tobias,
2015-04-30 Tobias Jakobi tjakobi@math.uni-bielefeld.de:
First step in allowing a more generic way to setup complex blending for the different layers.
Signed-off-by: Tobias Jakobi tjakobi@math.uni-bielefeld.de
drivers/gpu/drm/exynos/exynos_mixer.c | 74 +++++++++++++++++++++++++++++------ 1 file changed, 63 insertions(+), 11 deletions(-)
diff --git a/drivers/gpu/drm/exynos/exynos_mixer.c b/drivers/gpu/drm/exynos/exynos_mixer.c index 4155f43..a06b8e2 100644 --- a/drivers/gpu/drm/exynos/exynos_mixer.c +++ b/drivers/gpu/drm/exynos/exynos_mixer.c @@ -63,6 +63,12 @@ struct mixer_resources { struct clk *mout_mixer; };
+struct layer_config {
- unsigned int index;
- unsigned int priority;
- u32 cfg;
+};
I don't see why you are creating this struct, index and priority are never used in this patch series.
Good catch about 'priority'. But 'index' is used, see the second patch.
enum mixer_version_id { MXR_VER_0_0_0_16, MXR_VER_16_0_33_0, @@ -75,6 +81,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_config *layer_config;
- unsigned int num_layer; int pipe; bool interlace; bool powered;
@@ -95,6 +103,40 @@ struct mixer_drv_data { bool has_sclk; };
+/*
- The default layer priorities. A higher priority means that
- the layer is at the top of layer stack.
- The current configuration 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
- video processor is available.
- */
+static const struct layer_config default_layer_config[] = {
- {
.index = 0, .priority = 1, /* layer0 */.cfg = MXR_LAYER_CFG_GRP0_VAL(1)- }, {
.index = 1, .priority = 2, /* layer1 */.cfg = MXR_LAYER_CFG_GRP1_VAL(2)- }
+};
+static const struct layer_config vp_layer_config[] = {
- {
.index = 2, .priority = 1, /* video layer */.cfg = MXR_LAYER_CFG_VP_VAL(1)- }, {
.index = 0, .priority = 2, /* layer0 */.cfg = MXR_LAYER_CFG_GRP0_VAL(2)- }, {
.index = 1, .priority = 3, /* layer1 */.cfg = MXR_LAYER_CFG_GRP1_VAL(3)- }
+};
static const u8 filter_y_horiz_tap8[] = { 0, -1, -1, -1, -1, -1, -1, -1, -1, -1, -1, -1, -1, 0, 0, 0, @@ -253,6 +295,17 @@ 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;
- for (i = 0; i < ctx->num_layer; ++i)
val |= ctx->layer_config[i].cfg;- 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; @@ -655,17 +708,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);
I would move this exaclty piece of code into mixer_layer_priority().
Then we end up with the same static/hardcoded setup as before. That's something I want to move away from. The entire information about layer ordering should be stored in 'layer_config'.
mixer_layer_priority(ctx);
/* setting background color */ mixer_reg_write(res, MXR_BG_COLOR0, 0x008080);
@@ -1274,6 +1317,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_config = vp_layer_config;ctx->num_layer = ARRAY_SIZE(vp_layer_config);- } else {
ctx->layer_config = default_layer_config;ctx->num_layer = ARRAY_SIZE(default_layer_config);- }
Then this piece of code is useless.
No, since the second patch depends on it.
Right, I did a second look on patch 2 and realized that index is still used and this code is actually needed here.
Gustavo