Hi Michael, On Monday 07 March 2011 11:53:26 Michael Jones wrote: > On 03/04/2011 05:33 PM, Laurent Pinchart wrote: [snip] > >> + > >> + if (in_info->flavor != out_info->flavor) > >> + return 0; > >> + > >> + switch (in_info->bpp - out_info->bpp) { > >> + case 2: > >> + case 4: > >> + case 6: > >> + shiftable = 1; > >> + break; > >> + default: > >> + shiftable = 0; > >> + } > > > > What about > > > > return in_info->bpp - out_info->bpp <= 6; > > As long as there are never formats which are the same flavor but shifted > 1, 3, or 5 bits, that's fine. I suppose this is a safe assumption? I think so. If we need to add support for those formats later we can revisit the code. > >> + > >> + return shiftable; > >> +} > >> + > >> +/* > >> > >> * Configure the bridge and lane shifter. Valid inputs are > >> * > >> * CCDC_INPUT_PARALLEL: Parallel interface > >> [snip] > >> + /* find CCDC input format */ > >> + fmt_info = omap3isp_video_format_info > >> + (isp->isp_ccdc.formats[CCDC_PAD_SINK].code); > >> + depth_out = fmt_info ? fmt_info->bpp : 0; > >> + > >> + isp->isp_ccdc.syncif.datsz = depth_out; > >> + > >> + /* determine necessary shifting */ > >> + if (depth_in == depth_out + 6) > >> + shift = 3; > >> + else if (depth_in == depth_out + 4) > >> + shift = 2; > >> + else if (depth_in == depth_out + 2) > >> + shift = 1; > >> + else > >> + shift = 0; > > > > Maybe shift = (depth_out - depth_in) / 2; ? > > First of all, the other way around: (depth_in - depth_out). I suppose I > don't need to account for e.g. (depth_in - depth_out > 6) because then > the pipeline would've been invalid in the first place? If I do this, I > would at least use ISPCTRL_SHIFT_MASK when writing 'shift' into > ispctrl_val as a final catch if something went wrong with shift. Sounds good to me. -- Regards, Laurent Pinchart -- To unsubscribe from this list: send the line "unsubscribe linux-media" in the body of a message to majordomo@xxxxxxxxxxxxxxx More majordomo info at http://vger.kernel.org/majordomo-info.html