[PATCH v2 2/5] drm/exynos: mixer: introduce mixer_layer_blending()
2015-11-24 2:44 GMT+09:00 Tobias Jakobi :
> Hey Inki,
>
>
> Inki Dae wrote:
>>
>>
>> 2015ë
11ì 23ì¼ 01:09ì 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
>>> ---
>>> drivers/gpu/drm/exynos/exynos_mixer.c | 88
>>> +++
>>> drivers/gpu/drm/exynos/regs-mixer.h | 1 +
>>> 2 files changed, 89 insertions(+)
>>>
>>> diff --git a/drivers/gpu/drm/exynos/exynos_mixer.c
>>> b/drivers/gpu/drm/exynos/exynos_mixer.c
>>> index 2c1cea3..ec77aad 100644
>>> --- a/drivers/gpu/drm/exynos/exynos_mixer.c
>>> +++ b/drivers/gpu/drm/exynos/exynos_mixer.c
>>> @@ -174,6 +174,21 @@ 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)
>>> +{
>>> +const struct drm_plane_state *state = ctx->planes[win].base.state;
>>> +const struct drm_framebuffer *fb = state->fb;
>>> +
>>> +switch (fb->pixel_format) {
>>> +case DRM_FORMAT_ARGB:
>>> +case DRM_FORMAT_ARGB1555:
>>> +case DRM_FORMAT_ARGB:
>>> +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);
>>> @@ -331,6 +346,79 @@ static void mixer_layer_priority(struct mixer_context
>>> *ctx)
>>> mixer_reg_write(>mixer_res, MXR_LAYER_CFG, val);
>>> }
>>>
>>> +/* Configure blending for bottom-most layer. */
>>> +static void mixer_bottom_layer(struct mixer_context *ctx,
>>> +const struct layer_cfg *cfg)
>>> +{
>>> +u32 val;
>>> +struct mixer_resources *res = >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_cfg *cfg)
>>> +{
>>> +u32 val;
>>> +struct mixer_resources *res = >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. */
>>
>> Looks like above comment isn't related to graphics layer.
> Why do you think so? Because it certainly is.
Below line configures graphics layer not video layer. Why did you
comment it about video layer?
>
>
>>> +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);
>>
>> Above 'if statement' wouldn't be called because cfg->index has always a
>> value less than 2
>> so it should be removed.
> Can you explain why you think that index is always < 2 here?
Please, see mixer_layer_blending function. In this function, a local
variable, a bottom_layer is declared and
this has false as a default. At first loop, mixer_general_layer
function won't be called
but at second loop like below,
for (i = 0; i < ctx->num_layer; ++i) {
index = ctx->layer_cfg[i].index;
/* Skip layer if it's not enabled. */
if (!(enable_state & (1 << index)))
continue;
/* Bottom layer needs special handling. */
if (bottom_layer) {
mixer_general_layer(ctx, >layer_cfg[i]);<-
and second call.
} else {
mixer_bottom_layer(ctx, >layer_cfg[i]); <-- first
[PATCH v2 2/5] drm/exynos: mixer: introduce mixer_layer_blending()
Hey Inki,
Inki Dae wrote:
>
>
> 2015ë
11ì 23ì¼ 01:09ì 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
>> ---
>> drivers/gpu/drm/exynos/exynos_mixer.c | 88
>> +++
>> drivers/gpu/drm/exynos/regs-mixer.h | 1 +
>> 2 files changed, 89 insertions(+)
>>
>> diff --git a/drivers/gpu/drm/exynos/exynos_mixer.c
>> b/drivers/gpu/drm/exynos/exynos_mixer.c
>> index 2c1cea3..ec77aad 100644
>> --- a/drivers/gpu/drm/exynos/exynos_mixer.c
>> +++ b/drivers/gpu/drm/exynos/exynos_mixer.c
>> @@ -174,6 +174,21 @@ 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)
>> +{
>> +const struct drm_plane_state *state = ctx->planes[win].base.state;
>> +const struct drm_framebuffer *fb = state->fb;
>> +
>> +switch (fb->pixel_format) {
>> +case DRM_FORMAT_ARGB:
>> +case DRM_FORMAT_ARGB1555:
>> +case DRM_FORMAT_ARGB:
>> +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);
>> @@ -331,6 +346,79 @@ static void mixer_layer_priority(struct mixer_context
>> *ctx)
>> mixer_reg_write(>mixer_res, MXR_LAYER_CFG, val);
>> }
>>
>> +/* Configure blending for bottom-most layer. */
>> +static void mixer_bottom_layer(struct mixer_context *ctx,
>> +const struct layer_cfg *cfg)
>> +{
>> +u32 val;
>> +struct mixer_resources *res = >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_cfg *cfg)
>> +{
>> +u32 val;
>> +struct mixer_resources *res = >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. */
>
> Looks like above comment isn't related to graphics layer.
Why do you think so? Because it certainly is.
>> +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);
>
> Above 'if statement' wouldn't be called because cfg->index has always a value
> less than 2
> so it should be removed.
Can you explain why you think that index is always < 2 here?
>> +} 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)
>> +{
>> +unsigned int i, index;
>> +bool bottom_layer = false;
>> +
>> +for (i = 0; i < ctx->num_layer; ++i) {
>> +index = ctx->layer_cfg[i].index;
>> +
>> +/* Skip layer if it's not enabled. */
>> +if (!(enable_state & (1 << index)))
>> +continue;
>> +
>> +/* Bottom layer needs special handling. */
>> +if (bottom_layer) {
>> +mixer_general_layer(ctx, >layer_cfg[i]);
>> +} else {
>> +mixer_bottom_layer(ctx,
[PATCH v2 2/5] drm/exynos: mixer: introduce mixer_layer_blending()
2015ë
11ì 23ì¼ 01:09ì 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
> ---
> drivers/gpu/drm/exynos/exynos_mixer.c | 88
> +++
> drivers/gpu/drm/exynos/regs-mixer.h | 1 +
> 2 files changed, 89 insertions(+)
>
> diff --git a/drivers/gpu/drm/exynos/exynos_mixer.c
> b/drivers/gpu/drm/exynos/exynos_mixer.c
> index 2c1cea3..ec77aad 100644
> --- a/drivers/gpu/drm/exynos/exynos_mixer.c
> +++ b/drivers/gpu/drm/exynos/exynos_mixer.c
> @@ -174,6 +174,21 @@ 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)
> +{
> + const struct drm_plane_state *state = ctx->planes[win].base.state;
> + const struct drm_framebuffer *fb = state->fb;
> +
> + switch (fb->pixel_format) {
> + case DRM_FORMAT_ARGB:
> + case DRM_FORMAT_ARGB1555:
> + case DRM_FORMAT_ARGB:
> + 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);
> @@ -331,6 +346,79 @@ static void mixer_layer_priority(struct mixer_context
> *ctx)
> mixer_reg_write(>mixer_res, MXR_LAYER_CFG, val);
> }
>
> +/* Configure blending for bottom-most layer. */
> +static void mixer_bottom_layer(struct mixer_context *ctx,
> + const struct layer_cfg *cfg)
> +{
> + u32 val;
> + struct mixer_resources *res = >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_cfg *cfg)
> +{
> + u32 val;
> + struct mixer_resources *res = >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. */
Looks like above comment isn't related to graphics layer.
> + 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);
Above 'if statement' wouldn't be called because cfg->index has always a value
less than 2
so it should be removed.
> + } 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)
> +{
> + unsigned int i, index;
> + bool bottom_layer = false;
> +
> + for (i = 0; i < ctx->num_layer; ++i) {
> + index = ctx->layer_cfg[i].index;
> +
> + /* Skip layer if it's not enabled. */
> + if (!(enable_state & (1 << index)))
> + continue;
> +
> + /* Bottom layer needs special handling. */
> + if (bottom_layer) {
> + mixer_general_layer(ctx, >layer_cfg[i]);
> + } else {
> + mixer_bottom_layer(ctx, >layer_cfg[i]);
> + bottom_layer = true;
> + }
I think above codes could be more cleanned up like below if I understood
correctly,
if (cfg->index == 2) {
[PATCH v2 2/5] drm/exynos: mixer: introduce mixer_layer_blending()
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
---
drivers/gpu/drm/exynos/exynos_mixer.c | 88 +++
drivers/gpu/drm/exynos/regs-mixer.h | 1 +
2 files changed, 89 insertions(+)
diff --git a/drivers/gpu/drm/exynos/exynos_mixer.c
b/drivers/gpu/drm/exynos/exynos_mixer.c
index 2c1cea3..ec77aad 100644
--- a/drivers/gpu/drm/exynos/exynos_mixer.c
+++ b/drivers/gpu/drm/exynos/exynos_mixer.c
@@ -174,6 +174,21 @@ 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)
+{
+ const struct drm_plane_state *state = ctx->planes[win].base.state;
+ const struct drm_framebuffer *fb = state->fb;
+
+ switch (fb->pixel_format) {
+ case DRM_FORMAT_ARGB:
+ case DRM_FORMAT_ARGB1555:
+ case DRM_FORMAT_ARGB:
+ 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);
@@ -331,6 +346,79 @@ static void mixer_layer_priority(struct mixer_context *ctx)
mixer_reg_write(>mixer_res, MXR_LAYER_CFG, val);
}
+/* Configure blending for bottom-most layer. */
+static void mixer_bottom_layer(struct mixer_context *ctx,
+ const struct layer_cfg *cfg)
+{
+ u32 val;
+ struct mixer_resources *res = >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_cfg *cfg)
+{
+ u32 val;
+ struct mixer_resources *res = >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)
+{
+ unsigned int i, index;
+ bool bottom_layer = false;
+
+ for (i = 0; i < ctx->num_layer; ++i) {
+ index = ctx->layer_cfg[i].index;
+
+ /* Skip layer if it's not enabled. */
+ if (!(enable_state & (1 << index)))
+ continue;
+
+ /* Bottom layer needs special handling. */
+ if (bottom_layer) {
+ mixer_general_layer(ctx, >layer_cfg[i]);
+ } else {
+ mixer_bottom_layer(ctx, >layer_cfg[i]);
+ bottom_layer = true;
+ }
+ }
+}
+
static void mixer_vsync_set_update(struct mixer_context *ctx, bool enable)
{
struct mixer_resources *res = >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
