Re: [PATCH v2 1/2] media: staging/intel-ipu3: Fix memory leak in imu_fmt

2021-03-16 Thread Bingbu Cao
Hi, Ricardo

Thanks for your patch.
It looks fine for me, do you mind squash 2 patchsets into 1 commit?

On 3/15/21 8:34 PM, Ricardo Ribalda wrote:
> We are losing the reference to an allocated memory if try. Change the
> order of the check to avoid that.
> 
> Cc: sta...@vger.kernel.org
> Fixes: 6d5f26f2e045 ("media: staging/intel-ipu3-v4l: reduce kernel stack 
> usage")
> Signed-off-by: Ricardo Ribalda 
> ---
>  drivers/staging/media/ipu3/ipu3-v4l2.c | 11 +++
>  1 file changed, 7 insertions(+), 4 deletions(-)
> 
> diff --git a/drivers/staging/media/ipu3/ipu3-v4l2.c 
> b/drivers/staging/media/ipu3/ipu3-v4l2.c
> index 60aa02eb7d2a..35a74d99322f 100644
> --- a/drivers/staging/media/ipu3/ipu3-v4l2.c
> +++ b/drivers/staging/media/ipu3/ipu3-v4l2.c
> @@ -693,6 +693,13 @@ static int imgu_fmt(struct imgu_device *imgu, unsigned 
> int pipe, int node,
>   if (inode == IMGU_NODE_STAT_3A || inode == IMGU_NODE_PARAMS)
>   continue;
>  
> + /* CSS expects some format on OUT queue */
> + if (i != IPU3_CSS_QUEUE_OUT &&
> + !imgu_pipe->nodes[inode].enabled) {
> + fmts[i] = NULL;
> + continue;
> + }
> +
>   if (try) {
>   fmts[i] = 
> kmemdup(&imgu_pipe->nodes[inode].vdev_fmt.fmt.pix_mp,
> sizeof(struct v4l2_pix_format_mplane),
> @@ -705,10 +712,6 @@ static int imgu_fmt(struct imgu_device *imgu, unsigned 
> int pipe, int node,
>   fmts[i] = &imgu_pipe->nodes[inode].vdev_fmt.fmt.pix_mp;
>   }
>  
> - /* CSS expects some format on OUT queue */
> - if (i != IPU3_CSS_QUEUE_OUT &&
> - !imgu_pipe->nodes[inode].enabled)
> - fmts[i] = NULL;
>   }
>  
>   if (!try) {
> 

-- 
Best regards,
Bingbu Cao
___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel


Re: [PATCH v2 1/2] media: staging/intel-ipu3: Fix memory leak in imu_fmt

2021-03-16 Thread Bingbu Cao


On 3/17/21 1:50 AM, Ricardo Ribalda wrote:
> Hi Bingbu
> 
> Thanks for your review
> 
> On Tue, Mar 16, 2021 at 12:29 PM Bingbu Cao  
> wrote:
>>
>> Hi, Ricardo
>>
>> Thanks for your patch.
>> It looks fine for me, do you mind squash 2 patchsets into 1 commit?
> 
> Are you sure? There are two different issues that we are solving.

Oh, I see. I thought you were fixing 1 issue here.
Thanks!

> 
> Best regards!
> 
>>
>> On 3/15/21 8:34 PM, Ricardo Ribalda wrote:
>>> We are losing the reference to an allocated memory if try. Change the
>>> order of the check to avoid that.
>>>
>>> Cc: sta...@vger.kernel.org
>>> Fixes: 6d5f26f2e045 ("media: staging/intel-ipu3-v4l: reduce kernel stack 
>>> usage")
>>> Signed-off-by: Ricardo Ribalda 
>>> ---
>>>  drivers/staging/media/ipu3/ipu3-v4l2.c | 11 +++
>>>  1 file changed, 7 insertions(+), 4 deletions(-)
>>>
>>> diff --git a/drivers/staging/media/ipu3/ipu3-v4l2.c 
>>> b/drivers/staging/media/ipu3/ipu3-v4l2.c
>>> index 60aa02eb7d2a..35a74d99322f 100644
>>> --- a/drivers/staging/media/ipu3/ipu3-v4l2.c
>>> +++ b/drivers/staging/media/ipu3/ipu3-v4l2.c
>>> @@ -693,6 +693,13 @@ static int imgu_fmt(struct imgu_device *imgu, unsigned 
>>> int pipe, int node,
>>>   if (inode == IMGU_NODE_STAT_3A || inode == IMGU_NODE_PARAMS)
>>>   continue;
>>>
>>> + /* CSS expects some format on OUT queue */
>>> + if (i != IPU3_CSS_QUEUE_OUT &&
>>> + !imgu_pipe->nodes[inode].enabled) {
>>> + fmts[i] = NULL;
>>> + continue;
>>> + }
>>> +
>>>   if (try) {
>>>   fmts[i] = 
>>> kmemdup(&imgu_pipe->nodes[inode].vdev_fmt.fmt.pix_mp,
>>> sizeof(struct 
>>> v4l2_pix_format_mplane),
>>> @@ -705,10 +712,6 @@ static int imgu_fmt(struct imgu_device *imgu, unsigned 
>>> int pipe, int node,
>>>       fmts[i] = 
>>> &imgu_pipe->nodes[inode].vdev_fmt.fmt.pix_mp;
>>>   }
>>>
>>> - /* CSS expects some format on OUT queue */
>>> - if (i != IPU3_CSS_QUEUE_OUT &&
>>> - !imgu_pipe->nodes[inode].enabled)
>>> - fmts[i] = NULL;
>>>   }
>>>
>>>   if (!try) {
>>>
>>
>> --
>> Best regards,
>> Bingbu Cao
> 
> 
> 

-- 
Best regards,
Bingbu Cao
___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel


Re: [PATCH -next] media: staging/intel-ipu3: Fix err handle of ipu3_css_find_binary

2019-01-06 Thread Bingbu Cao

Hi, Haibing

Thanks for your patch, it looks fine for me.
Reviewed-by: Bingbu Cao 

On 12/29/2018 10:45 AM, YueHaibing wrote:

css->pipes[pipe].bindex = binary;


___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel


Re: [PATCH -next] media: staging/intel-ipu3: Fix err handle of ipu3_css_find_binary

2019-01-07 Thread Bingbu Cao



On 01/07/2019 07:00 PM, Sakari Ailus wrote:

Hi Bingbu,

On Mon, Jan 07, 2019 at 10:38:19AM +0800, Bingbu Cao wrote:

Hi, Haibing

Thanks for your patch, it looks fine for me.
Reviewed-by: Bingbu Cao 

On 12/29/2018 10:45 AM, YueHaibing wrote:

css->pipes[pipe].bindex = binary;

I'm taking Colin's patch with equivalent content; it was there first.

Sakari, good to know that, thanks!


Thanks!



___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel


Re: ipu3-imgu 0000:00:05.0: required queues are disabled

2019-01-29 Thread Bingbu Cao




On 01/28/2019 11:45 PM, Kai Heng Feng wrote:

Hi Kieran,


On Jan 28, 2019, at 4:48 PM, Kieran Bingham  
wrote:

Hi Kai-Heng,

On 27/01/2019 05:56, Kai-Heng Feng wrote:

Hi,

We have a bug report [1] that the ipu3 doesn’t work.
Does ipu3 need special userspace to work?

Yes, it will need further userspace support to configure the pipeline,
and to provide 3A algorithms for white balance, focus, and exposure
times to the sensor.

We are developing a stack called libcamera [0] to support this, but it's
still in active development and not yet ready for use. Fortunately
however, IPU3 is one of our primary initial targets.

Thanks for the info.

Hi, Kai-Heng,

Like Bingham said, for IPU3 some heavy control from the userspace is needed.
libcamera is a very good start.
If you just want to verify the driver firstly, you can use script to take a try.

[0] https://www.libcamera.org/


[1] https://bugs.launchpad.net/bugs/1812114

I have reported similar information to the launchpad bug entry.

It might help if we can get hold of a Dell 7275 sometime although I
think Mauro at least has one ?

If this is a priority for Canonical, please contact us directly.

Not really, just raise issues from Launchpad to appropriate mailing list.

Kai-Heng


Kai-Heng

--
Regards

Kieran




___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel


Re: [PATCH] staging: media: ipu3: Replace depracated MSI API.

2020-07-19 Thread Bingbu Cao
Upadhyay,

Thanks for your patch. Please correct the typo in message.

On 7/18/20 9:32 PM, Suraj Upadhyay wrote:
> Replace depracated psi_enable_msi with pci_alloc_irq_vectors.
> And as a result modify how the returned value is handled.
> 
> Signed-off-by: Suraj Upadhyay 
> ---
>  drivers/staging/media/ipu3/ipu3.c | 4 ++--
>  1 file changed, 2 insertions(+), 2 deletions(-)
> 
> diff --git a/drivers/staging/media/ipu3/ipu3.c 
> b/drivers/staging/media/ipu3/ipu3.c
> index ee1bba6bdcac..54690e7442be 100644
> --- a/drivers/staging/media/ipu3/ipu3.c
> +++ b/drivers/staging/media/ipu3/ipu3.c
> @@ -602,9 +602,9 @@ static irqreturn_t imgu_isr(int irq, void *imgu_ptr)
>  static int imgu_pci_config_setup(struct pci_dev *dev)
>  {
>   u16 pci_command;
> - int r = pci_enable_msi(dev);
> + int r = pci_alloc_irq_vectors(dev, 1, 1, PCI_IRQ_MSI);
>  
> - if (r) {
> + if (r < 0) {
>   dev_err(&dev->dev, "failed to enable MSI (%d)\n", r);
>   return r;
>   }
> 

-- 
Best regards,
Bingbu Cao
___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel


Re: [PATCH] media: staging/intel-ipu3: css: Correctly reset some memory

2020-08-23 Thread Bingbu Cao
Thanks for the patch.

On 8/22/20 9:11 PM, Christophe JAILLET wrote:
> The intent here is to reset the whole 'scaler_coeffs_luma' array, not just
> the first element.
> 
> Fixes:e0a5b744 ("media: staging/intel-ipu3: css: Compute and 
> program ccs")
> Signed-off-by: Christophe JAILLET 
> ---
>  drivers/staging/media/ipu3/ipu3-css-params.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/drivers/staging/media/ipu3/ipu3-css-params.c 
> b/drivers/staging/media/ipu3/ipu3-css-params.c
> index fbd53d7c097c..e9d6bd9e9332 100644
> --- a/drivers/staging/media/ipu3/ipu3-css-params.c
> +++ b/drivers/staging/media/ipu3/ipu3-css-params.c
> @@ -159,7 +159,7 @@ imgu_css_scaler_calc(u32 input_width, u32 input_height, 
> u32 target_width,
>  
>   memset(&cfg->scaler_coeffs_chroma, 0,
>  sizeof(cfg->scaler_coeffs_chroma));
> - memset(&cfg->scaler_coeffs_luma, 0, sizeof(*cfg->scaler_coeffs_luma));
> + memset(&cfg->scaler_coeffs_luma, 0, sizeof(cfg->scaler_coeffs_luma));
>   do {
>   phase_step_correction++;
>  
> 
Reviewed-by: Bingbu Cao 

-- 
Best regards,
Bingbu Cao
___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel