Re: [PATCH v13 5/6] drm/imx: Introduce i.MX8qm/qxp DPU DRM

From: Marcel Ziswiler <marcel.ziswiler@toradex.com>
To: "dri-devel@lists.freedesktop.org"
	<dri-devel@lists.freedesktop.org>,
	"linux-arm-kernel@lists.infradead.org" 
	<linux-arm-kernel@lists.infradead.org>,
	"victor.liu@nxp.com" <victor.liu@nxp.com>,
	"devicetree@vger.kernel.org" <devicetree@vger.kernel.org>,
	"linux-kernel@vger.kernel.org" <linux-kernel@vger.kernel.org>
Cc: "linux-imx@nxp.com" <linux-imx@nxp.com>,
	"robh@kernel.org" <robh@kernel.org>,
	"krzysztof.kozlowski+dt@linaro.org" 
	<krzysztof.kozlowski+dt@linaro.org>,
	"tzimmermann@suse.de" <tzimmermann@suse.de>,
	"guido.gunther@puri.sm" <guido.gunther@puri.sm>,
	"mripard@kernel.org" <mripard@kernel.org>,
	"laurentiu.palcu@oss.nxp.com" <laurentiu.palcu@oss.nxp.com>,
	"daniel@ffwll.ch" <daniel@ffwll.ch>,
	"maarten.lankhorst@linux.intel.com" 
	<maarten.lankhorst@linux.intel.com>,
	"shawnguo@kernel.org" <shawnguo@kernel.org>,
	"kernel@pengutronix.de" <kernel@pengutronix.de>,
	"s.hauer@pengutronix.de" <s.hauer@pengutronix.de>,
	"p.zabel@pengutronix.de" <p.zabel@pengutronix.de>,
	"robh+dt@kernel.org" <robh+dt@kernel.org>,
	"airlied@gmail.com" <airlied@gmail.com>,
	"festevam@gmail.com" <festevam@gmail.com>
Subject: Re: [PATCH v13 5/6] drm/imx: Introduce i.MX8qm/qxp DPU DRM
Date: Wed, 4 Jan 2023 10:01:08 +0000	[thread overview]
Message-ID: <549bf1f26b8212de2d4890a27e396250257aa027.camel@toradex.com> (raw)
In-Reply-To: <20221019020226.2340782-6-victor.liu@nxp.com>

Hi Liu

Thank you very much!

On Wed, 2022-10-19 at 10:02 +0800, Liu Ying wrote:
> This patch introduces i.MX8qm/qxp Display Processing Unit(DPU) DRM support.
> 
> DPU is comprised of two main components that include a blit engine for
> 2D graphics accelerations(with composition support) and a display
> controller for display output processing, as well as a command sequencer.
> Outside of DPU, optional prefetch engines, a.k.a, Prefetch Resolve
> Gasket(PRG) and Display Prefetch Resolve(DPR), can fetch data from memory
> prior to some DPU fetchunits of blit engine and display controller.  The
> prefetch engines support reading linear formats and resolving Vivante GPU
> tile formats.
> 
> This patch adds kernel modesetting support for the display controller part.
> The driver supports two CRTCs per display controller, planes backed by
> four fetchunits(decode0/1, fetchlayer, fetchwarp), fetchunit allocation
> logic for the two CRTCs, prefetch engines(with tile resolving supported),
> plane upscaling/deinterlacing/yuv2rgb CSC/alpha blending and CRTC gamma
> correction.  The registers of the controller is accessed without command
> sequencer involved, instead just by using CPU.
> 
> Reference manual can be found at:
> https://www.nxp.com/webapp/Download?colCode=IMX8DQXPRM
> 
> Reviewed-by: Laurentiu Palcu <laurentiu.palcu@oss.nxp.com>
> Signed-off-by: Liu Ying <victor.liu@nxp.com>
> ---
> v12->v13:
> * Drop 'drm->irq_enabled = true;' to fix a potential build break
>   reported by 'kernel test robot <lkp@intel.com>'. drm->irq_enabled
>   should not be used by imx-dpu drm as it is only used by legacy
>   drivers with userspace modesetting.
> 
> v11->v12:
> * Rebase upon v6.1-rc1.
> * Minor update on Kconfigs, struct names and macro names due to the rebase.
> 
> v10->v11:
> * Rebase upon v6.0-rc1.
> * Include drm_blend.h and drm_framebuffer.h in dpu-kms.c and dpu-plane.c
>   to fix build errors due to the rebase.
> * Fix a checkpatch warning for dpu-crtc.c.
> * Properly use dev_err_probe() to return it's return value directly where
>   possible.
> 
> v9->v10:
> * Make 'checkpatch.pl --strict' happier.
> * Add Laurentiu's R-b tag.
> 
> v8->v9:
> * Use drm_atomic_get_new_plane_state() in dpu_plane_atomic_update(). (Laurentiu)
> * Drop getting DPU DT alias ID, as it is unused.
> * Get the DPR interrupt(dpr_wrap) by name.
> 
> v7->v8:
> * Update dpu_plane_atomic_check() and dpu_plane_atomic_update(), due to DRM
>   plane helper functions API change(atomic_check and atomic_update) from DRM
>   atomic core.  Also, rename plane->state variables and relevant DPU plane
>   state variables in those two functions to reflect they are new states, like
>   the patch 'drm: Rename plane->state variables in atomic update and disable'
>   recently landed in drm-misc-next.
> * Replace drm_gem_fb_prepare_fb() with drm_gem_plane_helper_prepare_fb(),
>   due to DRM core API change.
> * Use 256byte DPR burst length for GPU standard tile and 128byte DPR burst
>   length for 32bpp GPU super tile to align with the latest version of internal
>   HW documention.
> 
> v6->v7:
> * Fix return value of dpu_get_irqs() if platform_get_irq() fails. (Laurentiu)
> * Use the function array dpu_irq_handler[] to store individual DPU irq handlers.
>   (Laurentiu)
> * Call get/put() hooks directly to get/put DPU fetchunits for DPU plane groups.
>   (Laurentiu)
> * Shorten the names of individual DPU irq handlers by using DPU unit abbrev
>   names to make writing dpu_irq_handler[] easier.
> 
> v5->v6:
> * Do not use macros where possible. (Laurentiu)
> * Break dpu_plane_atomic_check() into some smaller functions. (Laurentiu)
> * Address some minor comments from Laurentiu.
> * Add dpu_crtc_err() helper marco to tell dmesg which CRTC generates error.
> * Drop calling dev_set_drvdata() from dpu_drm_bind/unbind() as it is done
>   in dpu_drm_probe().
> * Some trivial tweaks.
> 
> v4->v5:
> * Rebase up onto the latest drm-misc-next branch and remove the hook to
>   drm_atomic_helper_legacy_gamma_set(), because it was dropped by the newly
>   landed commit 'drm: automatic legacy gamma support'.
> * Remove a redundant blank line from dpu_plane_atomic_update().
> 
> v3->v4:
> * No change.
> 
> v2->v3:
> * Fix build warnings Reported-by: kernel test robot <lkp@intel.com>.
> * Drop build dependency on IMX_SCU, as dummy SCU functions have been added in
>   header files by the patch 'firmware: imx: add dummy functions' which has
>   landed in linux-next/master branch.
> 
> v1->v2:
> * Add compatible for i.MX8qm DPU, as this is tested with i.MX8qm LVDS displays.
>   (Laurentiu)
> * Fix PRG burst size and stride. (Laurentiu)
> * Put 'ports' OF node to fix the bail-out logic in dpu_drm_probe(). (Laurentiu)
> 
>  drivers/gpu/drm/imx/Kconfig               |    1 +
>  drivers/gpu/drm/imx/Makefile              |    1 +
>  drivers/gpu/drm/imx/dpu/Kconfig           |    9 +
>  drivers/gpu/drm/imx/dpu/Makefile          |   10 +
>  drivers/gpu/drm/imx/dpu/dpu-constframe.c  |  171 ++++
>  drivers/gpu/drm/imx/dpu/dpu-core.c        | 1044 +++++++++++++++++++++
>  drivers/gpu/drm/imx/dpu/dpu-crtc.c        |  969 +++++++++++++++++++
>  drivers/gpu/drm/imx/dpu/dpu-crtc.h        |   72 ++
>  drivers/gpu/drm/imx/dpu/dpu-disengcfg.c   |  117 +++
>  drivers/gpu/drm/imx/dpu/dpu-dprc.c        |  715 ++++++++++++++
>  drivers/gpu/drm/imx/dpu/dpu-dprc.h        |   40 +
>  drivers/gpu/drm/imx/dpu/dpu-drv.c         |  290 ++++++
>  drivers/gpu/drm/imx/dpu/dpu-drv.h         |   28 +
>  drivers/gpu/drm/imx/dpu/dpu-extdst.c      |  299 ++++++
>  drivers/gpu/drm/imx/dpu/dpu-fetchdecode.c |  292 ++++++
>  drivers/gpu/drm/imx/dpu/dpu-fetcheco.c    |  224 +++++
>  drivers/gpu/drm/imx/dpu/dpu-fetchlayer.c  |  152 +++
>  drivers/gpu/drm/imx/dpu/dpu-fetchunit.c   |  610 ++++++++++++
>  drivers/gpu/drm/imx/dpu/dpu-fetchunit.h   |  195 ++++
>  drivers/gpu/drm/imx/dpu/dpu-fetchwarp.c   |  248 +++++
>  drivers/gpu/drm/imx/dpu/dpu-framegen.c    |  395 ++++++++
>  drivers/gpu/drm/imx/dpu/dpu-gammacor.c    |  223 +++++
>  drivers/gpu/drm/imx/dpu/dpu-hscaler.c     |  275 ++++++
>  drivers/gpu/drm/imx/dpu/dpu-kms.c         |  542 +++++++++++
>  drivers/gpu/drm/imx/dpu/dpu-kms.h         |   23 +
>  drivers/gpu/drm/imx/dpu/dpu-layerblend.c  |  348 +++++++
>  drivers/gpu/drm/imx/dpu/dpu-plane.c       |  804 ++++++++++++++++
>  drivers/gpu/drm/imx/dpu/dpu-plane.h       |   59 ++
>  drivers/gpu/drm/imx/dpu/dpu-prg.c         |  433 +++++++++
>  drivers/gpu/drm/imx/dpu/dpu-prg.h         |   45 +
>  drivers/gpu/drm/imx/dpu/dpu-prv.h         |  231 +++++
>  drivers/gpu/drm/imx/dpu/dpu-tcon.c        |  250 +++++
>  drivers/gpu/drm/imx/dpu/dpu-vscaler.c     |  308 ++++++
>  drivers/gpu/drm/imx/dpu/dpu.h             |  385 ++++++++
>  34 files changed, 9808 insertions(+)
>  create mode 100644 drivers/gpu/drm/imx/dpu/Kconfig
>  create mode 100644 drivers/gpu/drm/imx/dpu/Makefile
>  create mode 100644 drivers/gpu/drm/imx/dpu/dpu-constframe.c
>  create mode 100644 drivers/gpu/drm/imx/dpu/dpu-core.c
>  create mode 100644 drivers/gpu/drm/imx/dpu/dpu-crtc.c
>  create mode 100644 drivers/gpu/drm/imx/dpu/dpu-crtc.h
>  create mode 100644 drivers/gpu/drm/imx/dpu/dpu-disengcfg.c
>  create mode 100644 drivers/gpu/drm/imx/dpu/dpu-dprc.c
>  create mode 100644 drivers/gpu/drm/imx/dpu/dpu-dprc.h
>  create mode 100644 drivers/gpu/drm/imx/dpu/dpu-drv.c
>  create mode 100644 drivers/gpu/drm/imx/dpu/dpu-drv.h
>  create mode 100644 drivers/gpu/drm/imx/dpu/dpu-extdst.c
>  create mode 100644 drivers/gpu/drm/imx/dpu/dpu-fetchdecode.c
>  create mode 100644 drivers/gpu/drm/imx/dpu/dpu-fetcheco.c
>  create mode 100644 drivers/gpu/drm/imx/dpu/dpu-fetchlayer.c
>  create mode 100644 drivers/gpu/drm/imx/dpu/dpu-fetchunit.c
>  create mode 100644 drivers/gpu/drm/imx/dpu/dpu-fetchunit.h
>  create mode 100644 drivers/gpu/drm/imx/dpu/dpu-fetchwarp.c
>  create mode 100644 drivers/gpu/drm/imx/dpu/dpu-framegen.c
>  create mode 100644 drivers/gpu/drm/imx/dpu/dpu-gammacor.c
>  create mode 100644 drivers/gpu/drm/imx/dpu/dpu-hscaler.c
>  create mode 100644 drivers/gpu/drm/imx/dpu/dpu-kms.c
>  create mode 100644 drivers/gpu/drm/imx/dpu/dpu-kms.h
>  create mode 100644 drivers/gpu/drm/imx/dpu/dpu-layerblend.c
>  create mode 100644 drivers/gpu/drm/imx/dpu/dpu-plane.c
>  create mode 100644 drivers/gpu/drm/imx/dpu/dpu-plane.h
>  create mode 100644 drivers/gpu/drm/imx/dpu/dpu-prg.c
>  create mode 100644 drivers/gpu/drm/imx/dpu/dpu-prg.h
>  create mode 100644 drivers/gpu/drm/imx/dpu/dpu-prv.h
>  create mode 100644 drivers/gpu/drm/imx/dpu/dpu-tcon.c
>  create mode 100644 drivers/gpu/drm/imx/dpu/dpu-vscaler.c
>  create mode 100644 drivers/gpu/drm/imx/dpu/dpu.h

[snip]

> diff --git a/drivers/gpu/drm/imx/dpu/dpu-drv.c b/drivers/gpu/drm/imx/dpu/dpu-drv.c
> new file mode 100644
> index 000000000000..2a2dd92269c8
> --- /dev/null
> +++ b/drivers/gpu/drm/imx/dpu/dpu-drv.c
> @@ -0,0 +1,290 @@
> +// SPDX-License-Identifier: GPL-2.0+
> +
> +/*
> + * Copyright 2019,2020,2022 NXP
> + */
> +
> +#include <linux/component.h>
> +#include <linux/dma-mapping.h>
> +#include <linux/kernel.h>
> +#include <linux/module.h>
> +#include <linux/of.h>
> +#include <linux/platform_device.h>
> +
> +#include <drm/drm_atomic_helper.h>
> +#include <drm/drm_drv.h>

This may also need the following addition:

#include <drm/drm_fbdev_generic.h>

Otherwise I do get the following error on compilation:

  CC [M]  drivers/gpu/drm/imx/dpu/dpu-drv.o
/var/home/zim/Sources/linux-next.git/drivers/gpu/drm/imx/dpu/dpu-drv.c: In function 'dpu_drm_bind':
/var/home/zim/Sources/linux-next.git/drivers/gpu/drm/imx/dpu/dpu-drv.c:99:9: error: implicit declaration of
function 'drm_fbdev_generic_setup' [-Werror=implicit-function-declaration]
   99 |         drm_fbdev_generic_setup(drm, legacyfb_depth);
      |         ^~~~~~~~~~~~~~~~~~~~~~~
cc1: some warnings being treated as errors
make[7]: *** [/var/home/zim/Sources/linux-next.git/scripts/Makefile.build:252: drivers/gpu/drm/imx/dpu/dpu-
drv.o] Error 1
make[7]: *** Waiting for unfinished jobs....

> +#include <drm/drm_fb_helper.h>
> +#include <drm/drm_gem_dma_helper.h>
> +#include <drm/drm_modeset_helper.h>
> +#include <drm/drm_of.h>
> +#include <drm/drm_print.h>
> +#include <drm/drm_probe_helper.h>
> +
> +#include "dpu-drv.h"
> +#include "dpu-kms.h"
> +
> +#define DRIVER_NAME    "imx-dpu-drm"

[snip]

I am trying to actually get this to work on Apalis iMX8 [1] and Colibri iMX8X [2] but so far I am still missing
some crucial recent changes on the SoC device tree side for the dc-pixel-link (e.g. fsl,dc-id and fsl,dc-
stream-id),  lvds-csr (e.g. dropped clock-names) and/or dpr-channel (e.g. interrupts-extended especially the
resp. dc0_irqsteer numbers) parts. If you know of a recent complete git tree with working examples like back in
the day [3], let me know. Let's see...

[1] https://lore.kernel.org/all/20230102171023.33853-1-marcel@ziswiler.com/
[2]
https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/commit/?id=ba5a5615d54f8adfeb4edd005bbd0dfeb65feb9f
[3] https://lore.kernel.org/all/c4c93c217d21dc20435fd7615c146397d4fcafc7.camel@nxp.com/

Cheers

Marcel