* [PATCH] [RFC] drm: mxsfb: Implement LCDIF scanout CRC32 support
@ 2022-02-06 18:56 Marek Vasut
2022-02-06 21:50 ` kernel test robot
` (2 more replies)
0 siblings, 3 replies; 15+ messages in thread
From: Marek Vasut @ 2022-02-06 18:56 UTC (permalink / raw)
To: dri-devel
Cc: Marek Vasut, Peng Fan, Alexander Stein, Laurent Pinchart,
Sam Ravnborg, Robby Cai
The LCDIF controller as present in i.MX6SX/i.MX8M Mini/Nano has a CRC_STAT
register, which contains CRC32 of the frame as it was clocked out of the
DPI interface of the LCDIF. This is likely meant as a functional safety
register.
Unfortunatelly, there is zero documentation on how the CRC32 is calculated,
there is no documentation of the polynomial, the init value, nor on which
data is the checksum applied.
By applying brute-force on 8 pixel / 2 line frame, which is the minimum
size LCDIF would work with, it turns out the polynomial is CRC32_POLY_LE
0xedb88320 , init value is 0xffffffff , the input data are bitrev32()
of the entire frame and the resulting CRC has to be also bitrev32()ed.
Doing this calculation in software for each frame is unrealistic due to
the CPU demand, implement at least a sysfs attribute which permits testing
the current frame on demand.
Unfortunatelly, this functionality has another problem. On all of those SoCs,
it is possible to overload interconnect e.g. by concurrent USB and uSDHC
transfers, at which point the LCDIF LFIFO suffers an UNDERFLOW condition,
which results in the image being shifted to the right by exactly LFIFO size
pixels. On i.MX8M Mini, the LFIFO is 76x256 bits = 2432 Byte ~= 810 pixel
at 24bpp. In this case, the LCDIF does not assert UNDERFLOW_IRQ bit, the
frame CRC32 indicated in CRC_STAT register matches the CRC32 of the frame
in DRAM, the RECOVER_ON_UNDERFLOW bit has no effect, so if this mode of
failure occurs, the failure gets undetected and uncorrected.
Signed-off-by: Marek Vasut <marex@denx.de>
Cc: Alexander Stein <alexander.stein@ew.tq-group.com>
Cc: Laurent Pinchart <laurent.pinchart@ideasonboard.com>
Cc: Lucas Stach <l.stach@pengutronix.de>
Cc: Peng Fan <peng.fan@nxp.com>
Cc: Robby Cai <robby.cai@nxp.com>
Cc: Sam Ravnborg <sam@ravnborg.org>
Cc: Stefan Agner <stefan@agner.ch>
---
drivers/gpu/drm/mxsfb/mxsfb_drv.c | 38 ++++++++++++++++++++++++++++++
drivers/gpu/drm/mxsfb/mxsfb_drv.h | 3 +++
drivers/gpu/drm/mxsfb/mxsfb_kms.c | 11 +++++----
drivers/gpu/drm/mxsfb/mxsfb_regs.h | 1 +
4 files changed, 49 insertions(+), 4 deletions(-)
diff --git a/drivers/gpu/drm/mxsfb/mxsfb_drv.c b/drivers/gpu/drm/mxsfb/mxsfb_drv.c
index 4ff3c6195dd0c..6f296b398f28c 100644
--- a/drivers/gpu/drm/mxsfb/mxsfb_drv.c
+++ b/drivers/gpu/drm/mxsfb/mxsfb_drv.c
@@ -9,6 +9,7 @@
*/
#include <linux/clk.h>
+#include <linux/crc32.h>
#include <linux/dma-mapping.h>
#include <linux/io.h>
#include <linux/module.h>
@@ -292,6 +293,37 @@ static void mxsfb_unload(struct drm_device *drm)
pm_runtime_disable(drm->dev);
}
+static ssize_t mxsfb_frame_checksum_show(struct device *dev,
+ struct device_attribute *attr,
+ char *buf)
+{
+ struct drm_device *drm = dev_get_drvdata(dev);
+ struct mxsfb_drm_private *mxsfb = drm->dev_private;
+ u32 hwcrc = readl(mxsfb->base, LCDC_V4_CRC_STAT);
+ u32 swcrc = 0xffffffff;
+ int i;
+
+ if (mxsfb->gem_vaddr) {
+ for (i = 0; i < mxsfb->gem_size / 4; i++) {
+ u32 data = bitrev32(((u32 *)mxsfb->gem_vaddr)[i]);
+ swcrc = crc32(swcrc, &data, 4);
+ }
+ swcrc = bitrev32(swcrc);
+ }
+
+ return sysfs_emit(buf, "HW:%08x,SW:%08x,OK:%d\n", hwcrc, swcrc, hwcrc == swcrc);
+}
+static DEVICE_ATTR(frame_checksum, 0444, mxsfb_frame_checksum_show, NULL);
+
+static struct attribute *mxsfb_attributes[] = {
+ &dev_attr_frame_checksum.attr,
+ NULL,
+};
+
+static const struct attribute_group mxsfb_attr_group = {
+ .attrs = mxsfb_attributes,
+};
+
DEFINE_DRM_GEM_CMA_FOPS(fops);
static const struct drm_driver mxsfb_driver = {
@@ -335,10 +367,16 @@ static int mxsfb_probe(struct platform_device *pdev)
if (ret)
goto err_unload;
+ ret = devm_device_add_group(drm->dev, &mxsfb_attr_group);
+ if (ret)
+ goto err_attr;
+
drm_fbdev_generic_setup(drm, 32);
return 0;
+err_attr:
+ drm_dev_unregister(drm);
err_unload:
mxsfb_unload(drm);
err_free:
diff --git a/drivers/gpu/drm/mxsfb/mxsfb_drv.h b/drivers/gpu/drm/mxsfb/mxsfb_drv.h
index ddb5b0417a82c..0a3e5dd1e8bab 100644
--- a/drivers/gpu/drm/mxsfb/mxsfb_drv.h
+++ b/drivers/gpu/drm/mxsfb/mxsfb_drv.h
@@ -44,6 +44,9 @@ struct mxsfb_drm_private {
struct drm_encoder encoder;
struct drm_connector *connector;
struct drm_bridge *bridge;
+
+ void *gem_vaddr;
+ size_t gem_size;
};
static inline struct mxsfb_drm_private *
diff --git a/drivers/gpu/drm/mxsfb/mxsfb_kms.c b/drivers/gpu/drm/mxsfb/mxsfb_kms.c
index 03743a84c8e79..2a4edf5a2ac57 100644
--- a/drivers/gpu/drm/mxsfb/mxsfb_kms.c
+++ b/drivers/gpu/drm/mxsfb/mxsfb_kms.c
@@ -196,7 +196,7 @@ static int mxsfb_reset_block(struct mxsfb_drm_private *mxsfb)
return clear_poll_bit(mxsfb->base + LCDC_CTRL, CTRL_CLKGATE);
}
-static dma_addr_t mxsfb_get_fb_paddr(struct drm_plane *plane)
+static dma_addr_t mxsfb_get_fb_paddr(struct mxsfb_drm_private *mxsfb, struct drm_plane *plane)
{
struct drm_framebuffer *fb = plane->state->fb;
struct drm_gem_cma_object *gem;
@@ -208,6 +208,9 @@ static dma_addr_t mxsfb_get_fb_paddr(struct drm_plane *plane)
if (!gem)
return 0;
+ mxsfb->gem_vaddr = gem->vaddr;
+ mxsfb->gem_size = gem->base.size;
+
return gem->paddr;
}
@@ -370,7 +373,7 @@ static void mxsfb_crtc_atomic_enable(struct drm_crtc *crtc,
mxsfb_crtc_mode_set_nofb(mxsfb, bus_format);
/* Write cur_buf as well to avoid an initial corrupt frame */
- paddr = mxsfb_get_fb_paddr(crtc->primary);
+ paddr = mxsfb_get_fb_paddr(mxsfb, crtc->primary);
if (paddr) {
writel(paddr, mxsfb->base + mxsfb->devdata->cur_buf);
writel(paddr, mxsfb->base + mxsfb->devdata->next_buf);
@@ -476,7 +479,7 @@ static void mxsfb_plane_primary_atomic_update(struct drm_plane *plane,
struct mxsfb_drm_private *mxsfb = to_mxsfb_drm_private(plane->dev);
dma_addr_t paddr;
- paddr = mxsfb_get_fb_paddr(plane);
+ paddr = mxsfb_get_fb_paddr(mxsfb, plane);
if (paddr)
writel(paddr, mxsfb->base + mxsfb->devdata->next_buf);
}
@@ -492,7 +495,7 @@ static void mxsfb_plane_overlay_atomic_update(struct drm_plane *plane,
dma_addr_t paddr;
u32 ctrl;
- paddr = mxsfb_get_fb_paddr(plane);
+ paddr = mxsfb_get_fb_paddr(mxsfb, plane);
if (!paddr) {
writel(0, mxsfb->base + LCDC_AS_CTRL);
return;
diff --git a/drivers/gpu/drm/mxsfb/mxsfb_regs.h b/drivers/gpu/drm/mxsfb/mxsfb_regs.h
index 694fea13e893e..cf813a1da1d78 100644
--- a/drivers/gpu/drm/mxsfb/mxsfb_regs.h
+++ b/drivers/gpu/drm/mxsfb/mxsfb_regs.h
@@ -26,6 +26,7 @@
#define LCDC_VDCTRL2 0x90
#define LCDC_VDCTRL3 0xa0
#define LCDC_VDCTRL4 0xb0
+#define LCDC_V4_CRC_STAT 0x1a0
#define LCDC_V4_DEBUG0 0x1d0
#define LCDC_V3_DEBUG0 0x1f0
#define LCDC_AS_CTRL 0x210
--
2.34.1
^ permalink raw reply related [flat|nested] 15+ messages in thread
* Re: [PATCH] [RFC] drm: mxsfb: Implement LCDIF scanout CRC32 support
2022-02-06 18:56 [PATCH] [RFC] drm: mxsfb: Implement LCDIF scanout CRC32 support Marek Vasut
@ 2022-02-06 21:50 ` kernel test robot
2022-02-06 23:01 ` kernel test robot
2022-02-07 5:13 ` Liu Ying
2 siblings, 0 replies; 15+ messages in thread
From: kernel test robot @ 2022-02-06 21:50 UTC (permalink / raw)
To: Marek Vasut; +Cc: llvm, kbuild-all
Hi Marek,
[FYI, it's a private test report for your RFC patch.]
[auto build test ERROR on drm/drm-next]
[also build test ERROR on drm-intel/for-linux-next drm-tip/drm-tip drm-exynos/exynos-drm-next tegra-drm/drm/tegra/for-next v5.17-rc2 next-20220204]
[cannot apply to airlied/drm-next]
[If your patch is applied to the wrong git tree, kindly drop us a note.
And when submitting patch, we suggest to use '--base' as documented in
https://git-scm.com/docs/git-format-patch]
url: https://github.com/0day-ci/linux/commits/Marek-Vasut/drm-mxsfb-Implement-LCDIF-scanout-CRC32-support/20220207-025808
base: git://anongit.freedesktop.org/drm/drm drm-next
config: i386-randconfig-a002 (https://download.01.org/0day-ci/archive/20220207/202202070546.PJcFF726-lkp@intel.com/config)
compiler: clang version 15.0.0 (https://github.com/llvm/llvm-project 6daaf5a44925592c764c59219b0024ee06317028)
reproduce (this is a W=1 build):
wget https://raw.githubusercontent.com/intel/lkp-tests/master/sbin/make.cross -O ~/bin/make.cross
chmod +x ~/bin/make.cross
# https://github.com/0day-ci/linux/commit/75b77e11a90d2a041b405a10dce7e1be6aab61a0
git remote add linux-review https://github.com/0day-ci/linux
git fetch --no-tags linux-review Marek-Vasut/drm-mxsfb-Implement-LCDIF-scanout-CRC32-support/20220207-025808
git checkout 75b77e11a90d2a041b405a10dce7e1be6aab61a0
# save the config file to linux build tree
mkdir build_dir
COMPILER_INSTALL_PATH=$HOME/0day COMPILER=clang make.cross W=1 O=build_dir ARCH=i386 SHELL=/bin/bash drivers/gpu/drm/mxsfb/
If you fix the issue, kindly add following tag as appropriate
Reported-by: kernel test robot <lkp@intel.com>
All errors (new ones prefixed by >>):
>> drivers/gpu/drm/mxsfb/mxsfb_drv.c:324:33: error: too many arguments to function call, expected single argument 'addr', have 2 arguments
u32 hwcrc = readl(mxsfb->base, LCDC_V4_CRC_STAT);
~~~~~ ^~~~~~~~~~~~~~~~
drivers/gpu/drm/mxsfb/mxsfb_regs.h:29:27: note: expanded from macro 'LCDC_V4_CRC_STAT'
#define LCDC_V4_CRC_STAT 0x1a0
^~~~~
arch/x86/include/asm/io.h:60:17: note: 'readl' declared here
build_mmio_read(readl, "l", unsigned int, "=r", :"memory")
^
1 error generated.
vim +/addr +324 drivers/gpu/drm/mxsfb/mxsfb_drv.c
317
318 static ssize_t mxsfb_frame_checksum_show(struct device *dev,
319 struct device_attribute *attr,
320 char *buf)
321 {
322 struct drm_device *drm = dev_get_drvdata(dev);
323 struct mxsfb_drm_private *mxsfb = drm->dev_private;
> 324 u32 hwcrc = readl(mxsfb->base, LCDC_V4_CRC_STAT);
325 u32 swcrc = 0xffffffff;
326 int i;
327
328 if (mxsfb->gem_vaddr) {
329 for (i = 0; i < mxsfb->gem_size / 4; i++) {
330 u32 data = bitrev32(((u32 *)mxsfb->gem_vaddr)[i]);
331 swcrc = crc32(swcrc, &data, 4);
332 }
333 swcrc = bitrev32(swcrc);
334 }
335
336 return sysfs_emit(buf, "HW:%08x,SW:%08x,OK:%d\n", hwcrc, swcrc, hwcrc == swcrc);
337 }
338 static DEVICE_ATTR(frame_checksum, 0444, mxsfb_frame_checksum_show, NULL);
339
---
0-DAY CI Kernel Test Service, Intel Corporation
https://lists.01.org/hyperkitty/list/kbuild-all@lists.01.org
^ permalink raw reply [flat|nested] 15+ messages in thread
* Re: [PATCH] [RFC] drm: mxsfb: Implement LCDIF scanout CRC32 support
@ 2022-02-06 21:50 ` kernel test robot
0 siblings, 0 replies; 15+ messages in thread
From: kernel test robot @ 2022-02-06 21:50 UTC (permalink / raw)
To: kbuild-all
[-- Attachment #1: Type: text/plain, Size: 3442 bytes --]
Hi Marek,
[FYI, it's a private test report for your RFC patch.]
[auto build test ERROR on drm/drm-next]
[also build test ERROR on drm-intel/for-linux-next drm-tip/drm-tip drm-exynos/exynos-drm-next tegra-drm/drm/tegra/for-next v5.17-rc2 next-20220204]
[cannot apply to airlied/drm-next]
[If your patch is applied to the wrong git tree, kindly drop us a note.
And when submitting patch, we suggest to use '--base' as documented in
https://git-scm.com/docs/git-format-patch]
url: https://github.com/0day-ci/linux/commits/Marek-Vasut/drm-mxsfb-Implement-LCDIF-scanout-CRC32-support/20220207-025808
base: git://anongit.freedesktop.org/drm/drm drm-next
config: i386-randconfig-a002 (https://download.01.org/0day-ci/archive/20220207/202202070546.PJcFF726-lkp(a)intel.com/config)
compiler: clang version 15.0.0 (https://github.com/llvm/llvm-project 6daaf5a44925592c764c59219b0024ee06317028)
reproduce (this is a W=1 build):
wget https://raw.githubusercontent.com/intel/lkp-tests/master/sbin/make.cross -O ~/bin/make.cross
chmod +x ~/bin/make.cross
# https://github.com/0day-ci/linux/commit/75b77e11a90d2a041b405a10dce7e1be6aab61a0
git remote add linux-review https://github.com/0day-ci/linux
git fetch --no-tags linux-review Marek-Vasut/drm-mxsfb-Implement-LCDIF-scanout-CRC32-support/20220207-025808
git checkout 75b77e11a90d2a041b405a10dce7e1be6aab61a0
# save the config file to linux build tree
mkdir build_dir
COMPILER_INSTALL_PATH=$HOME/0day COMPILER=clang make.cross W=1 O=build_dir ARCH=i386 SHELL=/bin/bash drivers/gpu/drm/mxsfb/
If you fix the issue, kindly add following tag as appropriate
Reported-by: kernel test robot <lkp@intel.com>
All errors (new ones prefixed by >>):
>> drivers/gpu/drm/mxsfb/mxsfb_drv.c:324:33: error: too many arguments to function call, expected single argument 'addr', have 2 arguments
u32 hwcrc = readl(mxsfb->base, LCDC_V4_CRC_STAT);
~~~~~ ^~~~~~~~~~~~~~~~
drivers/gpu/drm/mxsfb/mxsfb_regs.h:29:27: note: expanded from macro 'LCDC_V4_CRC_STAT'
#define LCDC_V4_CRC_STAT 0x1a0
^~~~~
arch/x86/include/asm/io.h:60:17: note: 'readl' declared here
build_mmio_read(readl, "l", unsigned int, "=r", :"memory")
^
1 error generated.
vim +/addr +324 drivers/gpu/drm/mxsfb/mxsfb_drv.c
317
318 static ssize_t mxsfb_frame_checksum_show(struct device *dev,
319 struct device_attribute *attr,
320 char *buf)
321 {
322 struct drm_device *drm = dev_get_drvdata(dev);
323 struct mxsfb_drm_private *mxsfb = drm->dev_private;
> 324 u32 hwcrc = readl(mxsfb->base, LCDC_V4_CRC_STAT);
325 u32 swcrc = 0xffffffff;
326 int i;
327
328 if (mxsfb->gem_vaddr) {
329 for (i = 0; i < mxsfb->gem_size / 4; i++) {
330 u32 data = bitrev32(((u32 *)mxsfb->gem_vaddr)[i]);
331 swcrc = crc32(swcrc, &data, 4);
332 }
333 swcrc = bitrev32(swcrc);
334 }
335
336 return sysfs_emit(buf, "HW:%08x,SW:%08x,OK:%d\n", hwcrc, swcrc, hwcrc == swcrc);
337 }
338 static DEVICE_ATTR(frame_checksum, 0444, mxsfb_frame_checksum_show, NULL);
339
---
0-DAY CI Kernel Test Service, Intel Corporation
https://lists.01.org/hyperkitty/list/kbuild-all(a)lists.01.org
^ permalink raw reply [flat|nested] 15+ messages in thread
* Re: [PATCH] [RFC] drm: mxsfb: Implement LCDIF scanout CRC32 support
2022-02-06 18:56 [PATCH] [RFC] drm: mxsfb: Implement LCDIF scanout CRC32 support Marek Vasut
@ 2022-02-06 23:01 ` kernel test robot
2022-02-06 23:01 ` kernel test robot
2022-02-07 5:13 ` Liu Ying
2 siblings, 0 replies; 15+ messages in thread
From: kernel test robot @ 2022-02-06 23:01 UTC (permalink / raw)
To: Marek Vasut; +Cc: llvm, kbuild-all
Hi Marek,
[FYI, it's a private test report for your RFC patch.]
[auto build test ERROR on drm/drm-next]
[also build test ERROR on drm-intel/for-linux-next drm-tip/drm-tip drm-exynos/exynos-drm-next tegra-drm/drm/tegra/for-next v5.17-rc2 next-20220204]
[cannot apply to airlied/drm-next]
[If your patch is applied to the wrong git tree, kindly drop us a note.
And when submitting patch, we suggest to use '--base' as documented in
https://git-scm.com/docs/git-format-patch]
url: https://github.com/0day-ci/linux/commits/Marek-Vasut/drm-mxsfb-Implement-LCDIF-scanout-CRC32-support/20220207-025808
base: git://anongit.freedesktop.org/drm/drm drm-next
config: riscv-randconfig-r031-20220207 (https://download.01.org/0day-ci/archive/20220207/202202070617.cZxa0dIi-lkp@intel.com/config)
compiler: clang version 15.0.0 (https://github.com/llvm/llvm-project 6daaf5a44925592c764c59219b0024ee06317028)
reproduce (this is a W=1 build):
wget https://raw.githubusercontent.com/intel/lkp-tests/master/sbin/make.cross -O ~/bin/make.cross
chmod +x ~/bin/make.cross
# install riscv cross compiling tool for clang build
# apt-get install binutils-riscv64-linux-gnu
# https://github.com/0day-ci/linux/commit/75b77e11a90d2a041b405a10dce7e1be6aab61a0
git remote add linux-review https://github.com/0day-ci/linux
git fetch --no-tags linux-review Marek-Vasut/drm-mxsfb-Implement-LCDIF-scanout-CRC32-support/20220207-025808
git checkout 75b77e11a90d2a041b405a10dce7e1be6aab61a0
# save the config file to linux build tree
mkdir build_dir
COMPILER_INSTALL_PATH=$HOME/0day COMPILER=clang make.cross W=1 O=build_dir ARCH=riscv SHELL=/bin/bash drivers/gpu/drm/mxsfb/
If you fix the issue, kindly add following tag as appropriate
Reported-by: kernel test robot <lkp@intel.com>
All error/warnings (new ones prefixed by >>):
>> drivers/gpu/drm/mxsfb/mxsfb_drv.c:324:33: error: too many arguments provided to function-like macro invocation
u32 hwcrc = readl(mxsfb->base, LCDC_V4_CRC_STAT);
^
arch/riscv/include/asm/mmio.h:140:9: note: macro 'readl' defined here
#define readl(c) ({ u32 __v; __io_br(); __v = readl_cpu(c); __io_ar(__v); __v; })
^
>> drivers/gpu/drm/mxsfb/mxsfb_drv.c:324:14: error: use of undeclared identifier 'readl'; did you mean 'vread'?
u32 hwcrc = readl(mxsfb->base, LCDC_V4_CRC_STAT);
^~~~~
vread
include/linux/vmalloc.h:251:13: note: 'vread' declared here
extern long vread(char *buf, char *addr, unsigned long count);
^
>> drivers/gpu/drm/mxsfb/mxsfb_drv.c:324:6: warning: incompatible pointer to integer conversion initializing 'u32' (aka 'unsigned int') with an expression of type 'long (char *, char *, unsigned long)' [-Wint-conversion]
u32 hwcrc = readl(mxsfb->base, LCDC_V4_CRC_STAT);
^ ~~~~~
1 warning and 2 errors generated.
vim +324 drivers/gpu/drm/mxsfb/mxsfb_drv.c
317
318 static ssize_t mxsfb_frame_checksum_show(struct device *dev,
319 struct device_attribute *attr,
320 char *buf)
321 {
322 struct drm_device *drm = dev_get_drvdata(dev);
323 struct mxsfb_drm_private *mxsfb = drm->dev_private;
> 324 u32 hwcrc = readl(mxsfb->base, LCDC_V4_CRC_STAT);
325 u32 swcrc = 0xffffffff;
326 int i;
327
328 if (mxsfb->gem_vaddr) {
329 for (i = 0; i < mxsfb->gem_size / 4; i++) {
330 u32 data = bitrev32(((u32 *)mxsfb->gem_vaddr)[i]);
331 swcrc = crc32(swcrc, &data, 4);
332 }
333 swcrc = bitrev32(swcrc);
334 }
335
336 return sysfs_emit(buf, "HW:%08x,SW:%08x,OK:%d\n", hwcrc, swcrc, hwcrc == swcrc);
337 }
338 static DEVICE_ATTR(frame_checksum, 0444, mxsfb_frame_checksum_show, NULL);
339
---
0-DAY CI Kernel Test Service, Intel Corporation
https://lists.01.org/hyperkitty/list/kbuild-all@lists.01.org
^ permalink raw reply [flat|nested] 15+ messages in thread
* Re: [PATCH] [RFC] drm: mxsfb: Implement LCDIF scanout CRC32 support
@ 2022-02-06 23:01 ` kernel test robot
0 siblings, 0 replies; 15+ messages in thread
From: kernel test robot @ 2022-02-06 23:01 UTC (permalink / raw)
To: kbuild-all
[-- Attachment #1: Type: text/plain, Size: 4083 bytes --]
Hi Marek,
[FYI, it's a private test report for your RFC patch.]
[auto build test ERROR on drm/drm-next]
[also build test ERROR on drm-intel/for-linux-next drm-tip/drm-tip drm-exynos/exynos-drm-next tegra-drm/drm/tegra/for-next v5.17-rc2 next-20220204]
[cannot apply to airlied/drm-next]
[If your patch is applied to the wrong git tree, kindly drop us a note.
And when submitting patch, we suggest to use '--base' as documented in
https://git-scm.com/docs/git-format-patch]
url: https://github.com/0day-ci/linux/commits/Marek-Vasut/drm-mxsfb-Implement-LCDIF-scanout-CRC32-support/20220207-025808
base: git://anongit.freedesktop.org/drm/drm drm-next
config: riscv-randconfig-r031-20220207 (https://download.01.org/0day-ci/archive/20220207/202202070617.cZxa0dIi-lkp(a)intel.com/config)
compiler: clang version 15.0.0 (https://github.com/llvm/llvm-project 6daaf5a44925592c764c59219b0024ee06317028)
reproduce (this is a W=1 build):
wget https://raw.githubusercontent.com/intel/lkp-tests/master/sbin/make.cross -O ~/bin/make.cross
chmod +x ~/bin/make.cross
# install riscv cross compiling tool for clang build
# apt-get install binutils-riscv64-linux-gnu
# https://github.com/0day-ci/linux/commit/75b77e11a90d2a041b405a10dce7e1be6aab61a0
git remote add linux-review https://github.com/0day-ci/linux
git fetch --no-tags linux-review Marek-Vasut/drm-mxsfb-Implement-LCDIF-scanout-CRC32-support/20220207-025808
git checkout 75b77e11a90d2a041b405a10dce7e1be6aab61a0
# save the config file to linux build tree
mkdir build_dir
COMPILER_INSTALL_PATH=$HOME/0day COMPILER=clang make.cross W=1 O=build_dir ARCH=riscv SHELL=/bin/bash drivers/gpu/drm/mxsfb/
If you fix the issue, kindly add following tag as appropriate
Reported-by: kernel test robot <lkp@intel.com>
All error/warnings (new ones prefixed by >>):
>> drivers/gpu/drm/mxsfb/mxsfb_drv.c:324:33: error: too many arguments provided to function-like macro invocation
u32 hwcrc = readl(mxsfb->base, LCDC_V4_CRC_STAT);
^
arch/riscv/include/asm/mmio.h:140:9: note: macro 'readl' defined here
#define readl(c) ({ u32 __v; __io_br(); __v = readl_cpu(c); __io_ar(__v); __v; })
^
>> drivers/gpu/drm/mxsfb/mxsfb_drv.c:324:14: error: use of undeclared identifier 'readl'; did you mean 'vread'?
u32 hwcrc = readl(mxsfb->base, LCDC_V4_CRC_STAT);
^~~~~
vread
include/linux/vmalloc.h:251:13: note: 'vread' declared here
extern long vread(char *buf, char *addr, unsigned long count);
^
>> drivers/gpu/drm/mxsfb/mxsfb_drv.c:324:6: warning: incompatible pointer to integer conversion initializing 'u32' (aka 'unsigned int') with an expression of type 'long (char *, char *, unsigned long)' [-Wint-conversion]
u32 hwcrc = readl(mxsfb->base, LCDC_V4_CRC_STAT);
^ ~~~~~
1 warning and 2 errors generated.
vim +324 drivers/gpu/drm/mxsfb/mxsfb_drv.c
317
318 static ssize_t mxsfb_frame_checksum_show(struct device *dev,
319 struct device_attribute *attr,
320 char *buf)
321 {
322 struct drm_device *drm = dev_get_drvdata(dev);
323 struct mxsfb_drm_private *mxsfb = drm->dev_private;
> 324 u32 hwcrc = readl(mxsfb->base, LCDC_V4_CRC_STAT);
325 u32 swcrc = 0xffffffff;
326 int i;
327
328 if (mxsfb->gem_vaddr) {
329 for (i = 0; i < mxsfb->gem_size / 4; i++) {
330 u32 data = bitrev32(((u32 *)mxsfb->gem_vaddr)[i]);
331 swcrc = crc32(swcrc, &data, 4);
332 }
333 swcrc = bitrev32(swcrc);
334 }
335
336 return sysfs_emit(buf, "HW:%08x,SW:%08x,OK:%d\n", hwcrc, swcrc, hwcrc == swcrc);
337 }
338 static DEVICE_ATTR(frame_checksum, 0444, mxsfb_frame_checksum_show, NULL);
339
---
0-DAY CI Kernel Test Service, Intel Corporation
https://lists.01.org/hyperkitty/list/kbuild-all(a)lists.01.org
^ permalink raw reply [flat|nested] 15+ messages in thread
* Re: [PATCH] [RFC] drm: mxsfb: Implement LCDIF scanout CRC32 support
2022-02-06 18:56 [PATCH] [RFC] drm: mxsfb: Implement LCDIF scanout CRC32 support Marek Vasut
2022-02-06 21:50 ` kernel test robot
2022-02-06 23:01 ` kernel test robot
@ 2022-02-07 5:13 ` Liu Ying
2022-02-07 8:14 ` Marek Vasut
2 siblings, 1 reply; 15+ messages in thread
From: Liu Ying @ 2022-02-07 5:13 UTC (permalink / raw)
To: Marek Vasut, dri-devel
Cc: Alexander Stein, Peng Fan, Sam Ravnborg, Laurent Pinchart, Robby Cai
Hi Marek,
On Sun, 2022-02-06 at 19:56 +0100, Marek Vasut wrote:
> The LCDIF controller as present in i.MX6SX/i.MX8M Mini/Nano has a CRC_STAT
> register, which contains CRC32 of the frame as it was clocked out of the
> DPI interface of the LCDIF. This is likely meant as a functional safety
> register.
>
> Unfortunatelly, there is zero documentation on how the CRC32 is calculated,
> there is no documentation of the polynomial, the init value, nor on which
> data is the checksum applied.
>
> By applying brute-force on 8 pixel / 2 line frame, which is the minimum
> size LCDIF would work with, it turns out the polynomial is CRC32_POLY_LE
> 0xedb88320 , init value is 0xffffffff , the input data are bitrev32()
> of the entire frame and the resulting CRC has to be also bitrev32()ed.
No idea how the HW calculates the CRC value.
I didn't hear anyone internal tried this feature.
>
> Doing this calculation in software for each frame is unrealistic due to
> the CPU demand, implement at least a sysfs attribute which permits testing
> the current frame on demand.
Why not using the existing debugfs CRC support implemented
in drivers/gpu/drm/drm_debugfs_crc.c?
>
> Unfortunatelly, this functionality has another problem. On all of those SoCs,
> it is possible to overload interconnect e.g. by concurrent USB and uSDHC
> transfers, at which point the LCDIF LFIFO suffers an UNDERFLOW condition,
> which results in the image being shifted to the right by exactly LFIFO size
> pixels. On i.MX8M Mini, the LFIFO is 76x256 bits = 2432 Byte ~= 810 pixel
> at 24bpp. In this case, the LCDIF does not assert UNDERFLOW_IRQ bit, the
> frame CRC32 indicated in CRC_STAT register matches the CRC32 of the frame
> in DRAM, the RECOVER_ON_UNDERFLOW bit has no effect, so if this mode of
> failure occurs, the failure gets undetected and uncorrected.
Hmmm, interesting, no UNDERFLOW_IRQ bit asserted when LCDIF suffers an
UNDERFLOW condition? Are you sure LCDIF really underflows?
If the shifted image is seen on a MIPI DSI display, could that be a
MIPI DSI or DPHY issue, like wrong horizontal parameter(s)?
>
> Signed-off-by: Marek Vasut <marex@denx.de>
> Cc: Alexander Stein <alexander.stein@ew.tq-group.com>
> Cc: Laurent Pinchart <laurent.pinchart@ideasonboard.com>
> Cc: Lucas Stach <l.stach@pengutronix.de>
> Cc: Peng Fan <peng.fan@nxp.com>
> Cc: Robby Cai <robby.cai@nxp.com>
> Cc: Sam Ravnborg <sam@ravnborg.org>
> Cc: Stefan Agner <stefan@agner.ch>
> ---
> drivers/gpu/drm/mxsfb/mxsfb_drv.c | 38 ++++++++++++++++++++++++++++++
> drivers/gpu/drm/mxsfb/mxsfb_drv.h | 3 +++
> drivers/gpu/drm/mxsfb/mxsfb_kms.c | 11 +++++----
> drivers/gpu/drm/mxsfb/mxsfb_regs.h | 1 +
> 4 files changed, 49 insertions(+), 4 deletions(-)
>
> diff --git a/drivers/gpu/drm/mxsfb/mxsfb_drv.c b/drivers/gpu/drm/mxsfb/mxsfb_drv.c
> index 4ff3c6195dd0c..6f296b398f28c 100644
> --- a/drivers/gpu/drm/mxsfb/mxsfb_drv.c
> +++ b/drivers/gpu/drm/mxsfb/mxsfb_drv.c
> @@ -9,6 +9,7 @@
> */
>
> #include <linux/clk.h>
> +#include <linux/crc32.h>
> #include <linux/dma-mapping.h>
> #include <linux/io.h>
> #include <linux/module.h>
> @@ -292,6 +293,37 @@ static void mxsfb_unload(struct drm_device *drm)
> pm_runtime_disable(drm->dev);
> }
>
> +static ssize_t mxsfb_frame_checksum_show(struct device *dev,
> + struct device_attribute *attr,
> + char *buf)
> +{
> + struct drm_device *drm = dev_get_drvdata(dev);
> + struct mxsfb_drm_private *mxsfb = drm->dev_private;
> + u32 hwcrc = readl(mxsfb->base, LCDC_V4_CRC_STAT);
Access register without relevant clock(s) enabled?
LCDC_V4_CRC_STAT seems to hint that there should be some verion control
logic for MXSFB_V3/4/6.
Regards,
Liu Ying
> + u32 swcrc = 0xffffffff;
> + int i;
> +
> + if (mxsfb->gem_vaddr) {
> + for (i = 0; i < mxsfb->gem_size / 4; i++) {
> + u32 data = bitrev32(((u32 *)mxsfb->gem_vaddr)[i]);
> + swcrc = crc32(swcrc, &data, 4);
> + }
> + swcrc = bitrev32(swcrc);
> + }
> +
> + return sysfs_emit(buf, "HW:%08x,SW:%08x,OK:%d\n", hwcrc, swcrc, hwcrc == swcrc);
> +}
> +static DEVICE_ATTR(frame_checksum, 0444, mxsfb_frame_checksum_show, NULL);
> +
> +static struct attribute *mxsfb_attributes[] = {
> + &dev_attr_frame_checksum.attr,
> + NULL,
> +};
> +
> +static const struct attribute_group mxsfb_attr_group = {
> + .attrs = mxsfb_attributes,
> +};
> +
> DEFINE_DRM_GEM_CMA_FOPS(fops);
>
> static const struct drm_driver mxsfb_driver = {
> @@ -335,10 +367,16 @@ static int mxsfb_probe(struct platform_device *pdev)
> if (ret)
> goto err_unload;
>
> + ret = devm_device_add_group(drm->dev, &mxsfb_attr_group);
> + if (ret)
> + goto err_attr;
> +
> drm_fbdev_generic_setup(drm, 32);
>
> return 0;
>
> +err_attr:
> + drm_dev_unregister(drm);
> err_unload:
> mxsfb_unload(drm);
> err_free:
> diff --git a/drivers/gpu/drm/mxsfb/mxsfb_drv.h b/drivers/gpu/drm/mxsfb/mxsfb_drv.h
> index ddb5b0417a82c..0a3e5dd1e8bab 100644
> --- a/drivers/gpu/drm/mxsfb/mxsfb_drv.h
> +++ b/drivers/gpu/drm/mxsfb/mxsfb_drv.h
> @@ -44,6 +44,9 @@ struct mxsfb_drm_private {
> struct drm_encoder encoder;
> struct drm_connector *connector;
> struct drm_bridge *bridge;
> +
> + void *gem_vaddr;
> + size_t gem_size;
> };
>
> static inline struct mxsfb_drm_private *
> diff --git a/drivers/gpu/drm/mxsfb/mxsfb_kms.c b/drivers/gpu/drm/mxsfb/mxsfb_kms.c
> index 03743a84c8e79..2a4edf5a2ac57 100644
> --- a/drivers/gpu/drm/mxsfb/mxsfb_kms.c
> +++ b/drivers/gpu/drm/mxsfb/mxsfb_kms.c
> @@ -196,7 +196,7 @@ static int mxsfb_reset_block(struct mxsfb_drm_private *mxsfb)
> return clear_poll_bit(mxsfb->base + LCDC_CTRL, CTRL_CLKGATE);
> }
>
> -static dma_addr_t mxsfb_get_fb_paddr(struct drm_plane *plane)
> +static dma_addr_t mxsfb_get_fb_paddr(struct mxsfb_drm_private *mxsfb, struct drm_plane *plane)
> {
> struct drm_framebuffer *fb = plane->state->fb;
> struct drm_gem_cma_object *gem;
> @@ -208,6 +208,9 @@ static dma_addr_t mxsfb_get_fb_paddr(struct drm_plane *plane)
> if (!gem)
> return 0;
>
> + mxsfb->gem_vaddr = gem->vaddr;
> + mxsfb->gem_size = gem->base.size;
> +
> return gem->paddr;
> }
>
> @@ -370,7 +373,7 @@ static void mxsfb_crtc_atomic_enable(struct drm_crtc *crtc,
> mxsfb_crtc_mode_set_nofb(mxsfb, bus_format);
>
> /* Write cur_buf as well to avoid an initial corrupt frame */
> - paddr = mxsfb_get_fb_paddr(crtc->primary);
> + paddr = mxsfb_get_fb_paddr(mxsfb, crtc->primary);
> if (paddr) {
> writel(paddr, mxsfb->base + mxsfb->devdata->cur_buf);
> writel(paddr, mxsfb->base + mxsfb->devdata->next_buf);
> @@ -476,7 +479,7 @@ static void mxsfb_plane_primary_atomic_update(struct drm_plane *plane,
> struct mxsfb_drm_private *mxsfb = to_mxsfb_drm_private(plane->dev);
> dma_addr_t paddr;
>
> - paddr = mxsfb_get_fb_paddr(plane);
> + paddr = mxsfb_get_fb_paddr(mxsfb, plane);
> if (paddr)
> writel(paddr, mxsfb->base + mxsfb->devdata->next_buf);
> }
> @@ -492,7 +495,7 @@ static void mxsfb_plane_overlay_atomic_update(struct drm_plane *plane,
> dma_addr_t paddr;
> u32 ctrl;
>
> - paddr = mxsfb_get_fb_paddr(plane);
> + paddr = mxsfb_get_fb_paddr(mxsfb, plane);
> if (!paddr) {
> writel(0, mxsfb->base + LCDC_AS_CTRL);
> return;
> diff --git a/drivers/gpu/drm/mxsfb/mxsfb_regs.h b/drivers/gpu/drm/mxsfb/mxsfb_regs.h
> index 694fea13e893e..cf813a1da1d78 100644
> --- a/drivers/gpu/drm/mxsfb/mxsfb_regs.h
> +++ b/drivers/gpu/drm/mxsfb/mxsfb_regs.h
> @@ -26,6 +26,7 @@
> #define LCDC_VDCTRL2 0x90
> #define LCDC_VDCTRL3 0xa0
> #define LCDC_VDCTRL4 0xb0
> +#define LCDC_V4_CRC_STAT 0x1a0
> #define LCDC_V4_DEBUG0 0x1d0
> #define LCDC_V3_DEBUG0 0x1f0
> #define LCDC_AS_CTRL 0x210
^ permalink raw reply [flat|nested] 15+ messages in thread
* Re: [PATCH] [RFC] drm: mxsfb: Implement LCDIF scanout CRC32 support
2022-02-07 5:13 ` Liu Ying
@ 2022-02-07 8:14 ` Marek Vasut
2022-02-07 9:18 ` Liu Ying
0 siblings, 1 reply; 15+ messages in thread
From: Marek Vasut @ 2022-02-07 8:14 UTC (permalink / raw)
To: Liu Ying, dri-devel
Cc: Peng Fan, Alexander Stein, Sam Ravnborg, Laurent Pinchart, Robby Cai
On 2/7/22 06:13, Liu Ying wrote:
> Hi Marek,
Hi,
> On Sun, 2022-02-06 at 19:56 +0100, Marek Vasut wrote:
>> The LCDIF controller as present in i.MX6SX/i.MX8M Mini/Nano has a CRC_STAT
>> register, which contains CRC32 of the frame as it was clocked out of the
>> DPI interface of the LCDIF. This is likely meant as a functional safety
>> register.
>>
>> Unfortunatelly, there is zero documentation on how the CRC32 is calculated,
>> there is no documentation of the polynomial, the init value, nor on which
>> data is the checksum applied.
>>
>> By applying brute-force on 8 pixel / 2 line frame, which is the minimum
>> size LCDIF would work with, it turns out the polynomial is CRC32_POLY_LE
>> 0xedb88320 , init value is 0xffffffff , the input data are bitrev32()
>> of the entire frame and the resulting CRC has to be also bitrev32()ed.
>
> No idea how the HW calculates the CRC value.
> I didn't hear anyone internal tried this feature.
It would be nice if the datasheet could be improved.
There are many blank areas which are undocumented, this LCDIF CRC32
feature, i.MX8M Mini Arteris NOC at 0x32700000 , the ARM GPV NIC-301 at
0x32{0,1,2,3,4,5,6,8}00000 and their master/slave port mapping. The NOC
and NICs were documented at least up to i.MX6QP and then that
information disappeared from NXP datasheets. I think reconfiguring the
NOC/NIC QoS would help mitigate this shift issue described below (*).
Do you know if there is some additional NOC/NIC documentation for i.MX8M
Mini available ?
>> Doing this calculation in software for each frame is unrealistic due to
>> the CPU demand, implement at least a sysfs attribute which permits testing
>> the current frame on demand.
>
> Why not using the existing debugfs CRC support implemented
> in drivers/gpu/drm/drm_debugfs_crc.c?
I wasn't aware of that, thanks.
>> Unfortunatelly, this functionality has another problem. On all of those SoCs,
>> it is possible to overload interconnect e.g. by concurrent USB and uSDHC
>> transfers, at which point the LCDIF LFIFO suffers an UNDERFLOW condition,
>> which results in the image being shifted to the right by exactly LFIFO size
>> pixels. On i.MX8M Mini, the LFIFO is 76x256 bits = 2432 Byte ~= 810 pixel
>> at 24bpp. In this case, the LCDIF does not assert UNDERFLOW_IRQ bit, the
>> frame CRC32 indicated in CRC_STAT register matches the CRC32 of the frame
>> in DRAM, the RECOVER_ON_UNDERFLOW bit has no effect, so if this mode of
>> failure occurs, the failure gets undetected and uncorrected.
>
> Hmmm, interesting, no UNDERFLOW_IRQ bit asserted when LCDIF suffers an
> UNDERFLOW condition?
Yes
> Are you sure LCDIF really underflows?
Mostly sure.
This problem occurs also on i.MX6SX which has no DSIM.
The failure is triggered by many short writes into DRAM to different
addresses (I was successful at triggering it by using i.MX8M Mini with
ASIX 88772 USB ethernet adapter, running iperf3 on the device, iperf3 -c
... -t 0 -R -P 16 on the PC). This effectively makes the CI HDRC behave
as a DRAM thrashing AXI master, since it triggers a lot of short USB qTD
READs from DRAM and a lot of short ethernet packet WRITEs to DRAM. And
that either clogs the DRAM itself, or the NOC or DISPLAY/HSIO NIC-301,
and prevents LCDIF from getting data long enough for this underflow
condition to happen, LFIFO to underflow, and this shift to appear. And
the shift does not disappear automatically itself, it just stays there
until the LCDIF is reinitialized.
And it apparently also happens on iMXRT, where a suggestion was made to
tweak the QoS settings of the interconnect (which cannot be tested on
i.MX8M Mini, because neither of that documentation is available, see
above (*)):
https://community.nxp.com/t5/i-MX-RT/iMXRT1052-LCD-Screen-shifted/td-p/1069978
> If the shifted image is seen on a MIPI DSI display, could that be a
> MIPI DSI or DPHY issue, like wrong horizontal parameter(s)?
No, it happens also on i.MX6SX without DSIM, so this is LCDIF problem.
^ permalink raw reply [flat|nested] 15+ messages in thread
* Re: [PATCH] [RFC] drm: mxsfb: Implement LCDIF scanout CRC32 support
2022-02-07 8:14 ` Marek Vasut
@ 2022-02-07 9:18 ` Liu Ying
2022-02-07 10:43 ` Marek Vasut
0 siblings, 1 reply; 15+ messages in thread
From: Liu Ying @ 2022-02-07 9:18 UTC (permalink / raw)
To: Marek Vasut, dri-devel
Cc: Peng Fan, Alexander Stein, Sam Ravnborg, Laurent Pinchart, Robby Cai
On Mon, 2022-02-07 at 09:14 +0100, Marek Vasut wrote:
> On 2/7/22 06:13, Liu Ying wrote:
> > Hi Marek,
>
> Hi,
>
> > On Sun, 2022-02-06 at 19:56 +0100, Marek Vasut wrote:
> > > The LCDIF controller as present in i.MX6SX/i.MX8M Mini/Nano has a
> > > CRC_STAT
> > > register, which contains CRC32 of the frame as it was clocked out
> > > of the
> > > DPI interface of the LCDIF. This is likely meant as a functional
> > > safety
> > > register.
> > >
> > > Unfortunatelly, there is zero documentation on how the CRC32 is
> > > calculated,
> > > there is no documentation of the polynomial, the init value, nor
> > > on which
> > > data is the checksum applied.
> > >
> > > By applying brute-force on 8 pixel / 2 line frame, which is the
> > > minimum
> > > size LCDIF would work with, it turns out the polynomial is
> > > CRC32_POLY_LE
> > > 0xedb88320 , init value is 0xffffffff , the input data are
> > > bitrev32()
> > > of the entire frame and the resulting CRC has to be also
> > > bitrev32()ed.
> >
> > No idea how the HW calculates the CRC value.
> > I didn't hear anyone internal tried this feature.
>
> It would be nice if the datasheet could be improved.
Agreed.
>
> There are many blank areas which are undocumented, this LCDIF CRC32
> feature, i.MX8M Mini Arteris NOC at 0x32700000 , the ARM GPV NIC-301
> at
> 0x32{0,1,2,3,4,5,6,8}00000 and their master/slave port mapping. The
> NOC
> and NICs were documented at least up to i.MX6QP and then that
> information disappeared from NXP datasheets. I think reconfiguring
> the
> NOC/NIC QoS would help mitigate this shift issue described below (*).
I also think the QoS would help if it is configureable.
>
> Do you know if there is some additional NOC/NIC documentation for
> i.MX8M
> Mini available ?
No.
>
> > > Doing this calculation in software for each frame is unrealistic
> > > due to
> > > the CPU demand, implement at least a sysfs attribute which
> > > permits testing
> > > the current frame on demand.
> >
> > Why not using the existing debugfs CRC support implemented
> > in drivers/gpu/drm/drm_debugfs_crc.c?
>
> I wasn't aware of that, thanks.
No problem.
>
> > > Unfortunatelly, this functionality has another problem. On all of
> > > those SoCs,
> > > it is possible to overload interconnect e.g. by concurrent USB
> > > and uSDHC
> > > transfers, at which point the LCDIF LFIFO suffers an UNDERFLOW
> > > condition,
> > > which results in the image being shifted to the right by exactly
> > > LFIFO size
> > > pixels. On i.MX8M Mini, the LFIFO is 76x256 bits = 2432 Byte ~=
> > > 810 pixel
> > > at 24bpp. In this case, the LCDIF does not assert UNDERFLOW_IRQ
> > > bit, the
> > > frame CRC32 indicated in CRC_STAT register matches the CRC32 of
> > > the frame
> > > in DRAM, the RECOVER_ON_UNDERFLOW bit has no effect, so if this
> > > mode of
> > > failure occurs, the failure gets undetected and uncorrected.
> >
> > Hmmm, interesting, no UNDERFLOW_IRQ bit asserted when LCDIF suffers
> > an
> > UNDERFLOW condition?
>
> Yes
Did you ever see UNDERFLOW_IRQ bit asserted in any case?
Liu Ying
^ permalink raw reply [flat|nested] 15+ messages in thread
* Re: [PATCH] [RFC] drm: mxsfb: Implement LCDIF scanout CRC32 support
2022-02-07 9:18 ` Liu Ying
@ 2022-02-07 10:43 ` Marek Vasut
2022-02-08 2:41 ` Liu Ying
0 siblings, 1 reply; 15+ messages in thread
From: Marek Vasut @ 2022-02-07 10:43 UTC (permalink / raw)
To: Liu Ying, dri-devel
Cc: Peng Fan, Alexander Stein, Sam Ravnborg, Laurent Pinchart,
Robert Chiras, Robby Cai
On 2/7/22 10:18, Liu Ying wrote:
Hi,
>>> On Sun, 2022-02-06 at 19:56 +0100, Marek Vasut wrote:
>>>> The LCDIF controller as present in i.MX6SX/i.MX8M Mini/Nano has a
>>>> CRC_STAT
>>>> register, which contains CRC32 of the frame as it was clocked out
>>>> of the
>>>> DPI interface of the LCDIF. This is likely meant as a functional
>>>> safety
>>>> register.
>>>>
>>>> Unfortunatelly, there is zero documentation on how the CRC32 is
>>>> calculated,
>>>> there is no documentation of the polynomial, the init value, nor
>>>> on which
>>>> data is the checksum applied.
>>>>
>>>> By applying brute-force on 8 pixel / 2 line frame, which is the
>>>> minimum
>>>> size LCDIF would work with, it turns out the polynomial is
>>>> CRC32_POLY_LE
>>>> 0xedb88320 , init value is 0xffffffff , the input data are
>>>> bitrev32()
>>>> of the entire frame and the resulting CRC has to be also
>>>> bitrev32()ed.
>>>
>>> No idea how the HW calculates the CRC value.
>>> I didn't hear anyone internal tried this feature.
>>
>> It would be nice if the datasheet could be improved.
>
> Agreed.
>
>>
>> There are many blank areas which are undocumented, this LCDIF CRC32
>> feature, i.MX8M Mini Arteris NOC at 0x32700000 , the ARM GPV NIC-301
>> at
>> 0x32{0,1,2,3,4,5,6,8}00000 and their master/slave port mapping. The
>> NOC
>> and NICs were documented at least up to i.MX6QP and then that
>> information disappeared from NXP datasheets. I think reconfiguring
>> the
>> NOC/NIC QoS would help mitigate this shift issue described below (*).
>
> I also think the QoS would help if it is configureable.
It is programmable, it's just the port mapping which is undocumented.
>> Do you know if there is some additional NOC/NIC documentation for
>> i.MX8M
>> Mini available ?
>
> No.
Can you ask someone internally in NXP maybe ?
>>>> Doing this calculation in software for each frame is unrealistic
>>>> due to
>>>> the CPU demand, implement at least a sysfs attribute which
>>>> permits testing
>>>> the current frame on demand.
>>>
>>> Why not using the existing debugfs CRC support implemented
>>> in drivers/gpu/drm/drm_debugfs_crc.c?
>>
>> I wasn't aware of that, thanks.
>
> No problem.
>
>>
>>>> Unfortunatelly, this functionality has another problem. On all of
>>>> those SoCs,
>>>> it is possible to overload interconnect e.g. by concurrent USB
>>>> and uSDHC
>>>> transfers, at which point the LCDIF LFIFO suffers an UNDERFLOW
>>>> condition,
>>>> which results in the image being shifted to the right by exactly
>>>> LFIFO size
>>>> pixels. On i.MX8M Mini, the LFIFO is 76x256 bits = 2432 Byte ~=
>>>> 810 pixel
>>>> at 24bpp. In this case, the LCDIF does not assert UNDERFLOW_IRQ
>>>> bit, the
>>>> frame CRC32 indicated in CRC_STAT register matches the CRC32 of
>>>> the frame
>>>> in DRAM, the RECOVER_ON_UNDERFLOW bit has no effect, so if this
>>>> mode of
>>>> failure occurs, the failure gets undetected and uncorrected.
>>>
>>> Hmmm, interesting, no UNDERFLOW_IRQ bit asserted when LCDIF suffers
>>> an
>>> UNDERFLOW condition?
>>
>> Yes
>
> Did you ever see UNDERFLOW_IRQ bit asserted in any case?
I didn't see the UNDERFLOW_IRQ bit asserted during my tests, either with
this IRQ enabled (UNDERFLOW_IRQ_EN=1) or with the IRQ disabled
(UNDERFLOW_IRQ_EN=0) by reading the CTRL1 register in interrupt handler
when CUR_FRAME_DONE_IRQ triggered the IRQ handler.
I did see a few auto-recoveries of the panel back into non-shifted
image, that happened once in some 100-200 tests. Mostly the LCDIF does
not recover automatically.
^ permalink raw reply [flat|nested] 15+ messages in thread
* Re: [PATCH] [RFC] drm: mxsfb: Implement LCDIF scanout CRC32 support
2022-02-07 10:43 ` Marek Vasut
@ 2022-02-08 2:41 ` Liu Ying
2022-02-08 3:03 ` Laurent Pinchart
2022-02-08 10:02 ` Marek Vasut
0 siblings, 2 replies; 15+ messages in thread
From: Liu Ying @ 2022-02-08 2:41 UTC (permalink / raw)
To: Marek Vasut, dri-devel
Cc: Peng Fan, Alexander Stein, Sam Ravnborg, Laurent Pinchart,
Robert Chiras, Robby Cai
On Mon, 2022-02-07 at 11:43 +0100, Marek Vasut wrote:
> On 2/7/22 10:18, Liu Ying wrote:
>
> Hi,
>
> > > > On Sun, 2022-02-06 at 19:56 +0100, Marek Vasut wrote:
> > > > > The LCDIF controller as present in i.MX6SX/i.MX8M Mini/Nano
> > > > > has a
> > > > > CRC_STAT
> > > > > register, which contains CRC32 of the frame as it was clocked
> > > > > out
> > > > > of the
> > > > > DPI interface of the LCDIF. This is likely meant as a
> > > > > functional
> > > > > safety
> > > > > register.
> > > > >
> > > > > Unfortunatelly, there is zero documentation on how the CRC32
> > > > > is
> > > > > calculated,
> > > > > there is no documentation of the polynomial, the init value,
> > > > > nor
> > > > > on which
> > > > > data is the checksum applied.
> > > > >
> > > > > By applying brute-force on 8 pixel / 2 line frame, which is
> > > > > the
> > > > > minimum
> > > > > size LCDIF would work with, it turns out the polynomial is
> > > > > CRC32_POLY_LE
> > > > > 0xedb88320 , init value is 0xffffffff , the input data are
> > > > > bitrev32()
> > > > > of the entire frame and the resulting CRC has to be also
> > > > > bitrev32()ed.
> > > >
> > > > No idea how the HW calculates the CRC value.
> > > > I didn't hear anyone internal tried this feature.
> > >
> > > It would be nice if the datasheet could be improved.
> >
> > Agreed.
> >
> > >
> > > There are many blank areas which are undocumented, this LCDIF
> > > CRC32
> > > feature, i.MX8M Mini Arteris NOC at 0x32700000 , the ARM GPV NIC-
> > > 301
> > > at
> > > 0x32{0,1,2,3,4,5,6,8}00000 and their master/slave port mapping.
> > > The
> > > NOC
> > > and NICs were documented at least up to i.MX6QP and then that
> > > information disappeared from NXP datasheets. I think
> > > reconfiguring
> > > the
> > > NOC/NIC QoS would help mitigate this shift issue described below
> > > (*).
> >
> > I also think the QoS would help if it is configureable.
>
> It is programmable, it's just the port mapping which is undocumented.
>
> > > Do you know if there is some additional NOC/NIC documentation for
> > > i.MX8M
> > > Mini available ?
> >
> > No.
>
> Can you ask someone internally in NXP maybe ?
Maybe, you may try community.nxp.com, like the i.MXRT case.
Liu Ying
^ permalink raw reply [flat|nested] 15+ messages in thread
* Re: [PATCH] [RFC] drm: mxsfb: Implement LCDIF scanout CRC32 support
2022-02-08 2:41 ` Liu Ying
@ 2022-02-08 3:03 ` Laurent Pinchart
2022-02-10 5:09 ` Liu Ying
2022-02-08 10:02 ` Marek Vasut
1 sibling, 1 reply; 15+ messages in thread
From: Laurent Pinchart @ 2022-02-08 3:03 UTC (permalink / raw)
To: Liu Ying
Cc: Marek Vasut, Peng Fan, Alexander Stein, dri-devel, Robert Chiras,
Sam Ravnborg, Robby Cai
Hello Liu Ying,
On Tue, Feb 08, 2022 at 10:41:59AM +0800, Liu Ying wrote:
> On Mon, 2022-02-07 at 11:43 +0100, Marek Vasut wrote:
> > On 2/7/22 10:18, Liu Ying wrote:
> > > > > On Sun, 2022-02-06 at 19:56 +0100, Marek Vasut wrote:
> > > > > > The LCDIF controller as present in i.MX6SX/i.MX8M Mini/Nano has a CRC_STAT
> > > > > > register, which contains CRC32 of the frame as it was clocked out of the
> > > > > > DPI interface of the LCDIF. This is likely meant as a functional safety
> > > > > > register.
> > > > > >
> > > > > > Unfortunatelly, there is zero documentation on how the CRC32 is calculated,
> > > > > > there is no documentation of the polynomial, the init value, nor on which
> > > > > > data is the checksum applied.
> > > > > >
> > > > > > By applying brute-force on 8 pixel / 2 line frame, which is the minimum
> > > > > > size LCDIF would work with, it turns out the polynomial is CRC32_POLY_LE
> > > > > > 0xedb88320 , init value is 0xffffffff , the input data are bitrev32()
> > > > > > of the entire frame and the resulting CRC has to be also bitrev32()ed.
> > > > >
> > > > > No idea how the HW calculates the CRC value.
> > > > > I didn't hear anyone internal tried this feature.
> > > >
> > > > It would be nice if the datasheet could be improved.
> > >
> > > Agreed.
> > >
> > > > There are many blank areas which are undocumented, this LCDIF CRC32
> > > > feature, i.MX8M Mini Arteris NOC at 0x32700000 , the ARM GPV NIC-301 at
> > > > 0x32{0,1,2,3,4,5,6,8}00000 and their master/slave port mapping. The NOC
> > > > and NICs were documented at least up to i.MX6QP and then that
> > > > information disappeared from NXP datasheets. I think reconfiguring the
> > > > NOC/NIC QoS would help mitigate this shift issue described below
> > > > (*).
> > >
> > > I also think the QoS would help if it is configureable.
> >
> > It is programmable, it's just the port mapping which is undocumented.
> >
> > > > Do you know if there is some additional NOC/NIC documentation for i.MX8M
> > > > Mini available ?
> > >
> > > No.
> >
> > Can you ask someone internally in NXP maybe ?
>
> Maybe, you may try community.nxp.com, like the i.MXRT case.
Overall we seem to have had little luck with community.nxp.com. I wonder
if it would be possible for key community members to get some more
direct access to support when working on upstream drivers. I'm pretty
sure nobody will try to abuse it :-)
--
Regards,
Laurent Pinchart
^ permalink raw reply [flat|nested] 15+ messages in thread
* Re: [PATCH] [RFC] drm: mxsfb: Implement LCDIF scanout CRC32 support
2022-02-08 2:41 ` Liu Ying
2022-02-08 3:03 ` Laurent Pinchart
@ 2022-02-08 10:02 ` Marek Vasut
2022-02-10 5:22 ` Liu Ying
1 sibling, 1 reply; 15+ messages in thread
From: Marek Vasut @ 2022-02-08 10:02 UTC (permalink / raw)
To: Liu Ying, dri-devel
Cc: Peng Fan, Alexander Stein, Sam Ravnborg, Laurent Pinchart,
Robert Chiras, Robby Cai
On 2/8/22 03:41, Liu Ying wrote:
Hello everyone,
>>>> There are many blank areas which are undocumented, this LCDIF
>>>> CRC32
>>>> feature, i.MX8M Mini Arteris NOC at 0x32700000 , the ARM GPV NIC-
>>>> 301
>>>> at
>>>> 0x32{0,1,2,3,4,5,6,8}00000 and their master/slave port mapping.
>>>> The
>>>> NOC
>>>> and NICs were documented at least up to i.MX6QP and then that
>>>> information disappeared from NXP datasheets. I think
>>>> reconfiguring
>>>> the
>>>> NOC/NIC QoS would help mitigate this shift issue described below
>>>> (*).
>>>
>>> I also think the QoS would help if it is configureable.
>>
>> It is programmable, it's just the port mapping which is undocumented.
>>
>>>> Do you know if there is some additional NOC/NIC documentation for
>>>> i.MX8M
>>>> Mini available ?
>>>
>>> No.
>>
>> Can you ask someone internally in NXP maybe ?
>
> Maybe, you may try community.nxp.com, like the i.MXRT case.
The community.nxp.com is unhelpful, the i.MXRT case it a good example --
the solution to the problem has been found by the person who asked the
question on their own, and elsewhere too.
But note that the i.MXRT interconnect documentation is available in the
i.MXRT datasheet, which made that possible in the first place. On i.MX,
all that information has been removed from the datasheet in i.MX7 and
i.MX8M, so I cannot even help myself, even if I wanted to. This is very bad.
^ permalink raw reply [flat|nested] 15+ messages in thread
* Re: [PATCH] [RFC] drm: mxsfb: Implement LCDIF scanout CRC32 support
2022-02-08 3:03 ` Laurent Pinchart
@ 2022-02-10 5:09 ` Liu Ying
0 siblings, 0 replies; 15+ messages in thread
From: Liu Ying @ 2022-02-10 5:09 UTC (permalink / raw)
To: Laurent Pinchart
Cc: Marek Vasut, Peng Fan, Alexander Stein, dri-devel, Robert Chiras,
Sam Ravnborg, Robby Cai
Hello Laurent,
On Tue, 2022-02-08 at 05:03 +0200, Laurent Pinchart wrote:
> Hello Liu Ying,
>
> On Tue, Feb 08, 2022 at 10:41:59AM +0800, Liu Ying wrote:
> > On Mon, 2022-02-07 at 11:43 +0100, Marek Vasut wrote:
> > > On 2/7/22 10:18, Liu Ying wrote:
> > > > > > On Sun, 2022-02-06 at 19:56 +0100, Marek Vasut wrote:
> > > > > > > The LCDIF controller as present in i.MX6SX/i.MX8M
> > > > > > > Mini/Nano has a CRC_STAT
> > > > > > > register, which contains CRC32 of the frame as it was
> > > > > > > clocked out of the
> > > > > > > DPI interface of the LCDIF. This is likely meant as a
> > > > > > > functional safety
> > > > > > > register.
> > > > > > >
> > > > > > > Unfortunatelly, there is zero documentation on how the
> > > > > > > CRC32 is calculated,
> > > > > > > there is no documentation of the polynomial, the init
> > > > > > > value, nor on which
> > > > > > > data is the checksum applied.
> > > > > > >
> > > > > > > By applying brute-force on 8 pixel / 2 line frame, which
> > > > > > > is the minimum
> > > > > > > size LCDIF would work with, it turns out the polynomial
> > > > > > > is CRC32_POLY_LE
> > > > > > > 0xedb88320 , init value is 0xffffffff , the input data
> > > > > > > are bitrev32()
> > > > > > > of the entire frame and the resulting CRC has to be also
> > > > > > > bitrev32()ed.
> > > > > >
> > > > > > No idea how the HW calculates the CRC value.
> > > > > > I didn't hear anyone internal tried this feature.
> > > > >
> > > > > It would be nice if the datasheet could be improved.
> > > >
> > > > Agreed.
> > > >
> > > > > There are many blank areas which are undocumented, this LCDIF
> > > > > CRC32
> > > > > feature, i.MX8M Mini Arteris NOC at 0x32700000 , the ARM GPV
> > > > > NIC-301 at
> > > > > 0x32{0,1,2,3,4,5,6,8}00000 and their master/slave port
> > > > > mapping. The NOC
> > > > > and NICs were documented at least up to i.MX6QP and then that
> > > > > information disappeared from NXP datasheets. I think
> > > > > reconfiguring the
> > > > > NOC/NIC QoS would help mitigate this shift issue described
> > > > > below
> > > > > (*).
> > > >
> > > > I also think the QoS would help if it is configureable.
> > >
> > > It is programmable, it's just the port mapping which is
> > > undocumented.
> > >
> > > > > Do you know if there is some additional NOC/NIC documentation
> > > > > for i.MX8M
> > > > > Mini available ?
> > > >
> > > > No.
> > >
> > > Can you ask someone internally in NXP maybe ?
> >
> > Maybe, you may try community.nxp.com, like the i.MXRT case.
>
> Overall we seem to have had little luck with community.nxp.com. I
> wonder
> if it would be possible for key community members to get some more
> direct access to support when working on upstream drivers. I'm pretty
> sure nobody will try to abuse it :-)
I'm not sure if it is possible. It's not a bad idea in my personal
opinion.
Liu Ying
^ permalink raw reply [flat|nested] 15+ messages in thread
* Re: [PATCH] [RFC] drm: mxsfb: Implement LCDIF scanout CRC32 support
2022-02-08 10:02 ` Marek Vasut
@ 2022-02-10 5:22 ` Liu Ying
2022-02-10 9:11 ` Marek Vasut
0 siblings, 1 reply; 15+ messages in thread
From: Liu Ying @ 2022-02-10 5:22 UTC (permalink / raw)
To: Marek Vasut, dri-devel
Cc: Peng Fan, Alexander Stein, Sam Ravnborg, Laurent Pinchart,
Robert Chiras, Robby Cai
On Tue, 2022-02-08 at 11:02 +0100, Marek Vasut wrote:
> On 2/8/22 03:41, Liu Ying wrote:
>
> Hello everyone,
>
> > > > > There are many blank areas which are undocumented, this LCDIF
> > > > > CRC32
> > > > > feature, i.MX8M Mini Arteris NOC at 0x32700000 , the ARM GPV
> > > > > NIC-
> > > > > 301
> > > > > at
> > > > > 0x32{0,1,2,3,4,5,6,8}00000 and their master/slave port
> > > > > mapping.
> > > > > The
> > > > > NOC
> > > > > and NICs were documented at least up to i.MX6QP and then that
> > > > > information disappeared from NXP datasheets. I think
> > > > > reconfiguring
> > > > > the
> > > > > NOC/NIC QoS would help mitigate this shift issue described
> > > > > below
> > > > > (*).
> > > >
> > > > I also think the QoS would help if it is configureable.
> > >
> > > It is programmable, it's just the port mapping which is
> > > undocumented.
> > >
> > > > > Do you know if there is some additional NOC/NIC documentation
> > > > > for
> > > > > i.MX8M
> > > > > Mini available ?
> > > >
> > > > No.
> > >
> > > Can you ask someone internally in NXP maybe ?
> >
> > Maybe, you may try community.nxp.com, like the i.MXRT case.
>
> The community.nxp.com is unhelpful, the i.MXRT case it a good example
> --
> the solution to the problem has been found by the person who asked
> the
> question on their own, and elsewhere too.
AFAIK, there are questions answered by internal support team and RnD
team at that community. I personally take it as a resource to use.
>
> But note that the i.MXRT interconnect documentation is available in
> the
> i.MXRT datasheet, which made that possible in the first place. On
> i.MX,
> all that information has been removed from the datasheet in i.MX7
> and
> i.MX8M, so I cannot even help myself, even if I wanted to. This is
> very bad.
I'm not familiar with the documention in that area, so I personally
will not be helpful at the documention topic. The main purpose I
jumped in this thread is to review the patch and share the idea to use
the existing drm debugfs crc support instead of creating a sysfs
attribute.
Liu Ying
^ permalink raw reply [flat|nested] 15+ messages in thread
* Re: [PATCH] [RFC] drm: mxsfb: Implement LCDIF scanout CRC32 support
2022-02-10 5:22 ` Liu Ying
@ 2022-02-10 9:11 ` Marek Vasut
0 siblings, 0 replies; 15+ messages in thread
From: Marek Vasut @ 2022-02-10 9:11 UTC (permalink / raw)
To: Liu Ying, dri-devel
Cc: Peng Fan, Alexander Stein, Sam Ravnborg, Frieder Schrempf,
Marcel Ziswiler, Laurent Pinchart, Robert Chiras, Robby Cai
On 2/10/22 06:22, Liu Ying wrote:
Hi,
[...]
>>>>>> There are many blank areas which are undocumented, this LCDIF
>>>>>> CRC32
>>>>>> feature, i.MX8M Mini Arteris NOC at 0x32700000 , the ARM GPV
>>>>>> NIC-
>>>>>> 301
>>>>>> at
>>>>>> 0x32{0,1,2,3,4,5,6,8}00000 and their master/slave port
>>>>>> mapping.
>>>>>> The
>>>>>> NOC
>>>>>> and NICs were documented at least up to i.MX6QP and then that
>>>>>> information disappeared from NXP datasheets. I think
>>>>>> reconfiguring
>>>>>> the
>>>>>> NOC/NIC QoS would help mitigate this shift issue described
>>>>>> below
>>>>>> (*).
>>>>>
>>>>> I also think the QoS would help if it is configureable.
>>>>
>>>> It is programmable, it's just the port mapping which is
>>>> undocumented.
>>>>
>>>>>> Do you know if there is some additional NOC/NIC documentation
>>>>>> for
>>>>>> i.MX8M
>>>>>> Mini available ?
>>>>>
>>>>> No.
>>>>
>>>> Can you ask someone internally in NXP maybe ?
>>>
>>> Maybe, you may try community.nxp.com, like the i.MXRT case.
>>
>> The community.nxp.com is unhelpful, the i.MXRT case it a good example
>> --
>> the solution to the problem has been found by the person who asked
>> the
>> question on their own, and elsewhere too.
>
> AFAIK, there are questions answered by internal support team and RnD
> team at that community. I personally take it as a resource to use.
Sure, here is a list of links to similar problem triggered by various
people using the NXP BSP, neither of them in answered:
https://community.nxp.com/t5/i-MX-Processors/Image-shift-right-for-LVDS/td-p/969581
https://community.nxp.com/t5/i-MX-Processors/iMX8M-display-shifted-after-playing-decoded-video-with-gstreamer/td-p/928269
https://community.nxp.com/t5/i-MX-Processors/Display-Wrap-Around-Issue/td-p/1084052
https://community.nxp.com/t5/i-MX-Processors/Display-Vertically-shifted-IMX8mq-evk-board-in-dual-display-use/m-p/965726
https://community.nxp.com/t5/i-MX-RT/iMXRT1052-LCD-Screen-shifted/td-p/1069978
>> But note that the i.MXRT interconnect documentation is available in
>> the
>> i.MXRT datasheet, which made that possible in the first place. On
>> i.MX,
>> all that information has been removed from the datasheet in i.MX7
>> and
>> i.MX8M, so I cannot even help myself, even if I wanted to. This is
>> very bad.
>
> I'm not familiar with the documention in that area, so I personally
> will not be helpful at the documention topic. The main purpose I
> jumped in this thread is to review the patch and share the idea to use
> the existing drm debugfs crc support instead of creating a sysfs
> attribute.
Can you maybe ask someone inside NXP about this problem ?
^ permalink raw reply [flat|nested] 15+ messages in thread
end of thread, other threads:[~2022-02-10 9:11 UTC | newest]
Thread overview: 15+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2022-02-06 18:56 [PATCH] [RFC] drm: mxsfb: Implement LCDIF scanout CRC32 support Marek Vasut
2022-02-06 21:50 ` kernel test robot
2022-02-06 21:50 ` kernel test robot
2022-02-06 23:01 ` kernel test robot
2022-02-06 23:01 ` kernel test robot
2022-02-07 5:13 ` Liu Ying
2022-02-07 8:14 ` Marek Vasut
2022-02-07 9:18 ` Liu Ying
2022-02-07 10:43 ` Marek Vasut
2022-02-08 2:41 ` Liu Ying
2022-02-08 3:03 ` Laurent Pinchart
2022-02-10 5:09 ` Liu Ying
2022-02-08 10:02 ` Marek Vasut
2022-02-10 5:22 ` Liu Ying
2022-02-10 9:11 ` Marek Vasut
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.