<html><body><p>
<pre>
Hi, Shuijing:
On Mon, 2024-08-12 at 16:47 +0800, CK Hu wrote:
> Hi, Shuijing:
>
> On Mon, 2024-08-12 at 15:03 +0800, Shuijing Li wrote:
> > 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@mediatek.com>
> > ---
> > Changes in v5:
> > Fix code style issue and add per-line-lp function to be symmetrical with per-frame-lp.
> > per suggestion from previous thread:
> > https://urldefense.com/v3/__https://patchwork.kernel.org/project/linux-mediatek/patch/20240801081144.22372-1-shuijing.li@mediatek.com/__;!!CTRNKA9wMg0ARbw!lt0WeWrOtkG2s4UnUCtHx14j8FvTQK-U-DhEyw8YhsnpbUj2o2KSDk3D65k08G6gAQSxzATL4oc-q6wqHIJ6yQ$
> > Changes in v4:
> > Drop the code related to bllp_en and bllp_wc, adjust ps_wc to dsi->vm.hactive *
> > dsi_buf_bpp.
> > Changes in v3:
> > Use function in bitfield.h and get value from phy timing, per suggestion
> > from previous thread:
> > https://urldefense.com/v3/__https://patchwork.kernel.org/project/linux-mediatek/patch/20240424091639.22759-1-shuijing.li@mediatek.com/__;!!CTRNKA9wMg0ARbw!lt0WeWrOtkG2s4UnUCtHx14j8FvTQK-U-DhEyw8YhsnpbUj2o2KSDk3D65k08G6gAQSxzATL4oc-q6yKiR5BLQ$
> > Changes in v2:
> > Use bitfield macros and add new function for per prame lp and improve
> > the format, per suggestion from previous thread:
> > https://urldefense.com/v3/__https://patchwork.kernel.org/project/linux-mediatek/patch/20240314094238.3315-1-shuijing.li@mediatek.com/__;!!CTRNKA9wMg0ARbw!lt0WeWrOtkG2s4UnUCtHx14j8FvTQK-U-DhEyw8YhsnpbUj2o2KSDk3D65k08G6gAQSxzATL4oc-q6ykO7VhBw$
> > ---
> > drivers/gpu/drm/mediatek/mtk_dsi.c | 158 +++++++++++++++++++++++++----
> > 1 file changed, 139 insertions(+), 19 deletions(-)
> >
> > diff --git a/drivers/gpu/drm/mediatek/mtk_dsi.c b/drivers/gpu/drm/mediatek/mtk_dsi.c
> > index b6e3c011a12d..027cf719b078 100644
> > --- a/drivers/gpu/drm/mediatek/mtk_dsi.c
> > +++ b/drivers/gpu/drm/mediatek/mtk_dsi.c
> > @@ -88,12 +88,15 @@
> > #define DSI_HSA_WC0x50
> > #define DSI_HBP_WC0x54
> > #define DSI_HFP_WC0x58
> > +#define HFP_HS_VB_PS_WCGENMASK(30, 16)
> > +#define HFP_HS_ENBIT(31)
> >
> > #define DSI_CMDQ_SIZE0x60
> > #define CMDQ_SIZE0x3f
> > #define CMDQ_SIZE_SELBIT(15)
> >
> > #define DSI_HSTX_CKL_WC0x64
> > +#define HSTX_CKL_WCGENMASK(15, 2)
> >
> > #define DSI_RX_DATA00x74
> > #define DSI_RX_DATA10x78
> > @@ -187,6 +190,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 {
> > @@ -426,7 +430,112 @@ static void mtk_dsi_ps_control(struct mtk_dsi *dsi, bool config_vact)
> > writel(ps_val, dsi->regs + DSI_PSCTRL);
> > }
> >
> > -static void mtk_dsi_config_vdo_timing(struct mtk_dsi *dsi)
> > +static void mtk_dsi_config_vdo_timing_per_frame_lp(struct mtk_dsi *dsi)
> > +{
> > +u32 horizontal_sync_active_byte;
> > +u32 horizontal_backporch_byte;
> > +u32 horizontal_frontporch_byte;
> > +u32 hfp_byte_adjust;
> > +u32 dsi_tmp_buf_bpp;
> > +unsigned int lpx, da_hs_exit, da_hs_prep, da_hs_trail;
> > +unsigned int da_hs_zero, ps_wc, hs_vb_ps_wc;
> > +u32 v_active_roundup, hstx_cklp_wc;
> > +u32 hstx_cklp_wc_max, hstx_cklp_wc_min;
> > +struct videomode *vm = &dsi->vm;
> > +
> > +if (dsi->format == MIPI_DSI_FMT_RGB565)
> > +dsi_tmp_buf_bpp = 2;
> > +else
> > +dsi_tmp_buf_bpp = 3;
> > +
> > +da_hs_trail = dsi->phy_timing.da_hs_trail;
> > +ps_wc = dsi->vm.hactive * dsi_tmp_buf_bpp;
> > +
> > +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;
> > +hfp_byte_adjust = 12;
> > +
> > +v_active_roundup = (32 + horizontal_sync_active_byte +
> > +horizontal_backporch_byte + ps_wc +
> > +vm->hfront_porch * dsi_tmp_buf_bpp - hfp_byte_adjust) % 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 +
> > +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) {
> > +hfp_byte_adjust = 18;
> > +
> > +v_active_roundup = (28 + horizontal_backporch_byte + ps_wc +
> > +vm->hfront_porch * dsi_tmp_buf_bpp - hfp_byte_adjust) % 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;
> > +} else {
> > +hfp_byte_adjust = 12;
> > +
> > +v_active_roundup = (22 + horizontal_backporch_byte + ps_wc +
> > +vm->hfront_porch * dsi_tmp_buf_bpp - hfp_byte_adjust) % dsi->lanes;
> > +if (v_active_roundup)
> > +horizontal_backporch_byte = horizontal_backporch_byte +
> > +dsi->lanes - v_active_roundup;
>
> Try to simplify the code:
>
> if (dsi->mode_flags & MIPI_DSI_MODE_VIDEO_SYNC_PULSE) {
> v_active_adjust = 32 + horizontal_sync_active_byte;
>
> ...
> } else {
> if (dsi->mode_flags & MIPI_DSI_MODE_VIDEO_BURST) {
> v_active_adjust = 28;
> ...
> } else {
> v_active_adjust = 22;
> ...
> }
> }
>
> v_active_roundup = (v_active_adjust + horizontal_backporch_byte + ps_wc +
> vm->hfront_porch * dsi_tmp_buf_bpp - hfp_byte_adjust) % dsi->lanes;
> horizontal_backporch_byte += dsi->lanes - v_active_roundup;
>
> You need not to check v_active_roundup is zero or not.
Sorry, you need to check v_active_roundup is zero or not, so
if (v_active_roundup)
horizontal_backporch_byte += dsi->lanes - v_active_roundup;
Regards,
CK
>
> > +
> > +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;
> > +}
> > +}
> > +horizontal_frontporch_byte = vm->hfront_porch * dsi_tmp_buf_bpp - hfp_byte_adjust;
> > +hstx_cklp_wc = FIELD_GET(HSTX_CKL_WC, readl(dsi->regs + DSI_HSTX_CKL_WC));
> > +
> > +if (hstx_cklp_wc <= hstx_cklp_wc_min || hstx_cklp_wc >= hstx_cklp_wc_max) {
> > +hstx_cklp_wc = ((hstx_cklp_wc_min + hstx_cklp_wc_max) / 2);
> > +
> > +/* Check if the new setting is valid */
> > +if (hstx_cklp_wc <= hstx_cklp_wc_min ||hstx_cklp_wc >= hstx_cklp_wc_max)
> > +DRM_WARN("Wrong setting of hstx_ckl_wc\n");
> > +
> > +hstx_cklp_wc = FIELD_PREP(HSTX_CKL_WC, hstx_cklp_wc);
> > +writel(hstx_cklp_wc, dsi->regs + DSI_HSTX_CKL_WC);
> > +}
> > +
> > +lpx = dsi->phy_timing.lpx;
> > +da_hs_exit = dsi->phy_timing.da_hs_exit;
> > +da_hs_prep = dsi->phy_timing.da_hs_prepare;
> > +da_hs_zero = dsi->phy_timing.da_hs_zero;
> > +
> > +hs_vb_ps_wc = ps_wc -
> > +(lpx + da_hs_exit + da_hs_prep + da_hs_zero + 2)
> > +* dsi->lanes;
> > +horizontal_frontporch_byte = FIELD_PREP(HFP_HS_EN, 1) |
> > +FIELD_PREP(HFP_HS_VB_PS_WC, hs_vb_ps_wc) |
> > +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);
> > +}
> > +
> > +static void mtk_dsi_config_vdo_timing_per_line_lp(struct mtk_dsi *dsi)
> > {
> > u32 horizontal_sync_active_byte;
> > u32 horizontal_backporch_byte;
> > @@ -436,7 +545,6 @@ static void mtk_dsi_config_vdo_timing(struct mtk_dsi *dsi)
> > u32 dsi_tmp_buf_bpp, data_phy_cycles;
> > u32 delta;
> > struct mtk_phy_timing *timing = &dsi->phy_timing;
> > -
> > struct videomode *vm = &dsi->vm;
> >
> > if (dsi->format == MIPI_DSI_FMT_RGB565)
> > @@ -444,26 +552,16 @@ static void mtk_dsi_config_vdo_timing(struct mtk_dsi *dsi)
> > else
> > dsi_tmp_buf_bpp = 3;
> >
> > -writel(vm->vsync_len, dsi->regs + DSI_VSA_NL);
> > -writel(vm->vback_porch, dsi->regs + DSI_VBP_NL);
> > -writel(vm->vfront_porch, dsi->regs + DSI_VFP_NL);
> > -writel(vm->vactive, dsi->regs + DSI_VACT_NL);
> > -
> > -if (dsi->driver_data->has_size_ctl)
> > -writel(FIELD_PREP(DSI_HEIGHT, vm->vactive) |
> > - FIELD_PREP(DSI_WIDTH, vm->hactive),
> > - dsi->regs + DSI_SIZE_CON);
> > -
> > horizontal_sync_active_byte = (vm->hsync_len * dsi_tmp_buf_bpp - 10);
> >
> > if (dsi->mode_flags & MIPI_DSI_MODE_VIDEO_SYNC_PULSE)
> > horizontal_backporch_byte = vm->hback_porch * dsi_tmp_buf_bpp - 10;
> > else
> > horizontal_backporch_byte = (vm->hback_porch + vm->hsync_len) *
> > - dsi_tmp_buf_bpp - 10;
> > +dsi_tmp_buf_bpp - 10;
>
> Do not modify this.
>
> >
> > data_phy_cycles = timing->lpx + timing->da_hs_prepare +
> > - timing->da_hs_zero + timing->da_hs_exit + 3;
> > +timing->da_hs_zero + timing->da_hs_exit + 3;
>
> Ditto.
>
> >
> > delta = dsi->mode_flags & MIPI_DSI_MODE_VIDEO_BURST ? 18 : 12;
> > delta += dsi->mode_flags & MIPI_DSI_MODE_NO_EOT_PACKET ? 0 : 2;
> > @@ -474,18 +572,18 @@ static void mtk_dsi_config_vdo_timing(struct mtk_dsi *dsi)
> >
> > if (horizontal_front_back_byte > data_phy_cycles_byte) {
> > horizontal_frontporch_byte -= data_phy_cycles_byte *
> > - horizontal_frontporch_byte /
> > - horizontal_front_back_byte;
> > +horizontal_frontporch_byte /
> > +horizontal_front_back_byte;
>
> Ditto.
>
> >
> > horizontal_backporch_byte -= data_phy_cycles_byte *
> > - horizontal_backporch_byte /
> > - horizontal_front_back_byte;
> > +horizontal_backporch_byte /
> > +horizontal_front_back_byte;
>
> Ditto.
>
> > } else {
> > DRM_WARN("HFP + HBP less than d-phy, FPS will under 60Hz\n");
> > }
> >
> > if ((dsi->mode_flags & MIPI_DSI_HS_PKT_END_ALIGNED) &&
> > - (dsi->lanes == 4)) {
> > +(dsi->lanes == 4)) {
>
> Ditto.
>
> Regards,
> CK
>
> > horizontal_sync_active_byte =
> > roundup(horizontal_sync_active_byte, dsi->lanes) - 2;
> > horizontal_frontporch_byte =
> > @@ -499,6 +597,26 @@ static void mtk_dsi_config_vdo_timing(struct mtk_dsi *dsi)
> > 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);
> > +}
> > +
> > +static void mtk_dsi_config_vdo_timing(struct mtk_dsi *dsi)
> > +{
> > +struct videomode *vm = &dsi->vm;
> > +
> > +writel(vm->vsync_len, dsi->regs + DSI_VSA_NL);
> > +writel(vm->vback_porch, dsi->regs + DSI_VBP_NL);
> > +writel(vm->vfront_porch, dsi->regs + DSI_VFP_NL);
> > +writel(vm->vactive, dsi->regs + DSI_VACT_NL);
> > +
> > +if (dsi->driver_data->has_size_ctl)
> > +writel(FIELD_PREP(DSI_HEIGHT, vm->vactive) |
> > +FIELD_PREP(DSI_WIDTH, vm->hactive),
> > +dsi->regs + DSI_SIZE_CON);
> > +
> > +if (dsi->driver_data->support_per_frame_lp)
> > +mtk_dsi_config_vdo_timing_per_frame_lp(dsi);
> > +else
> > +mtk_dsi_config_vdo_timing_per_line_lp(dsi);
> >
> > mtk_dsi_ps_control(dsi, false);
> > }
> > @@ -1197,6 +1315,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,
> > };
> >
> > static const struct of_device_id mtk_dsi_of_match[] = {
</pre>
</p></body></html><!--type:text--><!--{--><pre>************* MEDIATEK Confidentiality Notice ********************
The information contained in this e-mail message (including any
attachments) may be confidential, proprietary, privileged, or otherwise
exempt from disclosure under applicable laws. It is intended to be
conveyed only to the designated recipient(s). Any use, dissemination,
distribution, printing, retaining or copying of this e-mail (including its
attachments) by unintended recipient(s) is strictly prohibited and may
be unlawful. If you are not an intended recipient of this e-mail, or believe
that you have received this e-mail in error, please notify the sender
immediately (by replying to this e-mail), delete any and all copies of
this e-mail (including any attachments) from your system, and do not
disclose the content of this e-mail to any other person. Thank you!
</pre><!--}-->