Hi,
Tomi Valkeinen wrote:
> On Wed, 2010-11-03 at 08:57 +0100, ext Taneja, Archit wrote:
>> Hi,
>>
>> linux-omap-owner@xxxxxxxxxxxxxxx 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 majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
