| lists.openwall.net | lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC | |
|
Open Source and information security mailing list archives
| ||
|
Message-ID: <b7addbd1-d199-4734-9db6-d7dbc3a7f7d9@collabora.com>
Date: Thu, 14 Mar 2024 11:10:48 +0100
From: AngeloGioacchino Del Regno <angelogioacchino.delregno@...labora.com>
To: Shuijing Li <shuijing.li@...iatek.com>, chunkuang.hu@...nel.org,
p.zabel@...gutronix.de, airlied@...il.com, daniel@...ll.ch,
matthias.bgg@...il.com, jitao.shi@...iatek.com
Cc: dri-devel@...ts.freedesktop.org, linux-mediatek@...ts.infradead.org,
linux-kernel@...r.kernel.org, linux-arm-kernel@...ts.infradead.org,
Project_Global_Chrome_Upstream_Group@...iatek.com
Subject: Re: [PATCH] mediatek: dsi: Add dsi per-frame lp code for mt8188
Il 14/03/24 10:41, Shuijing Li ha scritto:
> Adding the per-frame lp function of mt8188, which can keep HFP in HS and
> reduce the time required for each line to enter and exit low power.
> Per Frame LP:
> |<----------One Active Frame-------->|
> --______________________________________----___________________
> ^HSA+HBP^^RGB^^HFP^^HSA+HBP^^RGB^^HFP^ ^HSA+HBP^^RGB^^HFP^
>
> Per Line LP:
> |<---------------One Active Frame----------->|
> --______________--______________--______________----______________
> ^HSA+HBP^^RGB^ ^HSA+HBP^^RGB^ ^HSA+HBP^^RGB^ ^HSA+HBP^^RGB^
>
> Signed-off-by: Shuijing Li <shuijing.li@...iatek.com>
> ---
> drivers/gpu/drm/mediatek/mtk_dsi.c | 100 +++++++++++++++++++++++++++++
> 1 file changed, 100 insertions(+)
>
> diff --git a/drivers/gpu/drm/mediatek/mtk_dsi.c b/drivers/gpu/drm/mediatek/mtk_dsi.c
> index a2fdfc8ddb15..e6f4807c8711 100644
> --- a/drivers/gpu/drm/mediatek/mtk_dsi.c
> +++ b/drivers/gpu/drm/mediatek/mtk_dsi.c
> @@ -83,6 +83,7 @@
> #define DSI_HSA_WC 0x50
> #define DSI_HBP_WC 0x54
> #define DSI_HFP_WC 0x58
> +#define DSI_BLLP_WC 0x5C
>
> #define DSI_CMDQ_SIZE 0x60
> #define CMDQ_SIZE 0x3f
> @@ -180,6 +181,7 @@ struct mtk_dsi_driver_data {
> bool has_shadow_ctl;
> bool has_size_ctl;
> bool cmdq_long_packet_ctl;
> + bool support_per_frame_lp;
> };
>
> struct mtk_dsi {
> @@ -516,6 +518,103 @@ static void mtk_dsi_config_vdo_timing(struct mtk_dsi *dsi)
> (vm->hactive * dsi_tmp_buf_bpp + 2) % dsi->lanes;
> }
>
> + if (dsi->driver_data->support_per_frame_lp) {
This really looks like a good candidate to fit in a function on its own...
> + unsigned int lpx = 0, da_hs_exit = 0, da_hs_prep = 0, da_hs_trail = 0;
> + unsigned int da_hs_zero = 0, ps_wc = 0, hs_vb_ps_wc = 0;
> + u32 bllp_wc, bllp_en, v_active_roundup, hstx_cklp_wc;
> + u32 hstx_cklp_wc_max, hstx_cklp_wc_min;
> +
> + da_hs_trail = (readl(dsi->regs + DSI_PHY_TIMECON0) >> 24) & 0xff;
> + bllp_en = (readl(dsi->regs + DSI_TXRX_CTRL) >> 7) & 0x1;
(somewhere: #define HSTX_BLLP_EN BIT(7))
u32 timecon0, txrx_ctrl;
timecon0 = readl(....DSI_PHY_TIMECON0);
txrx_ctrl = readl(.....);
da_hs_trail = FIELD_GET(HS_TRAIL, timecon0);
bllp_en = FIELD_GET(HSTX_BLLP_EN, txrx_ctrl);
> + if (dsi->mode_flags & MIPI_DSI_MODE_VIDEO_SYNC_PULSE) {
> + horizontal_sync_active_byte =
> + (vm->hsync_len * dsi_tmp_buf_bpp - 10);
> + horizontal_backporch_byte =
> + (vm->hback_porch * dsi_tmp_buf_bpp - 10);
> + horizontal_frontporch_byte =
> + (vm->hfront_porch * dsi_tmp_buf_bpp - 12);
> +
> + ps_wc = readl(dsi->regs + DSI_PSCTRL) & 0x7fff;
> + v_active_roundup = (32 + horizontal_sync_active_byte +
> + horizontal_backporch_byte + ps_wc +
> + horizontal_frontporch_byte) % dsi->lanes;
> + if (v_active_roundup)
> + horizontal_backporch_byte = horizontal_backporch_byte +
> + dsi->lanes - v_active_roundup;
> + hstx_cklp_wc_min = (DIV_ROUND_UP((12 + 2 + 4 +
Please document those "magic numbers": why is this 12 + 2 + 4 ?
What is 12? what is 2? what is 4?
> + horizontal_sync_active_byte), dsi->lanes) + da_hs_trail + 1)
> + * dsi->lanes / 6 - 1;
> + hstx_cklp_wc_max = (DIV_ROUND_UP((20 + 6 + 4 +
> + horizontal_sync_active_byte + horizontal_backporch_byte +
> + ps_wc), dsi->lanes) + da_hs_trail + 1) * dsi->lanes / 6 - 1;
> + } else {
> + horizontal_sync_active_byte = vm->hsync_len * dsi_tmp_buf_bpp - 4;
> +
> + horizontal_backporch_byte = (vm->hback_porch + vm->hsync_len) *
> + dsi_tmp_buf_bpp - 10;
> + hstx_cklp_wc_min = (DIV_ROUND_UP(4, dsi->lanes) + da_hs_trail + 1)
> + * dsi->lanes / 6 - 1;
> +
> + if (dsi->mode_flags & MIPI_DSI_MODE_VIDEO_BURST) {
> + ps_wc = readl(dsi->regs + DSI_PSCTRL) & 0x7fff;
> + bllp_wc = readl(dsi->regs + DSI_BLLP_WC) & 0xfff;
Please use bitfield macros here as well.
> + horizontal_frontporch_byte = (vm->hfront_porch *
> + dsi_tmp_buf_bpp - 18);
> +
> + v_active_roundup = (28 + horizontal_backporch_byte + ps_wc +
> + horizontal_frontporch_byte + bllp_wc) % dsi->lanes;
> + if (v_active_roundup)
> + horizontal_backporch_byte = horizontal_backporch_byte +
> + dsi->lanes - v_active_roundup;
> + if (bllp_en) {
> + hstx_cklp_wc_max = (DIV_ROUND_UP((16 + 6 + 4 +
> + horizontal_backporch_byte + bllp_wc + ps_wc),
> + dsi->lanes) + da_hs_trail + 1) * dsi->lanes / 6 - 1;
> + } else {
> + hstx_cklp_wc_max = (DIV_ROUND_UP((12 + 4 + 4 +
> + horizontal_backporch_byte + bllp_wc + ps_wc),
> + dsi->lanes) + da_hs_trail + 1) * dsi->lanes / 6 - 1;
> + }
> + } else {
> + ps_wc = readl(dsi->regs + DSI_PSCTRL) & 0x7fff;
> + horizontal_frontporch_byte = (vm->hfront_porch *
> + dsi_tmp_buf_bpp - 12);
> +
> + v_active_roundup = (22 + horizontal_backporch_byte + ps_wc +
> + horizontal_frontporch_byte) % dsi->lanes;
> + if (v_active_roundup)
> + horizontal_backporch_byte = horizontal_backporch_byte +
> + dsi->lanes - v_active_roundup;
> +
> + hstx_cklp_wc_max = (DIV_ROUND_UP((12 + 4 + 4 +
> + horizontal_backporch_byte + ps_wc),
> + dsi->lanes) + da_hs_trail + 1) * dsi->lanes / 6 - 1;
> + }
> + }
> + hstx_cklp_wc = (readl(dsi->regs + DSI_HSTX_CKL_WC) >> 2) & 0x3fff;
same here
> + if (hstx_cklp_wc <= hstx_cklp_wc_min ||
> + hstx_cklp_wc >= hstx_cklp_wc_max) {
> + hstx_cklp_wc = (hstx_cklp_wc_max / 2) << 2;
and same here... and everywhere else.
> + writel(hstx_cklp_wc, dsi->regs + DSI_HSTX_CKL_WC);
> + }
> + hstx_cklp_wc = hstx_cklp_wc >> 2;
> + if (hstx_cklp_wc <= hstx_cklp_wc_min ||
> + hstx_cklp_wc >= hstx_cklp_wc_max) {
> + DRM_WARN("Wrong setting of hstx_ckl_wc\n");
> + }
> +
> + lpx = readl(dsi->regs + DSI_PHY_TIMECON0) & 0xff;
> + da_hs_exit = (readl(dsi->regs + DSI_PHY_TIMECON1) >> 24) & 0xff;
> + da_hs_prep = (readl(dsi->regs + DSI_PHY_TIMECON0) >> 8) & 0xff;
> + da_hs_zero = (readl(dsi->regs + DSI_PHY_TIMECON0) >> 16) & 0xff;
> + ps_wc = readl(dsi->regs + DSI_PSCTRL) & 0x7fff;
> + hs_vb_ps_wc = ps_wc -
> + (lpx + da_hs_exit + da_hs_prep + da_hs_zero + 2)
> + * dsi->lanes;
> + horizontal_frontporch_byte = (1 << 31)
> + | (hs_vb_ps_wc << 16)
> + | (horizontal_frontporch_byte);
> + }
> writel(horizontal_sync_active_byte, dsi->regs + DSI_HSA_WC);
> writel(horizontal_backporch_byte, dsi->regs + DSI_HBP_WC);
> writel(horizontal_frontporch_byte, dsi->regs + DSI_HFP_WC);
> @@ -1246,6 +1345,7 @@ static const struct mtk_dsi_driver_data mt8188_dsi_driver_data = {
> .has_shadow_ctl = true,
> .has_size_ctl = true,
> .cmdq_long_packet_ctl = true,
> + .support_per_frame_lp = true,
Is this supported only on MT8188? Are you sure that MT8195 doesn't support that?
Regards,
Angelo
> };
>
> static const struct of_device_id mtk_dsi_of_match[] = {
Powered by blists - more mailing lists