Hi Andy,
On Tue, Dec 19, 2023 at 6:50 AM Andy Yan <andys...@163.com> wrote:
>
>
> Hi Jaqan：
>
> At 2023-12-19 03:11:10, "Jagan Teki" <ja...@amarulasolutions.com> wrote:
> >From: Jagan Teki <ja...@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 <ja...@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(®s->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, ®s->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.
