Re: [RESEND v2] iommu/vt-d: Use passthrough mode for the Intel IPUs
Andy,
On 4/20/21 6:20 PM, Andy Shevchenko wrote:
> On Tue, Apr 20, 2021 at 10:48:33AM +0800, Bingbu Cao wrote:
>> Intel IPU(Image Processing Unit) has its own (IO)MMU hardware,
>> The IPU driver allocates its own page table that is not mapped
>> via the DMA, and thus the Intel IOMMU driver blocks access giving
>> this error:
>>
>> DMAR: DRHD: handling fault status reg 3
>> DMAR: [DMA Read] Request device [00:05.0] PASID
>> fault addr 76406000 [fault reason 06] PTE Read access is not set
>>
>> As IPU is not an external facing device which is not risky, so use
>> IOMMU passthrough mode for Intel IPUs.
>>
>> Fixes: 26f5689592e2 ("media: staging/intel-ipu3: mmu: Implement driver")
>> Signed-off-by: Bingbu Cao
>> ---
>> drivers/iommu/intel/iommu.c | 29 +
>
> This misses the changelog from v1 followed by the explanation why resent.
>
I noticed there was a typo in the recipient list:
stable.vger.kernel.org -> sta...@vger.kernel.org
no code change for resent.
--
Best regards,
Bingbu Cao
[RESEND v2] iommu/vt-d: Use passthrough mode for the Intel IPUs
Intel IPU(Image Processing Unit) has its own (IO)MMU hardware,
The IPU driver allocates its own page table that is not mapped
via the DMA, and thus the Intel IOMMU driver blocks access giving
this error:
DMAR: DRHD: handling fault status reg 3
DMAR: [DMA Read] Request device [00:05.0] PASID
fault addr 76406000 [fault reason 06] PTE Read access is not set
As IPU is not an external facing device which is not risky, so use
IOMMU passthrough mode for Intel IPUs.
Fixes: 26f5689592e2 ("media: staging/intel-ipu3: mmu: Implement driver")
Signed-off-by: Bingbu Cao
---
drivers/iommu/intel/iommu.c | 29 +
1 file changed, 29 insertions(+)
diff --git a/drivers/iommu/intel/iommu.c b/drivers/iommu/intel/iommu.c
index ee0932307d64..7e2fbdae467e 100644
--- a/drivers/iommu/intel/iommu.c
+++ b/drivers/iommu/intel/iommu.c
@@ -55,6 +55,12 @@
#define IS_GFX_DEVICE(pdev) ((pdev->class >> 16) == PCI_BASE_CLASS_DISPLAY)
#define IS_USB_DEVICE(pdev) ((pdev->class >> 8) == PCI_CLASS_SERIAL_USB)
#define IS_ISA_DEVICE(pdev) ((pdev->class >> 8) == PCI_CLASS_BRIDGE_ISA)
+#define IS_INTEL_IPU(pdev) ((pdev)->vendor == PCI_VENDOR_ID_INTEL && \
+ ((pdev)->device == 0x9a19 ||\
+(pdev)->device == 0x9a39 ||\
+(pdev)->device == 0x4e19 ||\
+(pdev)->device == 0x465d ||\
+(pdev)->device == 0x1919))
#define IS_AZALIA(pdev) ((pdev)->vendor == 0x8086 && (pdev)->device == 0x3a3e)
#define IOAPIC_RANGE_START (0xfee0)
@@ -360,6 +366,7 @@ int intel_iommu_enabled = 0;
EXPORT_SYMBOL_GPL(intel_iommu_enabled);
static int dmar_map_gfx = 1;
+static int dmar_map_ipu = 1;
static int dmar_forcedac;
static int intel_iommu_strict;
static int intel_iommu_superpage = 1;
@@ -368,6 +375,7 @@ static int iommu_skip_te_disable;
#define IDENTMAP_GFX 2
#define IDENTMAP_AZALIA4
+#define IDENTMAP_IPU 8
int intel_iommu_gfx_mapped;
EXPORT_SYMBOL_GPL(intel_iommu_gfx_mapped);
@@ -2839,6 +2847,9 @@ static int device_def_domain_type(struct device *dev)
if ((iommu_identity_mapping & IDENTMAP_GFX) &&
IS_GFX_DEVICE(pdev))
return IOMMU_DOMAIN_IDENTITY;
+
+ if ((iommu_identity_mapping & IDENTMAP_IPU) &&
IS_INTEL_IPU(pdev))
+ return IOMMU_DOMAIN_IDENTITY;
}
return 0;
@@ -3278,6 +3289,9 @@ static int __init init_dmars(void)
if (!dmar_map_gfx)
iommu_identity_mapping |= IDENTMAP_GFX;
+ if (!dmar_map_ipu)
+ iommu_identity_mapping |= IDENTMAP_IPU;
+
check_tylersburg_isoch();
ret = si_domain_init(hw_pass_through);
@@ -5622,6 +5636,18 @@ static void quirk_iommu_igfx(struct pci_dev *dev)
dmar_map_gfx = 0;
}
+static void quirk_iommu_ipu(struct pci_dev *dev)
+{
+ if (!IS_INTEL_IPU(dev))
+ return;
+
+ if (risky_device(dev))
+ return;
+
+ pci_info(dev, "Passthrough IOMMU for integrated Intel IPU\n");
+ dmar_map_ipu = 0;
+}
+
/* G4x/GM45 integrated gfx dmar support is totally busted. */
DECLARE_PCI_FIXUP_HEADER(PCI_VENDOR_ID_INTEL, 0x2a40, quirk_iommu_igfx);
DECLARE_PCI_FIXUP_HEADER(PCI_VENDOR_ID_INTEL, 0x2e00, quirk_iommu_igfx);
@@ -5657,6 +5683,9 @@ DECLARE_PCI_FIXUP_HEADER(PCI_VENDOR_ID_INTEL, 0x1632,
quirk_iommu_igfx);
DECLARE_PCI_FIXUP_HEADER(PCI_VENDOR_ID_INTEL, 0x163A, quirk_iommu_igfx);
DECLARE_PCI_FIXUP_HEADER(PCI_VENDOR_ID_INTEL, 0x163D, quirk_iommu_igfx);
+/* disable IPU dmar support */
+DECLARE_PCI_FIXUP_HEADER(PCI_VENDOR_ID_INTEL, PCI_ANY_ID, quirk_iommu_ipu);
+
static void quirk_iommu_rwbf(struct pci_dev *dev)
{
if (risky_device(dev))
--
2.7.4
[PATCH v2] iommu/vt-d: Use passthrough mode for the Intel IPUs
Intel IPU(Image Processing Unit) has its own (IO)MMU hardware,
The IPU driver allocates its own page table that is not mapped
via the DMA, and thus the Intel IOMMU driver blocks access giving
this error:
DMAR: DRHD: handling fault status reg 3
DMAR: [DMA Read] Request device [00:05.0] PASID
fault addr 76406000 [fault reason 06] PTE Read access is not set
As IPU is not an external facing device which is not risky, so use
IOMMU passthrough mode for Intel IPUs.
Fixes: 26f5689592e2 ("media: staging/intel-ipu3: mmu: Implement driver")
Signed-off-by: Bingbu Cao
---
Changes since v1:
- Use IPU PCI DID value instead of macros to align with others
- Check IPU PCI device ID in quirk
---
drivers/iommu/intel/iommu.c | 29 +
1 file changed, 29 insertions(+)
diff --git a/drivers/iommu/intel/iommu.c b/drivers/iommu/intel/iommu.c
index ee0932307d64..7e2fbdae467e 100644
--- a/drivers/iommu/intel/iommu.c
+++ b/drivers/iommu/intel/iommu.c
@@ -55,6 +55,12 @@
#define IS_GFX_DEVICE(pdev) ((pdev->class >> 16) == PCI_BASE_CLASS_DISPLAY)
#define IS_USB_DEVICE(pdev) ((pdev->class >> 8) == PCI_CLASS_SERIAL_USB)
#define IS_ISA_DEVICE(pdev) ((pdev->class >> 8) == PCI_CLASS_BRIDGE_ISA)
+#define IS_INTEL_IPU(pdev) ((pdev)->vendor == PCI_VENDOR_ID_INTEL && \
+ ((pdev)->device == 0x9a19 ||\
+(pdev)->device == 0x9a39 ||\
+(pdev)->device == 0x4e19 ||\
+(pdev)->device == 0x465d ||\
+(pdev)->device == 0x1919))
#define IS_AZALIA(pdev) ((pdev)->vendor == 0x8086 && (pdev)->device == 0x3a3e)
#define IOAPIC_RANGE_START (0xfee0)
@@ -360,6 +366,7 @@ int intel_iommu_enabled = 0;
EXPORT_SYMBOL_GPL(intel_iommu_enabled);
static int dmar_map_gfx = 1;
+static int dmar_map_ipu = 1;
static int dmar_forcedac;
static int intel_iommu_strict;
static int intel_iommu_superpage = 1;
@@ -368,6 +375,7 @@ static int iommu_skip_te_disable;
#define IDENTMAP_GFX 2
#define IDENTMAP_AZALIA4
+#define IDENTMAP_IPU 8
int intel_iommu_gfx_mapped;
EXPORT_SYMBOL_GPL(intel_iommu_gfx_mapped);
@@ -2839,6 +2847,9 @@ static int device_def_domain_type(struct device *dev)
if ((iommu_identity_mapping & IDENTMAP_GFX) &&
IS_GFX_DEVICE(pdev))
return IOMMU_DOMAIN_IDENTITY;
+
+ if ((iommu_identity_mapping & IDENTMAP_IPU) &&
IS_INTEL_IPU(pdev))
+ return IOMMU_DOMAIN_IDENTITY;
}
return 0;
@@ -3278,6 +3289,9 @@ static int __init init_dmars(void)
if (!dmar_map_gfx)
iommu_identity_mapping |= IDENTMAP_GFX;
+ if (!dmar_map_ipu)
+ iommu_identity_mapping |= IDENTMAP_IPU;
+
check_tylersburg_isoch();
ret = si_domain_init(hw_pass_through);
@@ -5622,6 +5636,18 @@ static void quirk_iommu_igfx(struct pci_dev *dev)
dmar_map_gfx = 0;
}
+static void quirk_iommu_ipu(struct pci_dev *dev)
+{
+ if (!IS_INTEL_IPU(dev))
+ return;
+
+ if (risky_device(dev))
+ return;
+
+ pci_info(dev, "Passthrough IOMMU for integrated Intel IPU\n");
+ dmar_map_ipu = 0;
+}
+
/* G4x/GM45 integrated gfx dmar support is totally busted. */
DECLARE_PCI_FIXUP_HEADER(PCI_VENDOR_ID_INTEL, 0x2a40, quirk_iommu_igfx);
DECLARE_PCI_FIXUP_HEADER(PCI_VENDOR_ID_INTEL, 0x2e00, quirk_iommu_igfx);
@@ -5657,6 +5683,9 @@ DECLARE_PCI_FIXUP_HEADER(PCI_VENDOR_ID_INTEL, 0x1632,
quirk_iommu_igfx);
DECLARE_PCI_FIXUP_HEADER(PCI_VENDOR_ID_INTEL, 0x163A, quirk_iommu_igfx);
DECLARE_PCI_FIXUP_HEADER(PCI_VENDOR_ID_INTEL, 0x163D, quirk_iommu_igfx);
+/* disable IPU dmar support */
+DECLARE_PCI_FIXUP_HEADER(PCI_VENDOR_ID_INTEL, PCI_ANY_ID, quirk_iommu_ipu);
+
static void quirk_iommu_rwbf(struct pci_dev *dev)
{
if (risky_device(dev))
--
2.7.4
[PATCH] iommu: Use passthrough mode for the Intel IPUs
Intel IPU(Image Processing Unit) has its own (IO)MMU hardware,
The IPU driver allocates its own page table that is not mapped
via the DMA, and thus the Intel IOMMU driver blocks access giving
this error:
DMAR: DRHD: handling fault status reg 3
DMAR: [DMA Read] Request device [00:05.0] PASID
fault addr 76406000 [fault reason 06] PTE Read access is not set
As IPU is not an external facing device which is not risky, so use
IOMMU passthrough mode for Intel IPUs.
Signed-off-by: Bingbu Cao
---
drivers/iommu/intel/iommu.c | 35 +++
drivers/staging/media/ipu3/ipu3.c | 2 +-
include/linux/pci_ids.h | 5 +
3 files changed, 41 insertions(+), 1 deletion(-)
diff --git a/drivers/iommu/intel/iommu.c b/drivers/iommu/intel/iommu.c
index ee0932307d64..59222d2fe73f 100644
--- a/drivers/iommu/intel/iommu.c
+++ b/drivers/iommu/intel/iommu.c
@@ -55,6 +55,12 @@
#define IS_GFX_DEVICE(pdev) ((pdev->class >> 16) == PCI_BASE_CLASS_DISPLAY)
#define IS_USB_DEVICE(pdev) ((pdev->class >> 8) == PCI_CLASS_SERIAL_USB)
#define IS_ISA_DEVICE(pdev) ((pdev->class >> 8) == PCI_CLASS_BRIDGE_ISA)
+#define IS_IPU_DEVICE(pdev) ((pdev)->vendor == PCI_VENDOR_ID_INTEL &&
\
+((pdev)->device == PCI_DEVICE_ID_INTEL_IPU3 ||
\
+(pdev)->device == PCI_DEVICE_ID_INTEL_IPU6 ||
\
+(pdev)->device == PCI_DEVICE_ID_INTEL_IPU6SE ||
\
+(pdev)->device == PCI_DEVICE_ID_INTEL_IPU6SE_P ||
\
+(pdev)->device == PCI_DEVICE_ID_INTEL_IPU6EP))
#define IS_AZALIA(pdev) ((pdev)->vendor == 0x8086 && (pdev)->device == 0x3a3e)
#define IOAPIC_RANGE_START (0xfee0)
@@ -360,6 +366,7 @@ int intel_iommu_enabled = 0;
EXPORT_SYMBOL_GPL(intel_iommu_enabled);
static int dmar_map_gfx = 1;
+static int dmar_map_ipu = 1;
static int dmar_forcedac;
static int intel_iommu_strict;
static int intel_iommu_superpage = 1;
@@ -368,6 +375,7 @@ static int iommu_skip_te_disable;
#define IDENTMAP_GFX 2
#define IDENTMAP_AZALIA4
+#define IDENTMAP_IPU 8
int intel_iommu_gfx_mapped;
EXPORT_SYMBOL_GPL(intel_iommu_gfx_mapped);
@@ -2839,6 +2847,9 @@ static int device_def_domain_type(struct device *dev)
if ((iommu_identity_mapping & IDENTMAP_GFX) &&
IS_GFX_DEVICE(pdev))
return IOMMU_DOMAIN_IDENTITY;
+
+ if ((iommu_identity_mapping & IDENTMAP_IPU) &&
IS_IPU_DEVICE(pdev))
+ return IOMMU_DOMAIN_IDENTITY;
}
return 0;
@@ -3278,6 +3289,9 @@ static int __init init_dmars(void)
if (!dmar_map_gfx)
iommu_identity_mapping |= IDENTMAP_GFX;
+ if (!dmar_map_ipu)
+ iommu_identity_mapping |= IDENTMAP_IPU;
+
check_tylersburg_isoch();
ret = si_domain_init(hw_pass_through);
@@ -5622,6 +5636,15 @@ static void quirk_iommu_igfx(struct pci_dev *dev)
dmar_map_gfx = 0;
}
+static void quirk_iommu_ipu(struct pci_dev *dev)
+{
+ if (risky_device(dev))
+ return;
+
+ pci_info(dev, "Passthrough IOMMU for integrated Intel IPU\n");
+ dmar_map_ipu = 0;
+}
+
/* G4x/GM45 integrated gfx dmar support is totally busted. */
DECLARE_PCI_FIXUP_HEADER(PCI_VENDOR_ID_INTEL, 0x2a40, quirk_iommu_igfx);
DECLARE_PCI_FIXUP_HEADER(PCI_VENDOR_ID_INTEL, 0x2e00, quirk_iommu_igfx);
@@ -5657,6 +5680,18 @@ DECLARE_PCI_FIXUP_HEADER(PCI_VENDOR_ID_INTEL, 0x1632,
quirk_iommu_igfx);
DECLARE_PCI_FIXUP_HEADER(PCI_VENDOR_ID_INTEL, 0x163A, quirk_iommu_igfx);
DECLARE_PCI_FIXUP_HEADER(PCI_VENDOR_ID_INTEL, 0x163D, quirk_iommu_igfx);
+/* disable IPU dmar support */
+DECLARE_PCI_FIXUP_HEADER(PCI_VENDOR_ID_INTEL, PCI_DEVICE_ID_INTEL_IPU3,
+quirk_iommu_ipu);
+DECLARE_PCI_FIXUP_HEADER(PCI_VENDOR_ID_INTEL, PCI_DEVICE_ID_INTEL_IPU6EP,
+quirk_iommu_ipu);
+DECLARE_PCI_FIXUP_HEADER(PCI_VENDOR_ID_INTEL, PCI_DEVICE_ID_INTEL_IPU6SE_P,
+quirk_iommu_ipu);
+DECLARE_PCI_FIXUP_HEADER(PCI_VENDOR_ID_INTEL, PCI_DEVICE_ID_INTEL_IPU6,
+quirk_iommu_ipu);
+DECLARE_PCI_FIXUP_HEADER(PCI_VENDOR_ID_INTEL, PCI_DEVICE_ID_INTEL_IPU6SE,
+quirk_iommu_ipu);
+
static void quirk_iommu_rwbf(struct pci_dev *dev)
{
if (risky_device(dev))
diff --git a/drivers/staging/media/ipu3/ipu3.c
b/drivers/staging/media/ipu3/ipu3.c
index ee1bba6bdcac..aee1130ac042 100644
--- a/drivers/staging/media/ipu3/ipu3.c
+++ b/drivers/staging/media/ipu3/ipu3.c
@@ -16,7 +16,7 @@
#include "ipu3-dmamap.h"
#include "ipu3-mmu.h"
-#define IMGU_PCI_ID0x1919
+#define IMGU_PCI_IDPCI_DEVICE_ID_INTEL_IPU3
#define IMGU_PCI_BAR 0
#define I
Re: [PATCH 1/1] staging: ipu3-imgu: Move the UAPI header from include under include/uapi
Reviewed-by: Bingbu Cao
On 4/12/21 7:11 PM, Sakari Ailus wrote:
> The header defines the user space interface but may be mistaken as
> kernel-only header due to its location. Add "uapi" directory under
> driver's include directory and move the header there.
>
> Suggested-by: Greg KH
> Signed-off-by: Sakari Ailus
> ---
> Documentation/admin-guide/media/ipu3.rst | 35 ++-
> .../media/v4l/pixfmt-meta-intel-ipu3.rst | 2 +-
> .../ipu3/include/{ => uapi}/intel-ipu3.h | 0
> drivers/staging/media/ipu3/ipu3-abi.h | 2 +-
> 4 files changed, 20 insertions(+), 19 deletions(-)
> rename drivers/staging/media/ipu3/include/{ => uapi}/intel-ipu3.h (100%)
>
> diff --git a/Documentation/admin-guide/media/ipu3.rst
> b/Documentation/admin-guide/media/ipu3.rst
> index f59697c7b374..d6454f637ff4 100644
> --- a/Documentation/admin-guide/media/ipu3.rst
> +++ b/Documentation/admin-guide/media/ipu3.rst
> @@ -234,22 +234,23 @@ The IPU3 ImgU pipelines can be configured using the
> Media Controller, defined at
> Running mode and firmware binary selection
> --
>
> -ImgU works based on firmware, currently the ImgU firmware support run 2
> pipes in
> -time-sharing with single input frame data. Each pipe can run at certain mode
> -
> -"VIDEO" or "STILL", "VIDEO" mode is commonly used for video frames capture,
> and
> -"STILL" is used for still frame capture. However, you can also select
> "VIDEO" to
> -capture still frames if you want to capture images with less system load and
> -power. For "STILL" mode, ImgU will try to use smaller BDS factor and output
> -larger bayer frame for further YUV processing than "VIDEO" mode to get high
> -quality images. Besides, "STILL" mode need XNR3 to do noise reduction, hence
> -"STILL" mode will need more power and memory bandwidth than "VIDEO" mode. TNR
> -will be enabled in "VIDEO" mode and bypassed by "STILL" mode. ImgU is
> running at
> -“VIDEO” mode by default, the user can use v4l2 control
> V4L2_CID_INTEL_IPU3_MODE
> -(currently defined in drivers/staging/media/ipu3/include/intel-ipu3.h) to
> query
> -and set the running mode. For user, there is no difference for buffer
> queueing
> -between the "VIDEO" and "STILL" mode, mandatory input and main output node
> -should be enabled and buffers need be queued, the statistics and the
> view-finder
> -queues are optional.
> +ImgU works based on firmware, currently the ImgU firmware support run 2 pipes
> +in time-sharing with single input frame data. Each pipe can run at certain
> mode
> +- "VIDEO" or "STILL", "VIDEO" mode is commonly used for video frames capture,
> +and "STILL" is used for still frame capture. However, you can also select
> +"VIDEO" to capture still frames if you want to capture images with less
> system
> +load and power. For "STILL" mode, ImgU will try to use smaller BDS factor and
> +output larger bayer frame for further YUV processing than "VIDEO" mode to get
> +high quality images. Besides, "STILL" mode need XNR3 to do noise reduction,
> +hence "STILL" mode will need more power and memory bandwidth than "VIDEO"
> mode.
> +TNR will be enabled in "VIDEO" mode and bypassed by "STILL" mode. ImgU is
> +running at “VIDEO” mode by default, the user can use v4l2 control
> +V4L2_CID_INTEL_IPU3_MODE (currently defined in
> +drivers/staging/media/ipu3/include/uapi/intel-ipu3.h) to query and set the
> +running mode. For user, there is no difference for buffer queueing between
> the
> +"VIDEO" and "STILL" mode, mandatory input and main output node should be
> +enabled and buffers need be queued, the statistics and the view-finder queues
> +are optional.
>
> The firmware binary will be selected according to current running mode, such
> log
> "using binary if_to_osys_striped " or "using binary
> if_to_osys_primary_striped"
> @@ -586,7 +587,7 @@ preserved.
> References
> ==
>
> -.. [#f5] drivers/staging/media/ipu3/include/intel-ipu3.h
> +.. [#f5] drivers/staging/media/ipu3/include/uapi/intel-ipu3.h
>
> .. [#f1] https://github.com/intel/nvt
>
> diff --git a/Documentation/userspace-api/media/v4l/pixfmt-meta-intel-ipu3.rst
> b/Documentation/userspace-api/media/v4l/pixfmt-meta-intel-ipu3.rst
> index 5f33d35532ef..84d81dd7a7b5 100644
> --- a/Documentation/userspace-api/media/v4l/pixfmt-meta-intel-ipu3.rst
> +++ b/Documentation/userspace-api/med
Re: [PATCH v2 1/2] media: staging/intel-ipu3: Fix memory leak in imu_fmt
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(_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] =
>>> _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
Re: [PATCH v2 1/2] media: staging/intel-ipu3: Fix memory leak in imu_fmt
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(_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] = _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
Re: [PATCH v9 6/7] media: i2c: imx319: Support probe while the device is off
Reviewed-by: Bingbu Cao
On 1/29/21 7:27 AM, Sakari Ailus wrote:
> From: Rajmohan Mani
>
> Tell ACPI device PM code that the driver supports the device being powered
> off when the driver's probe function is entered.
>
> Signed-off-by: Rajmohan Mani
> Signed-off-by: Sakari Ailus
> ---
> drivers/media/i2c/imx319.c | 72 +++---
> 1 file changed, 44 insertions(+), 28 deletions(-)
>
> diff --git a/drivers/media/i2c/imx319.c b/drivers/media/i2c/imx319.c
> index 8473c0bbb35d6..e0b22e9318fed 100644
> --- a/drivers/media/i2c/imx319.c
> +++ b/drivers/media/i2c/imx319.c
> @@ -140,6 +140,8 @@ struct imx319 {
>
> /* Streaming on/off */
> bool streaming;
> + /* True if the device has been identified */
> + bool identified;
> };
>
> static const struct imx319_reg imx319_global_regs[] = {
> @@ -2084,6 +2086,31 @@ imx319_set_pad_format(struct v4l2_subdev *sd,
> return 0;
> }
>
> +/* Verify chip ID */
> +static int imx319_identify_module(struct imx319 *imx319)
> +{
> + struct i2c_client *client = v4l2_get_subdevdata(>sd);
> + int ret;
> + u32 val;
> +
> + if (imx319->identified)
> + return 0;
> +
> + ret = imx319_read_reg(imx319, IMX319_REG_CHIP_ID, 2, );
> + if (ret)
> + return ret;
> +
> + if (val != IMX319_CHIP_ID) {
> + dev_err(>dev, "chip id mismatch: %x!=%x",
> + IMX319_CHIP_ID, val);
> + return -EIO;
> + }
> +
> + imx319->identified = true;
> +
> + return 0;
> +}
> +
> /* Start streaming */
> static int imx319_start_streaming(struct imx319 *imx319)
> {
> @@ -2091,6 +2118,10 @@ static int imx319_start_streaming(struct imx319
> *imx319)
> const struct imx319_reg_list *reg_list;
> int ret;
>
> + ret = imx319_identify_module(imx319);
> + if (ret)
> + return ret;
> +
> /* Global Setting */
> reg_list = _global_setting;
> ret = imx319_write_regs(imx319, reg_list->regs, reg_list->num_of_regs);
> @@ -2208,26 +2239,6 @@ static int __maybe_unused imx319_resume(struct device
> *dev)
> return ret;
> }
>
> -/* Verify chip ID */
> -static int imx319_identify_module(struct imx319 *imx319)
> -{
> - struct i2c_client *client = v4l2_get_subdevdata(>sd);
> - int ret;
> - u32 val;
> -
> - ret = imx319_read_reg(imx319, IMX319_REG_CHIP_ID, 2, );
> - if (ret)
> - return ret;
> -
> - if (val != IMX319_CHIP_ID) {
> - dev_err(>dev, "chip id mismatch: %x!=%x",
> - IMX319_CHIP_ID, val);
> - return -EIO;
> - }
> -
> - return 0;
> -}
> -
> static const struct v4l2_subdev_core_ops imx319_subdev_core_ops = {
> .subscribe_event = v4l2_ctrl_subdev_subscribe_event,
> .unsubscribe_event = v4l2_event_subdev_unsubscribe,
> @@ -2422,6 +2433,7 @@ static struct imx319_hwcfg *imx319_get_hwcfg(struct
> device *dev)
> static int imx319_probe(struct i2c_client *client)
> {
> struct imx319 *imx319;
> + bool low_power;
> int ret;
> u32 i;
>
> @@ -2434,11 +2446,14 @@ static int imx319_probe(struct i2c_client *client)
> /* Initialize subdev */
> v4l2_i2c_subdev_init(>sd, client, _subdev_ops);
>
> - /* Check module identity */
> - ret = imx319_identify_module(imx319);
> - if (ret) {
> - dev_err(>dev, "failed to find sensor: %d", ret);
> - goto error_probe;
> + low_power = acpi_dev_state_low_power(>dev);
> + if (!low_power) {
> + /* Check module identity */
> + ret = imx319_identify_module(imx319);
> + if (ret) {
> + dev_err(>dev, "failed to find sensor: %d", ret);
> + goto error_probe;
> + }
> }
>
> imx319->hwcfg = imx319_get_hwcfg(>dev);
> @@ -2491,10 +2506,10 @@ static int imx319_probe(struct i2c_client *client)
> goto error_media_entity;
>
> /*
> - * Device is already turned on by i2c-core with ACPI domain PM.
> - * Enable runtime PM and turn off the device.
> + * Don't set the device's state to active if it's in a low power state.
>*/
> - pm_runtime_set_active(>dev);
> + if (!low_power)
> + pm_runtime_set_active(>dev);
> pm_runtime_enable(>dev);
> pm_runtime_idle(>dev);
>
> @@ -2547,6 +2562,7 @@ static struct i2c_driver imx319_i2c_driver = {
> },
> .probe_new = imx319_probe,
> .remove = imx319_remove,
> + .flags = I2C_DRV_FL_ALLOW_LOW_POWER_PROBE,
> };
> module_i2c_driver(imx319_i2c_driver);
>
>
--
Best regards,
Bingbu Cao
Re: [PATCH v2 1/1] v4l: ioctl: Fix memory leak in video_usercopy
On 1/14/21 12:50 PM, Bingbu Cao wrote:
> Sakari,
>
> On 12/21/20 4:11 AM, Sakari Ailus wrote:
>> When an IOCTL with argument size larger than 128 that also used array
>> arguments were handled, two memory allocations were made but alas, only
>> the latter one of them was released. This happened because there was only
>> a single local variable to hold such a temporary allocation.
>>
>> Fix this by adding separate variables to hold the pointers to the
>> temporary allocations.
>>
>> Reported-by: Arnd Bergmann
>> Reported-by: syzbot+1115e79c8df6472c6...@syzkaller.appspotmail.com
>> Fixes: d14e6d76ebf7 ("[media] v4l: Add multi-planar ioctl handling code")
>> Cc: sta...@vger.kernel.org
>> Signed-off-by: Sakari Ailus
>> Reviewed-by: Laurent Pinchart
>> ---
>> drivers/media/v4l2-core/v4l2-ioctl.c | 32
>> 1 file changed, 14 insertions(+), 18 deletions(-)
>>
>> diff --git a/drivers/media/v4l2-core/v4l2-ioctl.c
>> b/drivers/media/v4l2-core/v4l2-ioctl.c
>> index 3198abdd538c..9906b41004e9 100644
>> --- a/drivers/media/v4l2-core/v4l2-ioctl.c
>> +++ b/drivers/media/v4l2-core/v4l2-ioctl.c
>> @@ -3283,7 +3283,7 @@ video_usercopy(struct file *file, unsigned int
>> orig_cmd, unsigned long arg,
>> v4l2_kioctl func)
>> {
>> charsbuf[128];
>> -void*mbuf = NULL;
>> +void*mbuf = NULL, *array_buf = NULL;
>> void*parg = (void *)arg;
>> longerr = -EINVAL;
>> boolhas_array_args;
>> @@ -3318,27 +3318,21 @@ video_usercopy(struct file *file, unsigned int
>> orig_cmd, unsigned long arg,
>> has_array_args = err;
>>
>> if (has_array_args) {
>> -/*
>> - * When adding new types of array args, make sure that the
>> - * parent argument to ioctl (which contains the pointer to the
>> - * array) fits into sbuf (so that mbuf will still remain
>> - * unused up to here).
>> - */
>> -mbuf = kvmalloc(array_size, GFP_KERNEL);
>> +array_buf = kvmalloc(array_size, GFP_KERNEL);
>> err = -ENOMEM;
>> -if (NULL == mbuf)
>> +if (array_buf == NULL)
>
> if (!array_buf)
> ?
>
Please ignore my previous comment, as the patch was landed. :)
--
Best regards,
Bingbu Cao
Re: [PATCH v2 1/1] v4l: ioctl: Fix memory leak in video_usercopy
Sakari,
On 12/21/20 4:11 AM, Sakari Ailus wrote:
> When an IOCTL with argument size larger than 128 that also used array
> arguments were handled, two memory allocations were made but alas, only
> the latter one of them was released. This happened because there was only
> a single local variable to hold such a temporary allocation.
>
> Fix this by adding separate variables to hold the pointers to the
> temporary allocations.
>
> Reported-by: Arnd Bergmann
> Reported-by: syzbot+1115e79c8df6472c6...@syzkaller.appspotmail.com
> Fixes: d14e6d76ebf7 ("[media] v4l: Add multi-planar ioctl handling code")
> Cc: sta...@vger.kernel.org
> Signed-off-by: Sakari Ailus
> Reviewed-by: Laurent Pinchart
> ---
> drivers/media/v4l2-core/v4l2-ioctl.c | 32
> 1 file changed, 14 insertions(+), 18 deletions(-)
>
> diff --git a/drivers/media/v4l2-core/v4l2-ioctl.c
> b/drivers/media/v4l2-core/v4l2-ioctl.c
> index 3198abdd538c..9906b41004e9 100644
> --- a/drivers/media/v4l2-core/v4l2-ioctl.c
> +++ b/drivers/media/v4l2-core/v4l2-ioctl.c
> @@ -3283,7 +3283,7 @@ video_usercopy(struct file *file, unsigned int
> orig_cmd, unsigned long arg,
> v4l2_kioctl func)
> {
> charsbuf[128];
> - void*mbuf = NULL;
> + void*mbuf = NULL, *array_buf = NULL;
> void*parg = (void *)arg;
> longerr = -EINVAL;
> boolhas_array_args;
> @@ -3318,27 +3318,21 @@ video_usercopy(struct file *file, unsigned int
> orig_cmd, unsigned long arg,
> has_array_args = err;
>
> if (has_array_args) {
> - /*
> - * When adding new types of array args, make sure that the
> - * parent argument to ioctl (which contains the pointer to the
> - * array) fits into sbuf (so that mbuf will still remain
> - * unused up to here).
> - */
> - mbuf = kvmalloc(array_size, GFP_KERNEL);
> + array_buf = kvmalloc(array_size, GFP_KERNEL);
> err = -ENOMEM;
> - if (NULL == mbuf)
> + if (array_buf == NULL)
if (!array_buf)
?
> goto out_array_args;
> err = -EFAULT;
> if (in_compat_syscall())
> - err = v4l2_compat_get_array_args(file, mbuf, user_ptr,
> - array_size, orig_cmd,
> - parg);
> + err = v4l2_compat_get_array_args(file, array_buf,
> + user_ptr, array_size,
> + orig_cmd, parg);
> else
> - err = copy_from_user(mbuf, user_ptr, array_size) ?
> + err = copy_from_user(array_buf, user_ptr, array_size) ?
> -EFAULT : 0;
> if (err)
> goto out_array_args;
> - *kernel_ptr = mbuf;
> + *kernel_ptr = array_buf;
> }
>
> /* Handles IOCTL */
> @@ -3360,12 +3354,13 @@ video_usercopy(struct file *file, unsigned int
> orig_cmd, unsigned long arg,
> if (in_compat_syscall()) {
> int put_err;
>
> - put_err = v4l2_compat_put_array_args(file, user_ptr,
> mbuf,
> - array_size,
> orig_cmd,
> - parg);
> + put_err = v4l2_compat_put_array_args(file, user_ptr,
> + array_buf,
> + array_size,
> + orig_cmd, parg);
> if (put_err)
> err = put_err;
> - } else if (copy_to_user(user_ptr, mbuf, array_size)) {
> + } else if (copy_to_user(user_ptr, array_buf, array_size)) {
> err = -EFAULT;
> }
> goto out_array_args;
> @@ -3381,6 +3376,7 @@ video_usercopy(struct file *file, unsigned int
> orig_cmd, unsigned long arg,
> if (video_put_user((void __user *)arg, parg, cmd, orig_cmd))
> err = -EFAULT;
> out:
> + kvfree(array_buf);
> kvfree(mbuf);
> return err;
> }
>
--
Best regards,
Bingbu Cao
Re: [PATCH][next] media: ov2740: fix dereference before null check on pointer nvm
Hi, Colin
Thanks for your patch.
On 11/26/20 7:49 PM, Colin King wrote:
> From: Colin Ian King
>
> Currently pointer nvm is being dereferenced before it is being null
> checked. Fix this by moving the assignments of pointers client and
> ov2740 so that are after the null check hence avoiding any potential
> null pointer dereferences on pointer nvm.
>
> Fixes: 5e6fd339b68d ("media: ov2740: allow OTP data access during streaming")
> Signed-off-by: Colin Ian King
> ---
> drivers/media/i2c/ov2740.c | 7 +--
> 1 file changed, 5 insertions(+), 2 deletions(-)
>
> diff --git a/drivers/media/i2c/ov2740.c b/drivers/media/i2c/ov2740.c
> index 99016546cbec..b41a90c2aed5 100644
> --- a/drivers/media/i2c/ov2740.c
> +++ b/drivers/media/i2c/ov2740.c
> @@ -600,8 +600,8 @@ static void ov2740_update_pad_format(const struct
> ov2740_mode *mode,
>
> static int ov2740_load_otp_data(struct nvm_data *nvm)
> {
> - struct i2c_client *client = nvm->client;
> - struct ov2740 *ov2740 = to_ov2740(i2c_get_clientdata(client));
> + struct i2c_client *client;
> + struct ov2740 *ov2740;
> u32 isp_ctrl00 = 0;
> u32 isp_ctrl01 = 0;
> int ret;
> @@ -612,6 +612,9 @@ static int ov2740_load_otp_data(struct nvm_data *nvm)
> if (nvm->nvm_buffer)
> return 0;
>
> + client = nvm->client;
> + ov2740 = to_ov2740(i2c_get_clientdata(client));
> +
> nvm->nvm_buffer = kzalloc(CUSTOMER_USE_OTP_SIZE, GFP_KERNEL);
> if (!nvm->nvm_buffer)
> return -ENOMEM;
>
Reviewed-by: Bingbu Cao
--
Best regards,
Bingbu Cao
Re: [PATCH 10/18] ipu3-cio2: Rename ipu3-cio2.c to allow module to be built from multiple source files retaining ipu3-cio2 name
On 12/1/20 2:56 PM, Bingbu Cao wrote:
> I see there will be multiple files, but there will be no conflict if keep as
> the main
> file name unchanged, right? If so, I prefer keep as it was.
Oops, I notice you try to build all the files into single module, so please
ignore my
comment above.
>
> On 11/30/20 9:31 PM, Daniel Scally wrote:
>> ipu3-cio2 driver needs extending with multiple files; rename the main
>> source file and specify the renamed file in Makefile to accommodate that.
>>
>> Suggested-by: Andy Shevchenko
>> Reviewed-by: Andy Shevchenko
>> Reviewed-by: Laurent Pinchart
>> Signed-off-by: Daniel Scally
>> ---
>> Changes since RFC v3:
>>
>> - None
>>
>> drivers/media/pci/intel/ipu3/Makefile | 2 ++
>> drivers/media/pci/intel/ipu3/{ipu3-cio2.c => ipu3-cio2-main.c} | 0
>> 2 files changed, 2 insertions(+)
>> rename drivers/media/pci/intel/ipu3/{ipu3-cio2.c => ipu3-cio2-main.c} (100%)
>>
>> diff --git a/drivers/media/pci/intel/ipu3/Makefile
>> b/drivers/media/pci/intel/ipu3/Makefile
>> index 98ddd5beafe0..429d516452e4 100644
>> --- a/drivers/media/pci/intel/ipu3/Makefile
>> +++ b/drivers/media/pci/intel/ipu3/Makefile
>> @@ -1,2 +1,4 @@
>> # SPDX-License-Identifier: GPL-2.0-only
>> obj-$(CONFIG_VIDEO_IPU3_CIO2) += ipu3-cio2.o
>> +
>> +ipu3-cio2-y += ipu3-cio2-main.o
>> diff --git a/drivers/media/pci/intel/ipu3/ipu3-cio2.c
>> b/drivers/media/pci/intel/ipu3/ipu3-cio2-main.c
>> similarity index 100%
>> rename from drivers/media/pci/intel/ipu3/ipu3-cio2.c
>> rename to drivers/media/pci/intel/ipu3/ipu3-cio2-main.c
>>
>
--
Best regards,
Bingbu Cao
Re: [PATCH 10/18] ipu3-cio2: Rename ipu3-cio2.c to allow module to be built from multiple source files retaining ipu3-cio2 name
I see there will be multiple files, but there will be no conflict if keep as
the main
file name unchanged, right? If so, I prefer keep as it was.
On 11/30/20 9:31 PM, Daniel Scally wrote:
> ipu3-cio2 driver needs extending with multiple files; rename the main
> source file and specify the renamed file in Makefile to accommodate that.
>
> Suggested-by: Andy Shevchenko
> Reviewed-by: Andy Shevchenko
> Reviewed-by: Laurent Pinchart
> Signed-off-by: Daniel Scally
> ---
> Changes since RFC v3:
>
> - None
>
> drivers/media/pci/intel/ipu3/Makefile | 2 ++
> drivers/media/pci/intel/ipu3/{ipu3-cio2.c => ipu3-cio2-main.c} | 0
> 2 files changed, 2 insertions(+)
> rename drivers/media/pci/intel/ipu3/{ipu3-cio2.c => ipu3-cio2-main.c} (100%)
>
> diff --git a/drivers/media/pci/intel/ipu3/Makefile
> b/drivers/media/pci/intel/ipu3/Makefile
> index 98ddd5beafe0..429d516452e4 100644
> --- a/drivers/media/pci/intel/ipu3/Makefile
> +++ b/drivers/media/pci/intel/ipu3/Makefile
> @@ -1,2 +1,4 @@
> # SPDX-License-Identifier: GPL-2.0-only
> obj-$(CONFIG_VIDEO_IPU3_CIO2) += ipu3-cio2.o
> +
> +ipu3-cio2-y += ipu3-cio2-main.o
> diff --git a/drivers/media/pci/intel/ipu3/ipu3-cio2.c
> b/drivers/media/pci/intel/ipu3/ipu3-cio2-main.c
> similarity index 100%
> rename from drivers/media/pci/intel/ipu3/ipu3-cio2.c
> rename to drivers/media/pci/intel/ipu3/ipu3-cio2-main.c
>
--
Best regards,
Bingbu Cao
Re: [PATCH 01/18] property: Return true in fwnode_device_is_available for node types that do not implement this operation
Daniel, thanks for your patch.
On 11/30/20 9:31 PM, Daniel Scally wrote:
> Some types of fwnode_handle do not implement the device_is_available()
> check, such as those created by software_nodes. There isn't really a
> meaningful way to check for the availability of a device that doesn't
> actually exist, so if the check isn't implemented just assume that the
> "device" is present.
>
> Suggested-by: Laurent Pinchart
> Signed-off-by: Daniel Scally
> ---
> Changes since RFC v3:
>
> patch introduced
>
> drivers/base/property.c | 5 +
> 1 file changed, 5 insertions(+)
>
> diff --git a/drivers/base/property.c b/drivers/base/property.c
> index 4c43d30145c6..a5ca2306796f 100644
> --- a/drivers/base/property.c
> +++ b/drivers/base/property.c
> @@ -785,9 +785,14 @@ EXPORT_SYMBOL_GPL(fwnode_handle_put);
> /**
> * fwnode_device_is_available - check if a device is available for use
> * @fwnode: Pointer to the fwnode of the device.
> + *
> + * For fwnode node types that don't implement the .device_is_available()
> + * operation, this function returns true.
> */
> bool fwnode_device_is_available(const struct fwnode_handle *fwnode)
> {
> + if (!fwnode_has_op(fwnode, device_is_available))
> + return true;
blank line here?
> return fwnode_call_bool_op(fwnode, device_is_available);
> }
> EXPORT_SYMBOL_GPL(fwnode_device_is_available);
>
--
Best regards,
Bingbu Cao
Re: [PATCH] media: ov8856: Remove 3280x2464 mode
On 11/24/20 6:20 PM, Robert Foss wrote: > On Tue, 24 Nov 2020 at 10:42, Bingbu Cao wrote: >> >> Hi, Robert >> >> I remember that the full size of ov8856 image sensor is 3296x2480 and we can >> get the 3280x2464 >> frames based on current settings. >> >> Do you have any issues with this mode? > > As far as I can tell using the 3280x2464 mode actually yields an > output resolution that is 3264x2448. > > What does your hardware setup look like? And which revision of the > sensor are you using? > Robert, the sensor revision I am using is v1.1. I just checked the actual output pixels on our hardware, the output resolution with 2464 mode is 3280x2464, no black pixels. As Tomasz said, some ISP has the requirement of extra pixel padding, From the ov8856 datasheet, the central 3264x2448 pixels are *suggested* to be output from the pixel array and the boundary pixels can be used for additional processing. In my understanding, the 32 dummy lines are not black lines. -- Best regards, Bingbu Cao
Re: [PATCH] media: ov8856: Remove 3280x2464 mode
Hi, Robert
I remember that the full size of ov8856 image sensor is 3296x2480 and we can
get the 3280x2464
frames based on current settings.
Do you have any issues with this mode?
On 11/16/20 11:50 PM, Robert Foss wrote:
> 0x3812, 0x00},
> 236 {0x3813, 0x01},
--
Best regards,
Bingbu Cao
Re: [PATCH] media: staging/intel-ipu3: css: Correctly reset some memory
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(>scaler_coeffs_chroma, 0,
> sizeof(cfg->scaler_coeffs_chroma));
> - memset(>scaler_coeffs_luma, 0, sizeof(*cfg->scaler_coeffs_luma));
> + memset(>scaler_coeffs_luma, 0, sizeof(cfg->scaler_coeffs_luma));
> do {
> phase_step_correction++;
>
>
Reviewed-by: Bingbu Cao
--
Best regards,
Bingbu Cao
Re: [PATCH v5 0/6] Support running driver's probe for a device powered off
On 8/14/20 12:11 PM, Bingbu Cao wrote: > > > On 8/10/20 10:27 PM, Sakari Ailus wrote: >> Hi all, >> > ...snip... >> >> The use case is such that there is a privacy LED next to an integrated >> user-facing laptop camera, and this LED is there to signal the user that >> the camera is recording a video or capturing images. That LED also happens >> to be wired to one of the power supplies of the camera, so whenever you >> power on the camera, the LED will be lit, whether images are captured from >> the camera --- or not. There's no way to implement this differently >> without additional software control (allowing of which is itself a >> hardware design decision) on most CSI-2-connected camera sensors as they >> simply have no pin to signal the camera streaming state. >> >> This is also what happens during driver probe: the camera will be powered >> on by the I²C subsystem calling dev_pm_domain_attach() and the device is >> already powered on when the driver's own probe function is called. To the >> user this visible during the boot process as a blink of the privacy LED, >> suggesting that the camera is recording without the user having used an >> application to do that. From the end user's point of view the behaviour is >> not expected and for someone unfamiliar with internal workings of a >> computer surely seems quite suspicious --- even if images are not being >> actually captured. >> >> I've tested these on linux-next master. They also apply to Wolfram's >> i2c/for-next branch, there's a patch that affects the I²C core changes >> here (see below). The patches apart from that apply to Bartosz's >> at24/for-next as well as Mauro's linux-media master branch. > > Sakari, we meet one issue - once the vcm sub-device registered, the user space > will try to open the VCM (I have not figure out who did that), it will also > trigger the acpi pm resume/suspend, as the VCM always shares same power rail > with camera sensor, so the privacy LED still has a blink. Sakari, please ignore my previous comment, it is not related to this change. I see the sub device open is caused by v4l_id program from udev. > >> > ...snip... > -- Best regards, Bingbu Cao
Re: [PATCH v5 3/6] ov5670: Support probe whilst the device is in a low power state
On 8/12/20 5:12 PM, Bingbu Cao wrote:
>
>
> On 8/10/20 10:27 PM, Sakari Ailus wrote:
>> Tell ACPI device PM code that the driver supports the device being in a
>> low power state when the driver's probe function is entered.
>>
>> Signed-off-by: Sakari Ailus
>> ---
>> drivers/media/i2c/ov5670.c | 23 ++-
>> 1 file changed, 14 insertions(+), 9 deletions(-)
>>
>> diff --git a/drivers/media/i2c/ov5670.c b/drivers/media/i2c/ov5670.c
>> index f26252e35e08d..1f75b888d2a18 100644
>> --- a/drivers/media/i2c/ov5670.c
>> +++ b/drivers/media/i2c/ov5670.c
>> @@ -2456,6 +2456,7 @@ static int ov5670_probe(struct i2c_client *client)
>> struct ov5670 *ov5670;
>> const char *err_msg;
>> u32 input_clk = 0;
>> +bool low_power;
>> int ret;
>>
>> device_property_read_u32(>dev, "clock-frequency", _clk);
>> @@ -2472,11 +2473,14 @@ static int ov5670_probe(struct i2c_client *client)
>> /* Initialize subdev */
>> v4l2_i2c_subdev_init(>sd, client, _subdev_ops);
>>
>> -/* Check module identity */
>> -ret = ov5670_identify_module(ov5670);
>> -if (ret) {
>> -err_msg = "ov5670_identify_module() error";
>> -goto error_print;
>> +low_power = acpi_dev_state_low_power(>dev);
>> +if (!low_power) {
>> +/* Check module identity */
>> +ret = ov5670_identify_module(ov5670);
>> +if (ret) {
>> +err_msg = "ov5670_identify_module() error";
>> +goto error_print;
>> +
>
> Sakari, thanks for your patch.
> one question - With this change, there will be no chance for driver to
> guarantee
> that the camera sensor plugged in is the camera that the matched driver
> actually
> can drive until try to streaming the camera, so is it necessary to return
> appropriate error in .s_stream ops to notify user it is not the hardware that
> current driver can drive? if no other better way.
Sakari, please ignore my previous comment, it is not related to this change. I
see the sub device open is caused by v4l_id program from udev.
>
--
Best regards,
Bingbu Cao
Re: [PATCH v5 0/6] Support running driver's probe for a device powered off
On 8/10/20 10:27 PM, Sakari Ailus wrote: > Hi all, > ...snip... > > The use case is such that there is a privacy LED next to an integrated > user-facing laptop camera, and this LED is there to signal the user that > the camera is recording a video or capturing images. That LED also happens > to be wired to one of the power supplies of the camera, so whenever you > power on the camera, the LED will be lit, whether images are captured from > the camera --- or not. There's no way to implement this differently > without additional software control (allowing of which is itself a > hardware design decision) on most CSI-2-connected camera sensors as they > simply have no pin to signal the camera streaming state. > > This is also what happens during driver probe: the camera will be powered > on by the I²C subsystem calling dev_pm_domain_attach() and the device is > already powered on when the driver's own probe function is called. To the > user this visible during the boot process as a blink of the privacy LED, > suggesting that the camera is recording without the user having used an > application to do that. From the end user's point of view the behaviour is > not expected and for someone unfamiliar with internal workings of a > computer surely seems quite suspicious --- even if images are not being > actually captured. > > I've tested these on linux-next master. They also apply to Wolfram's > i2c/for-next branch, there's a patch that affects the I²C core changes > here (see below). The patches apart from that apply to Bartosz's > at24/for-next as well as Mauro's linux-media master branch. Sakari, we meet one issue - once the vcm sub-device registered, the user space will try to open the VCM (I have not figure out who did that), it will also trigger the acpi pm resume/suspend, as the VCM always shares same power rail with camera sensor, so the privacy LED still has a blink. > ...snip... -- Best regards, Bingbu Cao
[PATCH v3] nvmem: core: add sanity check in nvmem_device_read()
nvmem device read/write could be called directly once nvmem
device registered, the sanity check should be done before each
nvmem_reg_read/write().
Signed-off-by: Bingbu Cao
---
drivers/nvmem/core.c | 6 ++
1 file changed, 6 insertions(+)
diff --git a/drivers/nvmem/core.c b/drivers/nvmem/core.c
index 927eb5f6003f..09ad5a06efee 100644
--- a/drivers/nvmem/core.c
+++ b/drivers/nvmem/core.c
@@ -69,6 +69,9 @@ static BLOCKING_NOTIFIER_HEAD(nvmem_notifier);
static int nvmem_reg_read(struct nvmem_device *nvmem, unsigned int offset,
void *val, size_t bytes)
{
+ if (bytes + offset > nvmem->size)
+ return -EINVAL;
+
if (nvmem->reg_read)
return nvmem->reg_read(nvmem->priv, offset, val, bytes);
@@ -80,6 +83,9 @@ static int nvmem_reg_write(struct nvmem_device *nvmem,
unsigned int offset,
{
int ret;
+ if (bytes + offset > nvmem->size)
+ return -EINVAL;
+
if (nvmem->reg_write) {
gpiod_set_value_cansleep(nvmem->wp_gpio, 0);
ret = nvmem->reg_write(nvmem->priv, offset, val, bytes);
--
2.7.4
Re: [PATCH v5 3/6] ov5670: Support probe whilst the device is in a low power state
On 8/10/20 10:27 PM, Sakari Ailus wrote:
> Tell ACPI device PM code that the driver supports the device being in a
> low power state when the driver's probe function is entered.
>
> Signed-off-by: Sakari Ailus
> ---
> drivers/media/i2c/ov5670.c | 23 ++-
> 1 file changed, 14 insertions(+), 9 deletions(-)
>
> diff --git a/drivers/media/i2c/ov5670.c b/drivers/media/i2c/ov5670.c
> index f26252e35e08d..1f75b888d2a18 100644
> --- a/drivers/media/i2c/ov5670.c
> +++ b/drivers/media/i2c/ov5670.c
> @@ -2456,6 +2456,7 @@ static int ov5670_probe(struct i2c_client *client)
> struct ov5670 *ov5670;
> const char *err_msg;
> u32 input_clk = 0;
> + bool low_power;
> int ret;
>
> device_property_read_u32(>dev, "clock-frequency", _clk);
> @@ -2472,11 +2473,14 @@ static int ov5670_probe(struct i2c_client *client)
> /* Initialize subdev */
> v4l2_i2c_subdev_init(>sd, client, _subdev_ops);
>
> - /* Check module identity */
> - ret = ov5670_identify_module(ov5670);
> - if (ret) {
> - err_msg = "ov5670_identify_module() error";
> - goto error_print;
> + low_power = acpi_dev_state_low_power(>dev);
> + if (!low_power) {
> + /* Check module identity */
> + ret = ov5670_identify_module(ov5670);
> + if (ret) {
> + err_msg = "ov5670_identify_module() error";
> + goto error_print;
> +
Sakari, thanks for your patch.
one question - With this change, there will be no chance for driver to guarantee
that the camera sensor plugged in is the camera that the matched driver actually
can drive until try to streaming the camera, so is it necessary to return
appropriate error in .s_stream ops to notify user it is not the hardware that
current driver can drive? if no other better way.
--
Best regards,
Bingbu Cao
Re: [PATCH] nvmem: core: add sanity check in nvmem_device_read()
On 8/4/20 6:03 PM, Srinivas Kandagatla wrote: > > > On 04/08/2020 10:58, Sakari Ailus wrote: >> Hi Bingbu, >> >> Thank you for the patch. >> >> On Tue, Aug 04, 2020 at 05:13:56PM +0800, Bingbu Cao wrote: >>> nvmem_device_read() could be called directly once nvmem device >>> registered, the sanity check should be done before call >>> nvmem_reg_read() as cell and sysfs read did now. >>> >>> Signed-off-by: Bingbu Cao >>> --- >>> drivers/nvmem/core.c | 7 +++ >>> 1 file changed, 7 insertions(+) >>> >>> diff --git a/drivers/nvmem/core.c b/drivers/nvmem/core.c >>> index 927eb5f6003f..c9a77993f008 100644 >>> --- a/drivers/nvmem/core.c >>> +++ b/drivers/nvmem/core.c >>> @@ -1491,6 +1491,13 @@ int nvmem_device_read(struct nvmem_device *nvmem, >>> if (!nvmem) >>> return -EINVAL; >>> + if (offset >= nvmem->size || bytes < nvmem->word_size) >>> + return 0; >>> + >>> + if (bytes + offset > nvmem->size) >>> + bytes = nvmem->size - offset; >> >> The check is relevant for nvmem_device_write(), too. >> >> There are also other ways to access nvmem devices such as nvmem_cell_read >> and others alike. Should they be considered as well? > > We should probably move these sanity checks to a common place like > nvmem_reg_read() and nvmem_reg_write(), so the callers need not duplicate the > same! > Srini and Sakari, thanks for your review. Is it OK just return INVAL with simple check like below? if (bytes + offset > nvmem->size || bytes != round_down(bytes, nvmem->word_size)) return -EINVAL; > --srini > >> >>> + >>> + bytes = round_down(bytes, nvmem->word_size); >>> rc = nvmem_reg_read(nvmem, offset, buf, bytes); >>> if (rc) >> -- Best regards, Bingbu Cao
[PATCH] nvmem: core: add sanity check in nvmem_device_read()
nvmem_device_read() could be called directly once nvmem device registered, the sanity check should be done before call nvmem_reg_read() as cell and sysfs read did now. Signed-off-by: Bingbu Cao --- drivers/nvmem/core.c | 7 +++ 1 file changed, 7 insertions(+) diff --git a/drivers/nvmem/core.c b/drivers/nvmem/core.c index 927eb5f6003f..c9a77993f008 100644 --- a/drivers/nvmem/core.c +++ b/drivers/nvmem/core.c @@ -1491,6 +1491,13 @@ int nvmem_device_read(struct nvmem_device *nvmem, if (!nvmem) return -EINVAL; + if (offset >= nvmem->size || bytes < nvmem->word_size) + return 0; + + if (bytes + offset > nvmem->size) + bytes = nvmem->size - offset; + + bytes = round_down(bytes, nvmem->word_size); rc = nvmem_reg_read(nvmem, offset, buf, bytes); if (rc) -- 2.7.4
[PATCH v2] MAINTAINERS: Fix email typo and correct name of Tianshu
Fix the typo in email address of Tianshu Qiu and correct the name. Reported-by: Bjorn Helgaas Signed-off-by: Tianshu Qiu Signed-off-by: Bingbu Cao --- MAINTAINERS | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/MAINTAINERS b/MAINTAINERS index 5392f00cec46..638dfa99751b 100644 --- a/MAINTAINERS +++ b/MAINTAINERS @@ -8765,7 +8765,7 @@ INTEL IPU3 CSI-2 CIO2 DRIVER M: Yong Zhi M: Sakari Ailus M: Bingbu Cao -R: Tian Shu Qiu +R: Tianshu Qiu L: linux-me...@vger.kernel.org S: Maintained F: Documentation/userspace-api/media/v4l/pixfmt-srggb10-ipu3.rst @@ -8774,7 +8774,7 @@ F:drivers/media/pci/intel/ipu3/ INTEL IPU3 CSI-2 IMGU DRIVER M: Sakari Ailus R: Bingbu Cao -R: Tian Shu Qiu +R: Tianshu Qiu L: linux-me...@vger.kernel.org S: Maintained F: Documentation/admin-guide/media/ipu3.rst @@ -12609,7 +12609,7 @@ T: git git://linuxtv.org/media_tree.git F: drivers/media/i2c/ov2685.c OMNIVISION OV2740 SENSOR DRIVER -M: Tianshu Qiu +M: Tianshu Qiu R: Shawn Tu R: Bingbu Cao L: linux-me...@vger.kernel.org -- 2.7.4
[PATCH] MAINTAINERS: Fix email typo and correct name of Tianshu
Fix the typo in email address of Tianshu Qiu and correct the name. Signed-off-by: Bingbu Cao Signed-off-by: Tianshu Qiu Reported-by: Bjorn Helgaas --- MAINTAINERS | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/MAINTAINERS b/MAINTAINERS index 5392f00cec46..638dfa99751b 100644 --- a/MAINTAINERS +++ b/MAINTAINERS @@ -8765,7 +8765,7 @@ INTEL IPU3 CSI-2 CIO2 DRIVER M: Yong Zhi M: Sakari Ailus M: Bingbu Cao -R: Tian Shu Qiu +R: Tianshu Qiu L: linux-me...@vger.kernel.org S: Maintained F: Documentation/userspace-api/media/v4l/pixfmt-srggb10-ipu3.rst @@ -8774,7 +8774,7 @@ F:drivers/media/pci/intel/ipu3/ INTEL IPU3 CSI-2 IMGU DRIVER M: Sakari Ailus R: Bingbu Cao -R: Tian Shu Qiu +R: Tianshu Qiu L: linux-me...@vger.kernel.org S: Maintained F: Documentation/admin-guide/media/ipu3.rst @@ -12609,7 +12609,7 @@ T: git git://linuxtv.org/media_tree.git F: drivers/media/i2c/ov2685.c OMNIVISION OV2740 SENSOR DRIVER -M: Tianshu Qiu +M: Tianshu Qiu R: Shawn Tu R: Bingbu Cao L: linux-me...@vger.kernel.org -- 2.7.4
Re: [PATCH] staging: media: ipu3: Replace depracated MSI API.
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, "failed to enable MSI (%d)\n", r);
> return r;
> }
>
--
Best regards,
Bingbu Cao
[PATCH] media: ov2740: add NVMEM interface to read customized OTP data
From: Qingwu Zhang
ov2740 includes 512bytes of one-time programmable memory and
256 bytes are reserved for customers which can be used to store
customized information. This patch provide an NVMEM interface
to support read out the customized data in OTP.
Signed-off-by: Bingbu Cao
Signed-off-by: Qingwu Zhang
---
drivers/media/i2c/ov2740.c | 145 +
1 file changed, 145 insertions(+)
diff --git a/drivers/media/i2c/ov2740.c b/drivers/media/i2c/ov2740.c
index 33025c6d23ac..fd0b6a903ec1 100644
--- a/drivers/media/i2c/ov2740.c
+++ b/drivers/media/i2c/ov2740.c
@@ -7,6 +7,8 @@
#include
#include
#include
+#include
+#include
#include
#include
#include
@@ -59,6 +61,21 @@
#define OV2740_TEST_PATTERN_ENABLE BIT(7)
#define OV2740_TEST_PATTERN_BAR_SHIFT 2
+/* ISP CTRL00 */
+#define OV2740_REG_ISP_CTRL00 0x5000
+/* ISP CTRL01 */
+#define OV2740_REG_ISP_CTRL01 0x5001
+/* Customer Addresses: 0x7010 - 0x710F */
+#define CUSTOMER_USE_OTP_SIZE 0x100
+/* OTP registers from sensor */
+#define OV2740_REG_OTP_CUSTOMER0x7010
+
+struct nvm_data {
+ char *nvm_buffer;
+ struct nvmem_device *nvmem;
+ struct regmap *regmap;
+};
+
enum {
OV2740_LINK_FREQ_360MHZ_INDEX,
};
@@ -915,6 +932,130 @@ static int ov2740_remove(struct i2c_client *client)
return 0;
}
+static int ov2740_load_otp_data(struct i2c_client *client, struct nvm_data
*nvm)
+{
+ struct ov2740 *ov2740 = to_ov2740(i2c_get_clientdata(client));
+ u32 isp_ctrl00 = 0;
+ u32 isp_ctrl01 = 0;
+ int ret;
+
+ ret = ov2740_read_reg(ov2740, OV2740_REG_ISP_CTRL00, 1, _ctrl00);
+ if (ret) {
+ dev_err(>dev, "failed to read ISP CTRL00\n");
+ goto exit;
+ }
+ ret = ov2740_read_reg(ov2740, OV2740_REG_ISP_CTRL01, 1, _ctrl01);
+ if (ret) {
+ dev_err(>dev, "failed to read ISP CTRL01\n");
+ goto exit;
+ }
+
+ /* Clear bit 5 of ISP CTRL00 */
+ ret = ov2740_write_reg(ov2740, OV2740_REG_ISP_CTRL00, 1,
+ isp_ctrl00 & ~BIT(5));
+ if (ret) {
+ dev_err(>dev, "failed to write ISP CTRL00\n");
+ goto exit;
+ }
+
+ /* Clear bit 7 of ISP CTRL01 */
+ ret = ov2740_write_reg(ov2740, OV2740_REG_ISP_CTRL01, 1,
+ isp_ctrl01 & ~BIT(7));
+ if (ret) {
+ dev_err(>dev, "failed to write ISP CTRL01\n");
+ goto exit;
+ }
+
+ ret = ov2740_write_reg(ov2740, OV2740_REG_MODE_SELECT, 1,
+ OV2740_MODE_STREAMING);
+ if (ret) {
+ dev_err(>dev, "failed to start streaming\n");
+ goto exit;
+ }
+
+ /*
+* Users are not allowed to access OTP-related registers and memory
+* during the 20 ms period after streaming starts (0x100 = 0x01).
+*/
+ msleep(20);
+
+ ret = regmap_bulk_read(nvm->regmap, OV2740_REG_OTP_CUSTOMER,
+ nvm->nvm_buffer, CUSTOMER_USE_OTP_SIZE);
+ if (ret) {
+ dev_err(>dev, "failed to read OTP data, ret %d\n", ret);
+ goto exit;
+ }
+
+ ov2740_write_reg(ov2740, OV2740_REG_MODE_SELECT, 1,
+OV2740_MODE_STANDBY);
+ ov2740_write_reg(ov2740, OV2740_REG_ISP_CTRL01, 1, isp_ctrl01);
+ ov2740_write_reg(ov2740, OV2740_REG_ISP_CTRL00, 1, isp_ctrl00);
+
+exit:
+ return ret;
+}
+
+static int ov2740_nvmem_read(void *priv, unsigned int off, void *val,
+size_t count)
+{
+ struct nvm_data *nvm = priv;
+
+ memcpy(val, nvm->nvm_buffer + off, count);
+
+ return 0;
+}
+
+static int ov2740_register_nvmem(struct i2c_client *client)
+{
+ struct nvm_data *nvm;
+ struct regmap_config regmap_config = { };
+ struct nvmem_config nvmem_config = { };
+ struct regmap *regmap;
+ struct device *dev = >dev;
+ int ret = 0;
+
+ nvm = devm_kzalloc(dev, sizeof(*nvm), GFP_KERNEL);
+ if (!nvm)
+ return -ENOMEM;
+
+ regmap_config.val_bits = 8;
+ regmap_config.reg_bits = 16;
+ regmap_config.disable_locking = true;
+ regmap = devm_regmap_init_i2c(client, _config);
+ if (IS_ERR(regmap))
+ return PTR_ERR(regmap);
+
+ nvm->regmap = regmap;
+
+ nvmem_config.name = dev_name(dev);
+ nvmem_config.dev = dev;
+ nvmem_config.read_only = true;
+ nvmem_config.root_only = true;
+ nvmem_config.owner = THIS_MODULE;
+ nvmem_config.compat = true;
+ nvmem_config.base_dev = dev;
+ nvmem_config.reg_read = ov2740_nvmem_read;
+ nvmem_config.reg_write = NULL;
+ nvmem_config.priv = nvm;
+ nvmem_config.stride = 1;
Re: ipu3-imgu 0000:00:05.0: required queues are disabled
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
Re: [PATCH -next] media: staging/intel-ipu3: Fix err handle of ipu3_css_find_binary
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!
Re: [PATCH -next] media: staging/intel-ipu3: Fix err handle of ipu3_css_find_binary
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;
