RE: [PATCH] OMAP3630: DSS2: Enable Pre-Multiplied Alpha Support

Hi,

Taneja, Archit wrote:

Hi,

Tomi Valkeinen wrote:

quoted

On Wed, 2010-11-03 at 08:57 +0100, ext Taneja, Archit wrote:

quoted

Hi,

linux-omap-owner@vger.kernel.org wrote:

quoted

Alpha Support

[snip]

quoted

quoted

+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