Re: [v9,2/3] drm/mediatek: implement the DSI hs packets aligned

From: Andrzej Hajda
Date: Fri Jan 14 2022 - 06:12:02 EST



On 14.01.2022 11:20, Rex-BC Chen wrote:
Hello AngeloGioacchino,

Thanks for your review.
I will modify this in next version.

BRs,
Rex-BC Chen

On Fri, 2022-01-14 at 10:36 +0100, AngeloGioacchino Del Regno wrote:
Il 14/01/22 10:21, Rex-BC Chen ha scritto:
Some DSI RX devices require the packets on all lanes aligned at the
end.
Otherwise, there will be some issues of shift or scroll for screen.

Signed-off-by: Jitao Shi <jitao.shi@xxxxxxxxxxxx>
Signed-off-by: Rex-BC Chen <rex-bc.chen@xxxxxxxxxxxx>
Hello,
thanks for the patch! However, there's something to improve...

---
drivers/gpu/drm/mediatek/mtk_dsi.c | 12 ++++++++++++
1 file changed, 12 insertions(+)

diff --git a/drivers/gpu/drm/mediatek/mtk_dsi.c
b/drivers/gpu/drm/mediatek/mtk_dsi.c
index 5d90d2eb0019..ccdda15f5a66 100644
--- a/drivers/gpu/drm/mediatek/mtk_dsi.c
+++ b/drivers/gpu/drm/mediatek/mtk_dsi.c
@@ -195,6 +195,8 @@ struct mtk_dsi {
struct clk *hs_clk;
u32 data_rate;
+ /* force dsi line end without dsi_null data */
+ bool hs_packet_end_aligned;
There's no need to introduce a new variable here...
unsigned long mode_flags;
enum mipi_dsi_pixel_format format;
@@ -500,6 +502,13 @@ static void mtk_dsi_config_vdo_timing(struct
mtk_dsi *dsi)
DRM_WARN("HFP + HBP less than d-phy, FPS will under
60Hz\n");
}
+ if (dsi->hs_packet_end_aligned) {
You can simply check mode_flags here:
if (dsi->mode_flags & MIPI_DSI_HS_PKT_END_ALIGNED) {

+ horizontal_sync_active_byte =
roundup(horizontal_sync_active_byte, dsi->lanes) - 2;
+ horizontal_frontporch_byte =
roundup(horizontal_frontporch_byte, dsi->lanes) - 2;
+ horizontal_backporch_byte =
roundup(horizontal_backporch_byte, dsi->lanes) - 2;
+ horizontal_backporch_byte -= (vm->hactive *
dsi_tmp_buf_bpp + 2) % dsi->lanes;
+ }
+


And if you could add comment explaining the magic here it would be nice.


Regards

Andrzej


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);
@@ -794,6 +803,9 @@ static int mtk_dsi_host_attach(struct
mipi_dsi_host *host,
dsi->lanes = device->lanes;
dsi->format = device->format;
dsi->mode_flags = device->mode_flags;
+ dsi->hs_packet_end_aligned = (dsi->mode_flags &
+ MIPI_DSI_HS_PKT_END_ALIGNED)
+ ? true : false;
...so there's no need for this one, either.

return 0;
}

Regards,
- Angelo