Hello,
here's the rework of the layer blending setup that I discussed with Joonyoung in the past days. There's still some TODOs in the code, but more or less it does what it's supposed to do. What still bothers me a bit is that I currently call blending reconfig in mixer_cfg_layer(). It would be nice if this could be reduced to one call per "frame" (so with the last win_{commit,disable} call). Maybe atomic provides such an infrastructure?
With best wishes, Tobias
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; +}; + 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); + 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); + } + init_waitqueue_head(&ctx->wait_vsync_queue); atomic_set(&ctx->wait_vsync_event, 0);
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.
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().
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.
Gustavo
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.
With best wishes, Tobias
Gustavo
To unsubscribe from this list: send the line "unsubscribe linux-samsung-soc" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
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
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@math.uni-bielefeld.de --- drivers/gpu/drm/exynos/exynos_mixer.c | 101 ++++++++++++++++++++++++++++++++++ drivers/gpu/drm/exynos/regs-mixer.h | 1 + 2 files changed, 102 insertions(+)
diff --git a/drivers/gpu/drm/exynos/exynos_mixer.c b/drivers/gpu/drm/exynos/exynos_mixer.c index a06b8e2..ff1168d 100644 --- a/drivers/gpu/drm/exynos/exynos_mixer.c +++ b/drivers/gpu/drm/exynos/exynos_mixer.c @@ -166,6 +166,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); @@ -306,6 +318,95 @@ 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; + + /* 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) { + // TODO + } 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; + + 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)
Previously blending setup was static and most of it was done in mixer_win_reset().
Signed-off-by: Tobias Jakobi tjakobi@math.uni-bielefeld.de --- drivers/gpu/drm/exynos/exynos_mixer.c | 22 ---------------------- 1 file changed, 22 deletions(-)
diff --git a/drivers/gpu/drm/exynos/exynos_mixer.c b/drivers/gpu/drm/exynos/exynos_mixer.c index ff1168d..3eec9ce 100644 --- a/drivers/gpu/drm/exynos/exynos_mixer.c +++ b/drivers/gpu/drm/exynos/exynos_mixer.c @@ -504,11 +504,6 @@ static void mixer_cfg_layer(struct mixer_context *ctx, int win, bool enable) vp_reg_writemask(res, VP_ENABLE, val, VP_ENABLE_ON); mixer_reg_writemask(res, MXR_CFG, val, MXR_CFG_VP_ENABLE); - - /* control blending of graphic layer 0 */ - mixer_reg_writemask(res, MXR_GRAPHIC_CFG(0), val, - MXR_GRP_CFG_BLEND_PRE_MUL | - MXR_GRP_CFG_PIXEL_BLEND_EN); } break; } @@ -816,23 +811,6 @@ static void mixer_win_reset(struct mixer_context *ctx) mixer_reg_write(res, MXR_BG_COLOR1, 0x008080); mixer_reg_write(res, MXR_BG_COLOR2, 0x008080);
- /* setting graphical layers */ - val = MXR_GRP_CFG_COLOR_KEY_DISABLE; /* no blank key */ - val |= MXR_GRP_CFG_WIN_BLEND_EN; - val |= MXR_GRP_CFG_ALPHA_VAL(0xff); /* non-transparent alpha */ - - /* Don't blend layer 0 onto the mixer background */ - mixer_reg_write(res, MXR_GRAPHIC_CFG(0), val); - - /* Blend layer 1 into layer 0 */ - val |= MXR_GRP_CFG_BLEND_PRE_MUL; - val |= MXR_GRP_CFG_PIXEL_BLEND_EN; - mixer_reg_write(res, MXR_GRAPHIC_CFG(1), val); - - /* setting video layers */ - val = MXR_GRP_CFG_ALPHA_VAL(0); - mixer_reg_write(res, MXR_VIDEO_CFG, val); - if (ctx->vp_enabled) { /* configuration of Video Processor Registers */ vp_win_reset(ctx);
This updates the blending setup when the layer configuration changes (triggered by mixer_win_{commit,disable}).
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 | 23 +++++++++++++++++++++++ 1 file changed, 23 insertions(+)
diff --git a/drivers/gpu/drm/exynos/exynos_mixer.c b/drivers/gpu/drm/exynos/exynos_mixer.c index 3eec9ce..809f840 100644 --- a/drivers/gpu/drm/exynos/exynos_mixer.c +++ b/drivers/gpu/drm/exynos/exynos_mixer.c @@ -178,6 +178,18 @@ static inline bool is_alpha_format(const struct mixer_context* ctx, unsigned int } }
+static inline unsigned int layer_bitmask(const struct mixer_context* ctx) +{ + unsigned int i, mask = 0; + + for (i = 0; i < MIXER_WIN_NR; ++i) { + if (ctx->planes[i].enabled) + mask |= (1 << i); + } + + return mask; +} + static inline u32 vp_reg_read(struct mixer_resources *res, u32 reg_id) { return readl(res->vp_regs + reg_id); @@ -490,6 +502,7 @@ static void mixer_cfg_rgb_fmt(struct mixer_context *ctx, unsigned int height) static void mixer_cfg_layer(struct mixer_context *ctx, int win, bool enable) { struct mixer_resources *res = &ctx->mixer_res; + unsigned int enable_state; u32 val = enable ? ~0 : 0;
switch (win) { @@ -507,6 +520,16 @@ static void mixer_cfg_layer(struct mixer_context *ctx, int win, bool enable) } break; } + + /* Determine the current enabled/disabled state of the layers. */ + enable_state = layer_bitmask(ctx); + if (enable) + enable_state |= (1 << win); + else + enable_state &= ~(1 << win); + + /* Layer configuration has changed, update blending setup. */ + mixer_layer_blending(ctx, enable_state); }
static void mixer_run(struct mixer_context *ctx)
Signed-off-by: Tobias Jakobi tjakobi@math.uni-bielefeld.de --- drivers/gpu/drm/exynos/exynos_mixer.c | 2 ++ 1 file changed, 2 insertions(+)
diff --git a/drivers/gpu/drm/exynos/exynos_mixer.c b/drivers/gpu/drm/exynos/exynos_mixer.c index 809f840..5a435aa 100644 --- a/drivers/gpu/drm/exynos/exynos_mixer.c +++ b/drivers/gpu/drm/exynos/exynos_mixer.c @@ -701,10 +701,12 @@ static void mixer_graph_buffer(struct mixer_context *ctx, int win)
switch (plane->pixel_format) { case DRM_FORMAT_XRGB4444: + case DRM_FORMAT_ARGB4444: fmt = MXR_FORMAT_ARGB4444; break;
case DRM_FORMAT_XRGB1555: + case DRM_FORMAT_ARGB1555: fmt = MXR_FORMAT_ARGB1555; break;
Hi Tobias,
To make patch files, you could use below command, #git format-patch --cover-letter from..to
With this command, a cover file will be created and you could describe what this patch series mean in the cover file.
Thanks, Inki Dae
On 2015년 04월 30일 23:56, Tobias Jakobi wrote:
Hello,
here's the rework of the layer blending setup that I discussed with Joonyoung in the past days. There's still some TODOs in the code, but more or less it does what it's supposed to do. What still bothers me a bit is that I currently call blending reconfig in mixer_cfg_layer(). It would be nice if this could be reduced to one call per "frame" (so with the last win_{commit,disable} call). Maybe atomic provides such an infrastructure?
With best wishes, Tobias
Hello Inki!
Inki Dae wrote:
Hi Tobias,
To make patch files, you could use below command, #git format-patch --cover-letter from..to
Thanks, I'm going to add the cover letter to the next revision.
With this command, a cover file will be created and you could describe what this patch series mean in the cover file.
I left out a detailed description since the series isn't really finished yet and I more or less wanted to hear Joonyoung' thoughts, who I discussed stuff with.
I still have to change some details and remove the TODOs. Once that's done I'm going to write a full description.
With best wishes, Tobias
Thanks, Inki Dae
On 2015년 04월 30일 23:56, Tobias Jakobi wrote:
Hello,
here's the rework of the layer blending setup that I discussed with Joonyoung in the past days. There's still some TODOs in the code, but more or less it does what it's supposed to do. What still bothers me a bit is that I currently call blending reconfig in mixer_cfg_layer(). It would be nice if this could be reduced to one call per "frame" (so with the last win_{commit,disable} call). Maybe atomic provides such an infrastructure?
With best wishes, Tobias
dri-devel@lists.freedesktop.org
-
Gustavo Padovan -
Gustavo Padovan -
Inki Dae -
Tobias Jakobi -
Tobias Jakobi