Re: [PATCH 11/23] media: atomisp: Remove test pattern generator (TPG) support

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



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






[Index of Archives]     [Linux Input]     [Video for Linux]     [Gstreamer Embedded]     [Mplayer Users]     [Linux USB Devel]     [Linux Audio Users]     [Linux Kernel]     [Linux SCSI]     [Yosemite Backpacking]

  Powered by Linux