Hi Andy, Thank you for all the reviews! On 4/15/24 4:40 PM, Andy Shevchenko wrote: > On Mon, Apr 15, 2024 at 02:02:08PM +0200, Hans de Goede wrote: >> The TPG support registers a v4l2-subdev for this, but this is not part of >> the media-controller graph in any way. After manually selecting the TPG >> as input using the s_input ioctl it does not work. >> >> Several supported sensors have their own working TPG and using the sensor's >> TPG means that the same data-flow is used as with actual sensors rather >> then the quite different data-flow from the ISP's builtin TPG. >> >> Remove the broken TPG support, when a test-pattern is needed for testing >> a sensor's TPG can be used. Using a sensor's TPG is actually better for >> testing since then the actual normal data-flow is being tested. > > ... > >> + if (mipi_info) >> + fc = atomisp_find_in_fmt_conv_by_atomisp_in_fmt(mipi_info->input_format); >> >> + if (!fc) >> + fc = atomisp_find_in_fmt_conv( >> + atomisp_subdev_get_ffmt(&asd->subdev, >> + NULL, V4L2_SUBDEV_FORMAT_ACTIVE, >> + ATOMISP_SUBDEV_PAD_SINK)->code); > > While it was in the original code, this is ugly. Can we split this to two > assignments? Ack, fixed. > >> + if (!fc) >> + return -EINVAL; >> + if (format->sh_fmt == IA_CSS_FRAME_FORMAT_RAW && > >> + raw_output_format_match_input(fc->atomisp_in_fmt, >> + pix->pixelformat)) > > Now a single line? Ack, fixed. > >> + return -EINVAL; > > ... > >> unsigned int hblank_cycles = 100, >> vblank_lines = 6, >> width, > > Urgh... These comma operators probably is subject to replace with separate > definitions or being grouped on a single line (as it suppose to be in this > case). That indeed is ugly, but fixing this is out of scope for this patch, so I've added an extra patch to the set to address this: https://git.kernel.org/pub/scm/linux/kernel/git/hansg/linux.git/commit/?h=media-atomisp&id=48d93af9d9b0fd40a21a656cb8cd8e25700bfed5 Regards, Hans
