| Message ID | 20181116125449.23581-6-matthias.bgg@kernel.org (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | arm/arm64: mediatek: Fix mmsys device probing | expand |
Hi, Matthias: On Fri, 2018-11-16 at 13:54 +0100, matthias.bgg@kernel.org wrote: > From: Matthias Brugger <mbrugger@suse.com> > > It can happen that the mmsys clock drivers aren't probed before the > platform driver gets invoked. The platform driver used to print a warning > that the driver failed to get the clocks. Omit this error on > the defered probe path. This patch looks good to me, but you have not modified the sub driver in HDMI path. We could let HDMI path print the warning and someone send another patch later, or you modify for HDMI path in this patch. > > Signed-off-by: Matthias Brugger <mbrugger@suse.com> > --- > drivers/gpu/drm/mediatek/mtk_disp_color.c | 4 +++- > drivers/gpu/drm/mediatek/mtk_disp_ovl.c | 4 +++- > drivers/gpu/drm/mediatek/mtk_disp_rdma.c | 4 +++- > drivers/gpu/drm/mediatek/mtk_drm_ddp.c | 3 ++- > drivers/gpu/drm/mediatek/mtk_dsi.c | 6 ++++-- > 5 files changed, 15 insertions(+), 6 deletions(-) > > diff --git a/drivers/gpu/drm/mediatek/mtk_disp_color.c b/drivers/gpu/drm/mediatek/mtk_disp_color.c > index f609b62b8be6..1ea3178d4c18 100644 > --- a/drivers/gpu/drm/mediatek/mtk_disp_color.c > +++ b/drivers/gpu/drm/mediatek/mtk_disp_color.c > @@ -126,7 +126,9 @@ static int mtk_disp_color_probe(struct platform_device *pdev) > ret = mtk_ddp_comp_init(dev, dev->of_node, &priv->ddp_comp, comp_id, > &mtk_disp_color_funcs); > if (ret) { > - dev_err(dev, "Failed to initialize component: %d\n", ret); > + if (ret != -EPROBE_DEFER) > + dev_err(dev, "Failed to initialize component: %d\n", > + ret); I would like one more blank line here. > return ret; > } > > diff --git a/drivers/gpu/drm/mediatek/mtk_disp_ovl.c b/drivers/gpu/drm/mediatek/mtk_disp_ovl.c > index 28d191192945..5ebbcaa4e70e 100644 > --- a/drivers/gpu/drm/mediatek/mtk_disp_ovl.c > +++ b/drivers/gpu/drm/mediatek/mtk_disp_ovl.c > @@ -293,7 +293,9 @@ static int mtk_disp_ovl_probe(struct platform_device *pdev) > ret = mtk_ddp_comp_init(dev, dev->of_node, &priv->ddp_comp, comp_id, > &mtk_disp_ovl_funcs); > if (ret) { > - dev_err(dev, "Failed to initialize component: %d\n", ret); > + if (ret != -EPROBE_DEFER) > + dev_err(dev, "Failed to initialize component: %d\n", > + ret); I would like to align to the right of '('. Regards, CK > return ret; > } > > diff --git a/drivers/gpu/drm/mediatek/mtk_disp_rdma.c b/drivers/gpu/drm/mediatek/mtk_disp_rdma.c > index b0a5cffe345a..59a08ed5fea5 100644 > --- a/drivers/gpu/drm/mediatek/mtk_disp_rdma.c > +++ b/drivers/gpu/drm/mediatek/mtk_disp_rdma.c > @@ -295,7 +295,9 @@ static int mtk_disp_rdma_probe(struct platform_device *pdev) > ret = mtk_ddp_comp_init(dev, dev->of_node, &priv->ddp_comp, comp_id, > &mtk_disp_rdma_funcs); > if (ret) { > - dev_err(dev, "Failed to initialize component: %d\n", ret); > + if (ret != -EPROBE_DEFER) > + dev_err(dev, "Failed to initialize component: %d\n", > + ret); > return ret; > } > > diff --git a/drivers/gpu/drm/mediatek/mtk_drm_ddp.c b/drivers/gpu/drm/mediatek/mtk_drm_ddp.c > index b06cd9d4b525..b76a2d071a97 100644 > --- a/drivers/gpu/drm/mediatek/mtk_drm_ddp.c > +++ b/drivers/gpu/drm/mediatek/mtk_drm_ddp.c > @@ -566,7 +566,8 @@ static int mtk_ddp_probe(struct platform_device *pdev) > > ddp->clk = devm_clk_get(dev, NULL); > if (IS_ERR(ddp->clk)) { > - dev_err(dev, "Failed to get clock\n"); > + if (PTR_ERR(ddp->clk) != -EPROBE_DEFER) > + dev_err(dev, "Failed to get clock\n"); > return PTR_ERR(ddp->clk); > } > > diff --git a/drivers/gpu/drm/mediatek/mtk_dsi.c b/drivers/gpu/drm/mediatek/mtk_dsi.c > index 90109a0d6fff..cc6de75636c3 100644 > --- a/drivers/gpu/drm/mediatek/mtk_dsi.c > +++ b/drivers/gpu/drm/mediatek/mtk_dsi.c > @@ -1103,14 +1103,16 @@ static int mtk_dsi_probe(struct platform_device *pdev) > dsi->engine_clk = devm_clk_get(dev, "engine"); > if (IS_ERR(dsi->engine_clk)) { > ret = PTR_ERR(dsi->engine_clk); > - dev_err(dev, "Failed to get engine clock: %d\n", ret); > + if (ret != -EPROBE_DEFER) > + dev_err(dev, "Failed to get engine clock: %d\n", ret); > return ret; > } > > dsi->digital_clk = devm_clk_get(dev, "digital"); > if (IS_ERR(dsi->digital_clk)) { > ret = PTR_ERR(dsi->digital_clk); > - dev_err(dev, "Failed to get digital clock: %d\n", ret); > + if (ret != -EPROBE_DEFER) > + dev_err(dev, "Failed to get digital clock: %d\n", ret); > return ret; > } >
On 19/11/2018 06:38, CK Hu wrote: > Hi, Matthias: > > On Fri, 2018-11-16 at 13:54 +0100, matthias.bgg@kernel.org wrote: >> From: Matthias Brugger <mbrugger@suse.com> >> >> It can happen that the mmsys clock drivers aren't probed before the >> platform driver gets invoked. The platform driver used to print a warning >> that the driver failed to get the clocks. Omit this error on >> the defered probe path. > > This patch looks good to me, but you have not modified the sub driver in > HDMI path. We could let HDMI path print the warning and someone send > another patch later, or you modify for HDMI path in this patch. Sure, I'll add this in v6. After inspecting the code, I think we will need to also check for not initialized clocks in mtk_mdp_comp_init, as the driver for now does not even check if the clocks are present. What do you think? I'll address the coding style issue you metioned below as well. Regards, Matthias >> >> Signed-off-by: Matthias Brugger <mbrugger@suse.com> >> --- >> drivers/gpu/drm/mediatek/mtk_disp_color.c | 4 +++- >> drivers/gpu/drm/mediatek/mtk_disp_ovl.c | 4 +++- >> drivers/gpu/drm/mediatek/mtk_disp_rdma.c | 4 +++- >> drivers/gpu/drm/mediatek/mtk_drm_ddp.c | 3 ++- >> drivers/gpu/drm/mediatek/mtk_dsi.c | 6 ++++-- >> 5 files changed, 15 insertions(+), 6 deletions(-) >> >> diff --git a/drivers/gpu/drm/mediatek/mtk_disp_color.c b/drivers/gpu/drm/mediatek/mtk_disp_color.c >> index f609b62b8be6..1ea3178d4c18 100644 >> --- a/drivers/gpu/drm/mediatek/mtk_disp_color.c >> +++ b/drivers/gpu/drm/mediatek/mtk_disp_color.c >> @@ -126,7 +126,9 @@ static int mtk_disp_color_probe(struct platform_device *pdev) >> ret = mtk_ddp_comp_init(dev, dev->of_node, &priv->ddp_comp, comp_id, >> &mtk_disp_color_funcs); >> if (ret) { >> - dev_err(dev, "Failed to initialize component: %d\n", ret); >> + if (ret != -EPROBE_DEFER) >> + dev_err(dev, "Failed to initialize component: %d\n", >> + ret); > > I would like one more blank line here. > >> return ret; >> } >> >> diff --git a/drivers/gpu/drm/mediatek/mtk_disp_ovl.c b/drivers/gpu/drm/mediatek/mtk_disp_ovl.c >> index 28d191192945..5ebbcaa4e70e 100644 >> --- a/drivers/gpu/drm/mediatek/mtk_disp_ovl.c >> +++ b/drivers/gpu/drm/mediatek/mtk_disp_ovl.c >> @@ -293,7 +293,9 @@ static int mtk_disp_ovl_probe(struct platform_device *pdev) >> ret = mtk_ddp_comp_init(dev, dev->of_node, &priv->ddp_comp, comp_id, >> &mtk_disp_ovl_funcs); >> if (ret) { >> - dev_err(dev, "Failed to initialize component: %d\n", ret); >> + if (ret != -EPROBE_DEFER) >> + dev_err(dev, "Failed to initialize component: %d\n", >> + ret); > > I would like to align to the right of '('. > > Regards, > CK > >> return ret; >> } >> >> diff --git a/drivers/gpu/drm/mediatek/mtk_disp_rdma.c b/drivers/gpu/drm/mediatek/mtk_disp_rdma.c >> index b0a5cffe345a..59a08ed5fea5 100644 >> --- a/drivers/gpu/drm/mediatek/mtk_disp_rdma.c >> +++ b/drivers/gpu/drm/mediatek/mtk_disp_rdma.c >> @@ -295,7 +295,9 @@ static int mtk_disp_rdma_probe(struct platform_device *pdev) >> ret = mtk_ddp_comp_init(dev, dev->of_node, &priv->ddp_comp, comp_id, >> &mtk_disp_rdma_funcs); >> if (ret) { >> - dev_err(dev, "Failed to initialize component: %d\n", ret); >> + if (ret != -EPROBE_DEFER) >> + dev_err(dev, "Failed to initialize component: %d\n", >> + ret); >> return ret; >> } >> >> diff --git a/drivers/gpu/drm/mediatek/mtk_drm_ddp.c b/drivers/gpu/drm/mediatek/mtk_drm_ddp.c >> index b06cd9d4b525..b76a2d071a97 100644 >> --- a/drivers/gpu/drm/mediatek/mtk_drm_ddp.c >> +++ b/drivers/gpu/drm/mediatek/mtk_drm_ddp.c >> @@ -566,7 +566,8 @@ static int mtk_ddp_probe(struct platform_device *pdev) >> >> ddp->clk = devm_clk_get(dev, NULL); >> if (IS_ERR(ddp->clk)) { >> - dev_err(dev, "Failed to get clock\n"); >> + if (PTR_ERR(ddp->clk) != -EPROBE_DEFER) >> + dev_err(dev, "Failed to get clock\n"); >> return PTR_ERR(ddp->clk); >> } >> >> diff --git a/drivers/gpu/drm/mediatek/mtk_dsi.c b/drivers/gpu/drm/mediatek/mtk_dsi.c >> index 90109a0d6fff..cc6de75636c3 100644 >> --- a/drivers/gpu/drm/mediatek/mtk_dsi.c >> +++ b/drivers/gpu/drm/mediatek/mtk_dsi.c >> @@ -1103,14 +1103,16 @@ static int mtk_dsi_probe(struct platform_device *pdev) >> dsi->engine_clk = devm_clk_get(dev, "engine"); >> if (IS_ERR(dsi->engine_clk)) { >> ret = PTR_ERR(dsi->engine_clk); >> - dev_err(dev, "Failed to get engine clock: %d\n", ret); >> + if (ret != -EPROBE_DEFER) >> + dev_err(dev, "Failed to get engine clock: %d\n", ret); >> return ret; >> } >> >> dsi->digital_clk = devm_clk_get(dev, "digital"); >> if (IS_ERR(dsi->digital_clk)) { >> ret = PTR_ERR(dsi->digital_clk); >> - dev_err(dev, "Failed to get digital clock: %d\n", ret); >> + if (ret != -EPROBE_DEFER) >> + dev_err(dev, "Failed to get digital clock: %d\n", ret); >> return ret; >> } >> > >
Hi, Matthias: On Mon, 2018-11-19 at 10:26 +0100, Matthias Brugger wrote: > > On 19/11/2018 06:38, CK Hu wrote: > > Hi, Matthias: > > > > On Fri, 2018-11-16 at 13:54 +0100, matthias.bgg@kernel.org wrote: > >> From: Matthias Brugger <mbrugger@suse.com> > >> > >> It can happen that the mmsys clock drivers aren't probed before the > >> platform driver gets invoked. The platform driver used to print a warning > >> that the driver failed to get the clocks. Omit this error on > >> the defered probe path. > > > > This patch looks good to me, but you have not modified the sub driver in > > HDMI path. We could let HDMI path print the warning and someone send > > another patch later, or you modify for HDMI path in this patch. > > Sure, I'll add this in v6. After inspecting the code, I think we will need to > also check for not initialized clocks in mtk_mdp_comp_init, as the driver for > now does not even check if the clocks are present. What do you think? Yes, we do really need to consider mdp driver because mmsys clock include mdp clock. You remind me that mmsys control 4 major function: drm routing, drm clock, mdp routing, and mdp clock. Your design let the mmsys device as drm device (control drm routing) and create a sub device as clock device (control drm clock, mdp clock). If one day mdp device (may need control drm routing) need to control the register of mdp routing, would mdp device be a sub device? Or we need not to consider this because it need not to access mmsys register now? Regards, CK > > I'll address the coding style issue you metioned below as well. > > Regards, > Matthias > > >> > >> Signed-off-by: Matthias Brugger <mbrugger@suse.com> > >> --- > >> drivers/gpu/drm/mediatek/mtk_disp_color.c | 4 +++- > >> drivers/gpu/drm/mediatek/mtk_disp_ovl.c | 4 +++- > >> drivers/gpu/drm/mediatek/mtk_disp_rdma.c | 4 +++- > >> drivers/gpu/drm/mediatek/mtk_drm_ddp.c | 3 ++- > >> drivers/gpu/drm/mediatek/mtk_dsi.c | 6 ++++-- > >> 5 files changed, 15 insertions(+), 6 deletions(-) > >> > >> diff --git a/drivers/gpu/drm/mediatek/mtk_disp_color.c b/drivers/gpu/drm/mediatek/mtk_disp_color.c > >> index f609b62b8be6..1ea3178d4c18 100644 > >> --- a/drivers/gpu/drm/mediatek/mtk_disp_color.c > >> +++ b/drivers/gpu/drm/mediatek/mtk_disp_color.c > >> @@ -126,7 +126,9 @@ static int mtk_disp_color_probe(struct platform_device *pdev) > >> ret = mtk_ddp_comp_init(dev, dev->of_node, &priv->ddp_comp, comp_id, > >> &mtk_disp_color_funcs); > >> if (ret) { > >> - dev_err(dev, "Failed to initialize component: %d\n", ret); > >> + if (ret != -EPROBE_DEFER) > >> + dev_err(dev, "Failed to initialize component: %d\n", > >> + ret); > > > > I would like one more blank line here. > > > >> return ret; > >> } > >> > >> diff --git a/drivers/gpu/drm/mediatek/mtk_disp_ovl.c b/drivers/gpu/drm/mediatek/mtk_disp_ovl.c > >> index 28d191192945..5ebbcaa4e70e 100644 > >> --- a/drivers/gpu/drm/mediatek/mtk_disp_ovl.c > >> +++ b/drivers/gpu/drm/mediatek/mtk_disp_ovl.c > >> @@ -293,7 +293,9 @@ static int mtk_disp_ovl_probe(struct platform_device *pdev) > >> ret = mtk_ddp_comp_init(dev, dev->of_node, &priv->ddp_comp, comp_id, > >> &mtk_disp_ovl_funcs); > >> if (ret) { > >> - dev_err(dev, "Failed to initialize component: %d\n", ret); > >> + if (ret != -EPROBE_DEFER) > >> + dev_err(dev, "Failed to initialize component: %d\n", > >> + ret); > > > > I would like to align to the right of '('. > > > > Regards, > > CK > > > >> return ret; > >> } > >> > >> diff --git a/drivers/gpu/drm/mediatek/mtk_disp_rdma.c b/drivers/gpu/drm/mediatek/mtk_disp_rdma.c > >> index b0a5cffe345a..59a08ed5fea5 100644 > >> --- a/drivers/gpu/drm/mediatek/mtk_disp_rdma.c > >> +++ b/drivers/gpu/drm/mediatek/mtk_disp_rdma.c > >> @@ -295,7 +295,9 @@ static int mtk_disp_rdma_probe(struct platform_device *pdev) > >> ret = mtk_ddp_comp_init(dev, dev->of_node, &priv->ddp_comp, comp_id, > >> &mtk_disp_rdma_funcs); > >> if (ret) { > >> - dev_err(dev, "Failed to initialize component: %d\n", ret); > >> + if (ret != -EPROBE_DEFER) > >> + dev_err(dev, "Failed to initialize component: %d\n", > >> + ret); > >> return ret; > >> } > >> > >> diff --git a/drivers/gpu/drm/mediatek/mtk_drm_ddp.c b/drivers/gpu/drm/mediatek/mtk_drm_ddp.c > >> index b06cd9d4b525..b76a2d071a97 100644 > >> --- a/drivers/gpu/drm/mediatek/mtk_drm_ddp.c > >> +++ b/drivers/gpu/drm/mediatek/mtk_drm_ddp.c > >> @@ -566,7 +566,8 @@ static int mtk_ddp_probe(struct platform_device *pdev) > >> > >> ddp->clk = devm_clk_get(dev, NULL); > >> if (IS_ERR(ddp->clk)) { > >> - dev_err(dev, "Failed to get clock\n"); > >> + if (PTR_ERR(ddp->clk) != -EPROBE_DEFER) > >> + dev_err(dev, "Failed to get clock\n"); > >> return PTR_ERR(ddp->clk); > >> } > >> > >> diff --git a/drivers/gpu/drm/mediatek/mtk_dsi.c b/drivers/gpu/drm/mediatek/mtk_dsi.c > >> index 90109a0d6fff..cc6de75636c3 100644 > >> --- a/drivers/gpu/drm/mediatek/mtk_dsi.c > >> +++ b/drivers/gpu/drm/mediatek/mtk_dsi.c > >> @@ -1103,14 +1103,16 @@ static int mtk_dsi_probe(struct platform_device *pdev) > >> dsi->engine_clk = devm_clk_get(dev, "engine"); > >> if (IS_ERR(dsi->engine_clk)) { > >> ret = PTR_ERR(dsi->engine_clk); > >> - dev_err(dev, "Failed to get engine clock: %d\n", ret); > >> + if (ret != -EPROBE_DEFER) > >> + dev_err(dev, "Failed to get engine clock: %d\n", ret); > >> return ret; > >> } > >> > >> dsi->digital_clk = devm_clk_get(dev, "digital"); > >> if (IS_ERR(dsi->digital_clk)) { > >> ret = PTR_ERR(dsi->digital_clk); > >> - dev_err(dev, "Failed to get digital clock: %d\n", ret); > >> + if (ret != -EPROBE_DEFER) > >> + dev_err(dev, "Failed to get digital clock: %d\n", ret); > >> return ret; > >> } > >> > > > >
Hi, On Tue, 2018-11-20 at 12:05 +0800, CK Hu wrote: > Hi, Matthias: > > On Mon, 2018-11-19 at 10:26 +0100, Matthias Brugger wrote: > > > > On 19/11/2018 06:38, CK Hu wrote: > > > Hi, Matthias: > > > > > > On Fri, 2018-11-16 at 13:54 +0100, matthias.bgg@kernel.org wrote: > > >> From: Matthias Brugger <mbrugger@suse.com> > > >> > > >> It can happen that the mmsys clock drivers aren't probed before the > > >> platform driver gets invoked. The platform driver used to print a warning > > >> that the driver failed to get the clocks. Omit this error on > > >> the defered probe path. > > > > > > This patch looks good to me, but you have not modified the sub driver in > > > HDMI path. We could let HDMI path print the warning and someone send > > > another patch later, or you modify for HDMI path in this patch. > > > > Sure, I'll add this in v6. After inspecting the code, I think we will need to > > also check for not initialized clocks in mtk_mdp_comp_init, as the driver for > > now does not even check if the clocks are present. What do you think? > > Yes, we do really need to consider mdp driver because mmsys clock > include mdp clock. You remind me that mmsys control 4 major function: > drm routing, drm clock, mdp routing, and mdp clock. Your design let the > mmsys device as drm device (control drm routing) and create a sub device > as clock device (control drm clock, mdp clock). If one day mdp device > (may need control drm routing) need to control the register of mdp Sorry for typo: 'mdp device (may need control mdp routing)' Regards, CK > routing, would mdp device be a sub device? Or we need not to consider > this because it need not to access mmsys register now? > > Regards, > CK > > > > > I'll address the coding style issue you metioned below as well. > > > > Regards, > > Matthias > > > > >> > > >> Signed-off-by: Matthias Brugger <mbrugger@suse.com> > > >> --- > > >> drivers/gpu/drm/mediatek/mtk_disp_color.c | 4 +++- > > >> drivers/gpu/drm/mediatek/mtk_disp_ovl.c | 4 +++- > > >> drivers/gpu/drm/mediatek/mtk_disp_rdma.c | 4 +++- > > >> drivers/gpu/drm/mediatek/mtk_drm_ddp.c | 3 ++- > > >> drivers/gpu/drm/mediatek/mtk_dsi.c | 6 ++++-- > > >> 5 files changed, 15 insertions(+), 6 deletions(-) > > >> > > >> diff --git a/drivers/gpu/drm/mediatek/mtk_disp_color.c b/drivers/gpu/drm/mediatek/mtk_disp_color.c > > >> index f609b62b8be6..1ea3178d4c18 100644 > > >> --- a/drivers/gpu/drm/mediatek/mtk_disp_color.c > > >> +++ b/drivers/gpu/drm/mediatek/mtk_disp_color.c > > >> @@ -126,7 +126,9 @@ static int mtk_disp_color_probe(struct platform_device *pdev) > > >> ret = mtk_ddp_comp_init(dev, dev->of_node, &priv->ddp_comp, comp_id, > > >> &mtk_disp_color_funcs); > > >> if (ret) { > > >> - dev_err(dev, "Failed to initialize component: %d\n", ret); > > >> + if (ret != -EPROBE_DEFER) > > >> + dev_err(dev, "Failed to initialize component: %d\n", > > >> + ret); > > > > > > I would like one more blank line here. > > > > > >> return ret; > > >> } > > >> > > >> diff --git a/drivers/gpu/drm/mediatek/mtk_disp_ovl.c b/drivers/gpu/drm/mediatek/mtk_disp_ovl.c > > >> index 28d191192945..5ebbcaa4e70e 100644 > > >> --- a/drivers/gpu/drm/mediatek/mtk_disp_ovl.c > > >> +++ b/drivers/gpu/drm/mediatek/mtk_disp_ovl.c > > >> @@ -293,7 +293,9 @@ static int mtk_disp_ovl_probe(struct platform_device *pdev) > > >> ret = mtk_ddp_comp_init(dev, dev->of_node, &priv->ddp_comp, comp_id, > > >> &mtk_disp_ovl_funcs); > > >> if (ret) { > > >> - dev_err(dev, "Failed to initialize component: %d\n", ret); > > >> + if (ret != -EPROBE_DEFER) > > >> + dev_err(dev, "Failed to initialize component: %d\n", > > >> + ret); > > > > > > I would like to align to the right of '('. > > > > > > Regards, > > > CK > > > > > >> return ret; > > >> } > > >> > > >> diff --git a/drivers/gpu/drm/mediatek/mtk_disp_rdma.c b/drivers/gpu/drm/mediatek/mtk_disp_rdma.c > > >> index b0a5cffe345a..59a08ed5fea5 100644 > > >> --- a/drivers/gpu/drm/mediatek/mtk_disp_rdma.c > > >> +++ b/drivers/gpu/drm/mediatek/mtk_disp_rdma.c > > >> @@ -295,7 +295,9 @@ static int mtk_disp_rdma_probe(struct platform_device *pdev) > > >> ret = mtk_ddp_comp_init(dev, dev->of_node, &priv->ddp_comp, comp_id, > > >> &mtk_disp_rdma_funcs); > > >> if (ret) { > > >> - dev_err(dev, "Failed to initialize component: %d\n", ret); > > >> + if (ret != -EPROBE_DEFER) > > >> + dev_err(dev, "Failed to initialize component: %d\n", > > >> + ret); > > >> return ret; > > >> } > > >> > > >> diff --git a/drivers/gpu/drm/mediatek/mtk_drm_ddp.c b/drivers/gpu/drm/mediatek/mtk_drm_ddp.c > > >> index b06cd9d4b525..b76a2d071a97 100644 > > >> --- a/drivers/gpu/drm/mediatek/mtk_drm_ddp.c > > >> +++ b/drivers/gpu/drm/mediatek/mtk_drm_ddp.c > > >> @@ -566,7 +566,8 @@ static int mtk_ddp_probe(struct platform_device *pdev) > > >> > > >> ddp->clk = devm_clk_get(dev, NULL); > > >> if (IS_ERR(ddp->clk)) { > > >> - dev_err(dev, "Failed to get clock\n"); > > >> + if (PTR_ERR(ddp->clk) != -EPROBE_DEFER) > > >> + dev_err(dev, "Failed to get clock\n"); > > >> return PTR_ERR(ddp->clk); > > >> } > > >> > > >> diff --git a/drivers/gpu/drm/mediatek/mtk_dsi.c b/drivers/gpu/drm/mediatek/mtk_dsi.c > > >> index 90109a0d6fff..cc6de75636c3 100644 > > >> --- a/drivers/gpu/drm/mediatek/mtk_dsi.c > > >> +++ b/drivers/gpu/drm/mediatek/mtk_dsi.c > > >> @@ -1103,14 +1103,16 @@ static int mtk_dsi_probe(struct platform_device *pdev) > > >> dsi->engine_clk = devm_clk_get(dev, "engine"); > > >> if (IS_ERR(dsi->engine_clk)) { > > >> ret = PTR_ERR(dsi->engine_clk); > > >> - dev_err(dev, "Failed to get engine clock: %d\n", ret); > > >> + if (ret != -EPROBE_DEFER) > > >> + dev_err(dev, "Failed to get engine clock: %d\n", ret); > > >> return ret; > > >> } > > >> > > >> dsi->digital_clk = devm_clk_get(dev, "digital"); > > >> if (IS_ERR(dsi->digital_clk)) { > > >> ret = PTR_ERR(dsi->digital_clk); > > >> - dev_err(dev, "Failed to get digital clock: %d\n", ret); > > >> + if (ret != -EPROBE_DEFER) > > >> + dev_err(dev, "Failed to get digital clock: %d\n", ret); > > >> return ret; > > >> } > > >> > > > > > > >
Hi, i got v5-patches working on bpi-r2 (mt7623) with a Patch from Ryder.lee and 2 from Bibby Hsieh on 4.19-rc1 https://github.com/frank-w/BPI-R2-4.14/commits/4.19-hdmiv5 but after i tried to include them on 4.19.2, i got a strange behaviour (stretched and pink font instead of white/grey) see here for detailed info: http://forum.banana-pi.org/t/kernel-4-19-rc1-for-testers/6618/75 there are some patches between rc1 and final which i tried to revert, but no luck till now (cannot revert, no effect on the issue or no output). Had informed Bibby Hsieh... is there any idea, why this happen? regards Frank
On 20/11/2018 09:26, Frank Wunderlich wrote: > Hi, > > i got v5-patches working on bpi-r2 (mt7623) with a Patch from Ryder.lee and 2 from Bibby Hsieh on 4.19-rc1 > > https://github.com/frank-w/BPI-R2-4.14/commits/4.19-hdmiv5 > I don't see the patches applied to this tree. Apart from that this tree has a lot of other patches applied. It differs greatly from mainline, so nothing we should discuss on this mailinglist. Regards, Matthias > but after i tried to include them on 4.19.2, i got a strange behaviour (stretched and pink font instead of white/grey) > > see here for detailed info: > > http://forum.banana-pi.org/t/kernel-4-19-rc1-for-testers/6618/75 > > there are some patches between rc1 and final which i tried to revert, but no luck till now (cannot revert, no effect on the issue or no output). Had informed Bibby Hsieh... > > is there any idea, why this happen? > > regards Frank >
On 20/11/2018 05:05, CK Hu wrote: > Hi, Matthias: > > On Mon, 2018-11-19 at 10:26 +0100, Matthias Brugger wrote: >> >> On 19/11/2018 06:38, CK Hu wrote: >>> Hi, Matthias: >>> >>> On Fri, 2018-11-16 at 13:54 +0100, matthias.bgg@kernel.org wrote: >>>> From: Matthias Brugger <mbrugger@suse.com> >>>> >>>> It can happen that the mmsys clock drivers aren't probed before the >>>> platform driver gets invoked. The platform driver used to print a warning >>>> that the driver failed to get the clocks. Omit this error on >>>> the defered probe path. >>> >>> This patch looks good to me, but you have not modified the sub driver in >>> HDMI path. We could let HDMI path print the warning and someone send >>> another patch later, or you modify for HDMI path in this patch. >> >> Sure, I'll add this in v6. After inspecting the code, I think we will need to >> also check for not initialized clocks in mtk_mdp_comp_init, as the driver for >> now does not even check if the clocks are present. What do you think? > > Yes, we do really need to consider mdp driver because mmsys clock > include mdp clock. You remind me that mmsys control 4 major function: > drm routing, drm clock, mdp routing, and mdp clock. Your design let the > mmsys device as drm device (control drm routing) and create a sub device > as clock device (control drm clock, mdp clock). If one day mdp device > (may need control drm routing) need to control the register of mdp > routing, would mdp device be a sub device? Or we need not to consider > this because it need not to access mmsys register now? > I think we should for now concentrate to fix the clock probing issue. If in the future we will need to access drm routing from the mdp device, we can have a look into this. Sounds good? Regards, Matthias
Hi, Matthias: On Tue, 2018-11-20 at 11:19 +0100, Matthias Brugger wrote: > > On 20/11/2018 05:05, CK Hu wrote: > > Hi, Matthias: > > > > On Mon, 2018-11-19 at 10:26 +0100, Matthias Brugger wrote: > >> > >> On 19/11/2018 06:38, CK Hu wrote: > >>> Hi, Matthias: > >>> > >>> On Fri, 2018-11-16 at 13:54 +0100, matthias.bgg@kernel.org wrote: > >>>> From: Matthias Brugger <mbrugger@suse.com> > >>>> > >>>> It can happen that the mmsys clock drivers aren't probed before the > >>>> platform driver gets invoked. The platform driver used to print a warning > >>>> that the driver failed to get the clocks. Omit this error on > >>>> the defered probe path. > >>> > >>> This patch looks good to me, but you have not modified the sub driver in > >>> HDMI path. We could let HDMI path print the warning and someone send > >>> another patch later, or you modify for HDMI path in this patch. > >> > >> Sure, I'll add this in v6. After inspecting the code, I think we will need to > >> also check for not initialized clocks in mtk_mdp_comp_init, as the driver for > >> now does not even check if the clocks are present. What do you think? > > > > Yes, we do really need to consider mdp driver because mmsys clock > > include mdp clock. You remind me that mmsys control 4 major function: > > drm routing, drm clock, mdp routing, and mdp clock. Your design let the > > mmsys device as drm device (control drm routing) and create a sub device > > as clock device (control drm clock, mdp clock). If one day mdp device > > (may need control drm routing) need to control the register of mdp > > routing, would mdp device be a sub device? Or we need not to consider > > this because it need not to access mmsys register now? > > > > I think we should for now concentrate to fix the clock probing issue. If in the > future we will need to access drm routing from the mdp device, we can have a > look into this. > > Sounds good? Sounds good to me. Regards, CK > Regards, > Matthias
right this branch based on rc1 with some other commits, but v5-patches are applied on Oct 3rd (listed in Oct 5th) frank@frank-N56VZ:/media/data_ext/bpi-r2-kernel/github [11:29:10]$ git checkout 4.19-hdmiv5 Already on '4.19-hdmiv5' Your branch is up to date with 'origin/4.19-hdmiv5'. frank@frank-N56VZ:/media/data_ext/bpi-r2-kernel/github [11:29:13]$ make kernelversion 4.19.0-rc1 frank@frank-N56VZ:/media/data_ext/bpi-r2-kernel/github [11:29:19]$ git log --oneline ... fc14b8c515de drm/mediatek: add a error return value when clock driver has been prepared 0dc856306aaf drm/mediatek: implement connection from BLS to DPI0 3d0a6749bfe3 drm/mediatek: add hdmi driver for MT2701 and MT7623 b03e1b353b28 drm/mediatek: add support for SPDIF audio in HDMI c5564966272e drm/mediatek: separate hdmi phy to different file dee3954828db drm/mediatek: add dpi driver for mt2701 and mt7623 a838ae8b415c drm/mediatek: convert dpi driver to use drm_of_find_panel_or_bridge f7f9f7c080ae drm/mediatek: add clock factor for different IC be1a5447330f drm/mediatek: adjust EDGE to match clock and data fec4504ea318 drm/mediatek: move hardware register to node data db992e429b9c drm/mediatek: add refcount for DPI power on/off ... compared with 4.19.0-release there is merge https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/commit/drivers/gpu/drm/mediatek?id=53b3b6bbfde6aae8d1ededc86ad4e0e1e00eb5f8 which seems to break v5-patches > Gesendet: Dienstag, 20. November 2018 um 11:14 Uhr > Von: "Matthias Brugger" <matthias.bgg@gmail.com> > An: "Frank Wunderlich" <frank-w@public-files.de>, "CK Hu" <ck.hu@mediatek.com> > Cc: mark.rutland@arm.com, devicetree@vger.kernel.org, sean.wang@kernel.org, "Matthias Brugger" <mbrugger@suse.com>, airlied@linux.ie, mturquette@baylibre.com, sean.wang@mediatek.com, sboyd@codeaurora.org, linux-kernel@vger.kernel.org, dri-devel@lists.freedesktop.org, wens@csie.org, robh+dt@kernel.org, rdunlap@infradead.org, laurent.pinchart@ideasonboard.com, p.zabel@pengutronix.de, matthias.bgg@kernel.org, ulrich.hecht+renesas@gmail.com, linux-mediatek@lists.infradead.org, linux-clk@vger.kernel.org, linux-arm-kernel@lists.infradead.org > Betreff: Re: Aw: Re: [PATCH v5 05/12] drm: mediatek > > > > On 20/11/2018 09:26, Frank Wunderlich wrote: > > Hi, > > > > i got v5-patches working on bpi-r2 (mt7623) with a Patch from Ryder.lee and 2 from Bibby Hsieh on 4.19-rc1 > > > > https://github.com/frank-w/BPI-R2-4.14/commits/4.19-hdmiv5 > > > > I don't see the patches applied to this tree. Apart from that this tree has a > lot of other patches applied. It differs greatly from mainline, so nothing we > should discuss on this mailinglist. > > Regards, > Matthias > > > but after i tried to include them on 4.19.2, i got a strange behaviour (stretched and pink font instead of white/grey) > > > > see here for detailed info: > > > > http://forum.banana-pi.org/t/kernel-4-19-rc1-for-testers/6618/75 > > > > there are some patches between rc1 and final which i tried to revert, but no luck till now (cannot revert, no effect on the issue or no output). Had informed Bibby Hsieh... > > > > is there any idea, why this happen? > > > > regards Frank > > >
On 20/11/2018 11:34, Frank Wunderlich wrote: > right this branch based on rc1 with some other commits, but v5-patches are applied on Oct 3rd (listed in Oct 5th) > > frank@frank-N56VZ:/media/data_ext/bpi-r2-kernel/github > [11:29:10]$ git checkout 4.19-hdmiv5 > Already on '4.19-hdmiv5' > Your branch is up to date with 'origin/4.19-hdmiv5'. > frank@frank-N56VZ:/media/data_ext/bpi-r2-kernel/github > [11:29:13]$ make kernelversion > 4.19.0-rc1 > frank@frank-N56VZ:/media/data_ext/bpi-r2-kernel/github > [11:29:19]$ git log --oneline > ... > fc14b8c515de drm/mediatek: add a error return value when clock driver has been prepared > 0dc856306aaf drm/mediatek: implement connection from BLS to DPI0 > 3d0a6749bfe3 drm/mediatek: add hdmi driver for MT2701 and MT7623 > b03e1b353b28 drm/mediatek: add support for SPDIF audio in HDMI > c5564966272e drm/mediatek: separate hdmi phy to different file > dee3954828db drm/mediatek: add dpi driver for mt2701 and mt7623 > a838ae8b415c drm/mediatek: convert dpi driver to use drm_of_find_panel_or_bridge > f7f9f7c080ae drm/mediatek: add clock factor for different IC > be1a5447330f drm/mediatek: adjust EDGE to match clock and data > fec4504ea318 drm/mediatek: move hardware register to node data > db992e429b9c drm/mediatek: add refcount for DPI power on/off > ... This patches are not part of this patch series. > > compared with 4.19.0-release there is merge https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/commit/drivers/gpu/drm/mediatek?id=53b3b6bbfde6aae8d1ededc86ad4e0e1e00eb5f8 which seems to break v5-patches > >> Gesendet: Dienstag, 20. November 2018 um 11:14 Uhr >> Von: "Matthias Brugger" <matthias.bgg@gmail.com> >> An: "Frank Wunderlich" <frank-w@public-files.de>, "CK Hu" <ck.hu@mediatek.com> >> Cc: mark.rutland@arm.com, devicetree@vger.kernel.org, sean.wang@kernel.org, "Matthias Brugger" <mbrugger@suse.com>, airlied@linux.ie, mturquette@baylibre.com, sean.wang@mediatek.com, sboyd@codeaurora.org, linux-kernel@vger.kernel.org, dri-devel@lists.freedesktop.org, wens@csie.org, robh+dt@kernel.org, rdunlap@infradead.org, laurent.pinchart@ideasonboard.com, p.zabel@pengutronix.de, matthias.bgg@kernel.org, ulrich.hecht+renesas@gmail.com, linux-mediatek@lists.infradead.org, linux-clk@vger.kernel.org, linux-arm-kernel@lists.infradead.org >> Betreff: Re: Aw: Re: [PATCH v5 05/12] drm: mediatek >> >> >> >> On 20/11/2018 09:26, Frank Wunderlich wrote: >>> Hi, >>> >>> i got v5-patches working on bpi-r2 (mt7623) with a Patch from Ryder.lee and 2 from Bibby Hsieh on 4.19-rc1 >>> >>> https://github.com/frank-w/BPI-R2-4.14/commits/4.19-hdmiv5 >>> >> >> I don't see the patches applied to this tree. Apart from that this tree has a >> lot of other patches applied. It differs greatly from mainline, so nothing we >> should discuss on this mailinglist. >> >> Regards, >> Matthias >> >>> but after i tried to include them on 4.19.2, i got a strange behaviour (stretched and pink font instead of white/grey) >>> >>> see here for detailed info: >>> >>> http://forum.banana-pi.org/t/kernel-4-19-rc1-for-testers/6618/75 >>> >>> there are some patches between rc1 and final which i tried to revert, but no luck till now (cannot revert, no effect on the issue or no output). Had informed Bibby Hsieh... >>> >>> is there any idea, why this happen? >>> >>> regards Frank >>> >>
diff --git a/drivers/gpu/drm/mediatek/mtk_disp_color.c b/drivers/gpu/drm/mediatek/mtk_disp_color.c index f609b62b8be6..1ea3178d4c18 100644 --- a/drivers/gpu/drm/mediatek/mtk_disp_color.c +++ b/drivers/gpu/drm/mediatek/mtk_disp_color.c @@ -126,7 +126,9 @@ static int mtk_disp_color_probe(struct platform_device *pdev) ret = mtk_ddp_comp_init(dev, dev->of_node, &priv->ddp_comp, comp_id, &mtk_disp_color_funcs); if (ret) { - dev_err(dev, "Failed to initialize component: %d\n", ret); + if (ret != -EPROBE_DEFER) + dev_err(dev, "Failed to initialize component: %d\n", + ret); return ret; } diff --git a/drivers/gpu/drm/mediatek/mtk_disp_ovl.c b/drivers/gpu/drm/mediatek/mtk_disp_ovl.c index 28d191192945..5ebbcaa4e70e 100644 --- a/drivers/gpu/drm/mediatek/mtk_disp_ovl.c +++ b/drivers/gpu/drm/mediatek/mtk_disp_ovl.c @@ -293,7 +293,9 @@ static int mtk_disp_ovl_probe(struct platform_device *pdev) ret = mtk_ddp_comp_init(dev, dev->of_node, &priv->ddp_comp, comp_id, &mtk_disp_ovl_funcs); if (ret) { - dev_err(dev, "Failed to initialize component: %d\n", ret); + if (ret != -EPROBE_DEFER) + dev_err(dev, "Failed to initialize component: %d\n", + ret); return ret; } diff --git a/drivers/gpu/drm/mediatek/mtk_disp_rdma.c b/drivers/gpu/drm/mediatek/mtk_disp_rdma.c index b0a5cffe345a..59a08ed5fea5 100644 --- a/drivers/gpu/drm/mediatek/mtk_disp_rdma.c +++ b/drivers/gpu/drm/mediatek/mtk_disp_rdma.c @@ -295,7 +295,9 @@ static int mtk_disp_rdma_probe(struct platform_device *pdev) ret = mtk_ddp_comp_init(dev, dev->of_node, &priv->ddp_comp, comp_id, &mtk_disp_rdma_funcs); if (ret) { - dev_err(dev, "Failed to initialize component: %d\n", ret); + if (ret != -EPROBE_DEFER) + dev_err(dev, "Failed to initialize component: %d\n", + ret); return ret; } diff --git a/drivers/gpu/drm/mediatek/mtk_drm_ddp.c b/drivers/gpu/drm/mediatek/mtk_drm_ddp.c index b06cd9d4b525..b76a2d071a97 100644 --- a/drivers/gpu/drm/mediatek/mtk_drm_ddp.c +++ b/drivers/gpu/drm/mediatek/mtk_drm_ddp.c @@ -566,7 +566,8 @@ static int mtk_ddp_probe(struct platform_device *pdev) ddp->clk = devm_clk_get(dev, NULL); if (IS_ERR(ddp->clk)) { - dev_err(dev, "Failed to get clock\n"); + if (PTR_ERR(ddp->clk) != -EPROBE_DEFER) + dev_err(dev, "Failed to get clock\n"); return PTR_ERR(ddp->clk); } diff --git a/drivers/gpu/drm/mediatek/mtk_dsi.c b/drivers/gpu/drm/mediatek/mtk_dsi.c index 90109a0d6fff..cc6de75636c3 100644 --- a/drivers/gpu/drm/mediatek/mtk_dsi.c +++ b/drivers/gpu/drm/mediatek/mtk_dsi.c @@ -1103,14 +1103,16 @@ static int mtk_dsi_probe(struct platform_device *pdev) dsi->engine_clk = devm_clk_get(dev, "engine"); if (IS_ERR(dsi->engine_clk)) { ret = PTR_ERR(dsi->engine_clk); - dev_err(dev, "Failed to get engine clock: %d\n", ret); + if (ret != -EPROBE_DEFER) + dev_err(dev, "Failed to get engine clock: %d\n", ret); return ret; } dsi->digital_clk = devm_clk_get(dev, "digital"); if (IS_ERR(dsi->digital_clk)) { ret = PTR_ERR(dsi->digital_clk); - dev_err(dev, "Failed to get digital clock: %d\n", ret); + if (ret != -EPROBE_DEFER) + dev_err(dev, "Failed to get digital clock: %d\n", ret); return ret; }