Re: [PATCH v2 13/17] video: rockchip: Add rk3328 vop support

Hi Andy,

On Tue, Dec 19, 2023 at 6:50 AM Andy Yan andyshrk@163.com wrote:

Hi Jaqan：

At 2023-12-19 03:11:10, "Jagan Teki" jagan@amarulasolutions.com wrote:

...

From: Jagan Teki jagan@edgeble.ai

Add support for Rockchip RK3328 VOP.

Require VOP cleanup before handoff to Linux by writing reset values to WIN registers. Without this Linux VOP trigger page fault as below [ 0.752016] Loading compiled-in X.509 certificates [ 0.787796] inno_hdmi_phy_rk3328_clk_recalc_rate: parent 24000000 [ 0.788391] inno-hdmi-phy ff430000.phy: inno_hdmi_phy_rk3328_clk_recalc_rate rate 148500000 vco 148500000 [ 0.798353] rockchip-drm display-subsystem: bound ff370000.vop (ops vop_component_ops) [ 0.799403] dwhdmi-rockchip ff3c0000.hdmi: supply avdd-0v9 not found, using dummy regulator [ 0.800288] rk_iommu ff373f00.iommu: Enable stall request timed out, status: 0x00004b [ 0.801131] dwhdmi-rockchip ff3c0000.hdmi: supply avdd-1v8 not found, using dummy regulator [ 0.802056] rk_iommu ff373f00.iommu: Disable paging request timed out, status: 0x00004b [ 0.803233] dwhdmi-rockchip ff3c0000.hdmi: Detected HDMI TX controller v2.11a with HDCP (inno_dw_hdmi_phy2) [ 0.805355] dwhdmi-rockchip ff3c0000.hdmi: registered DesignWare HDMI I2C bus driver [ 0.808769] rockchip-drm display-subsystem: bound ff3c0000.hdmi (ops dw_hdmi_rockchip_ops) [ 0.810869] [drm] Initialized rockchip 1.0.0 20140818 for display-subsystem on minor 0

Signed-off-by: Jagan Teki jagan@edgeble.ai

Changes for v2:

• Add VOP cleanup
• Update commit

drivers/video/rockchip/Makefile | 1 + drivers/video/rockchip/rk3328_vop.c | 83 +++++++++++++++++++++++++++++ 2 files changed, 84 insertions(+) create mode 100644 drivers/video/rockchip/rk3328_vop.c

diff --git a/drivers/video/rockchip/Makefile b/drivers/video/rockchip/Makefile index 4991303c73..f55beceebf 100644 --- a/drivers/video/rockchip/Makefile +++ b/drivers/video/rockchip/Makefile @@ -6,6 +6,7 @@ ifdef CONFIG_VIDEO_ROCKCHIP obj-y += rk_vop.o obj-$(CONFIG_ROCKCHIP_RK3288) += rk3288_vop.o +obj-$(CONFIG_ROCKCHIP_RK3328) += rk3328_vop.o obj-$(CONFIG_ROCKCHIP_RK3399) += rk3399_vop.o obj-$(CONFIG_DISPLAY_ROCKCHIP_EDP) += rk_edp.o obj-$(CONFIG_DISPLAY_ROCKCHIP_LVDS) += rk_lvds.o diff --git a/drivers/video/rockchip/rk3328_vop.c b/drivers/video/rockchip/rk3328_vop.c new file mode 100644 index 0000000000..a4da3a91e8 --- /dev/null +++ b/drivers/video/rockchip/rk3328_vop.c @@ -0,0 +1,83 @@ +// SPDX-License-Identifier: GPL-2.0+ +/*

• • Copyright (c) 2023 Edgeble AI Technologies Pvt. Ltd.
• */

+#include <dm.h> +#include <video.h> +#include <asm/io.h> +#include "rk_vop.h"

+DECLARE_GLOBAL_DATA_PTR;

+static void rk3328_set_pin_polarity(struct udevice *dev,

•                              enum vop_modes mode, u32 polarity)
  

+{

•  struct rk_vop_priv *priv = dev_get_priv(dev);
  

•  struct rk3288_vop *regs = priv->regs;
  

•  switch (mode) {
  

•  case VOP_MODE_HDMI:
  

•          clrsetbits_le32(&regs->dsp_ctrl1,
  

•                          M_RK3399_DSP_HDMI_POL,
  

•                          V_RK3399_DSP_HDMI_POL(polarity));
  

•          break;
  

•  default:
  

•          debug("%s: unsupported output mode %x\n", __func__, mode);
  

•  }
  

+}

+static int rk3328_vop_probe(struct udevice *dev) +{

•  /* Before relocation we don't need to do anything */
  

•  if (!(gd->flags & GD_FLG_RELOC))
  

•          return 0;
  

•  return rk_vop_probe(dev);
  

+}

+static int rk3328_vop_remove(struct udevice *dev) +{

•  struct rk_vop_priv *priv = dev_get_priv(dev);
  

•  struct rk3288_vop *regs = priv->regs;
  

•  struct rk3288_vop *win_regs = priv->regs + priv->win_offset;
  

•  /* write reset values */
  

•  writel(0xef013f, &win_regs->win0_act_info);
  

•  writel(0xef013f, &win_regs->win0_dsp_info);
  

•  writel(0xa000a, &win_regs->win0_dsp_st);
  

•  writel(0x0, &win_regs->win0_yrgb_mst);
  

•  writel(0x01, &regs->reg_cfg_done);
  

•  return 0;
  

+}

I think this just workaround Linux iommu page fault by luck。 The reset value（what you called it is）your write just let win0 read a 320x240 rectangular from address 0 and display it at next frame（maybe 16ms later if your current display is run at 60HZ）。

1. we don't know what content is at address 0, so you will see something strange on your monitor.
2. there is no guarantee that address 0 is really readable（maybe a security memory space， or maybe it is not a valid address）， this may cause another issue that not easy to detect。

Okay. Can you suggest any proper way to clean up VOP? All these reset values are referred to as per the TRM and read before enabling the video.

Thanks, Jagan.