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 [email protected]
More majordomo info at http://vger.kernel.org/majordomo-info.html