Hi Jaqan，
在 2023-12-19 15:42:26，"Jagan Teki" <ja...@amarulasolutions.com> 写道：
>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.
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.red...@gmail.com/
>
>Thanks,
>Jagan.
