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

Jagan Teki jagan at amarulasolutions.com
Thu Dec 21 09:24:30 CET 2023


Hi Andy,

On Tue, Dec 19, 2023 at 2:17 PM Andy Yan <andyshrk at 163.com> wrote:
>
>
>
> Hi Jaqan,
> 在 2023-12-19 15:42:26,"Jagan Teki" <jagan at amarulasolutions.com> 写道:
> >Hi Andy,
> >
> >On Tue, Dec 19, 2023 at 6:50 AM Andy Yan <andyshrk at 163.com> wrote:
> >>
> >>
> >> Hi Jaqan:
> >>
> >> At 2023-12-19 03:11:10, "Jagan Teki" <jagan at amarulasolutions.com> wrote:
> >> >From: Jagan Teki <jagan at 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 at 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.
>
>
> Maybe write 0 to the windows enable bit, disable this window。
> But this is a little rude,I wonder if the mainline has a way to handleoff
> bootloader framebuffer to linux kernel drm system seamless。

I don't think any sort of handoff procedure in U-Boot - Anatolij any
comments on this?

I did see some nxp platform drivers turn off the DE during handoff. So
disabling WIN0 is the only option that I'm thinking of as of now.

>
> [0] is also what Robin mention is V1,  maybe a better direction.
>
>
> [0]https://lore.kernel.org/all/20230120174251.4004100-1-thierry.reding@gmail.com/

Yes, it looks like a proper solution to check but it requires some
iommu re-work to handle iommu-address.

Jagan.


More information about the U-Boot mailing list