Re: [PATCH 14/18] drm/mediatek: dpi: add support for dpi clock

2024-04-16 Thread Alexandre Mergnat 




On 24/10/2023 11:12, AngeloGioacchino Del Regno wrote:

Il 23/10/23 16:40, amerg...@baylibre.com ha scritto:

From: Fabien Parent 

MT8365 requires an additional clock for DPI. Add support for that
additional clock.

Signed-off-by: Fabien Parent 
Signed-off-by: Alexandre Mergnat 


I'm not convinced that this is right... at all.

 From a fast check of the MT8365 DPI clocks, I can see that the DPI0 
clock declares
parent VPLL_DPIX (a fixed clock), but nothing ever has VPLL_DPIX_EN 
(which is the

GATE clock, enabling output of DPIx VPLL?).

But then, there's even more: no clock ever references the 
CLK_TOP_DPI0_SEL nor the
CLK_TOP_DPI1_SEL gate, which is a PLL parent selector... in other 
platforms, that

is muxing through the TVDPLL, but on MT8365 that is LVDSPLL?!


AFAI see into mt8365 documentation, there is no TVDPLL, only LVDSPLL



I have many questions now:
* Two PLLs are apparently brought up, but which one is the right one?!
   * Is the LVDS PLL really used for DisplayPort? (dpi0_sel)


Seems to be LVDS

 enable  prepare  protect   
 duty  hardware
   clock  countcountcountrate   
accuracy phase  cycleenable
---
 clk26m  18   1912600  
0 0 Y
vpll_dpix 1107500  
0 0  5 Y
   mm_flvdstx_pxl 0007500  
0 0  5 N
   mm_dpi0_dpi0   1107500  
0 0  5 Y
   vpll_dpix_en   0007500  
0 0  5 N
lvdspll   110   283999497  
0 0  5 Y
   lvdspll_d1600017749968  
0 0  5 Y
   lvdspll_d8 00035499937  
0 0  5 Y
   lvdspll_d4 00070999874  
0 0  5 Y
   lvdspll_d2 110   141999748  
0 0  5 Y
  dpi0_sel110   141999748  
0 0  5 Y
  dpi1_sel000   141999748  
0 0  5 N
mmpll 110   45609  
0 0  5 Y
   mmpll_ck   110   45609  
0 0  5 Y
  mm_sel 15   150   45609  
0 0  5 Y
 mm_dpi0  110   45609  
0 0  5 Y



* Are you sure that CLK_MM_DPI0_DPI0's parent shouldn't be dpi0_sel 
instead?


I'm agree with you. After few change, it works.

-   GATE_MM0(CLK_MM_DPI0_DPI0, "mm_dpi0_dpi0", "vpll_dpix", 20),
+   GATE_MM0(CLK_MM_DPI0_DPI0, "mm_dpi0_dpi0", "dpi0_sel", 20),

-   clocks = < CLK_TOP_DPI0_SEL>,
+   clocks = < CLK_MM_DPI0_DPI0>,


 enable  prepare  protect   
 duty  hardware
   clock  countcountcountrate   
accuracy phase  cycleenable
---
vpll_dpix0   0075000
  0 5  Y
   mm_flvdstx_pxl0   0075000
  0 5  N
   vpll_dpix_en  0   0075000
  0 5  N

lvdspll  1   10283999497   0
  0 5  Y
   lvdspll_d16   0   00177499680
  0 5  Y
   lvdspll_d80   00354999370
  0 5  Y
   lvdspll_d40   00709998740
  0 5  Y
   lvdspll_d21   10141999748   0
  0 5  Y
  dpi0_sel   1   10141999748   0
  0 5  Y
 mm_dpi0_dpi01   10141999748   0
  0 5  Y
  dpi1_sel   0   00141999748   0
  0 5  N

mmpll1   1045609   0
  0 5  Y

Re: [PATCH 14/18] drm/mediatek: dpi: add support for dpi clock

2023-10-24 Thread AngeloGioacchino Del Regno 

Il 23/10/23 16:40, amerg...@baylibre.com ha scritto:

From: Fabien Parent 

MT8365 requires an additional clock for DPI. Add support for that
additional clock.

Signed-off-by: Fabien Parent 
Signed-off-by: Alexandre Mergnat 


I'm not convinced that this is right... at all.

From a fast check of the MT8365 DPI clocks, I can see that the DPI0 clock 
declares
parent VPLL_DPIX (a fixed clock), but nothing ever has VPLL_DPIX_EN (which is 
the
GATE clock, enabling output of DPIx VPLL?).

But then, there's even more: no clock ever references the CLK_TOP_DPI0_SEL nor 
the
CLK_TOP_DPI1_SEL gate, which is a PLL parent selector... in other platforms, 
that
is muxing through the TVDPLL, but on MT8365 that is LVDSPLL?!

I have many questions now:
* Two PLLs are apparently brought up, but which one is the right one?!
  * Is the LVDS PLL really used for DisplayPort? (dpi0_sel)
  * Is the VPLL_DPIx PLL used for DisplayPort instead? (dpi0_dpi0)
* Why is the LVDSTX_PXL clock using the same PLL as DPI0?!
  * Why is the VPLL_DPIx gate never enabled?
* Are you sure that CLK_MM_DPI0_DPI0's parent shouldn't be dpi0_sel instead?
* Where is DPI1 in this SoC? Why is there a dpi1_sel clock, but no MM clock
  for the DPI1 controller? Is there any DPI1 controller, even?!
  * Why is there a DPI1 MUX, if there's no DPI1 controller?!

Answering all those questions will lead you to the right change, which I believe
to be in the clock drivers, not here in mtk_dpi.c.

Cheers!
Angelo

[PATCH 14/18] drm/mediatek: dpi: add support for dpi clock

2023-10-23 Thread amergnat 
From: Fabien Parent 

MT8365 requires an additional clock for DPI. Add support for that
additional clock.

Signed-off-by: Fabien Parent 
Signed-off-by: Alexandre Mergnat 
---
 drivers/gpu/drm/mediatek/mtk_dpi.c | 50 +-
 1 file changed, 49 insertions(+), 1 deletion(-)

diff --git a/drivers/gpu/drm/mediatek/mtk_dpi.c 
b/drivers/gpu/drm/mediatek/mtk_dpi.c
index 2f931e4e2b60..ddd7c54febe6 100644
--- a/drivers/gpu/drm/mediatek/mtk_dpi.c
+++ b/drivers/gpu/drm/mediatek/mtk_dpi.c
@@ -70,6 +70,7 @@ struct mtk_dpi {
struct device *mmsys_dev;
struct clk *engine_clk;
struct clk *pixel_clk;
+   struct clk *dpi_clk;
struct clk *tvd_clk;
int irq;
struct drm_display_mode mode;
@@ -137,6 +138,7 @@ struct mtk_dpi_yc_limit {
  * @csc_enable_bit: Enable bit of CSC.
  * @pixels_per_iter: Quantity of transferred pixels per iteration.
  * @edge_cfg_in_mmsys: If the edge configuration for DPI's output needs to be 
set in MMSYS.
+ * @is_dpi_clk_req: Support the additionnal DPI clock.
  */
 struct mtk_dpi_conf {
unsigned int (*cal_factor)(int clock);
@@ -156,6 +158,7 @@ struct mtk_dpi_conf {
u32 csc_enable_bit;
u32 pixels_per_iter;
bool edge_cfg_in_mmsys;
+   bool is_dpi_clk_req;
 };
 
 static void mtk_dpi_mask(struct mtk_dpi *dpi, u32 offset, u32 val, u32 mask)
@@ -472,6 +475,7 @@ static void mtk_dpi_power_off(struct mtk_dpi *dpi)
mtk_dpi_disable(dpi);
clk_disable_unprepare(dpi->pixel_clk);
clk_disable_unprepare(dpi->engine_clk);
+   clk_disable_unprepare(dpi->dpi_clk);
 }
 
 static int mtk_dpi_power_on(struct mtk_dpi *dpi)
@@ -481,10 +485,16 @@ static int mtk_dpi_power_on(struct mtk_dpi *dpi)
if (++dpi->refcount != 1)
return 0;
 
+   ret = clk_prepare_enable(dpi->dpi_clk);
+   if (ret) {
+   dev_err(dpi->dev, "failed to enable dpi clock: %d\n", ret);
+   goto err_refcount;
+   }
+
ret = clk_prepare_enable(dpi->engine_clk);
if (ret) {
dev_err(dpi->dev, "Failed to enable engine clock: %d\n", ret);
-   goto err_refcount;
+   goto err_engine;
}
 
ret = clk_prepare_enable(dpi->pixel_clk);
@@ -497,6 +507,8 @@ static int mtk_dpi_power_on(struct mtk_dpi *dpi)
 
 err_pixel:
clk_disable_unprepare(dpi->engine_clk);
+err_engine:
+   clk_disable_unprepare(dpi->dpi_clk);
 err_refcount:
dpi->refcount--;
return ret;
@@ -902,6 +914,7 @@ static const struct mtk_dpi_conf mt8173_conf = {
.channel_swap_shift = CH_SWAP,
.yuv422_en_bit = YUV422_EN,
.csc_enable_bit = CSC_ENABLE,
+   .is_dpi_clk_req = false,
 };
 
 static const struct mtk_dpi_conf mt2701_conf = {
@@ -920,6 +933,7 @@ static const struct mtk_dpi_conf mt2701_conf = {
.channel_swap_shift = CH_SWAP,
.yuv422_en_bit = YUV422_EN,
.csc_enable_bit = CSC_ENABLE,
+   .is_dpi_clk_req = false,
 };
 
 static const struct mtk_dpi_conf mt8183_conf = {
@@ -937,6 +951,7 @@ static const struct mtk_dpi_conf mt8183_conf = {
.channel_swap_shift = CH_SWAP,
.yuv422_en_bit = YUV422_EN,
.csc_enable_bit = CSC_ENABLE,
+   .is_dpi_clk_req = false,
 };
 
 static const struct mtk_dpi_conf mt8186_conf = {
@@ -969,6 +984,7 @@ static const struct mtk_dpi_conf mt8188_dpintf_conf = {
.channel_swap_shift = DPINTF_CH_SWAP,
.yuv422_en_bit = DPINTF_YUV422_EN,
.csc_enable_bit = DPINTF_CSC_ENABLE,
+   .is_dpi_clk_req = false,
 };
 
 static const struct mtk_dpi_conf mt8192_conf = {
@@ -986,6 +1002,7 @@ static const struct mtk_dpi_conf mt8192_conf = {
.channel_swap_shift = CH_SWAP,
.yuv422_en_bit = YUV422_EN,
.csc_enable_bit = CSC_ENABLE,
+   .is_dpi_clk_req = false,
 };
 
 static const struct mtk_dpi_conf mt8195_dpintf_conf = {
@@ -1000,6 +1017,25 @@ static const struct mtk_dpi_conf mt8195_dpintf_conf = {
.channel_swap_shift = DPINTF_CH_SWAP,
.yuv422_en_bit = DPINTF_YUV422_EN,
.csc_enable_bit = DPINTF_CSC_ENABLE,
+   .is_dpi_clk_req = false,
+};
+
+static const struct mtk_dpi_conf mt8365_conf = {
+   .cal_factor = mt8183_calculate_factor,
+   .channel_swap_shift = CH_SWAP,
+   .csc_enable_bit = CSC_ENABLE,
+   .dimension_mask = HPW_MASK,
+   .hvsize_mask = HSIZE_MASK,
+   .is_ck_de_pol = true,
+   .is_dpi_clk_req = true,
+   .max_clock_khz = 15,
+   .num_output_fmts = ARRAY_SIZE(mt8183_output_fmts),
+   .output_fmts = mt8183_output_fmts,
+   .pixels_per_iter = 1,
+   .reg_h_fre_con = 0xe0,
+   .support_direct_pin = true,
+   .swap_input_support = true,
+   .yuv422_en_bit = YUV422_EN,
 };
 
 static int mtk_dpi_probe(struct platform_device *pdev)
@@ -1056,6 +1092,17 @@ static int mtk_dpi_probe(struct platform_device *pdev)
return dev_err_probe(dev, PTR_ERR(dpi->tvd_clk),