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-omap" in the body of a message to majordomo@xxxxxxxxxxxxxxx More majordomo info at http://vger.kernel.org/majordomo-info.html