Tomasz, On 10/6/21 1:03 PM, Tomasz Figa wrote: > Hi Bingbu, > > On Thu, Sep 23, 2021 at 1:11 PM Bingbu Cao <bingbu.cao@xxxxxxxxx> wrote: >> >> CIO2 worked well with most camera sensors so far, but CIO2 will meet SRAM >> overflow when working with higher data rate camera sensors such as 13M@30fps. >> We must set lower high watermark value to trigger the DRAM write to support >> such camera sensors. >> >> Signed-off-by: Bingbu Cao <bingbu.cao@xxxxxxxxx> >> --- >> drivers/media/pci/intel/ipu3/ipu3-cio2.h | 2 +- >> 1 file changed, 1 insertion(+), 1 deletion(-) >> > > Thanks for the patch. Would this have any implications for other > (lower) operating modes, such as increased power consumption, or it's > harmless? If so, what's the reason we didn't use the value from the > very beginning? Yes, we meet that the frame data corruption for some high data rate camera sensors like imx258 (13M@30fps) with current watermark settings. The higher watermark potentially has power concern as it request DMA transfer more than before. To keep the old settings for low data rate camera sensor, I am thinking the rationality to determine the HWM value based on the link_frequency? Apparently, it is not reliable to determine by the format. > > Best regards, > Tomasz > >> diff --git a/drivers/media/pci/intel/ipu3/ipu3-cio2.h b/drivers/media/pci/intel/ipu3/ipu3-cio2.h >> index 3806d7f04d69..fde80d48533b 100644 >> --- a/drivers/media/pci/intel/ipu3/ipu3-cio2.h >> +++ b/drivers/media/pci/intel/ipu3/ipu3-cio2.h >> @@ -181,7 +181,7 @@ struct pci_dev; >> #define CIO2_PBM_WMCTRL1_MID1_2CK (16 << CIO2_PBM_WMCTRL1_MID1_2CK_SHIFT) >> #define CIO2_PBM_WMCTRL1_MID2_2CK (21 << CIO2_PBM_WMCTRL1_MID2_2CK_SHIFT) >> #define CIO2_REG_PBM_WMCTRL2 0x1468 >> -#define CIO2_PBM_WMCTRL2_HWM_2CK 40U >> +#define CIO2_PBM_WMCTRL2_HWM_2CK 30U >> #define CIO2_PBM_WMCTRL2_HWM_2CK_SHIFT 0U >> #define CIO2_PBM_WMCTRL2_LWM_2CK 22U >> #define CIO2_PBM_WMCTRL2_LWM_2CK_SHIFT 8U >> -- >> 2.7.4 >> -- Best regards, Bingbu Cao
