[PATCH] media: intel/ipu6: set the bus_info of the v4l2_capability

[PATCH] media: intel/ipu6: set the bus_info of the v4l2_capability

[bingbu.cao]

From: Bingbu Cao <bingbu.cao@intel.com>

IPU6 isys driver missed setting the bus_info of its v4l2_capability.
`v4l2-ctl --all` cannot show the bus_info. This patch copy the bus_info
from the media device to fill the v4l2_capability.

Fixes: 3c1dfb5a69cf ("media: intel/ipu6: input system video nodes and buffer queues")
Signed-off-by: Hidenori Kobayashi <hidenorik@chromium.org>
Signed-off-by: Bingbu Cao <bingbu.cao@intel.com>
---
 drivers/media/pci/intel/ipu6/ipu6-isys-video.c | 2 ++
 1 file changed, 2 insertions(+)

diff --git a/drivers/media/pci/intel/ipu6/ipu6-isys-video.c b/drivers/media/pci/intel/ipu6/ipu6-isys-video.c
index 387963529adb..3ca3f44da387 100644
--- a/drivers/media/pci/intel/ipu6/ipu6-isys-video.c
+++ b/drivers/media/pci/intel/ipu6/ipu6-isys-video.c
@@ -146,6 +146,8 @@ static int ipu6_isys_vidioc_querycap(struct file *file, void *fh,
 
 	strscpy(cap->driver, IPU6_ISYS_NAME, sizeof(cap->driver));
 	strscpy(cap->card, av->isys->media_dev.model, sizeof(cap->card));
+	strscpy(cap->bus_info, av->isys->media_dev.bus_info,
+		sizeof(cap->bus_info));
 
 	return 0;
 }
-- 
2.34.1

Re: [PATCH] media: intel/ipu6: set the bus_info of the v4l2_capability

[Sakari Ailus]

Hi Bingbu,

Thanks for the patch.

On Wed, Feb 12, 2025 at 03:53:14PM +0800, bingbu.cao@intel.com wrote:
> From: Bingbu Cao <bingbu.cao@intel.com>
> 
> IPU6 isys driver missed setting the bus_info of its v4l2_capability.
> `v4l2-ctl --all` cannot show the bus_info. This patch copy the bus_info
> from the media device to fill the v4l2_capability.
> 
> Fixes: 3c1dfb5a69cf ("media: intel/ipu6: input system video nodes and buffer queues")
> Signed-off-by: Hidenori Kobayashi <hidenorik@chromium.org>
> Signed-off-by: Bingbu Cao <bingbu.cao@intel.com>
> ---
>  drivers/media/pci/intel/ipu6/ipu6-isys-video.c | 2 ++
>  1 file changed, 2 insertions(+)
> 
> diff --git a/drivers/media/pci/intel/ipu6/ipu6-isys-video.c b/drivers/media/pci/intel/ipu6/ipu6-isys-video.c
> index 387963529adb..3ca3f44da387 100644
> --- a/drivers/media/pci/intel/ipu6/ipu6-isys-video.c
> +++ b/drivers/media/pci/intel/ipu6/ipu6-isys-video.c
> @@ -146,6 +146,8 @@ static int ipu6_isys_vidioc_querycap(struct file *file, void *fh,
>  
>  	strscpy(cap->driver, IPU6_ISYS_NAME, sizeof(cap->driver));
>  	strscpy(cap->card, av->isys->media_dev.model, sizeof(cap->card));
> +	strscpy(cap->bus_info, av->isys->media_dev.bus_info,
> +		sizeof(cap->bus_info));

Is there a need to do this? The bus_info is set by the framework based on
struct video_device dev_parent field and that comes from struct v4l2_device
dev field.

>  
>  	return 0;
>  }

-- 
Kind regards,

Sakari Ailus

Re: [PATCH] media: intel/ipu6: set the bus_info of the v4l2_capability

[Bingbu Cao]

On 2/12/25 4:25 PM, Sakari Ailus wrote:
> Hi Bingbu,
> 
> Thanks for the patch.
> 
> On Wed, Feb 12, 2025 at 03:53:14PM +0800, bingbu.cao@intel.com wrote:
>> From: Bingbu Cao <bingbu.cao@intel.com>
>>
>> IPU6 isys driver missed setting the bus_info of its v4l2_capability.
>> `v4l2-ctl --all` cannot show the bus_info. This patch copy the bus_info
>> from the media device to fill the v4l2_capability.
>>
>> Fixes: 3c1dfb5a69cf ("media: intel/ipu6: input system video nodes and buffer queues")
>> Signed-off-by: Hidenori Kobayashi <hidenorik@chromium.org>
>> Signed-off-by: Bingbu Cao <bingbu.cao@intel.com>
>> ---
>>  drivers/media/pci/intel/ipu6/ipu6-isys-video.c | 2 ++
>>  1 file changed, 2 insertions(+)
>>
>> diff --git a/drivers/media/pci/intel/ipu6/ipu6-isys-video.c b/drivers/media/pci/intel/ipu6/ipu6-isys-video.c
>> index 387963529adb..3ca3f44da387 100644
>> --- a/drivers/media/pci/intel/ipu6/ipu6-isys-video.c
>> +++ b/drivers/media/pci/intel/ipu6/ipu6-isys-video.c
>> @@ -146,6 +146,8 @@ static int ipu6_isys_vidioc_querycap(struct file *file, void *fh,
>>  
>>  	strscpy(cap->driver, IPU6_ISYS_NAME, sizeof(cap->driver));
>>  	strscpy(cap->card, av->isys->media_dev.model, sizeof(cap->card));
>> +	strscpy(cap->bus_info, av->isys->media_dev.bus_info,
>> +		sizeof(cap->bus_info));
> 
> Is there a need to do this? The bus_info is set by the framework based on
> struct video_device dev_parent field and that comes from struct v4l2_device
> dev field.
>

The v4l2_device.dev filed is set to the dev_parent which is auxdev.dev
instead of pdev. So the bus_info was not set by framework.

>>  
>>  	return 0;
>>  }
> 

-- 
Best regards,
Bingbu Cao

Re: [PATCH] media: intel/ipu6: set the bus_info of the v4l2_capability

[Bingbu Cao]

On 2/12/25 5:14 PM, Bingbu Cao wrote:
> 
> 
> On 2/12/25 4:25 PM, Sakari Ailus wrote:
>> Hi Bingbu,
>>
>> Thanks for the patch.
>>
>> On Wed, Feb 12, 2025 at 03:53:14PM +0800, bingbu.cao@intel.com wrote:
>>> From: Bingbu Cao <bingbu.cao@intel.com>
>>>
>>> IPU6 isys driver missed setting the bus_info of its v4l2_capability.
>>> `v4l2-ctl --all` cannot show the bus_info. This patch copy the bus_info
>>> from the media device to fill the v4l2_capability.
>>>
>>> Fixes: 3c1dfb5a69cf ("media: intel/ipu6: input system video nodes and buffer queues")
>>> Signed-off-by: Hidenori Kobayashi <hidenorik@chromium.org>
>>> Signed-off-by: Bingbu Cao <bingbu.cao@intel.com>
>>> ---
>>>  drivers/media/pci/intel/ipu6/ipu6-isys-video.c | 2 ++
>>>  1 file changed, 2 insertions(+)
>>>
>>> diff --git a/drivers/media/pci/intel/ipu6/ipu6-isys-video.c b/drivers/media/pci/intel/ipu6/ipu6-isys-video.c
>>> index 387963529adb..3ca3f44da387 100644
>>> --- a/drivers/media/pci/intel/ipu6/ipu6-isys-video.c
>>> +++ b/drivers/media/pci/intel/ipu6/ipu6-isys-video.c
>>> @@ -146,6 +146,8 @@ static int ipu6_isys_vidioc_querycap(struct file *file, void *fh,
>>>  
>>>  	strscpy(cap->driver, IPU6_ISYS_NAME, sizeof(cap->driver));
>>>  	strscpy(cap->card, av->isys->media_dev.model, sizeof(cap->card));
>>> +	strscpy(cap->bus_info, av->isys->media_dev.bus_info,
>>> +		sizeof(cap->bus_info));
>>
>> Is there a need to do this? The bus_info is set by the framework based on
>> struct video_device dev_parent field and that comes from struct v4l2_device
>> dev field.
>>
> 
> The v4l2_device.dev filed is set to the dev_parent which is auxdev.dev
> instead of pdev. So the bus_info was not set by framework.
>

I remember we made a change in isys_register_devices() :

- 	ret = v4l2_device_register(dev->parent, &isys->v4l2_dev);
+	ret = v4l2_device_register(dev, &isys->v4l2_dev);

>>>  
>>>  	return 0;
>>>  }
>>
> 

-- 
Best regards,
Bingbu Cao

Re: [PATCH] media: intel/ipu6: set the bus_info of the v4l2_capability

[Sakari Ailus]

Hi Bingbu,

On Wed, Feb 12, 2025 at 05:20:54PM +0800, Bingbu Cao wrote:
> 
> On 2/12/25 5:14 PM, Bingbu Cao wrote:
> > 
> > 
> > On 2/12/25 4:25 PM, Sakari Ailus wrote:
> >> Hi Bingbu,
> >>
> >> Thanks for the patch.
> >>
> >> On Wed, Feb 12, 2025 at 03:53:14PM +0800, bingbu.cao@intel.com wrote:
> >>> From: Bingbu Cao <bingbu.cao@intel.com>
> >>>
> >>> IPU6 isys driver missed setting the bus_info of its v4l2_capability.
> >>> `v4l2-ctl --all` cannot show the bus_info. This patch copy the bus_info
> >>> from the media device to fill the v4l2_capability.
> >>>
> >>> Fixes: 3c1dfb5a69cf ("media: intel/ipu6: input system video nodes and buffer queues")
> >>> Signed-off-by: Hidenori Kobayashi <hidenorik@chromium.org>
> >>> Signed-off-by: Bingbu Cao <bingbu.cao@intel.com>
> >>> ---
> >>>  drivers/media/pci/intel/ipu6/ipu6-isys-video.c | 2 ++
> >>>  1 file changed, 2 insertions(+)
> >>>
> >>> diff --git a/drivers/media/pci/intel/ipu6/ipu6-isys-video.c b/drivers/media/pci/intel/ipu6/ipu6-isys-video.c
> >>> index 387963529adb..3ca3f44da387 100644
> >>> --- a/drivers/media/pci/intel/ipu6/ipu6-isys-video.c
> >>> +++ b/drivers/media/pci/intel/ipu6/ipu6-isys-video.c
> >>> @@ -146,6 +146,8 @@ static int ipu6_isys_vidioc_querycap(struct file *file, void *fh,
> >>>  
> >>>  	strscpy(cap->driver, IPU6_ISYS_NAME, sizeof(cap->driver));
> >>>  	strscpy(cap->card, av->isys->media_dev.model, sizeof(cap->card));
> >>> +	strscpy(cap->bus_info, av->isys->media_dev.bus_info,
> >>> +		sizeof(cap->bus_info));
> >>
> >> Is there a need to do this? The bus_info is set by the framework based on
> >> struct video_device dev_parent field and that comes from struct v4l2_device
> >> dev field.
> >>
> > 
> > The v4l2_device.dev filed is set to the dev_parent which is auxdev.dev
> > instead of pdev. So the bus_info was not set by framework.
> >
> 
> I remember we made a change in isys_register_devices() :
> 
> - 	ret = v4l2_device_register(dev->parent, &isys->v4l2_dev);
> +	ret = v4l2_device_register(dev, &isys->v4l2_dev);

Right. I vaguely recall there were changes in device assignments over the
course of upstreaming, due to the sub-device driver module refcounting. So
I think this needs to stay as-is.

Instead, would assigning the dev_parent field in struct video_device
achieve this or would there be side effects? The other alternative is to
use media_set_bus_info().

-- 
Regards,

Sakari Ailus

Re: [PATCH] media: intel/ipu6: set the bus_info of the v4l2_capability

[Bingbu Cao]

Sakari,

On 2/12/25 5:39 PM, Sakari Ailus wrote:
> Hi Bingbu,
> 
> On Wed, Feb 12, 2025 at 05:20:54PM +0800, Bingbu Cao wrote:
>>
>> On 2/12/25 5:14 PM, Bingbu Cao wrote:
>>>
>>>
>>> On 2/12/25 4:25 PM, Sakari Ailus wrote:
>>>> Hi Bingbu,
>>>>
>>>> Thanks for the patch.
>>>>
>>>> On Wed, Feb 12, 2025 at 03:53:14PM +0800, bingbu.cao@intel.com wrote:
>>>>> From: Bingbu Cao <bingbu.cao@intel.com>
>>>>>
>>>>> IPU6 isys driver missed setting the bus_info of its v4l2_capability.
>>>>> `v4l2-ctl --all` cannot show the bus_info. This patch copy the bus_info
>>>>> from the media device to fill the v4l2_capability.
>>>>>
>>>>> Fixes: 3c1dfb5a69cf ("media: intel/ipu6: input system video nodes and buffer queues")
>>>>> Signed-off-by: Hidenori Kobayashi <hidenorik@chromium.org>
>>>>> Signed-off-by: Bingbu Cao <bingbu.cao@intel.com>
>>>>> ---
>>>>>  drivers/media/pci/intel/ipu6/ipu6-isys-video.c | 2 ++
>>>>>  1 file changed, 2 insertions(+)
>>>>>
>>>>> diff --git a/drivers/media/pci/intel/ipu6/ipu6-isys-video.c b/drivers/media/pci/intel/ipu6/ipu6-isys-video.c
>>>>> index 387963529adb..3ca3f44da387 100644
>>>>> --- a/drivers/media/pci/intel/ipu6/ipu6-isys-video.c
>>>>> +++ b/drivers/media/pci/intel/ipu6/ipu6-isys-video.c
>>>>> @@ -146,6 +146,8 @@ static int ipu6_isys_vidioc_querycap(struct file *file, void *fh,
>>>>>  
>>>>>  	strscpy(cap->driver, IPU6_ISYS_NAME, sizeof(cap->driver));
>>>>>  	strscpy(cap->card, av->isys->media_dev.model, sizeof(cap->card));
>>>>> +	strscpy(cap->bus_info, av->isys->media_dev.bus_info,
>>>>> +		sizeof(cap->bus_info));
>>>>
>>>> Is there a need to do this? The bus_info is set by the framework based on
>>>> struct video_device dev_parent field and that comes from struct v4l2_device
>>>> dev field.
>>>>
>>>
>>> The v4l2_device.dev filed is set to the dev_parent which is auxdev.dev
>>> instead of pdev. So the bus_info was not set by framework.
>>>
>>
>> I remember we made a change in isys_register_devices() :
>>
>> - 	ret = v4l2_device_register(dev->parent, &isys->v4l2_dev);
>> +	ret = v4l2_device_register(dev, &isys->v4l2_dev);
> 
> Right. I vaguely recall there were changes in device assignments over the
> course of upstreaming, due to the sub-device driver module refcounting. So
> I think this needs to stay as-is.
> 
> Instead, would assigning the dev_parent field in struct video_device
> achieve this or would there be side effects? The other alternative is to
> use media_set_bus_info().
>

I prefer assigning the dev_parent. I did some simple test and did not
observe side effects. Let me do more tests and then send v2.

-- 
Best regards,
Bingbu Cao