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

[Andy Yan]

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。

[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/ 

>
>Thanks,
>Jagan.