Re: [PATCH 00/15] Intel IPU6 and IPU6 input system drivers

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

 



Hans and Laurent,

On 10/3/23 1:41 AM, Hans de Goede wrote:
> Hi,
> 
> On 10/2/23 19:38, Laurent Pinchart wrote:
>> On Mon, Oct 02, 2023 at 07:19:13PM +0200, Hans de Goede wrote:
>>> Hi,
>>>
>>> On 9/4/23 05:13, Cao, Bingbu wrote:
>>>> Hans,
>>>>
>>>> Thanks for your test and report.
>>>>
>>>> I have made some changes locally which will support the latest
>>>> v4l2-async APIs, I will also add the fix for this issue and merge
>>>> them in v3.
>>>
>>> I have been trying to make rawbayer capture with the mainline isys code
>>> work in libcamera and I have hit several short comings in the ipu6-isys
>>> code. I have attached 3 patches to fix various issues please integrate
>>> these into the next version of this series.
>>
>> They look good to me.
> 
> Actually I just realized I forgot to commit + squash in a bug fix:
> 
>>> Talking about the next version of this series, I think it would be
>>> good to post a new version soon ?
>>>
>>
>>> From 14f42fd3071a7aba8b83b98802ca3b413794296d Mon Sep 17 00:00:00 2001
>>> From: Hans de Goede <hdegoede@xxxxxxxxxx>
>>> Date: Mon, 2 Oct 2023 16:37:11 +0200
>>> Subject: [PATCH 1/3] media: intel/ipu6: Add mbus code filtering to isys
>>>  /dev/video enum_fmt
>>>
>>> Add mbus code filtering to ipu6_isys_vidioc_enum_fmt().
>>>
>>> Signed-off-by: Hans de Goede <hdegoede@xxxxxxxxxx>
>>> ---
>>>  .../media/pci/intel/ipu6/ipu6-isys-video.c    | 29 +++++++++++++++----
>>>  1 file changed, 23 insertions(+), 6 deletions(-)
>>>
>>> diff --git a/drivers/media/pci/intel/ipu6/ipu6-isys-video.c b/drivers/media/pci/intel/ipu6/ipu6-isys-video.c
>>> index dc1605491352..20fd03f6f204 100644
>>> --- a/drivers/media/pci/intel/ipu6/ipu6-isys-video.c
>>> +++ b/drivers/media/pci/intel/ipu6/ipu6-isys-video.c
>>> @@ -130,14 +130,31 @@ int ipu6_isys_vidioc_querycap(struct file *file, void *fh,
>>>  int ipu6_isys_vidioc_enum_fmt(struct file *file, void *fh,
>>>  			      struct v4l2_fmtdesc *f)
>>>  {
>>> -	if (f->index >= ARRAY_SIZE(ipu6_isys_pfmts))
>>> -		return -EINVAL;
>>> +	unsigned int i, found = 0;
>>>  
>>> -	f->flags = 0;
>>> -	f->pixelformat = ipu6_isys_pfmts[f->index].pixelformat;
>>> -	f->mbus_code = ipu6_isys_pfmts[f->index].code;
>>> +	if (!f->mbus_code) {
>>> +		if (f->index >= ARRAY_SIZE(ipu6_isys_pfmts))
>>> +			return -EINVAL;
>>>  
>>> -	return 0;
>>> +		f->flags = 0;
>>> +		f->pixelformat = ipu6_isys_pfmts[f->index].pixelformat;
>>> +		f->mbus_code = ipu6_isys_pfmts[f->index].code;
> 
> There is a:
> 		return 0;
> 
> missing here.		
> 
>>> +	}
>>> +
> 
> Regards,
> 
> Hans
> 
> 
> 
>>> +	for (i = 0; i < ARRAY_SIZE(ipu6_isys_pfmts); i++) {
>>> +		if (f->mbus_code != ipu6_isys_pfmts[i].code)
>>> +			continue;
>>> +
>>> +		if (f->index == found) {
>>> +			f->flags = 0;
>>> +			f->pixelformat = ipu6_isys_pfmts[i].pixelformat;
>>> +			return 0;
>>> +		}
>>> +
>>> +		found++;
>>> +	}

A little confused here -

If the `mbus_code` argument here is not zero, does the user expect that
the first try (f->index == 0) should be found and return? `found` is
always 0 here?

My understanding is - user will try to enum again if return -EINVAL.

>>> +
>>> +	return -EINVAL;
>>>  }
>>>


<snip>
-- 
Best regards,
Bingbu Cao



[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