RE: [PATCH] OMAP3630: DSS2: Enable Pre-Multiplied Alpha Support
On Wed, 2010-11-03 at 12:57 +0100, ext Taneja, Archit wrote:
Hi,
Tomi Valkeinen wrote:
On Wed, 2010-11-03 at 08:57 +0100, ext Taneja, Archit wrote:
Hi,
linux-omap-ow...@vger.kernel.org wrote:
Alpha Support
[snip]
+static void _dispc_set_pre_mult_alpha(enum omap_plane plane, bool
+enable) { + if (!dss_has_feature(FEAT_PREMUL_ALPHA))
+return;
+
+BUG_ON(!dss_has_feature(FEAT_GLOBAL_ALPHA_VID1)
+plane == OMAP_DSS_VIDEO1);
What is the rationale for having the function return, if
FEAT_PREMUL_ALPHA is not supported, but BUG if plane is video1 and
GLOBAL_ALPHA_VID1 is not supported?
Premultiplied alpha is available on omap36xx and above, but on 36xx
since global alpha itself isn't supported for video1 then writing to
the pre multiplied alpha bit is incorrect.
It could have been possible to make a new feature like
FEAT_PRE_MULT_VID1 but I think its redundant as FEAT_GLOBAL_ALPHA_VID1
is enough to determine if we should set the pre multiplied
alpha bit for video1 plane or not.
I was referring to the first check using return, and the
second using BUG(). If nobody is supposed to call that function when
!dss_has_feature(FEAT_GLOBAL_ALPHA_VID1) plane == OMAP_DSS_VIDEO1)
then why is it ok to call that function when
!dss_has_feature(FEAT_PREMUL_ALPHA)
Shouldn't they both be either returns or BUGs?
If you see the current code, _dispc_setup_global_alpha() does the same thing,
and in the call to it in _dispc_setup_plane() is:
...
...
if (plane != OMAP_DSS_VIDEO1)
_dispc_setup_global_alpha(plane, global_alpha);
...
...
So, I guess this BUG_ON is probably not required for both setup_global_alpha
and setup_pre_multiplied_alpha if you take care while calling these functions
from _dispc_setup_plane().
BUGs should be used exactly in those cases, where you are sure you're
always calling the function with right parameters. BUGs are never meant
to trigger, so they should be used as a safeguard against broken
changes.
But this is the same with _dispc_set_scaling(), _dispc_set_vid_size() etc
where
we have a BUG_ON(plane == OMAP_DSS_GFX) even when we take care of not calling
it
for the GFX pipe.
Yes, I'm sure there are functions that are not perfect =).
So to summarize: if _dispc_set_pre_mult_alpha() is only called from
inside DSS, and it will never be called with a wrong plane argument,
then it should only use BUGs, not return.
Now, the question is, is it better to do the argument checking in the
caller or in the callee? In the case of DSS internal functions it's
perhaps easier if the callee does the checking. So:
_dispc_setup_plane() should always call _dispc_setup_global_alpha(), as
future OMAP versions may support global alpha on any overlay, and I
don't think it's _dispc_setup_plane's job to do that check as the checks
may be lenghty.
_dispc_setup_global_alpha() would do the check if global_alpha is
supported on that plane, and do nothing if not (or, return an error,
which _dispc_setup_plane() would ignore, but I don't think that's
needed).
And similarly for premult alpha. And I'm not saying all these have to be
changed now, but as general thoughts about the matter.
Tomi
--
To unsubscribe from this list: send the line unsubscribe linux-omap in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
RE: [PATCH] OMAP3630: DSS2: Enable Pre-Multiplied Alpha Support
Hi,
linux-omap-ow...@vger.kernel.org wrote:
Alpha Support
[snip]
+static void _dispc_set_pre_mult_alpha(enum omap_plane plane, bool +enable) {
+if (!dss_has_feature(FEAT_PREMUL_ALPHA))
+return;
+
+BUG_ON(!dss_has_feature(FEAT_GLOBAL_ALPHA_VID1)
+plane == OMAP_DSS_VIDEO1);
What is the rationale for having the function return, if
FEAT_PREMUL_ALPHA is not supported, but BUG if plane is video1 and
GLOBAL_ALPHA_VID1 is not supported?
Premultiplied alpha is available on omap36xx and above, but on 36xx
since global alpha itself isn't supported for video1 then writing to
the pre multiplied alpha bit is incorrect.
It could have been possible to make a new feature like FEAT_PRE_MULT_VID1
but I think its redundant as FEAT_GLOBAL_ALPHA_VID1 is enough to determine
if we should set the pre multiplied alpha bit for video1 plane or not.
+
+REG_FLD_MOD(dispc_reg_att[plane], enable ? 1 : 0, 28, 28); } +
static void _dispc_setup_global_alpha(enum omap_plane plane, u8
global_alpha) { if (!dss_has_feature(FEAT_GLOBAL_ALPHA))
@@ -1507,7 +1518,8 @@ static int _dispc_setup_plane(enum omap_plane plane,
bool ilace, enum omap_dss_rotation_type rotation_type,
u8 rotation, int mirror,
-u8 global_alpha)
+u8 global_alpha,
+u8 pre_mult_alpha)
{
const int maxdownscale = cpu_is_omap34xx() ? 4 : 2; bool five_taps
= 0;
@@ -1693,8 +1705,11 @@ static int _dispc_setup_plane(enum omap_plane plane,
_dispc_set_rotation_attrs(plane, rotation, mirror, color_mode);
-if (plane != OMAP_DSS_VIDEO1)
+if ((plane != OMAP_DSS_VIDEO1) ||
+dss_has_feature(FEAT_GLOBAL_ALPHA_VID1)) {
+_dispc_set_pre_mult_alpha(plane, pre_mult_alpha);
_dispc_setup_global_alpha(plane, global_alpha);
+}
return 0;
}
@@ -3139,7 +3154,8 @@ int dispc_setup_plane(enum omap_plane plane,
enum omap_color_mode color_mode,
bool ilace,
enum omap_dss_rotation_type rotation_type,
- u8 rotation, bool mirror, u8 global_alpha)
+ u8 rotation, bool mirror, u8 global_alpha, +
u8
pre_mult_alpha) {
int r = 0;
@@ -3161,7 +3177,8 @@ int dispc_setup_plane(enum omap_plane plane,
color_mode, ilace, rotation_type,
rotation, mirror,
- global_alpha);
+ global_alpha,
+ pre_mult_alpha);
enable_clocks(0);
diff --git a/drivers/video/omap2/dss/dss.h
b/drivers/video/omap2/dss/dss.h index 5c7940d..2bb515c 100644
--- a/drivers/video/omap2/dss/dss.h
+++ b/drivers/video/omap2/dss/dss.h
@@ -359,7 +359,8 @@ int dispc_setup_plane(enum omap_plane plane,
bool ilace, enum omap_dss_rotation_type rotation_type,
u8 rotation, bool mirror,
- u8 global_alpha);
+ u8 global_alpha,
+ u8 pre_mult_alpha);
bool dispc_go_busy(enum omap_channel channel); void dispc_go(enum
omap_channel channel); diff --git
a/drivers/video/omap2/dss/dss_features.c
b/drivers/video/omap2/dss/dss_features.c
index 867f68d..1abb8c5 100644
--- a/drivers/video/omap2/dss/dss_features.c
+++ b/drivers/video/omap2/dss/dss_features.c
@@ -134,7 +134,7 @@ static struct omap_dss_features omap2_dss_features = {
};
/* OMAP3 DSS Features */
-static struct omap_dss_features omap3_dss_features = {
+static struct omap_dss_features omap3430_dss_features = {
.reg_fields = omap3_dss_reg_fields,
.num_reg_fields = ARRAY_SIZE(omap3_dss_reg_fields),
@@ -146,6 +146,18 @@ static struct omap_dss_features omap3_dss_features = {
.supported_color_modes = omap3_dss_supported_color_modes, };
+static struct omap_dss_features omap3630_dss_features = {
+.reg_fields = omap3_dss_reg_fields,
+.num_reg_fields = ARRAY_SIZE(omap3_dss_reg_fields), +
+.has_feature= FEAT_GLOBAL_ALPHA | FEAT_PREMUL_ALPHA, +
+.num_mgrs = 2,
+.num_ovls = 3,
+.supported_displays = omap3_dss_supported_displays,
+.supported_color_modes = omap3_dss_supported_color_modes, }; +
/* Functions returning values related to a DSS feature */ int
dss_feat_get_num_mgrs(void) { @@ -186,6 +198,8 @@ void
dss_features_init(void) { if (cpu_is_omap24xx())
omap_current_dss_features = omap2_dss_features;
-else
-omap_current_dss_features = omap3_dss_features; + else if
(cpu_is_omap3630()) +omap_current_dss_features =
omap3630_dss_features;
+else if (cpu_is_omap34xx()) + omap_current_dss_features =
omap3430_dss_features; }
diff --git a/drivers/video/omap2/dss/dss_features.h
b/drivers/video/omap2/dss/dss_features.h
index cb231ea..f287da8 100644
---
RE: [PATCH] OMAP3630: DSS2: Enable Pre-Multiplied Alpha Support
On Wed, 2010-11-03 at 08:57 +0100, ext Taneja, Archit wrote:
Hi,
linux-omap-ow...@vger.kernel.org wrote:
Alpha Support
[snip]
+static void _dispc_set_pre_mult_alpha(enum omap_plane plane, bool
+enable) {
+ if (!dss_has_feature(FEAT_PREMUL_ALPHA))
+ return;
+
+ BUG_ON(!dss_has_feature(FEAT_GLOBAL_ALPHA_VID1)
+ plane == OMAP_DSS_VIDEO1);
What is the rationale for having the function return, if
FEAT_PREMUL_ALPHA is not supported, but BUG if plane is video1 and
GLOBAL_ALPHA_VID1 is not supported?
Premultiplied alpha is available on omap36xx and above, but on 36xx
since global alpha itself isn't supported for video1 then writing to
the pre multiplied alpha bit is incorrect.
It could have been possible to make a new feature like FEAT_PRE_MULT_VID1
but I think its redundant as FEAT_GLOBAL_ALPHA_VID1 is enough to determine
if we should set the pre multiplied alpha bit for video1 plane or not.
I was referring to the first check using return, and the second using
BUG(). If nobody is supposed to call that function when
!dss_has_feature(FEAT_GLOBAL_ALPHA_VID1) plane == OMAP_DSS_VIDEO1)
then why is it ok to call that function when
!dss_has_feature(FEAT_PREMUL_ALPHA)
Shouldn't they both be either returns or BUGs?
Tomi
--
To unsubscribe from this list: send the line unsubscribe linux-omap in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
RE: [PATCH] OMAP3630: DSS2: Enable Pre-Multiplied Alpha Support
Hi,
Tomi Valkeinen wrote:
On Wed, 2010-11-03 at 08:57 +0100, ext Taneja, Archit wrote:
Hi,
linux-omap-ow...@vger.kernel.org wrote:
Alpha Support
[snip]
+static void _dispc_set_pre_mult_alpha(enum omap_plane plane, bool
+enable) { + if (!dss_has_feature(FEAT_PREMUL_ALPHA))
+ return;
+
+ BUG_ON(!dss_has_feature(FEAT_GLOBAL_ALPHA_VID1)
+ plane == OMAP_DSS_VIDEO1);
What is the rationale for having the function return, if
FEAT_PREMUL_ALPHA is not supported, but BUG if plane is video1 and
GLOBAL_ALPHA_VID1 is not supported?
Premultiplied alpha is available on omap36xx and above, but on 36xx
since global alpha itself isn't supported for video1 then writing to
the pre multiplied alpha bit is incorrect.
It could have been possible to make a new feature like
FEAT_PRE_MULT_VID1 but I think its redundant as FEAT_GLOBAL_ALPHA_VID1
is enough to determine if we should set the pre multiplied
alpha bit for video1 plane or not.
I was referring to the first check using return, and the
second using BUG(). If nobody is supposed to call that function when
!dss_has_feature(FEAT_GLOBAL_ALPHA_VID1) plane == OMAP_DSS_VIDEO1)
then why is it ok to call that function when
!dss_has_feature(FEAT_PREMUL_ALPHA)
Shouldn't they both be either returns or BUGs?
If you see the current code, _dispc_setup_global_alpha() does the same thing,
and in the call to it in _dispc_setup_plane() is:
...
...
if (plane != OMAP_DSS_VIDEO1)
_dispc_setup_global_alpha(plane, global_alpha);
...
...
So, I guess this BUG_ON is probably not required for both setup_global_alpha
and setup_pre_multiplied_alpha if you take care while calling these functions
from _dispc_setup_plane().
But this is the same with _dispc_set_scaling(), _dispc_set_vid_size() etc where
we have a BUG_ON(plane == OMAP_DSS_GFX) even when we take care of not calling it
for the GFX pipe.
Archit--
To unsubscribe from this list: send the line unsubscribe linux-omap in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
RE: [PATCH] OMAP3630: DSS2: Enable Pre-Multiplied Alpha Support
Hi,
Taneja, Archit wrote:
Hi,
Tomi Valkeinen wrote:
On Wed, 2010-11-03 at 08:57 +0100, ext Taneja, Archit wrote:
Hi,
linux-omap-ow...@vger.kernel.org wrote:
Alpha Support
[snip]
+static void _dispc_set_pre_mult_alpha(enum omap_plane plane, bool
+enable) { + if (!dss_has_feature(FEAT_PREMUL_ALPHA)) +
return;
+
+ BUG_ON(!dss_has_feature(FEAT_GLOBAL_ALPHA_VID1)
+ plane == OMAP_DSS_VIDEO1);
What is the rationale for having the function return, if
FEAT_PREMUL_ALPHA is not supported, but BUG if plane is video1 and
GLOBAL_ALPHA_VID1 is not supported?
Premultiplied alpha is available on omap36xx and above, but on 36xx
since global alpha itself isn't supported for video1 then writing to
the pre multiplied alpha bit is incorrect.
It could have been possible to make a new feature like
FEAT_PRE_MULT_VID1 but I think its redundant as
FEAT_GLOBAL_ALPHA_VID1 is enough to determine if we should set the
pre multiplied
alpha bit for video1 plane or not.
I was referring to the first check using return, and the second using
BUG(). If nobody is supposed to call that function when
!dss_has_feature(FEAT_GLOBAL_ALPHA_VID1) plane == OMAP_DSS_VIDEO1)
then why is it ok to call that function when
!dss_has_feature(FEAT_PREMUL_ALPHA)
Shouldn't they both be either returns or BUGs?
If you see the current code, _dispc_setup_global_alpha() does
the same thing, and in the call to it in _dispc_setup_plane() is:
...
...
if (plane != OMAP_DSS_VIDEO1)
_dispc_setup_global_alpha(plane, global_alpha); ... ...
So, I guess this BUG_ON is probably not required for both
setup_global_alpha and setup_pre_multiplied_alpha if you take
care while calling these functions from _dispc_setup_plane().
But this is the same with _dispc_set_scaling(),
_dispc_set_vid_size() etc where we have a BUG_ON(plane ==
OMAP_DSS_GFX) even when we take care of not calling it for
the GFX pipe.
Archit
[Samreen]
If this clarification is aligned, should I go ahead and post the patch
with the remaining review comments
Regards,
Samreen--
To unsubscribe from this list: send the line unsubscribe linux-omap in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH] OMAP3630: DSS2: Enable Pre-Multiplied Alpha Support
Hi,
On Wed, 2010-10-27 at 12:16 +0200, ext Samreen wrote:
From: Rajkumar N rajkumar.nagara...@ti.com
Enable dss to process color formats with pre-mulitplied alpha.
With this we can have alpha values defined for each pixel
and hence can have different blending values for each pixel.
sysfs entry has been created for this and pre-multiplied alpha
support is turned off by default.
Also, the check for '(plane != OMAP_DSS_VIDEO1)'
in _dispc_setup_plane has been replaced by the correct
dss_has_feature check
You're changing too many things in one patch. Please separate the
dss_feature changes and the actual implementation for the pre-multiplied
alpha.
Your naming of this feature is also bit varying: you have pre_mult,
PREMUL, pre_multiplication_alpha. The last one is quite wrong, as
the feature is premultiplied alpha.
Also a few comments inline.
Signed-off-by: Sudeep Basavaraj sudeep.basava...@ti.com
Signed-off-by: Rajkumar N rajkumar.nagara...@ti.com
Signed-off-by: Samreen samr...@ti.com
---
arch/arm/plat-omap/include/plat/display.h |1 +
drivers/video/omap2/dss/dispc.c | 25 +---
drivers/video/omap2/dss/dss.h |3 +-
drivers/video/omap2/dss/dss_features.c| 20 ++--
drivers/video/omap2/dss/dss_features.h|1 +
drivers/video/omap2/dss/manager.c |5 +++-
drivers/video/omap2/dss/overlay.c | 35
+
7 files changed, 81 insertions(+), 9 deletions(-)
diff --git a/arch/arm/plat-omap/include/plat/display.h
b/arch/arm/plat-omap/include/plat/display.h
index c915a66..d433baf 100644
--- a/arch/arm/plat-omap/include/plat/display.h
+++ b/arch/arm/plat-omap/include/plat/display.h
@@ -268,6 +268,7 @@ struct omap_overlay_info {
u16 out_width; /* if 0, out_width == width */
u16 out_height; /* if 0, out_height == height */
u8 global_alpha;
+ u8 pre_mult_alpha;
};
struct omap_overlay {
diff --git a/drivers/video/omap2/dss/dispc.c b/drivers/video/omap2/dss/dispc.c
index fa40fa5..f1d1797 100644
--- a/drivers/video/omap2/dss/dispc.c
+++ b/drivers/video/omap2/dss/dispc.c
@@ -773,6 +773,17 @@ static void _dispc_set_vid_size(enum omap_plane plane,
int width, int height)
dispc_write_reg(vsi_reg[plane-1], val);
}
+static void _dispc_set_pre_mult_alpha(enum omap_plane plane, bool enable)
+{
+ if (!dss_has_feature(FEAT_PREMUL_ALPHA))
+ return;
+
+ BUG_ON(!dss_has_feature(FEAT_GLOBAL_ALPHA_VID1)
+ plane == OMAP_DSS_VIDEO1);
What is the rationale for having the function return, if
FEAT_PREMUL_ALPHA is not supported, but BUG if plane is video1 and
GLOBAL_ALPHA_VID1 is not supported?
+
+ REG_FLD_MOD(dispc_reg_att[plane], enable ? 1 : 0, 28, 28);
+}
+
static void _dispc_setup_global_alpha(enum omap_plane plane, u8 global_alpha)
{
if (!dss_has_feature(FEAT_GLOBAL_ALPHA))
@@ -1507,7 +1518,8 @@ static int _dispc_setup_plane(enum omap_plane plane,
bool ilace,
enum omap_dss_rotation_type rotation_type,
u8 rotation, int mirror,
- u8 global_alpha)
+ u8 global_alpha,
+ u8 pre_mult_alpha)
{
const int maxdownscale = cpu_is_omap34xx() ? 4 : 2;
bool five_taps = 0;
@@ -1693,8 +1705,11 @@ static int _dispc_setup_plane(enum omap_plane plane,
_dispc_set_rotation_attrs(plane, rotation, mirror, color_mode);
- if (plane != OMAP_DSS_VIDEO1)
+ if ((plane != OMAP_DSS_VIDEO1) ||
+ dss_has_feature(FEAT_GLOBAL_ALPHA_VID1)) {
+ _dispc_set_pre_mult_alpha(plane, pre_mult_alpha);
_dispc_setup_global_alpha(plane, global_alpha);
+ }
return 0;
}
@@ -3139,7 +3154,8 @@ int dispc_setup_plane(enum omap_plane plane,
enum omap_color_mode color_mode,
bool ilace,
enum omap_dss_rotation_type rotation_type,
-u8 rotation, bool mirror, u8 global_alpha)
+u8 rotation, bool mirror, u8 global_alpha,
+u8 pre_mult_alpha)
{
int r = 0;
@@ -3161,7 +3177,8 @@ int dispc_setup_plane(enum omap_plane plane,
color_mode, ilace,
rotation_type,
rotation, mirror,
-global_alpha);
+global_alpha,
+pre_mult_alpha);
enable_clocks(0);
diff --git a/drivers/video/omap2/dss/dss.h b/drivers/video/omap2/dss/dss.h
index 5c7940d..2bb515c 100644
--- a/drivers/video/omap2/dss/dss.h
+++ b/drivers/video/omap2/dss/dss.h
@@ -359,7 +359,8 @@ int dispc_setup_plane(enum omap_plane plane,
bool ilace,
enum omap_dss_rotation_type rotation_type,
u8 rotation, bool mirror,
- u8 global_alpha);
RE: [PATCH] OMAP3630:DSS2:Enable Pre-Multiplied Alpha Support
-Original Message-
From: Tomi Valkeinen [mailto:tomi.valkei...@nokia.com]
Sent: Monday, January 18, 2010 5:54 PM
To: Y, Kishore
Cc: linux-omap@vger.kernel.org; Mande, Nikhil
Subject: RE: [PATCH] OMAP3630:DSS2:Enable Pre-Multiplied Alpha Support
On Mon, 2010-01-18 at 12:26 +0100, ext Y, Kishore wrote:
-Original Message-
From: Tomi Valkeinen [mailto:tomi.valkei...@nokia.com]
Sent: Friday, January 15, 2010 4:00 PM
To: Y, Kishore
Cc: linux-omap@vger.kernel.org
Subject: Re: [PATCH] OMAP3630:DSS2:Enable Pre-Multiplied Alpha Support
Hi,
On Mon, 2009-12-21 at 16:06 +0100, ext Y, Kishore wrote:
From 2f873819a4b9eb0bd658db1e59408d8f0aeb14b6 Mon Sep 17 00:00:00
2001
From: Sudeep Basavaraj sudeep.basava...@ti.com
Date: Mon, 14 Dec 2009 18:54:51 +0530
Subject: [PATCH] OMAP3630:DSS2:Enable Pre-Multiplied Alpha Support
Enables dss to process color formats with pre-mulitplied alpha
values.
With this we can have alpha values defined for each pixel
and hence can have different blending values for each pixel.
What does pre-multiplied alpha mean? The TRM didn't really open it
up...
Don't we already have per pixel alpha when using ARGB/RGBA?
When we set pixel format to ARGB/RGBA dss alpha blender would multiply
the value present in 'A' with 'RGB'.
By setting this bit display hardware assumes that the R,G,B are already
multiplied with the alpha value and there is no need to multiply again.
Ex:-
A R G B
argb data 128 255 128 100
pre-multiplied 128 128 64 50
So this bit, when set, would not multiply the pixel with alpha value.
Ok, after staring long enough to the TRM's alpha-picture, I got it =).
But having this bit always on on 3630 would mean that the software
designed for 3430 would not work properly on 3630, wouldn't it?
This patch seems to always set the bit on, never set it off. Is that
the
purpose?
As per TRM, this bit is valid only for ARGB formats and experts
suggested that we can safely assume pre-multiplied data always in real
world
I asked a few experts here, and they weren't so sure, and neither am I.
I don't see any problems making this feature configurable, but there may
be problems if it's hardcoded. So, it should be configurable. I think
the default should be the same as on 3430, so that they will work
similarly.
Tomi
I haven't considered the backward compatibility with 3430 apps.
I will make this configurable.
kishore
N�r��yb�X��ǧv�^�){.n�+{��f��{ay� ʇڙ�,j��f���h���z� �w���
���j:+v���w�j�mzZ+�ݢj��!�i
Re: [PATCH] OMAP3630:DSS2:Enable Pre-Multiplied Alpha Support
Y, Kishore wrote: As per TRM, this bit is valid only for ARGB formats and experts suggested that we can safely assume pre-multiplied data always in real world I asked a few experts here, and they weren't so sure, and neither am I. I don't see any problems making this feature configurable, but there may be problems if it's hardcoded. So, it should be configurable. I think the default should be the same as on 3430, so that they will work similarly. Tomi I haven't considered the backward compatibility with 3430 apps. I will make this configurable. And please defaulting to non-pre-multiplied. All 3430 apps will be non-pre-multiplied as this is what the 3430 supports only. So apps need to be actively changed to use pre-multiplied. -- To unsubscribe from this list: send the line unsubscribe linux-omap in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
RE: [PATCH] OMAP3630:DSS2:Enable Pre-Multiplied Alpha Support
-Original Message-
From: Tomi Valkeinen [mailto:tomi.valkei...@nokia.com]
Sent: Friday, January 15, 2010 4:00 PM
To: Y, Kishore
Cc: linux-omap@vger.kernel.org
Subject: Re: [PATCH] OMAP3630:DSS2:Enable Pre-Multiplied Alpha Support
Hi,
On Mon, 2009-12-21 at 16:06 +0100, ext Y, Kishore wrote:
From 2f873819a4b9eb0bd658db1e59408d8f0aeb14b6 Mon Sep 17 00:00:00 2001
From: Sudeep Basavaraj sudeep.basava...@ti.com
Date: Mon, 14 Dec 2009 18:54:51 +0530
Subject: [PATCH] OMAP3630:DSS2:Enable Pre-Multiplied Alpha Support
Enables dss to process color formats with pre-mulitplied alpha values.
With this we can have alpha values defined for each pixel
and hence can have different blending values for each pixel.
What does pre-multiplied alpha mean? The TRM didn't really open it up...
Don't we already have per pixel alpha when using ARGB/RGBA?
When we set pixel format to ARGB/RGBA dss alpha blender would multiply the
value present in 'A' with 'RGB'.
By setting this bit display hardware assumes that the R,G,B are already
multiplied with the alpha value and there is no need to multiply again.
Ex:-
A R G B
argb data 128 255 128 100
pre-multiplied 128 128 64 50
So this bit, when set, would not multiply the pixel with alpha value.
This patch seems to always set the bit on, never set it off. Is that the
purpose?
As per TRM, this bit is valid only for ARGB formats and experts suggested that
we can safely assume pre-multiplied data always in real world
Tomi
Signed-off-by: Sudeep Basavaraj sudeep.basava...@ti.com
Signed-off-by: Kishore Y kishor...@ti.com
---
drivers/video/omap2/dss/dispc.c |8
1 files changed, 8 insertions(+), 0 deletions(-)
diff --git a/drivers/video/omap2/dss/dispc.c
b/drivers/video/omap2/dss/dispc.c
index 6dabf4b..5f7819b 100644
--- a/drivers/video/omap2/dss/dispc.c
+++ b/drivers/video/omap2/dss/dispc.c
@@ -913,6 +913,11 @@ static void _dispc_set_vid_color_conv(enum
omap_plane plane, bool enable)
dispc_write_reg(dispc_reg_att[plane], val);
}
+static void _dispc_set_alpha_blend_attrs(enum omap_plane plane, bool
enable)
+{
+ REG_FLD_MOD(dispc_reg_att[plane], enable ? 1 : 0, 28, 28);
+}
+
void dispc_enable_replication(enum omap_plane plane, bool enable)
{
int bit;
@@ -1689,6 +1694,9 @@ static int _dispc_setup_plane(enum omap_plane
plane,
_dispc_set_rotation_attrs(plane, rotation, mirror, color_mode);
+ if (cpu_is_omap3630() (plane != OMAP_DSS_VIDEO1))
+ _dispc_set_alpha_blend_attrs(plane, 1);
+
if (plane != OMAP_DSS_VIDEO1)
_dispc_setup_global_alpha(plane, global_alpha);
RE: [PATCH] OMAP3630:DSS2:Enable Pre-Multiplied Alpha Support
On Mon, 2010-01-18 at 12:26 +0100, ext Y, Kishore wrote: -Original Message- From: Tomi Valkeinen [mailto:tomi.valkei...@nokia.com] Sent: Friday, January 15, 2010 4:00 PM To: Y, Kishore Cc: linux-omap@vger.kernel.org Subject: Re: [PATCH] OMAP3630:DSS2:Enable Pre-Multiplied Alpha Support Hi, On Mon, 2009-12-21 at 16:06 +0100, ext Y, Kishore wrote: From 2f873819a4b9eb0bd658db1e59408d8f0aeb14b6 Mon Sep 17 00:00:00 2001 From: Sudeep Basavaraj sudeep.basava...@ti.com Date: Mon, 14 Dec 2009 18:54:51 +0530 Subject: [PATCH] OMAP3630:DSS2:Enable Pre-Multiplied Alpha Support Enables dss to process color formats with pre-mulitplied alpha values. With this we can have alpha values defined for each pixel and hence can have different blending values for each pixel. What does pre-multiplied alpha mean? The TRM didn't really open it up... Don't we already have per pixel alpha when using ARGB/RGBA? When we set pixel format to ARGB/RGBA dss alpha blender would multiply the value present in 'A' with 'RGB'. By setting this bit display hardware assumes that the R,G,B are already multiplied with the alpha value and there is no need to multiply again. Ex:- A R G B argb data 128 255 128 100 pre-multiplied128 128 64 50 So this bit, when set, would not multiply the pixel with alpha value. Ok, after staring long enough to the TRM's alpha-picture, I got it =). But having this bit always on on 3630 would mean that the software designed for 3430 would not work properly on 3630, wouldn't it? This patch seems to always set the bit on, never set it off. Is that the purpose? As per TRM, this bit is valid only for ARGB formats and experts suggested that we can safely assume pre-multiplied data always in real world I asked a few experts here, and they weren't so sure, and neither am I. I don't see any problems making this feature configurable, but there may be problems if it's hardcoded. So, it should be configurable. I think the default should be the same as on 3430, so that they will work similarly. Tomi -- To unsubscribe from this list: send the line unsubscribe linux-omap in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH] OMAP3630:DSS2:Enable Pre-Multiplied Alpha Support
Hi,
On Mon, 2009-12-21 at 16:06 +0100, ext Y, Kishore wrote:
From 2f873819a4b9eb0bd658db1e59408d8f0aeb14b6 Mon Sep 17 00:00:00 2001
From: Sudeep Basavaraj sudeep.basava...@ti.com
Date: Mon, 14 Dec 2009 18:54:51 +0530
Subject: [PATCH] OMAP3630:DSS2:Enable Pre-Multiplied Alpha Support
Enables dss to process color formats with pre-mulitplied alpha values.
With this we can have alpha values defined for each pixel
and hence can have different blending values for each pixel.
What does pre-multiplied alpha mean? The TRM didn't really open it up...
Don't we already have per pixel alpha when using ARGB/RGBA?
This patch seems to always set the bit on, never set it off. Is that the
purpose?
Tomi
Signed-off-by: Sudeep Basavaraj sudeep.basava...@ti.com
Signed-off-by: Kishore Y kishor...@ti.com
---
drivers/video/omap2/dss/dispc.c |8
1 files changed, 8 insertions(+), 0 deletions(-)
diff --git a/drivers/video/omap2/dss/dispc.c b/drivers/video/omap2/dss/dispc.c
index 6dabf4b..5f7819b 100644
--- a/drivers/video/omap2/dss/dispc.c
+++ b/drivers/video/omap2/dss/dispc.c
@@ -913,6 +913,11 @@ static void _dispc_set_vid_color_conv(enum omap_plane
plane, bool enable)
dispc_write_reg(dispc_reg_att[plane], val);
}
+static void _dispc_set_alpha_blend_attrs(enum omap_plane plane, bool enable)
+{
+ REG_FLD_MOD(dispc_reg_att[plane], enable ? 1 : 0, 28, 28);
+}
+
void dispc_enable_replication(enum omap_plane plane, bool enable)
{
int bit;
@@ -1689,6 +1694,9 @@ static int _dispc_setup_plane(enum omap_plane plane,
_dispc_set_rotation_attrs(plane, rotation, mirror, color_mode);
+ if (cpu_is_omap3630() (plane != OMAP_DSS_VIDEO1))
+ _dispc_set_alpha_blend_attrs(plane, 1);
+
if (plane != OMAP_DSS_VIDEO1)
_dispc_setup_global_alpha(plane, global_alpha);
--
To unsubscribe from this list: send the line unsubscribe linux-omap in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
