Hi,
Taneja, Archit wrote:
> 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
[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-fbdev" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
