Changes in v5:
- Changed patch 1 to not fix the DSI_PS_SEL mask for newer SoCs
Changes in v4:
- Added a fix for RGB666 formats setting and for wrong register
definitions for packed vs loosely packed formats
- Added a commit to make use of mipi_dsi_pixel_format_to_bpp() helper
instead of open coding the same
Changes in v3:
- Rebased over next-20240131
- Added bitfield.h inclusion in patch 3
- Added three more cleanup commits to the mix to simplify
the probe function and remove gotos.
Changes in v2:
- Rebased over next-20231213
This series performs some cleanups for mtk_dsi, enhancing human
readability, using kernel provided macros where possible and
also reducing code size.
Tested on MT8173 and MT8192 Chromebooks (using a DSI<->DP bridge)
and on MT6795 Sony Xperia M5 (DSI video mode panel).
AngeloGioacchino Del Regno (9):
drm/mediatek: dsi: Use GENMASK() for register mask definitions
drm/mediatek: dsi: Fix DSI RGB666 formats and definitions
drm/mediatek: dsi: Cleanup functions mtk_dsi_ps_control{_vact}()
drm/mediatek: dsi: Use bitfield macros where useful
drm/mediatek: dsi: Replace open-coded instance of HZ_PER_MHZ
drm/mediatek: dsi: Register DSI host after acquiring clocks and PHY
drm/mediatek: dsi: Simplify with dev_err_probe and remove gotos
drm/mediatek: dsi: Compress of_device_id entries and add sentinel
drm/mediatek: dsi: Use mipi_dsi_pixel_format_to_bpp() helper function
drivers/gpu/drm/mediatek/mtk_dsi.c | 310 ++++++++++++-----------------
1 file changed, 126 insertions(+), 184 deletions(-)
--
2.43.0
Function mtk_dsi_ps_control() is a subset of mtk_dsi_ps_control_vact():
merge the two in one mtk_dsi_ps_control() function by adding one
function parameter `config_vact` which, when true, writes the VACT
related registers.
Reviewed-by: Fei Shao <[email protected]>
Reviewed-by: Alexandre Mergnat <[email protected]>
Signed-off-by: AngeloGioacchino Del Regno <[email protected]>
---
drivers/gpu/drm/mediatek/mtk_dsi.c | 76 +++++++++---------------------
1 file changed, 23 insertions(+), 53 deletions(-)
diff --git a/drivers/gpu/drm/mediatek/mtk_dsi.c b/drivers/gpu/drm/mediatek/mtk_dsi.c
index 8af0afbe9e3d..7d38e9500700 100644
--- a/drivers/gpu/drm/mediatek/mtk_dsi.c
+++ b/drivers/gpu/drm/mediatek/mtk_dsi.c
@@ -351,40 +351,6 @@ static void mtk_dsi_set_vm_cmd(struct mtk_dsi *dsi)
mtk_dsi_mask(dsi, DSI_VM_CMD_CON, TS_VFP_EN, TS_VFP_EN);
}
-static void mtk_dsi_ps_control_vact(struct mtk_dsi *dsi)
-{
- struct videomode *vm = &dsi->vm;
- u32 dsi_buf_bpp, ps_wc;
- u32 ps_bpp_mode;
-
- if (dsi->format == MIPI_DSI_FMT_RGB565)
- dsi_buf_bpp = 2;
- else
- dsi_buf_bpp = 3;
-
- ps_wc = vm->hactive * dsi_buf_bpp;
- ps_bpp_mode = ps_wc;
-
- switch (dsi->format) {
- case MIPI_DSI_FMT_RGB888:
- ps_bpp_mode |= PACKED_PS_24BIT_RGB888;
- break;
- case MIPI_DSI_FMT_RGB666:
- ps_bpp_mode |= LOOSELY_PS_24BIT_RGB666;
- break;
- case MIPI_DSI_FMT_RGB666_PACKED:
- ps_bpp_mode |= PACKED_PS_18BIT_RGB666;
- break;
- case MIPI_DSI_FMT_RGB565:
- ps_bpp_mode |= PACKED_PS_16BIT_RGB565;
- break;
- }
-
- writel(vm->vactive, dsi->regs + DSI_VACT_NL);
- writel(ps_bpp_mode, dsi->regs + DSI_PSCTRL);
- writel(ps_wc, dsi->regs + DSI_HSTX_CKL_WC);
-}
-
static void mtk_dsi_rxtx_control(struct mtk_dsi *dsi)
{
u32 tmp_reg;
@@ -416,36 +382,40 @@ static void mtk_dsi_rxtx_control(struct mtk_dsi *dsi)
writel(tmp_reg, dsi->regs + DSI_TXRX_CTRL);
}
-static void mtk_dsi_ps_control(struct mtk_dsi *dsi)
+static void mtk_dsi_ps_control(struct mtk_dsi *dsi, bool config_vact)
{
- u32 dsi_tmp_buf_bpp;
- u32 tmp_reg;
+ struct videomode *vm = &dsi->vm;
+ u32 dsi_buf_bpp, ps_wc;
+ u32 ps_bpp_mode;
+
+ if (dsi->format == MIPI_DSI_FMT_RGB565)
+ dsi_buf_bpp = 2;
+ else
+ dsi_buf_bpp = 3;
+
+ ps_wc = vm->hactive * dsi_buf_bpp;
+ ps_bpp_mode = ps_wc;
switch (dsi->format) {
case MIPI_DSI_FMT_RGB888:
- tmp_reg = PACKED_PS_24BIT_RGB888;
- dsi_tmp_buf_bpp = 3;
+ ps_bpp_mode |= PACKED_PS_24BIT_RGB888;
break;
case MIPI_DSI_FMT_RGB666:
- tmp_reg = LOOSELY_PS_24BIT_RGB666;
- dsi_tmp_buf_bpp = 3;
+ ps_bpp_mode |= LOOSELY_PS_24BIT_RGB666;
break;
case MIPI_DSI_FMT_RGB666_PACKED:
- tmp_reg = PACKED_PS_18BIT_RGB666;
- dsi_tmp_buf_bpp = 3;
+ ps_bpp_mode |= PACKED_PS_18BIT_RGB666;
break;
case MIPI_DSI_FMT_RGB565:
- tmp_reg = PACKED_PS_16BIT_RGB565;
- dsi_tmp_buf_bpp = 2;
- break;
- default:
- tmp_reg = PACKED_PS_24BIT_RGB888;
- dsi_tmp_buf_bpp = 3;
+ ps_bpp_mode |= PACKED_PS_16BIT_RGB565;
break;
}
- tmp_reg += dsi->vm.hactive * dsi_tmp_buf_bpp & DSI_PS_WC;
- writel(tmp_reg, dsi->regs + DSI_PSCTRL);
+ if (config_vact) {
+ writel(vm->vactive, dsi->regs + DSI_VACT_NL);
+ writel(ps_wc, dsi->regs + DSI_HSTX_CKL_WC);
+ }
+ writel(ps_bpp_mode, dsi->regs + DSI_PSCTRL);
}
static void mtk_dsi_config_vdo_timing(struct mtk_dsi *dsi)
@@ -521,7 +491,7 @@ static void mtk_dsi_config_vdo_timing(struct mtk_dsi *dsi)
writel(horizontal_backporch_byte, dsi->regs + DSI_HBP_WC);
writel(horizontal_frontporch_byte, dsi->regs + DSI_HFP_WC);
- mtk_dsi_ps_control(dsi);
+ mtk_dsi_ps_control(dsi, false);
}
static void mtk_dsi_start(struct mtk_dsi *dsi)
@@ -666,7 +636,7 @@ static int mtk_dsi_poweron(struct mtk_dsi *dsi)
mtk_dsi_reset_engine(dsi);
mtk_dsi_phy_timconfig(dsi);
- mtk_dsi_ps_control_vact(dsi);
+ mtk_dsi_ps_control(dsi, true);
mtk_dsi_set_vm_cmd(dsi);
mtk_dsi_config_vdo_timing(dsi);
mtk_dsi_set_interrupt_enable(dsi);
--
2.43.0
Hi, Angelo:
On Wed, 2024-02-07 at 15:53 +0100, AngeloGioacchino Del Regno wrote:
> Function mtk_dsi_ps_control() is a subset of
> mtk_dsi_ps_control_vact():
> merge the two in one mtk_dsi_ps_control() function by adding one
> function parameter `config_vact` which, when true, writes the VACT
> related registers.
Reviewed-by: CK Hu <[email protected]>
>
> Reviewed-by: Fei Shao <[email protected]>
> Reviewed-by: Alexandre Mergnat <[email protected]>
> Signed-off-by: AngeloGioacchino Del Regno <
> [email protected]>
> ---
> drivers/gpu/drm/mediatek/mtk_dsi.c | 76 +++++++++-------------------
> --
> 1 file changed, 23 insertions(+), 53 deletions(-)
>
> diff --git a/drivers/gpu/drm/mediatek/mtk_dsi.c
> b/drivers/gpu/drm/mediatek/mtk_dsi.c
> index 8af0afbe9e3d..7d38e9500700 100644
> --- a/drivers/gpu/drm/mediatek/mtk_dsi.c
> +++ b/drivers/gpu/drm/mediatek/mtk_dsi.c
> @@ -351,40 +351,6 @@ static void mtk_dsi_set_vm_cmd(struct mtk_dsi
> *dsi)
> mtk_dsi_mask(dsi, DSI_VM_CMD_CON, TS_VFP_EN, TS_VFP_EN);
> }
>
> -static void mtk_dsi_ps_control_vact(struct mtk_dsi *dsi)
> -{
> - struct videomode *vm = &dsi->vm;
> - u32 dsi_buf_bpp, ps_wc;
> - u32 ps_bpp_mode;
> -
> - if (dsi->format == MIPI_DSI_FMT_RGB565)
> - dsi_buf_bpp = 2;
> - else
> - dsi_buf_bpp = 3;
> -
> - ps_wc = vm->hactive * dsi_buf_bpp;
> - ps_bpp_mode = ps_wc;
> -
> - switch (dsi->format) {
> - case MIPI_DSI_FMT_RGB888:
> - ps_bpp_mode |= PACKED_PS_24BIT_RGB888;
> - break;
> - case MIPI_DSI_FMT_RGB666:
> - ps_bpp_mode |= LOOSELY_PS_24BIT_RGB666;
> - break;
> - case MIPI_DSI_FMT_RGB666_PACKED:
> - ps_bpp_mode |= PACKED_PS_18BIT_RGB666;
> - break;
> - case MIPI_DSI_FMT_RGB565:
> - ps_bpp_mode |= PACKED_PS_16BIT_RGB565;
> - break;
> - }
> -
> - writel(vm->vactive, dsi->regs + DSI_VACT_NL);
> - writel(ps_bpp_mode, dsi->regs + DSI_PSCTRL);
> - writel(ps_wc, dsi->regs + DSI_HSTX_CKL_WC);
> -}
> -
> static void mtk_dsi_rxtx_control(struct mtk_dsi *dsi)
> {
> u32 tmp_reg;
> @@ -416,36 +382,40 @@ static void mtk_dsi_rxtx_control(struct mtk_dsi
> *dsi)
> writel(tmp_reg, dsi->regs + DSI_TXRX_CTRL);
> }
>
> -static void mtk_dsi_ps_control(struct mtk_dsi *dsi)
> +static void mtk_dsi_ps_control(struct mtk_dsi *dsi, bool
> config_vact)
> {
> - u32 dsi_tmp_buf_bpp;
> - u32 tmp_reg;
> + struct videomode *vm = &dsi->vm;
> + u32 dsi_buf_bpp, ps_wc;
> + u32 ps_bpp_mode;
> +
> + if (dsi->format == MIPI_DSI_FMT_RGB565)
> + dsi_buf_bpp = 2;
> + else
> + dsi_buf_bpp = 3;
> +
> + ps_wc = vm->hactive * dsi_buf_bpp;
> + ps_bpp_mode = ps_wc;
>
> switch (dsi->format) {
> case MIPI_DSI_FMT_RGB888:
> - tmp_reg = PACKED_PS_24BIT_RGB888;
> - dsi_tmp_buf_bpp = 3;
> + ps_bpp_mode |= PACKED_PS_24BIT_RGB888;
> break;
> case MIPI_DSI_FMT_RGB666:
> - tmp_reg = LOOSELY_PS_24BIT_RGB666;
> - dsi_tmp_buf_bpp = 3;
> + ps_bpp_mode |= LOOSELY_PS_24BIT_RGB666;
> break;
> case MIPI_DSI_FMT_RGB666_PACKED:
> - tmp_reg = PACKED_PS_18BIT_RGB666;
> - dsi_tmp_buf_bpp = 3;
> + ps_bpp_mode |= PACKED_PS_18BIT_RGB666;
> break;
> case MIPI_DSI_FMT_RGB565:
> - tmp_reg = PACKED_PS_16BIT_RGB565;
> - dsi_tmp_buf_bpp = 2;
> - break;
> - default:
> - tmp_reg = PACKED_PS_24BIT_RGB888;
> - dsi_tmp_buf_bpp = 3;
> + ps_bpp_mode |= PACKED_PS_16BIT_RGB565;
> break;
> }
>
> - tmp_reg += dsi->vm.hactive * dsi_tmp_buf_bpp & DSI_PS_WC;
> - writel(tmp_reg, dsi->regs + DSI_PSCTRL);
> + if (config_vact) {
> + writel(vm->vactive, dsi->regs + DSI_VACT_NL);
> + writel(ps_wc, dsi->regs + DSI_HSTX_CKL_WC);
> + }
> + writel(ps_bpp_mode, dsi->regs + DSI_PSCTRL);
> }
>
> static void mtk_dsi_config_vdo_timing(struct mtk_dsi *dsi)
> @@ -521,7 +491,7 @@ static void mtk_dsi_config_vdo_timing(struct
> mtk_dsi *dsi)
> writel(horizontal_backporch_byte, dsi->regs + DSI_HBP_WC);
> writel(horizontal_frontporch_byte, dsi->regs + DSI_HFP_WC);
>
> - mtk_dsi_ps_control(dsi);
> + mtk_dsi_ps_control(dsi, false);
> }
>
> static void mtk_dsi_start(struct mtk_dsi *dsi)
> @@ -666,7 +636,7 @@ static int mtk_dsi_poweron(struct mtk_dsi *dsi)
> mtk_dsi_reset_engine(dsi);
> mtk_dsi_phy_timconfig(dsi);
>
> - mtk_dsi_ps_control_vact(dsi);
> + mtk_dsi_ps_control(dsi, true);
> mtk_dsi_set_vm_cmd(dsi);
> mtk_dsi_config_vdo_timing(dsi);
> mtk_dsi_set_interrupt_enable(dsi);