[PATCH 0/5] drm/exynos: rework layer blending setup

[PATCH 0/5] drm/exynos: rework layer blending setup

[Tobias Jakobi]

Hello,

this is a rework of the layer blending setup in the Exynos DRM mixer. The current setup is static and spread out through the mixer code. This rework pushes all the configuration details into a layer_config array, which specifies the priority of each layer.

Two arrays are currently found in the code, one for SoC versions with a video processor (VP) and one for SoC versions without VP. The VP gives us one additional layer, the video layer, which natively supports the NV12/NV21 pixelformat.

The blending setup roughly works like this:
1) Find the bottom-most enabled layer. Disable all blending for this layer. This is done because we currently don't expose modification of the mixer background to userspace. Once this is done we can add more flexibility here.
2) Find the next enabled layer in our layer stack. If the layer has a framebuffer with an alpha-pixelformat attached, enable blending for this layer. If not, disable blending.
3) Iterate (2) until all enabled layers are processed.

The series has been tested on a Hardkernel Odroid-X2 (Exynos4412, which has a VP).

If you want to use libdrm's modetest to check the series, please apply patches [1] and [2]. This should make it possible to also test a plane with NV12 format (which is located 'behind' the primary plane).

With best wishes,
Tobias

[1] https://patchwork.kernel.org/patch/6349241/
[2] https://patchwork.kernel.org/patch/6349261/


Tobias Jakobi (5):
  drm/exynos: mixer: refactor layer setup
  drm/exynos: mixer: introduce mixer_layer_blending()
  drm/exynos: mixer: remove all static blending setup
  drm/exynos: mixer: do blending setup in mixer_cfg_layer()
  drm/exynos: mixer: also allow ARGB1555 and ARGB4444

 drivers/gpu/drm/exynos/exynos_mixer.c | 246 +++++++++++++++++++++++++++++-----
 drivers/gpu/drm/exynos/regs-mixer.h   |   1 +
 2 files changed, 213 insertions(+), 34 deletions(-)

-- 
2.0.5

[PATCH 1/5] drm/exynos: mixer: refactor layer setup

[Tobias Jakobi]

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 | 90 ++++++++++++++++++++++++++++++-----
 1 file changed, 79 insertions(+), 11 deletions(-)

diff --git a/drivers/gpu/drm/exynos/exynos_mixer.c b/drivers/gpu/drm/exynos/exynos_mixer.c
index 4a1656b..e4a5e76 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_config {
+	unsigned int index;
+	unsigned int priority;
+};
+
 enum mixer_version_id {
 	MXR_VER_0_0_0_16,
 	MXR_VER_16_0_33_0,
@@ -75,6 +80,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 +102,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,			/* layer0 */
+		.priority = 1
+	}, {
+		.index = 1,			/* layer1 */
+		.priority = 2
+	}
+};
+
+static const struct layer_config vp_layer_config[] = {
+	{
+		.index = 2,			/* video layer */
+		.priority = 1
+	}, {
+		.index = 0,			/* layer0 */
+		.priority = 2
+	}, {
+		.index = 1,			/* layer1 */
+		.priority = 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 +294,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_config[i].priority;
+		BUG_ON(priority > 15);
+
+		switch (ctx->layer_config[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;
@@ -654,17 +723,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 +1333,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);
 
-- 
2.0.5

[PATCH 2/5] drm/exynos: mixer: introduce mixer_layer_blending()

[Tobias Jakobi]

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 | 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;
+
+		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)
-- 
2.0.5

[PATCH 3/5] drm/exynos: mixer: remove all static blending setup

[Tobias Jakobi]

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 | 23 -----------------------
 1 file changed, 23 deletions(-)

diff --git a/drivers/gpu/drm/exynos/exynos_mixer.c b/drivers/gpu/drm/exynos/exynos_mixer.c
index 5e95ef2..3ab7a01 100644
--- a/drivers/gpu/drm/exynos/exynos_mixer.c
+++ b/drivers/gpu/drm/exynos/exynos_mixer.c
@@ -527,11 +527,6 @@ static void mixer_cfg_layer(struct mixer_context *ctx, unsigned int win, bool en
 			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;
 	}
@@ -817,7 +812,6 @@ static void mixer_win_reset(struct mixer_context *ctx)
 {
 	struct mixer_resources *res = &ctx->mixer_res;
 	unsigned long flags;
-	u32 val; /* value stored to register */
 
 	spin_lock_irqsave(&res->reg_slock, flags);
 	mixer_vsync_set_update(ctx, false);
@@ -838,23 +832,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);
-- 
2.0.5

_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/dri-devel

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

[Tobias Jakobi]

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 3ab7a01..430d10c 100644
--- a/drivers/gpu/drm/exynos/exynos_mixer.c
+++ b/drivers/gpu/drm/exynos/exynos_mixer.c
@@ -177,6 +177,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);
@@ -513,6 +525,7 @@ static void mixer_cfg_rgb_fmt(struct mixer_context *ctx, unsigned int height)
 static void mixer_cfg_layer(struct mixer_context *ctx, unsigned int win, bool enable)
 {
 	struct mixer_resources *res = &ctx->mixer_res;
+	unsigned int enable_state;
 	u32 val = enable ? ~0 : 0;
 
 	switch (win) {
@@ -530,6 +543,16 @@ static void mixer_cfg_layer(struct mixer_context *ctx, unsigned int win, bool en
 		}
 		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)
-- 
2.0.5

[PATCH 5/5] drm/exynos: mixer: also allow ARGB1555 and ARGB4444

[Tobias Jakobi]

Allow the remaining alpha formats now that blending
is properly setup.

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 430d10c..54e79e0 100644
--- a/drivers/gpu/drm/exynos/exynos_mixer.c
+++ b/drivers/gpu/drm/exynos/exynos_mixer.c
@@ -721,10 +721,12 @@ static void mixer_graph_buffer(struct mixer_context *ctx, unsigned 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;
 
-- 
2.0.5

_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/dri-devel

Re: [PATCH 0/5] drm/exynos: rework layer blending setup

[Gustavo Padovan]

2015-05-06 Tobias Jakobi <tjakobi@math.uni-bielefeld.de>:

> Hello,
> 
> this is a rework of the layer blending setup in the Exynos DRM mixer. The current setup is static and spread out through the mixer code. This rework pushes all the configuration details into a layer_config array, which specifies the priority of each layer.
> 
> Two arrays are currently found in the code, one for SoC versions with a video processor (VP) and one for SoC versions without VP. The VP gives us one additional layer, the video layer, which natively supports the NV12/NV21 pixelformat.
> 
> The blending setup roughly works like this:
> 1) Find the bottom-most enabled layer. Disable all blending for this layer. This is done because we currently don't expose modification of the mixer background to userspace. Once this is done we can add more flexibility here.
> 2) Find the next enabled layer in our layer stack. If the layer has a framebuffer with an alpha-pixelformat attached, enable blending for this layer. If not, disable blending.
> 3) Iterate (2) until all enabled layers are processed.
> 
> The series has been tested on a Hardkernel Odroid-X2 (Exynos4412, which has a VP).
> 
> If you want to use libdrm's modetest to check the series, please apply patches [1] and [2]. This should make it possible to also test a plane with NV12 format (which is located 'behind' the primary plane).


The whole series works fine for me on Samsung Snow 5250 (which doesn't
have a VP).

Tested-by: Gustavo Padovan <gustavo.padovan@collabora.co.uk>

	Gustavo

Re: [PATCH 0/5] drm/exynos: rework layer blending setup

[Tobias Jakobi]

Gentle reminder that I still haven't heard anything about the series by
the Samsung guys ;)

With best wishes,
Tobias


Tobias Jakobi wrote:
> Hello,
> 
> this is a rework of the layer blending setup in the Exynos DRM mixer. The current setup is static and spread out through the mixer code. This rework pushes all the configuration details into a layer_config array, which specifies the priority of each layer.
> 
> Two arrays are currently found in the code, one for SoC versions with a video processor (VP) and one for SoC versions without VP. The VP gives us one additional layer, the video layer, which natively supports the NV12/NV21 pixelformat.
> 
> The blending setup roughly works like this:
> 1) Find the bottom-most enabled layer. Disable all blending for this layer. This is done because we currently don't expose modification of the mixer background to userspace. Once this is done we can add more flexibility here.
> 2) Find the next enabled layer in our layer stack. If the layer has a framebuffer with an alpha-pixelformat attached, enable blending for this layer. If not, disable blending.
> 3) Iterate (2) until all enabled layers are processed.
> 
> The series has been tested on a Hardkernel Odroid-X2 (Exynos4412, which has a VP).
> 
> If you want to use libdrm's modetest to check the series, please apply patches [1] and [2]. This should make it possible to also test a plane with NV12 format (which is located 'behind' the primary plane).
> 
> With best wishes,
> Tobias
> 
> [1] https://patchwork.kernel.org/patch/6349241/
> [2] https://patchwork.kernel.org/patch/6349261/
> 
> 
> Tobias Jakobi (5):
>   drm/exynos: mixer: refactor layer setup
>   drm/exynos: mixer: introduce mixer_layer_blending()
>   drm/exynos: mixer: remove all static blending setup
>   drm/exynos: mixer: do blending setup in mixer_cfg_layer()
>   drm/exynos: mixer: also allow ARGB1555 and ARGB4444
> 
>  drivers/gpu/drm/exynos/exynos_mixer.c | 246 +++++++++++++++++++++++++++++-----
>  drivers/gpu/drm/exynos/regs-mixer.h   |   1 +
>  2 files changed, 213 insertions(+), 34 deletions(-)
> 

_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/dri-devel

Re: [PATCH 0/5] drm/exynos: rework layer blending setup

[Joonyoung Shim]

Hi Tobias,

On 05/22/2015 05:44 AM, Tobias Jakobi wrote:
> Gentle reminder that I still haven't heard anything about the series by
> the Samsung guys ;)
> 

Sorry for late, i'm busy and i think whether it's possible any way for
this blending from exynos drm framework after atomic feature of exynos
merged.

Thanks.

> With best wishes,
> Tobias
> 
> 
> Tobias Jakobi wrote:
>> Hello,
>>
>> this is a rework of the layer blending setup in the Exynos DRM mixer. The current setup is static and spread out through the mixer code. This rework pushes all the configuration details into a layer_config array, which specifies the priority of each layer.
>>
>> Two arrays are currently found in the code, one for SoC versions with a video processor (VP) and one for SoC versions without VP. The VP gives us one additional layer, the video layer, which natively supports the NV12/NV21 pixelformat.
>>
>> The blending setup roughly works like this:
>> 1) Find the bottom-most enabled layer. Disable all blending for this layer. This is done because we currently don't expose modification of the mixer background to userspace. Once this is done we can add more flexibility here.
>> 2) Find the next enabled layer in our layer stack. If the layer has a framebuffer with an alpha-pixelformat attached, enable blending for this layer. If not, disable blending.
>> 3) Iterate (2) until all enabled layers are processed.
>>
>> The series has been tested on a Hardkernel Odroid-X2 (Exynos4412, which has a VP).
>>
>> If you want to use libdrm's modetest to check the series, please apply patches [1] and [2]. This should make it possible to also test a plane with NV12 format (which is located 'behind' the primary plane).
>>
>> With best wishes,
>> Tobias
>>
>> [1] https://patchwork.kernel.org/patch/6349241/
>> [2] https://patchwork.kernel.org/patch/6349261/
>>
>>
>> Tobias Jakobi (5):
>>   drm/exynos: mixer: refactor layer setup
>>   drm/exynos: mixer: introduce mixer_layer_blending()
>>   drm/exynos: mixer: remove all static blending setup
>>   drm/exynos: mixer: do blending setup in mixer_cfg_layer()
>>   drm/exynos: mixer: also allow ARGB1555 and ARGB4444
>>
>>  drivers/gpu/drm/exynos/exynos_mixer.c | 246 +++++++++++++++++++++++++++++-----
>>  drivers/gpu/drm/exynos/regs-mixer.h   |   1 +
>>  2 files changed, 213 insertions(+), 34 deletions(-)
>>
> 
> 

Re: [PATCH 0/5] drm/exynos: rework layer blending setup

[Tobias Jakobi]

Hello Joonyoung,

On 2015-05-22 06:04, Joonyoung Shim wrote:
> Hi Tobias,
> 
> On 05/22/2015 05:44 AM, Tobias Jakobi wrote:
>> Gentle reminder that I still haven't heard anything about the series 
>> by
>> the Samsung guys ;)
>> 
> 
> Sorry for late, i'm busy and i think whether it's possible any way for
> this blending from exynos drm framework after atomic feature of exynos
> merged.
the code is largely independent of Gustavo's patches. It doesn't really 
make a lot of difference if it's merged before or after atomic. In fact 
I've already tested it with and without atomic.

If possible I would like this to be merged now, so that it can go into 
4.2. I also have the patches fixing the zpos issue waiting, which I'm 
planning to base on Gustavo's next cleanup series. But for this both the 
atomic and the layer work series need to get merged first.

With best wishes,
Tobias


> 
> Thanks.
> 
>> With best wishes,
>> Tobias
>> 
>> 
>> Tobias Jakobi wrote:
>>> Hello,
>>> 
>>> this is a rework of the layer blending setup in the Exynos DRM mixer. 
>>> The current setup is static and spread out through the mixer code. 
>>> This rework pushes all the configuration details into a layer_config 
>>> array, which specifies the priority of each layer.
>>> 
>>> Two arrays are currently found in the code, one for SoC versions with 
>>> a video processor (VP) and one for SoC versions without VP. The VP 
>>> gives us one additional layer, the video layer, which natively 
>>> supports the NV12/NV21 pixelformat.
>>> 
>>> The blending setup roughly works like this:
>>> 1) Find the bottom-most enabled layer. Disable all blending for this 
>>> layer. This is done because we currently don't expose modification of 
>>> the mixer background to userspace. Once this is done we can add more 
>>> flexibility here.
>>> 2) Find the next enabled layer in our layer stack. If the layer has a 
>>> framebuffer with an alpha-pixelformat attached, enable blending for 
>>> this layer. If not, disable blending.
>>> 3) Iterate (2) until all enabled layers are processed.
>>> 
>>> The series has been tested on a Hardkernel Odroid-X2 (Exynos4412, 
>>> which has a VP).
>>> 
>>> If you want to use libdrm's modetest to check the series, please 
>>> apply patches [1] and [2]. This should make it possible to also test 
>>> a plane with NV12 format (which is located 'behind' the primary 
>>> plane).
>>> 
>>> With best wishes,
>>> Tobias
>>> 
>>> [1] https://patchwork.kernel.org/patch/6349241/
>>> [2] https://patchwork.kernel.org/patch/6349261/
>>> 
>>> 
>>> Tobias Jakobi (5):
>>>   drm/exynos: mixer: refactor layer setup
>>>   drm/exynos: mixer: introduce mixer_layer_blending()
>>>   drm/exynos: mixer: remove all static blending setup
>>>   drm/exynos: mixer: do blending setup in mixer_cfg_layer()
>>>   drm/exynos: mixer: also allow ARGB1555 and ARGB4444
>>> 
>>>  drivers/gpu/drm/exynos/exynos_mixer.c | 246 
>>> +++++++++++++++++++++++++++++-----
>>>  drivers/gpu/drm/exynos/regs-mixer.h   |   1 +
>>>  2 files changed, 213 insertions(+), 34 deletions(-)
>>> 
>> 
>> 

Re: [PATCH 1/5] drm/exynos: mixer: refactor layer setup

[Joonyoung Shim]

On 05/06/2015 10:36 PM, Tobias Jakobi wrote:
> 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 | 90 ++++++++++++++++++++++++++++++-----
>  1 file changed, 79 insertions(+), 11 deletions(-)
> 
> diff --git a/drivers/gpu/drm/exynos/exynos_mixer.c b/drivers/gpu/drm/exynos/exynos_mixer.c
> index 4a1656b..e4a5e76 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_config {
> +	unsigned int index;
> +	unsigned int priority;

I think layer index and priority information can go to struct
exynos_drm_plane.

> +};
> +
>  enum mixer_version_id {
>  	MXR_VER_0_0_0_16,
>  	MXR_VER_16_0_33_0,
> @@ -75,6 +80,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 +102,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,			/* layer0 */
> +		.priority = 1
> +	}, {
> +		.index = 1,			/* layer1 */
> +		.priority = 2
> +	}
> +};
> +
> +static const struct layer_config vp_layer_config[] = {
> +	{
> +		.index = 2,			/* video layer */
> +		.priority = 1
> +	}, {
> +		.index = 0,			/* layer0 */
> +		.priority = 2
> +	}, {
> +		.index = 1,			/* layer1 */
> +		.priority = 3
> +	}
> +};
> +

I feel there is no reason to split config, actually video layer exists
and video layer isn't enabled if no vp even if we use 3 layer.

Re: [PATCH 2/5] drm/exynos: mixer: introduce mixer_layer_blending()

[Joonyoung Shim]

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@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?

> +
> +		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) ...
_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/dri-devel

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

[Joonyoung Shim]

On 05/06/2015 10:36 PM, Tobias Jakobi wrote:
> 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 3ab7a01..430d10c 100644
> --- a/drivers/gpu/drm/exynos/exynos_mixer.c
> +++ b/drivers/gpu/drm/exynos/exynos_mixer.c
> @@ -177,6 +177,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);
> @@ -513,6 +525,7 @@ static void mixer_cfg_rgb_fmt(struct mixer_context *ctx, unsigned int height)
>  static void mixer_cfg_layer(struct mixer_context *ctx, unsigned int win, bool enable)
>  {
>  	struct mixer_resources *res = &ctx->mixer_res;
> +	unsigned int enable_state;
>  	u32 val = enable ? ~0 : 0;
>  
>  	switch (win) {
> @@ -530,6 +543,16 @@ static void mixer_cfg_layer(struct mixer_context *ctx, unsigned int win, bool en
>  		}
>  		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);

I think way to remove to get enable_state of plane everytime, maybe
using atomic state information?

> +
> +	/* Layer configuration has changed, update blending setup. */
> +	mixer_layer_blending(ctx, enable_state);
>  }
>  
>  static void mixer_run(struct mixer_context *ctx)
> 

_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/dri-devel

Re: [PATCH 0/5] drm/exynos: rework layer blending setup

[Joonyoung Shim]

Hi Tobias,

On 05/22/2015 05:07 PM, Tobias Jakobi wrote:
> Hello Joonyoung,
> 
> On 2015-05-22 06:04, Joonyoung Shim wrote:
>> Hi Tobias,
>>
>> On 05/22/2015 05:44 AM, Tobias Jakobi wrote:
>>> Gentle reminder that I still haven't heard anything about the series by
>>> the Samsung guys ;)
>>>
>>
>> Sorry for late, i'm busy and i think whether it's possible any way for
>> this blending from exynos drm framework after atomic feature of exynos
>> merged.
> the code is largely independent of Gustavo's patches. It doesn't really make a lot of difference if it's merged before or after atomic. In fact I've already tested it with and without atomic.
> 
> If possible I would like this to be merged now, so that it can go into 4.2. I also have the patches fixing the zpos issue waiting, which I'm planning to base on Gustavo's next cleanup series. But for this both the atomic and the layer work series need to get merged first.
> 

Sorry for not detailed review, i don't know what is Inki's opinion but i
feel they need to be improved more.

Please run ./scripts/checkpatch.pl before posting and could you use word
wrap? it's too long so difficult to read.

Thanks.
_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/dri-devel

Re: [PATCH 1/5] drm/exynos: mixer: refactor layer setup

[Tobias Jakobi]

Hello Joonyoung,

On 2015-05-22 11:12, Joonyoung Shim wrote:
> On 05/06/2015 10:36 PM, Tobias Jakobi wrote:
>> 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 | 90 
>> ++++++++++++++++++++++++++++++-----
>>  1 file changed, 79 insertions(+), 11 deletions(-)
>> 
>> diff --git a/drivers/gpu/drm/exynos/exynos_mixer.c 
>> b/drivers/gpu/drm/exynos/exynos_mixer.c
>> index 4a1656b..e4a5e76 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_config {
>> +	unsigned int index;
>> +	unsigned int priority;
> 
> I think layer index and priority information can go to struct
> exynos_drm_plane.
No, because of two reasons:
- plane configuration is done at run-time while this here is const 
static
- this is specific to the mixer


> 
>> +};
>> +
>>  enum mixer_version_id {
>>  	MXR_VER_0_0_0_16,
>>  	MXR_VER_16_0_33_0,
>> @@ -75,6 +80,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 +102,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,			/* layer0 */
>> +		.priority = 1
>> +	}, {
>> +		.index = 1,			/* layer1 */
>> +		.priority = 2
>> +	}
>> +};
>> +
>> +static const struct layer_config vp_layer_config[] = {
>> +	{
>> +		.index = 2,			/* video layer */
>> +		.priority = 1
>> +	}, {
>> +		.index = 0,			/* layer0 */
>> +		.priority = 2
>> +	}, {
>> +		.index = 1,			/* layer1 */
>> +		.priority = 3
>> +	}
>> +};
>> +
> 
> I feel there is no reason to split config, actually video layer exists
> and video layer isn't enabled if no vp even if we use 3 layer.
Well, as you point out yourself the current code is totally broken. Even 
if we don't support the video processor we still expose the 
corresponding DRM plane to userspace, but then just ignore any 
operations to it. This needs fixing (!), and my series is a first step 
in that direction.


With best wishes,
Tobias

Re: [PATCH 2/5] drm/exynos: mixer: introduce mixer_layer_blending()

[Tobias Jakobi]

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@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

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

[Tobias Jakobi]

Hello Joonyoung,

On 2015-05-22 11:12, Joonyoung Shim wrote:
> On 05/06/2015 10:36 PM, Tobias Jakobi wrote:
>> 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 3ab7a01..430d10c 100644
>> --- a/drivers/gpu/drm/exynos/exynos_mixer.c
>> +++ b/drivers/gpu/drm/exynos/exynos_mixer.c
>> @@ -177,6 +177,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);
>> @@ -513,6 +525,7 @@ static void mixer_cfg_rgb_fmt(struct mixer_context 
>> *ctx, unsigned int height)
>>  static void mixer_cfg_layer(struct mixer_context *ctx, unsigned int 
>> win, bool enable)
>>  {
>>  	struct mixer_resources *res = &ctx->mixer_res;
>> +	unsigned int enable_state;
>>  	u32 val = enable ? ~0 : 0;
>> 
>>  	switch (win) {
>> @@ -530,6 +543,16 @@ static void mixer_cfg_layer(struct mixer_context 
>> *ctx, unsigned int win, bool en
>>  		}
>>  		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);
> 
> I think way to remove to get enable_state of plane everytime, maybe
> using atomic state information?
I had the same idea, but I don't know even about the atomic 
infrastructure to evaluate if this is possible. I guess Gustavo would be 
the one who could answer this.



> 
>> +
>> +	/* Layer configuration has changed, update blending setup. */
>> +	mixer_layer_blending(ctx, enable_state);
>>  }
>> 
>>  static void mixer_run(struct mixer_context *ctx)
>> 

With best wishes,
Tobias

_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/dri-devel

Re: [PATCH 1/5] drm/exynos: mixer: refactor layer setup

[Tobias Jakobi]

On 2015-05-22 11:12, Joonyoung Shim wrote:
> On 05/06/2015 10:36 PM, Tobias Jakobi wrote:
>> 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 | 90 
>> ++++++++++++++++++++++++++++++-----
>>  1 file changed, 79 insertions(+), 11 deletions(-)
>> 
>> diff --git a/drivers/gpu/drm/exynos/exynos_mixer.c 
>> b/drivers/gpu/drm/exynos/exynos_mixer.c
>> index 4a1656b..e4a5e76 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_config {
>> +	unsigned int index;
>> +	unsigned int priority;
> 
> I think layer index and priority information can go to struct
> exynos_drm_plane.
> 
>> +};
>> +
>>  enum mixer_version_id {
>>  	MXR_VER_0_0_0_16,
>>  	MXR_VER_16_0_33_0,
>> @@ -75,6 +80,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 +102,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,			/* layer0 */
>> +		.priority = 1
>> +	}, {
>> +		.index = 1,			/* layer1 */
>> +		.priority = 2
>> +	}
>> +};
>> +
>> +static const struct layer_config vp_layer_config[] = {
>> +	{
>> +		.index = 2,			/* video layer */
>> +		.priority = 1
>> +	}, {
>> +		.index = 0,			/* layer0 */
>> +		.priority = 2
>> +	}, {
>> +		.index = 1,			/* layer1 */
>> +		.priority = 3
>> +	}
>> +};
>> +
> 
> I feel there is no reason to split config, actually video layer exists
> and video layer isn't enabled if no vp even if we use 3 layer.
Maybe I should add this here. Both default_layer_config and 
vp_layer_config function as default layer configurations. My plan is to 
expose custom configuration through a exynosdrm module parameter. The 
zpos property is immutable, so we can't modify it anymore after mixer 
probing (I understand that the immutabilility is a requirement of 
atomic). But we still can allow the user to supply his configuraton 
through the module parameter approach.


With best wishes,
Tobias