[RESEND PATCH] drm/rockchip: vop: Correct enabled clocks during setup

[Sjoerd Simons]

When doing the initial setup both the hclk and the aclk need to be
enabled otherwise the board will simply hang. This only occurs when
building the vop driver as a module, when its built-in the initial setup
happens to run before the clock framework shuts of unused clocks
(including the aclk).

While there also switch to doing prepare and enable in one step rather
then separate steps to reduce the amount of code required.

Signed-off-by: Sjoerd Simons 
Acked-by: Mark Yao 
Tested-by: Yakir Yang 
Tested-by: Romain Perier 
---

 drivers/gpu/drm/rockchip/rockchip_drm_vop.c | 36 +++--
 1 file changed, 14 insertions(+), 22 deletions(-)

diff --git a/drivers/gpu/drm/rockchip/rockchip_drm_vop.c 
b/drivers/gpu/drm/rockchip/rockchip_drm_vop.c
index 5d8ae5e..48719df 100644
--- a/drivers/gpu/drm/rockchip/rockchip_drm_vop.c
+++ b/drivers/gpu/drm/rockchip/rockchip_drm_vop.c
@@ -1575,32 +1575,25 @@ static int vop_initial(struct vop *vop)
return PTR_ERR(vop->dclk);
}

-   ret = clk_prepare(vop->hclk);
-   if (ret < 0) {
-   dev_err(vop->dev, "failed to prepare hclk\n");
-   return ret;
-   }
-
ret = clk_prepare(vop->dclk);
if (ret < 0) {
dev_err(vop->dev, "failed to prepare dclk\n");
-   goto err_unprepare_hclk;
+   return ret;
}

-   ret = clk_prepare(vop->aclk);
+   /* Enable both the hclk and aclk to setup the vop */
+   ret = clk_prepare_enable(vop->hclk);
if (ret < 0) {
-   dev_err(vop->dev, "failed to prepare aclk\n");
+   dev_err(vop->dev, "failed to prepare/enable hclk\n");
goto err_unprepare_dclk;
}

-   /*
-* enable hclk, so that we can config vop register.
-*/
-   ret = clk_enable(vop->hclk);
+   ret = clk_prepare_enable(vop->aclk);
if (ret < 0) {
-   dev_err(vop->dev, "failed to prepare aclk\n");
-   goto err_unprepare_aclk;
+   dev_err(vop->dev, "failed to prepare/enable aclk\n");
+   goto err_disable_hclk;
}
+
/*
 * do hclk_reset, reset all vop registers.
 */
@@ -1608,7 +1601,7 @@ static int vop_initial(struct vop *vop)
if (IS_ERR(ahb_rst)) {
dev_err(vop->dev, "failed to get ahb reset\n");
ret = PTR_ERR(ahb_rst);
-   goto err_disable_hclk;
+   goto err_disable_aclk;
}
reset_control_assert(ahb_rst);
usleep_range(10, 20);
@@ -1634,26 +1627,25 @@ static int vop_initial(struct vop *vop)
if (IS_ERR(vop->dclk_rst)) {
dev_err(vop->dev, "failed to get dclk reset\n");
ret = PTR_ERR(vop->dclk_rst);
-   goto err_unprepare_aclk;
+   goto err_disable_aclk;
}
reset_control_assert(vop->dclk_rst);
usleep_range(10, 20);
reset_control_deassert(vop->dclk_rst);

clk_disable(vop->hclk);
+   clk_disable(vop->aclk);

vop->is_enabled = false;

return 0;

+err_disable_aclk:
+   clk_disable_unprepare(vop->aclk);
 err_disable_hclk:
-   clk_disable(vop->hclk);
-err_unprepare_aclk:
-   clk_unprepare(vop->aclk);
+   clk_disable_unprepare(vop->hclk);
 err_unprepare_dclk:
clk_unprepare(vop->dclk);
-err_unprepare_hclk:
-   clk_unprepare(vop->hclk);
return ret;
 }

-- 
2.6.2

[PATCH 1/2] drm/rockchip: Don't build rockchip_drm_vop as modules

[Sjoerd Simons]

On Wed, 2016-01-13 at 16:22 +0800, Mark Yao wrote:
> rockchip_drm_vop's module init had moved to rockchip_vop_reg.c
> so no need to build rockchip_drm_vop.ko

Looks like this didn't make it for the Linux 4.5 merge window, which
unfortunately means in 4.5-rc1 DRM on rockchip is broken :/

> Signed-off-by: Mark Yao 
> ---
>  drivers/gpu/drm/rockchip/Makefile |    5 ++---
>  1 file changed, 2 insertions(+), 3 deletions(-)
> 
> diff --git a/drivers/gpu/drm/rockchip/Makefile
> b/drivers/gpu/drm/rockchip/Makefile
> index a9d380f..9e6e992 100644
> --- a/drivers/gpu/drm/rockchip/Makefile
> +++ b/drivers/gpu/drm/rockchip/Makefile
> @@ -3,9 +3,8 @@
>  # Direct Rendering Infrastructure (DRI) in XFree86 4.1.0 and higher.
>  
>  rockchipdrm-y := rockchip_drm_drv.o rockchip_drm_fb.o
> rockchip_drm_fbdev.o \
> - rockchip_drm_gem.o
> + rockchip_drm_gem.o rockchip_drm_vop.o
>  
>  obj-$(CONFIG_ROCKCHIP_DW_HDMI) += dw_hdmi-rockchip.o
>  
> -obj-$(CONFIG_DRM_ROCKCHIP) += rockchipdrm.o rockchip_drm_vop.o \
> - rockchip_vop_reg.o
> +obj-$(CONFIG_DRM_ROCKCHIP) += rockchipdrm.o rockchip_vop_reg.o

-- 
Sjoerd Simons
Collabora Ltd.

[PATCH 0/3] drm/exynos: Allow module to be autoloaded

[Sjoerd Simons]

The exynos DRM module currently is not automatically loaded when build as a
module. This is due to the simple fact that it doesn't have any
MODULE_DEVICE_TABLE entries whatsoever... Most of these were removed previously
as it wasn't possible at the time to have multiple calls to MODULE_DEVICE_TABLE
in one module, however commit 21bdd17b21b45ea solved that.

The first two patches revert the previous removals of MODULE_DEVICE_TABLE
calls, while the last one adds calls for the remaining OF match tables without a
MODULE_DEVICE_TABLE call.

Sjoerd Simons (3):
  Revert "drm/exynos: fix module build error"
  Revert "drm/exynos: remove MODULE_DEVICE_TABLE definitions"
  drm/exynos: Add MODULE_DEVICE_TABLE entries for various components

 drivers/gpu/drm/exynos/exynos_dp_core.c | 1 +
 drivers/gpu/drm/exynos/exynos_drm_dsi.c | 1 +
 drivers/gpu/drm/exynos/exynos_drm_fimc.c| 1 +
 drivers/gpu/drm/exynos/exynos_drm_fimd.c| 1 +
 drivers/gpu/drm/exynos/exynos_drm_g2d.c | 1 +
 drivers/gpu/drm/exynos/exynos_drm_rotator.c | 1 +
 drivers/gpu/drm/exynos/exynos_hdmi.c| 1 +
 drivers/gpu/drm/exynos/exynos_mixer.c   | 1 +
 8 files changed, 8 insertions(+)

-- 
2.0.1

[PATCH 1/3] Revert "drm/exynos: fix module build error"

[Sjoerd Simons]

This reverts commit de1d3677017a1d58419722b60564cb56bd9462c3, which was
original added to fix build errors when building exynosdrm as a single
module caused by multiple MODULE_DEVICE_TABLE in one module. Which, as
a side-effect broke autoloading of the module.

Since 21bdd17b21b45ea48e06e23918d681afbe0622e9 it is possible to have
multiple calls to MODULE_DEVICE_TABLE, so the patch can be
reverted to restore support for autoloading

Conflicts:
drivers/gpu/drm/exynos/exynos_drm_fimd.c
drivers/gpu/drm/exynos/exynos_drm_g2d.c

Signed-off-by: Sjoerd Simons 
---
 drivers/gpu/drm/exynos/exynos_drm_fimd.c | 1 +
 drivers/gpu/drm/exynos/exynos_drm_g2d.c  | 1 +
 2 files changed, 2 insertions(+)

diff --git a/drivers/gpu/drm/exynos/exynos_drm_fimd.c 
b/drivers/gpu/drm/exynos/exynos_drm_fimd.c
index 33161ad..081eb15 100644
--- a/drivers/gpu/drm/exynos/exynos_drm_fimd.c
+++ b/drivers/gpu/drm/exynos/exynos_drm_fimd.c
@@ -136,6 +136,7 @@ static const struct of_device_id fimd_driver_dt_match[] = {
  .data = &exynos5_fimd_driver_data },
{},
 };
+MODULE_DEVICE_TABLE(of, fimd_driver_dt_match);

 static inline struct fimd_driver_data *drm_fimd_get_driver_data(
struct platform_device *pdev)
diff --git a/drivers/gpu/drm/exynos/exynos_drm_g2d.c 
b/drivers/gpu/drm/exynos/exynos_drm_g2d.c
index 8001587..bb728c8 100644
--- a/drivers/gpu/drm/exynos/exynos_drm_g2d.c
+++ b/drivers/gpu/drm/exynos/exynos_drm_g2d.c
@@ -1546,6 +1546,7 @@ static const struct of_device_id exynos_g2d_match[] = {
{ .compatible = "samsung,exynos5250-g2d" },
{},
 };
+MODULE_DEVICE_TABLE(of, exynos_g2d_match);

 struct platform_driver g2d_driver = {
.probe  = g2d_probe,
-- 
2.0.1

[PATCH 3/3] drm/exynos: Add MODULE_DEVICE_TABLE entries for various components

[Sjoerd Simons]

Add MODULE_DEVICE_TABLE calls for the various OF match tables that
currently don't have one. This allows the module to be
autoloaded based on devicetree information.

Signed-off-by: Sjoerd Simons 
---
 drivers/gpu/drm/exynos/exynos_drm_fimc.c| 1 +
 drivers/gpu/drm/exynos/exynos_drm_rotator.c | 1 +
 drivers/gpu/drm/exynos/exynos_hdmi.c| 1 +
 drivers/gpu/drm/exynos/exynos_mixer.c   | 1 +
 4 files changed, 4 insertions(+)

diff --git a/drivers/gpu/drm/exynos/exynos_drm_fimc.c 
b/drivers/gpu/drm/exynos/exynos_drm_fimc.c
index 831dde9..ec7cc9e 100644
--- a/drivers/gpu/drm/exynos/exynos_drm_fimc.c
+++ b/drivers/gpu/drm/exynos/exynos_drm_fimc.c
@@ -1887,6 +1887,7 @@ static const struct of_device_id fimc_of_match[] = {
{ .compatible = "samsung,exynos4212-fimc" },
{ },
 };
+MODULE_DEVICE_TABLE(of, fimc_of_match);

 struct platform_driver fimc_driver = {
.probe  = fimc_probe,
diff --git a/drivers/gpu/drm/exynos/exynos_drm_rotator.c 
b/drivers/gpu/drm/exynos/exynos_drm_rotator.c
index f01fbb6..55af6b4 100644
--- a/drivers/gpu/drm/exynos/exynos_drm_rotator.c
+++ b/drivers/gpu/drm/exynos/exynos_drm_rotator.c
@@ -691,6 +691,7 @@ static const struct of_device_id exynos_rotator_match[] = {
},
{},
 };
+MODULE_DEVICE_TABLE(of, exynos_rotator_match);

 static int rotator_probe(struct platform_device *pdev)
 {
diff --git a/drivers/gpu/drm/exynos/exynos_hdmi.c 
b/drivers/gpu/drm/exynos/exynos_hdmi.c
index fd8141f..d08e00d 100644
--- a/drivers/gpu/drm/exynos/exynos_hdmi.c
+++ b/drivers/gpu/drm/exynos/exynos_hdmi.c
@@ -2295,6 +2295,7 @@ static struct of_device_id hdmi_match_types[] = {
/* end node */
}
 };
+MODULE_DEVICE_TABLE (of, hdmi_match_types);

 static int hdmi_bind(struct device *dev, struct device *master, void *data)
 {
diff --git a/drivers/gpu/drm/exynos/exynos_mixer.c 
b/drivers/gpu/drm/exynos/exynos_mixer.c
index 9d0c21a..6756d1c 100644
--- a/drivers/gpu/drm/exynos/exynos_mixer.c
+++ b/drivers/gpu/drm/exynos/exynos_mixer.c
@@ -1240,6 +1240,7 @@ static struct of_device_id mixer_match_types[] = {
/* end node */
}
 };
+MODULE_DEVICE_TABLE(of, mixer_match_types);

 static int mixer_bind(struct device *dev, struct device *manager, void *data)
 {
-- 
2.0.1

[PATCH 2/3] Revert "drm/exynos: remove MODULE_DEVICE_TABLE definitions"

[Sjoerd Simons]

This reverts commit d089621896c3530a9bd309f96e9c9124d07f6c3f was
original to prevent multiple MODULE_DEVICE_TABLE in one module.
Which, as a side-effect broke autoloading of the module.

Since 21bdd17b21b45ea48e06e23918d681afbe0622e9 it is possible to have
multiple calls to MODULE_DEVICE_TABLE, so the patch can be
reverted to restore support for autoloading

Signed-off-by: Sjoerd Simons 
---
 drivers/gpu/drm/exynos/exynos_dp_core.c | 1 +
 drivers/gpu/drm/exynos/exynos_drm_dsi.c | 1 +
 2 files changed, 2 insertions(+)

diff --git a/drivers/gpu/drm/exynos/exynos_dp_core.c 
b/drivers/gpu/drm/exynos/exynos_dp_core.c
index 845d766..31c3de9 100644
--- a/drivers/gpu/drm/exynos/exynos_dp_core.c
+++ b/drivers/gpu/drm/exynos/exynos_dp_core.c
@@ -1376,6 +1376,7 @@ static const struct of_device_id exynos_dp_match[] = {
{ .compatible = "samsung,exynos5-dp" },
{},
 };
+MODULE_DEVICE_TABLE(of, exynos_dp_match);

 struct platform_driver dp_driver = {
.probe  = exynos_dp_probe,
diff --git a/drivers/gpu/drm/exynos/exynos_drm_dsi.c 
b/drivers/gpu/drm/exynos/exynos_drm_dsi.c
index 2df3592..46b7bf6 100644
--- a/drivers/gpu/drm/exynos/exynos_drm_dsi.c
+++ b/drivers/gpu/drm/exynos/exynos_drm_dsi.c
@@ -1529,6 +1529,7 @@ static struct of_device_id exynos_dsi_of_match[] = {
{ .compatible = "samsung,exynos4210-mipi-dsi" },
{ }
 };
+MODULE_DEVICE_TABLE(of, exynos_dsi_of_match);

 struct platform_driver dsi_driver = {
.probe = exynos_dsi_probe,
-- 
2.0.1

[PATCH 0/3] drm/exynos: Allow module to be autoloaded

[Sjoerd Simons]

Hey Inki,

On Mon, 2014-07-21 at 12:02 +0900, Inki Dae wrote:
> On 2014? 07? 19? 05:36, Sjoerd Simons wrote:
> > The exynos DRM module currently is not automatically loaded when build as a
> > module. This is due to the simple fact that it doesn't have any
> > MODULE_DEVICE_TABLE entries whatsoever... Most of these were removed 
> > previously
> > as it wasn't possible at the time to have multiple calls to 
> > MODULE_DEVICE_TABLE
> > in one module, however commit 21bdd17b21b45ea solved that.
> > 
> > The first two patches revert the previous removals of MODULE_DEVICE_TABLE
> > calls, while the last one adds calls for the remaining OF match tables 
> > without a
> > MODULE_DEVICE_TABLE call.

> Exynos drm follows single-driver model. So each usb driver of Exynos drm
> wouldn't need its own MODULE_DEVICE_TABLE.

Strictly speaking you're right, for module autoloading to work the
module just needs to have one that matches. So in principle all other
entries are redundant.

However for exynos drm there does not seem to be one main device which
is guaranteed to always be present which can be used to key the module
autoloading of. So you still need seperate MODULE_DEVICE_TABLE entries
for all the various subdrivers to ensure autoloading actually happens,
especially since the various subdrivers can be seperately enabled
at build time. 

The one exception from the above might be the HDMI sub-driver, which is
always build together with the mixer (And i asume the HDMI hw block
depends on the mixer block for its input). However it seems more elegant
and less error-prone to have simply entries for both, rather then
implicitly replying on the other (sub)driver to trigger module loading.
Especially given there is essentially no cost in having another module
alias.

-- 
Sjoerd Simons 
Collabora Ltd.
-- next part --
A non-text attachment was scrubbed...
Name: smime.p7s
Type: application/x-pkcs7-signature
Size: 6170 bytes
Desc: not available
URL: 
<http://lists.freedesktop.org/archives/dri-devel/attachments/20140721/609438dd/attachment.bin>

[PATCH 0/3] drm/exynos: Allow module to be autoloaded

[Sjoerd Simons]

Hey Inki,

On Mon, 2014-07-21 at 08:50 +0200, Sjoerd Simons wrote:
> Hey Inki,
> 
> On Mon, 2014-07-21 at 12:02 +0900, Inki Dae wrote:
> > On 2014? 07? 19? 05:36, Sjoerd Simons wrote:
> > > The exynos DRM module currently is not automatically loaded when build as 
> > > a
> > > module. This is due to the simple fact that it doesn't have any
> > > MODULE_DEVICE_TABLE entries whatsoever... Most of these were removed 
> > > previously
> > > as it wasn't possible at the time to have multiple calls to 
> > > MODULE_DEVICE_TABLE
> > > in one module, however commit 21bdd17b21b45ea solved that.
> > > 
> > > The first two patches revert the previous removals of MODULE_DEVICE_TABLE
> > > calls, while the last one adds calls for the remaining OF match tables 
> > > without a
> > > MODULE_DEVICE_TABLE call.
> 
> > Exynos drm follows single-driver model. So each usb driver of Exynos drm
> > wouldn't need its own MODULE_DEVICE_TABLE.
> 
> Strictly speaking you're right, for module autoloading to work the
> module just needs to have one that matches. So in principle all other
> entries are redundant.
> 
> However for exynos drm there does not seem to be one main device which
> is guaranteed to always be present which can be used to key the module
> autoloading of. So you still need seperate MODULE_DEVICE_TABLE entries
> for all the various subdrivers to ensure autoloading actually happens,
> especially since the various subdrivers can be seperately enabled
> at build time. 

Been about a week since this last mail. If you have any suggestions on a
better approach or on how to move this forward, i'd be very grateful to
hear as i think i've addressed your original comment on the set in the
previous reply?

-- 
Sjoerd Simons 
Collabora Ltd.
-- next part --
A non-text attachment was scrubbed...
Name: smime.p7s
Type: application/x-pkcs7-signature
Size: 6170 bytes
Desc: not available
URL: 
<http://lists.freedesktop.org/archives/dri-devel/attachments/20140728/16c29e60/attachment-0001.bin>

[PATCH 0/3] drm/exynos: Allow module to be autoloaded

[Sjoerd Simons]

Hey Inki,

On Mon, 2014-07-28 at 23:17 +0900, Inki Dae wrote:
> On 2014? 07? 28? 17:30, Sjoerd Simons wrote:
> Sorry for late,
> 
> I don't see why Exynos drm driver should be auto-loaded module. I think
> all devices covered by Exynos drm framework are not hot-plugged. Maybe
> there is my missing point. So can you explain why Exynos drm driver
> should be auto-loaded module?

The background for this is that I'm building a distribution-style
multiplatform kernel, that is to say a kernel which can boot on a big
set of different ARM boards. As such, the intention is to keep the core
zImage as small as possible and essentially build things as far as
possible as loadable modules. So in a sense, all of the hardware is
"hotplugged", depending on which board the kernel is actually booted on!

For that use-case, exynosdrm needs to be able to build as a module
(which it already can!) and it needs the required meta-data for
userspace to know when it should be loaded. The latter is what my patch
adds. 

-- 
Sjoerd Simons 
Collabora Ltd.
-- next part --
A non-text attachment was scrubbed...
Name: smime.p7s
Type: application/x-pkcs7-signature
Size: 6170 bytes
Desc: not available
URL: 
<http://lists.freedesktop.org/archives/dri-devel/attachments/20140728/14894732/attachment.bin>

[PATCH 0/3] drm/exynos: Allow module to be autoloaded

[Sjoerd Simons]

On Tue, 2014-07-29 at 14:38 +0900, Inki Dae wrote:
> On 2014? 07? 28? 23:45, Sjoerd Simons wrote:
> > Hey Inki,
> > 
> > On Mon, 2014-07-28 at 23:17 +0900, Inki Dae wrote:
> >> On 2014? 07? 28? 17:30, Sjoerd Simons wrote:
> >> Sorry for late,
> >>
> >> I don't see why Exynos drm driver should be auto-loaded module. I think
> >> all devices covered by Exynos drm framework are not hot-plugged. Maybe
> >> there is my missing point. So can you explain why Exynos drm driver
> >> should be auto-loaded module?
> > 
> > The background for this is that I'm building a distribution-style
> > multiplatform kernel, that is to say a kernel which can boot on a big
> > set of different ARM boards. As such, the intention is to keep the core
> > zImage as small as possible and essentially build things as far as
> > possible as loadable modules. So in a sense, all of the hardware is
> > "hotplugged", depending on which board the kernel is actually booted on!
> > 
> > For that use-case, exynosdrm needs to be able to build as a module
> > (which it already can!) and it needs the required meta-data for
> > userspace to know when it should be loaded. The latter is what my patch
> > adds. 
> 
> It seems that you want that module data of sub drivers are added by
> depmod to /lib/modules/KERNEL_VERSION/modules.xxxmap because some
> hot-plug system should use modules.xxxmap file to find the proper driver
> to load.

Yes. I would like the module to export its module alias information for
the subdrivers such that depmod can add it to its databases and the
normal module autoloading mechanisms work as intended. Note that in my
case, "some hot-plug" system is really just udev, not something
special..


> Ok, then does exynos drm driver is loaded well with your patches? 

It is indeed.

> My concern is that device_id of exynos drm core driver ,
> exynos_drm_drv.c, wouldn't be exported to userspace, which means that
> exynos drm subsystem aren't bound by component framework because most
> sub drivers except vidi are bound by component interfaces of exynos drm
> core: exynos drm drv is not device tree base driver.

This patchset doesn't change how that works. Really all it does is to
tell userspace which devices exynosdrm supports. From the kernel side of
things, there is no difference between the module being loaded based on
that information vs. it being loaded by hand.

-- 
Sjoerd Simons 
Collabora Ltd.
-- next part --
A non-text attachment was scrubbed...
Name: smime.p7s
Type: application/x-pkcs7-signature
Size: 6170 bytes
Desc: not available
URL: 
<http://lists.freedesktop.org/archives/dri-devel/attachments/20140729/fc644e93/attachment-0001.bin>

[PATCH] drm/exynos: resolve infinite loop issue on non multi-platform

[Sjoerd Simons]

On Thu, 2014-11-06 at 23:10 +0900, Inki Dae wrote:
> This patch resovles the infinite loop issue incurred
> when Exyno drm driver is enabled but all kms drivers
> are disabled on Exynos board by returning -EPROBE_DEFER
> only in case that there is kms device registered.

It would be nice if you could explain in the commit message/comment why
returning -EPROBE_DEFER causes an infinite loop and why it's the wrong
thing to do in this case? 

Even if you know this probe will never succeed in the future (so
deferring is actually pointless), deferring really shouldn't trigger
infinte loops in calling code

> 
> Signed-off-by: Inki Dae 
> ---
>  drivers/gpu/drm/exynos/exynos_drm_drv.c |6 ++
>  1 file changed, 6 insertions(+)
> 
> diff --git a/drivers/gpu/drm/exynos/exynos_drm_drv.c 
> b/drivers/gpu/drm/exynos/exynos_drm_drv.c
> index ecc86aa..14c6af7 100644
> --- a/drivers/gpu/drm/exynos/exynos_drm_drv.c
> +++ b/drivers/gpu/drm/exynos/exynos_drm_drv.c
> @@ -488,6 +488,12 @@ static struct component_match 
> *exynos_drm_match_add(struct device *dev)
>  
>   mutex_lock(&drm_component_lock);
>  
> + /* Do not retry to probe if there is no any kms driver regitered. */
> + if (list_empty(&drm_component_list)) {
> + mutex_unlock(&drm_component_lock);
> + return ERR_PTR(-ENODEV);
> + }
> +
>   list_for_each_entry(cdev, &drm_component_list, list) {
>   /*
>* Add components to master only in case that crtc and


-- next part --
A non-text attachment was scrubbed...
Name: smime.p7s
Type: application/x-pkcs7-signature
Size: 6170 bytes
Desc: not available
URL: 

[PATCH] drm/exynos: fix infinite loop issue incurred by no pair

[Sjoerd Simons]

On Tue, 2014-11-18 at 12:20 +0900, Inki Dae wrote:
> This patch makes the deferred probe is tried up to 3 times in maximum.
> However, this is considered only for Exynos drm so I think other SoC
> drivers could also produce same issue. Therefore, the best way to resolve
> this issue, infinite loop incurred by defered probe, would be that dd core
> is fixed up corrrectly.

At first sight this seems to make little to no sense. Unless i'm
mistaken this would cause the exynos drm probe return -ENODEV to the dd
core, causing it to stop trying to probe. Which obviously breaks your
infinite loop, it also breaks situations where the probe needs to be
retried more then 3 times. 

I suspect with this patch once exynos DRM is loaded and actually validly
needs to defer (iotw when the required modules do exist but simply
aren't loaded just yet), it still jumps into an infinite loop which you
break after 3 tries after which the display will simply never come up
even if everything is in place because the core doesn't know it should
re-probe



> Signed-off-by: Inki Dae 
> ---
>  drivers/gpu/drm/exynos/exynos_drm_drv.c |8 
>  1 file changed, 8 insertions(+)
> 
> diff --git a/drivers/gpu/drm/exynos/exynos_drm_drv.c 
> b/drivers/gpu/drm/exynos/exynos_drm_drv.c
> index eab12f0..4d84f3a 100644
> --- a/drivers/gpu/drm/exynos/exynos_drm_drv.c
> +++ b/drivers/gpu/drm/exynos/exynos_drm_drv.c
> @@ -38,6 +38,8 @@
>  #define DRIVER_MAJOR 1
>  #define DRIVER_MINOR 0
>  
> +#define MAX_TRY_PROBE_DEFER  3
> +
>  static struct platform_device *exynos_drm_pdev;
>  
>  static DEFINE_MUTEX(drm_component_lock);
> @@ -481,6 +483,7 @@ static struct component_match 
> *exynos_drm_match_add(struct device *dev)
>   struct component_match *match = NULL;
>   struct component_dev *cdev;
>   unsigned int attach_cnt = 0;
> + static unsigned int try_probe_defer;
>  
>   mutex_lock(&drm_component_lock);
>  
> @@ -527,6 +530,11 @@ out_lock:
>  
>   mutex_unlock(&drm_component_lock);
>  
> + if (++try_probe_defer > MAX_TRY_PROBE_DEFER) {
> + try_probe_defer = 0;
> + return ERR_PTR(-ENODEV);
> + }
> +
>   return attach_cnt ? match : ERR_PTR(-EPROBE_DEFER);
>  }
>  


-- next part --
A non-text attachment was scrubbed...
Name: smime.p7s
Type: application/x-pkcs7-signature
Size: 6170 bytes
Desc: not available
URL: 

[5/5] ARM: tegra: jetson-tk1: enable GK20A GPU

[Sjoerd Simons]

Playing a bit with todays linux-next on my jetson, it seems this patch is
still required for enabling the GPU. Is there anything blocking it (firmware
not available yet in liux-firmware?)

On Mon, May 19, 2014 at 06:24:10PM +0900, Alexandre Courbot wrote:
> Signed-off-by: Alexandre Courbot 
> ---
>  arch/arm/boot/dts/tegra124-jetson-tk1.dts | 8 +++-
>  1 file changed, 7 insertions(+), 1 deletion(-)
> 
> diff --git a/arch/arm/boot/dts/tegra124-jetson-tk1.dts 
> b/arch/arm/boot/dts/tegra124-jetson-tk1.dts
> index e31fb61a81d3..15a194d1277f 100644
> --- a/arch/arm/boot/dts/tegra124-jetson-tk1.dts
> +++ b/arch/arm/boot/dts/tegra124-jetson-tk1.dts
> @@ -30,6 +30,12 @@
>   };
>   };
>  
> + gpu at 0,5700 {
> + status = "okay";
> +
> + vdd-supply = <&vdd_gpu>;
> + };
> +
>   pinmux: pinmux at 0,7868 {
>   pinctrl-names = "default";
>   pinctrl-0 = <&state_default>;
> @@ -1505,7 +1511,7 @@
>   regulator-always-on;
>   };
>  
> - sd6 {
> + vdd_gpu: sd6 {
>   regulator-name = "+VDD_GPU_AP";
>   regulator-min-microvolt = <65>;
>   regulator-max-microvolt = <120>;

-- 
If you put it off long enough, it might go away.

[5/5] ARM: tegra: jetson-tk1: enable GK20A GPU

[Sjoerd Simons]

On Thu, 2014-09-25 at 18:41 +0200, Thierry Reding wrote:
> On Thu, Sep 25, 2014 at 09:48:01AM -0600, Stephen Warren wrote:
> > On 09/25/2014 07:27 AM, Sjoerd Simons wrote:
> > >Playing a bit with todays linux-next on my jetson, it seems this patch is
> > >still required for enabling the GPU. Is there anything blocking it 
> > >(firmware
> > >not available yet in liux-firmware?)
> > 
> > I think initially I was waiting for the DRM patch "drm/nouvea: support for
> > probing platform devices" to be applied, but it looks like that's been
> > applied already, so only patches 4 and 5 in this series are still
> > outstanding.
> > 
> > Alex, wasn't there also some issue where the VPR register had to be
> > programmed, and if it wasn't there'd be a hang when the GPU registers were
> > touched? If we've added code to Nouveau/tegradrm to detect that and avoid
> > the problem, then I guess we can commit these last two patches for 3.19. A
> > resend after the 3.18 merge window might help.
> 
> A patch that programs VPR was merged into U-Boot (though I don't think
> it's made it into master yet).

Assuming you're talking about "ARM: tegra: Disable VPR",that has landed
in u-boot master and released as part of v2014.10-rc2 [0]

>  I'm not sure we can reasonably check for
> that in Nouveau, given that the register is somewhere completely
> unrelated. In fact I think the U-Boot patch was triggered by some
> discussion about how to solve this and it was decided that it shouldn't
> be done in the kernel, but U-Boot should set it up.
> 
> That said, perhaps one solution would be to make U-Boot enable the gk20a
> device if it's set up the VPR and disable it otherwise?

I guess in that case the vdd-supply should still be added to the dts
with u-boot toggling the status field of the node?


0: 
http://git.denx.de/?p=u-boot.git;a=commit;h=df3443dfa449ad02bef8ddf6e2c90a6fd9394fc9
-- 
Sjoerd Simons 

[PATCH] drm/rockchip: vop: Correct enabled clocks during setup

[Sjoerd Simons]

When doing the initial setup both the hclk and the aclk need to be
enabled otherwise the board will simply hang. This only occurs when
building the vop driver as a module, when its built-in the initial setup
happens to run before the clock framework shuts of unused clocks
(including the aclk).

While there also switch to doing prepare and enable in one step rather
then separate steps to reduce the amount of code required.

Signed-off-by: Sjoerd Simons 

---

 drivers/gpu/drm/rockchip/rockchip_drm_vop.c | 36 +++--
 1 file changed, 14 insertions(+), 22 deletions(-)

diff --git a/drivers/gpu/drm/rockchip/rockchip_drm_vop.c 
b/drivers/gpu/drm/rockchip/rockchip_drm_vop.c
index 5d8ae5e..48719df 100644
--- a/drivers/gpu/drm/rockchip/rockchip_drm_vop.c
+++ b/drivers/gpu/drm/rockchip/rockchip_drm_vop.c
@@ -1575,32 +1575,25 @@ static int vop_initial(struct vop *vop)
return PTR_ERR(vop->dclk);
}

-   ret = clk_prepare(vop->hclk);
-   if (ret < 0) {
-   dev_err(vop->dev, "failed to prepare hclk\n");
-   return ret;
-   }
-
ret = clk_prepare(vop->dclk);
if (ret < 0) {
dev_err(vop->dev, "failed to prepare dclk\n");
-   goto err_unprepare_hclk;
+   return ret;
}

-   ret = clk_prepare(vop->aclk);
+   /* Enable both the hclk and aclk to setup the vop */
+   ret = clk_prepare_enable(vop->hclk);
if (ret < 0) {
-   dev_err(vop->dev, "failed to prepare aclk\n");
+   dev_err(vop->dev, "failed to prepare/enable hclk\n");
goto err_unprepare_dclk;
}

-   /*
-* enable hclk, so that we can config vop register.
-*/
-   ret = clk_enable(vop->hclk);
+   ret = clk_prepare_enable(vop->aclk);
if (ret < 0) {
-   dev_err(vop->dev, "failed to prepare aclk\n");
-   goto err_unprepare_aclk;
+   dev_err(vop->dev, "failed to prepare/enable aclk\n");
+   goto err_disable_hclk;
}
+
/*
 * do hclk_reset, reset all vop registers.
 */
@@ -1608,7 +1601,7 @@ static int vop_initial(struct vop *vop)
if (IS_ERR(ahb_rst)) {
dev_err(vop->dev, "failed to get ahb reset\n");
ret = PTR_ERR(ahb_rst);
-   goto err_disable_hclk;
+   goto err_disable_aclk;
}
reset_control_assert(ahb_rst);
usleep_range(10, 20);
@@ -1634,26 +1627,25 @@ static int vop_initial(struct vop *vop)
if (IS_ERR(vop->dclk_rst)) {
dev_err(vop->dev, "failed to get dclk reset\n");
ret = PTR_ERR(vop->dclk_rst);
-   goto err_unprepare_aclk;
+   goto err_disable_aclk;
}
reset_control_assert(vop->dclk_rst);
usleep_range(10, 20);
reset_control_deassert(vop->dclk_rst);

clk_disable(vop->hclk);
+   clk_disable(vop->aclk);

vop->is_enabled = false;

return 0;

+err_disable_aclk:
+   clk_disable_unprepare(vop->aclk);
 err_disable_hclk:
-   clk_disable(vop->hclk);
-err_unprepare_aclk:
-   clk_unprepare(vop->aclk);
+   clk_disable_unprepare(vop->hclk);
 err_unprepare_dclk:
clk_unprepare(vop->dclk);
-err_unprepare_hclk:
-   clk_unprepare(vop->hclk);
return ret;
 }

-- 
2.5.3

[PATCH] drm/rockchip: vop: Correct enabled clocks during setup

[Sjoerd Simons]

On Tue, 2015-09-29 at 18:58 +0800, Yakir Yang wrote:
> 
> On 09/29/2015 05:55 PM, Yakir Yang wrote:
> > 
> > 
> > On 09/29/2015 05:28 PM, Sjoerd Simons wrote:
> > > When doing the initial setup both the hclk and the aclk need to
> > > be
> > > enabled otherwise the board will simply hang. This only occurs
> > > when
> > > building the vop driver as a module, when its built-in the
> > > initial setup
> 
> Hmm... My previous test was built-in the vop driver, and just notice
> that
> you say problem only occurred when building the vop driver as module.
> That's to say my test was wrong, so I try to do the right things.
> 
> But I found that vop driver module and rockchipdrm driver module in
> dependency cycles, here are the build message:
>  depmod: ERROR: Found 2 modules in dependency cycles!


I've only tested with mainline which doesn't seem to have that issue?
So can't easily help you there unfortunately.


> Thanks,
> - Yakir
> 
> > > happens to run before the clock framework shuts of unused clocks
> > > (including the aclk).
> > > 
> > > While there also switch to doing prepare and enable in one step
> > > rather
> > > then separate steps to reduce the amount of code required.
> > > 
> > > Signed-off-by: Sjoerd Simons 
> > 
> > Looks good and test on chromeos-3.14 tree, no problem, so
> > 
> > Tested-by: Yakir Yang 
> > 
> > > ---
> > > 
> > >   drivers/gpu/drm/rockchip/rockchip_drm_vop.c | 36 
> > > +++--
> > >   1 file changed, 14 insertions(+), 22 deletions(-)
> > > 
> > > diff --git a/drivers/gpu/drm/rockchip/rockchip_drm_vop.c 
> > > b/drivers/gpu/drm/rockchip/rockchip_drm_vop.c
> > > index 5d8ae5e..48719df 100644
> > > --- a/drivers/gpu/drm/rockchip/rockchip_drm_vop.c
> > > +++ b/drivers/gpu/drm/rockchip/rockchip_drm_vop.c
> > > @@ -1575,32 +1575,25 @@ static int vop_initial(struct vop *vop)
> > >   return PTR_ERR(vop->dclk);
> > >   }
> > >   -ret = clk_prepare(vop->hclk);
> > > -if (ret < 0) {
> > > -dev_err(vop->dev, "failed to prepare hclk\n");
> > > -return ret;
> > > -}
> > > -
> > >   ret = clk_prepare(vop->dclk);
> > >   if (ret < 0) {
> > >   dev_err(vop->dev, "failed to prepare dclk\n");
> > > -goto err_unprepare_hclk;
> > > +return ret;
> > >   }
> > >   -ret = clk_prepare(vop->aclk);
> > > +/* Enable both the hclk and aclk to setup the vop */
> > > +ret = clk_prepare_enable(vop->hclk);
> > >   if (ret < 0) {
> > > -dev_err(vop->dev, "failed to prepare aclk\n");
> > > +dev_err(vop->dev, "failed to prepare/enable hclk\n");
> > >   goto err_unprepare_dclk;
> > >   }
> > >   -/*
> > > - * enable hclk, so that we can config vop register.
> > > - */
> > > -ret = clk_enable(vop->hclk);
> > > +ret = clk_prepare_enable(vop->aclk);
> > >   if (ret < 0) {
> > > -dev_err(vop->dev, "failed to prepare aclk\n");
> > > -goto err_unprepare_aclk;
> > > +dev_err(vop->dev, "failed to prepare/enable aclk\n");
> > > +goto err_disable_hclk;
> > >   }
> > > +
> > >   /*
> > >* do hclk_reset, reset all vop registers.
> > >*/
> > > @@ -1608,7 +1601,7 @@ static int vop_initial(struct vop *vop)
> > >   if (IS_ERR(ahb_rst)) {
> > >   dev_err(vop->dev, "failed to get ahb reset\n");
> > >   ret = PTR_ERR(ahb_rst);
> > > -goto err_disable_hclk;
> > > +goto err_disable_aclk;
> > >   }
> > >   reset_control_assert(ahb_rst);
> > >   usleep_range(10, 20);
> > > @@ -1634,26 +1627,25 @@ static int vop_initial(struct vop *vop)
> > >   if (IS_ERR(vop->dclk_rst)) {
> > >   dev_err(vop->dev, "failed to get dclk reset\n");
> > >   ret = PTR_ERR(vop->dclk_rst);
> > > -goto err_unprepare_aclk;
> > > +goto err_disable_aclk;
> > >   }
> > >   reset_control_assert(vop->dclk_rst);
> > >   usleep_range(10, 20);
> > >   reset_control_deassert(vop->dclk_rst);
> > > clk_disable(vop->hclk);
> > > +clk_disable(vop->aclk);
> > > vop->is_enabled = false;
> > > return 0;
> > >   +err_disable_aclk:
> > > +clk_disable_unprepare(vop->aclk);
> > >   err_disable_hclk:
> > > -clk_disable(vop->hclk);
> > > -err_unprepare_aclk:
> > > -clk_unprepare(vop->aclk);
> > > +clk_disable_unprepare(vop->hclk);
> > >   err_unprepare_dclk:
> > >   clk_unprepare(vop->dclk);
> > > -err_unprepare_hclk:
> > > -clk_unprepare(vop->hclk);
> > >   return ret;
> > >   }
> > 
> 
> 

-- 
Sjoerd Simons
Collabora Ltd.

[PATCH] drm: rcar-du: Only unwindow as necessary on probe failure

[Sjoerd Simons]

Simply calling rcar_du_remove on any error can cause unbalanced calls to
various functions (e.g. drm_connector_register/unregister,
drm_dev_register/drm_dev_unref).

This can be seen at bootup of my Porter boards with current next:

[2.789322] rcar-du feb0.display: failed to initialize DRM/KMS (-517)
[2.796267] [ cut here ]
[2.800996] WARNING: CPU: 1 PID: 1 at include/drm/drm_crtc.h:2623 
drm_connector_unregister_all+0x60/0x68
[2.810689] CPU: 1 PID: 1 Comm: swapper/0 Not tainted 
4.6.0-rc2-next-20160405-dirty #11
[2.818864] Hardware name: Generic R8A7791 (Flattened Device Tree)
[2.825174] Backtrace:
[2.827699] [] (dump_backtrace) from [] 
(show_stack+0x18/0x1c)
[2.835430]  r7:c042da78 r6:0009 r5:6013 r4:
[2.841251] [] (show_stack) from [] 
(dump_stack+0x84/0xa4)
[2.848632] [] (dump_stack) from [] (__warn+0xd0/0x100)
[2.855739]  r5: r4:
[2.859409] [] (__warn) from [] 
(warn_slowpath_null+0x28/0x30)
[2.867139]  r9:0001 r8:ef1dc610 r7:ef114010 r6:ef1dc600 r5:ef114010 
r4:ef2f2400
[2.875096] [] (warn_slowpath_null) from [] 
(drm_connector_unregister_all+0x60/0x68)
[2.884785] [] (drm_connector_unregister_all) from [] 
(rcar_du_remove+0x1c/0x5c)
[2.894111]  r5:ef114010 r4:ef2f2400
[2.897780] [] (rcar_du_remove) from [] 
(rcar_du_probe+0x1d0/0x210)
[2.905953]  r5:ef2f2400 r4:fdfb
[2.909625] [] (rcar_du_probe) from [] 
(platform_drv_probe+0x58/0xa8)
[2.917976]  r9: r8:c0a1cca4 r7:c0a71a48 r6:c0a1cca4 r5:ef1dc610 
r4:c0440b80

Adjust the code to only unwind as necessary on probe failures

Signed-off-by: Sjoerd Simons 

---

 drivers/gpu/drm/rcar-du/rcar_du_drv.c | 32 +---
 1 file changed, 25 insertions(+), 7 deletions(-)

diff --git a/drivers/gpu/drm/rcar-du/rcar_du_drv.c 
b/drivers/gpu/drm/rcar-du/rcar_du_drv.c
index 644db36..deb75fa 100644
--- a/drivers/gpu/drm/rcar-du/rcar_du_drv.c
+++ b/drivers/gpu/drm/rcar-du/rcar_du_drv.c
@@ -333,7 +333,7 @@ static int rcar_du_probe(struct platform_device *pdev)
rcdu->mmio = devm_ioremap_resource(&pdev->dev, mem);
if (IS_ERR(rcdu->mmio)) {
ret = PTR_ERR(rcdu->mmio);
-   goto error;
+   goto error_dev;
}

/* Initialize vertical blanking interrupts handling. Start with vblank
@@ -342,14 +342,17 @@ static int rcar_du_probe(struct platform_device *pdev)
ret = drm_vblank_init(ddev, (1 << rcdu->info->num_crtcs) - 1);
if (ret < 0) {
dev_err(&pdev->dev, "failed to initialize vblank\n");
-   goto error;
+   goto error_dev;
}

/* DRM/KMS objects */
ret = rcar_du_modeset_init(rcdu);
if (ret < 0) {
+   /* modeset_init could have failed partway through and doesn't
+* do its own cleanup, so needs to be completely undone.
+*/
dev_err(&pdev->dev, "failed to initialize DRM/KMS (%d)\n", ret);
-   goto error;
+   goto error_modeset;
}

ddev->irq_enabled = 1;
@@ -359,7 +362,7 @@ static int rcar_du_probe(struct platform_device *pdev)
 */
ret = drm_dev_register(ddev, 0);
if (ret)
-   goto error;
+   goto error_modeset;

mutex_lock(&ddev->mode_config.mutex);
drm_for_each_connector(connector, ddev) {
@@ -369,15 +372,30 @@ static int rcar_du_probe(struct platform_device *pdev)
}
mutex_unlock(&ddev->mode_config.mutex);

+   /* One or more connects could have been registered, so unregister all
+* connectors.
+*/
if (ret < 0)
-   goto error;
+   goto error_connector;

DRM_INFO("Device %s probed\n", dev_name(&pdev->dev));

return 0;

-error:
-   rcar_du_remove(pdev);
+error_connector:
+   drm_connector_unregister_all(ddev);
+   drm_dev_unregister(ddev);
+
+error_modeset:
+   if (rcdu->fbdev)
+   drm_fbdev_cma_fini(rcdu->fbdev);
+
+   drm_kms_helper_poll_fini(ddev);
+   drm_mode_config_cleanup(ddev);
+   drm_vblank_cleanup(ddev);
+
+error_dev:
+   drm_dev_unref(ddev);

return ret;
 }
-- 
2.8.0.rc3

[PATCH] drm/tilcdc: Defer probe if there are no connectors

[Sjoerd Simons]

During probe there may not be any connectors yet if e.g. the panel
failed or hasn't been probed yet. I hitting this in practice the panels
probing was being delayed due to using a gpio backlight.

Fix this by returning -EPROBE_DEFER so the probing will be retried.

Signed-off-by: Sjoerd Simons 

---

 drivers/gpu/drm/tilcdc/tilcdc_drv.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/gpu/drm/tilcdc/tilcdc_drv.c 
b/drivers/gpu/drm/tilcdc/tilcdc_drv.c
index 1afde61f1247..59f0a44bb6e3 100644
--- a/drivers/gpu/drm/tilcdc/tilcdc_drv.c
+++ b/drivers/gpu/drm/tilcdc/tilcdc_drv.c
@@ -380,7 +380,7 @@ static int tilcdc_init(struct drm_driver *ddrv, struct 
device *dev)
if (!priv->external_connector &&
((priv->num_encoders == 0) || (priv->num_connectors == 0))) {
dev_err(dev, "no encoders/connectors found\n");
-   ret = -ENXIO;
+   ret = -EPROBE_DEFER;
goto init_failed;
}
 
-- 
2.16.3

___
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

[PATCH] drm/i915: no lvds quirk for Zotac ZDBOX SD ID12/ID13

[Sjoerd Simons]

This box claims to have an LVDS interface but doesn't
actually have one.

Signed-off-by: Sjoerd Simons 
---
 drivers/gpu/drm/i915/intel_lvds.c |8 
 1 file changed, 8 insertions(+)

diff --git a/drivers/gpu/drm/i915/intel_lvds.c 
b/drivers/gpu/drm/i915/intel_lvds.c
index 08eb04c..9393860 100644
--- a/drivers/gpu/drm/i915/intel_lvds.c
+++ b/drivers/gpu/drm/i915/intel_lvds.c
@@ -777,6 +777,14 @@ static const struct dmi_system_id intel_no_lvds[] = {
DMI_MATCH(DMI_BOARD_NAME, "MS-7469"),
},
},
+   {
+   .callback = intel_no_lvds_dmi_callback,
+   .ident = "ZOTAC ZBOXSD-ID12/ID13",
+   .matches = {
+   DMI_MATCH(DMI_BOARD_VENDOR, "ZOTAC"),
+   DMI_MATCH(DMI_BOARD_NAME, "ZBOXSD-ID12/ID13"),
+   },
+   },
 
{ } /* terminating entry */
 };
-- 
1.7.10

___
dri-devel mailing list
dri-devel@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/dri-devel

[PATCH] drm/i915: no lvds quirk for Zotac ZDBOX SD ID12/ID13

[Sjoerd Simons]

This box claims to have an LVDS interface but doesn't
actually have one.

Signed-off-by: Sjoerd Simons 
---
 drivers/gpu/drm/i915/intel_lvds.c |8 
 1 file changed, 8 insertions(+)

diff --git a/drivers/gpu/drm/i915/intel_lvds.c 
b/drivers/gpu/drm/i915/intel_lvds.c
index 08eb04c..9393860 100644
--- a/drivers/gpu/drm/i915/intel_lvds.c
+++ b/drivers/gpu/drm/i915/intel_lvds.c
@@ -777,6 +777,14 @@ static const struct dmi_system_id intel_no_lvds[] = {
DMI_MATCH(DMI_BOARD_NAME, "MS-7469"),
},
},
+   {
+   .callback = intel_no_lvds_dmi_callback,
+   .ident = "ZOTAC ZBOXSD-ID12/ID13",
+   .matches = {
+   DMI_MATCH(DMI_BOARD_VENDOR, "ZOTAC"),
+   DMI_MATCH(DMI_BOARD_NAME, "ZBOXSD-ID12/ID13"),
+   },
+   },

{ } /* terminating entry */
 };
-- 
1.7.10

[PATCH 1/2] drm/rockchip: Use CRTC vblank event interface

[Sjoerd Simons]

On Mon, 2015-11-16 at 12:50 +, Daniel Stone wrote:
> Passing -1 as the pipe for vblank events now triggers a WARN_ON, but
> had
> previously made multi-screen unusable anyway. Pass the correct pipe
> to
> the event-send function, and use the new API to make this a bit
> easier
> for us.

Tested on a Radxa Rock 2 square board.

Tested-By: Sjoerd Simons 

> Fixes WARN present since cc1ef118fc for every pageflip event sent:
> [  209.549969] [ cut here ]
> [  209.554592] WARNING: CPU: 3 PID: 238 at
> drivers/gpu/drm/drm_irq.c:924 drm_vblank_count_and_time+0x80/0x88
> [drm]()
> [  209.564832] Modules linked in: [...]
> [  209.612401] CPU: 3 PID: 238 Comm: irq/41-ff94 Tainted:
> G        W       4.3.0-rc6+ #71
> [  209.620647] Hardware name: Rockchip (Device Tree)
> [  209.625348] [] (unwind_backtrace) from []
> (show_stack+0x20/0x24)
> [  209.633079] [] (show_stack) from []
> (dump_stack+0x8c/0x9c)
> [  209.640289] [] (dump_stack) from []
> (warn_slowpath_common+0x94/0xc4)
> [  209.648364] [] (warn_slowpath_common) from []
> (warn_slowpath_null+0x2c/0x34)
> [  209.657139] [] (warn_slowpath_null) from []
> (drm_vblank_count_and_time+0x80/0x88 [drm])
> [  209.666875] [] (drm_vblank_count_and_time [drm]) from
> [] (drm_send_vblank_event+0x74/0x7c [drm])
> [  209.677385] [] (drm_send_vblank_event [drm]) from
> [] (vop_win_state_complete+0x4c/0x70 [rockchip_drm_vop])
> [  209.688757] [] (vop_win_state_complete
> [rockchip_drm_vop]) from [] (vop_isr_thread+0x170/0x1d4
> [rockchip_drm_vop])
> [  209.700822] [] (vop_isr_thread [rockchip_drm_vop]) from
> [] (irq_thread_fn+0x2c/0x50)
> [  209.710284] [] (irq_thread_fn) from []
> (irq_thread+0x13c/0x188)
> [  209.717927] [] (irq_thread) from []
> (kthread+0xec/0x104)
> [  209.724965] [] (kthread) from []
> (ret_from_fork+0x14/0x3c)
> [  209.732171] ---[ end trace 0690bc604f5d535d ]---
> 
> Signed-off-by: Daniel Stone 
> Cc: Sjoerd Simons 
> Cc: Thierry Reding 
> Cc: Heiko Stuebner 
> ---
>  drivers/gpu/drm/rockchip/rockchip_drm_vop.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/drivers/gpu/drm/rockchip/rockchip_drm_vop.c
> b/drivers/gpu/drm/rockchip/rockchip_drm_vop.c
> index 5d8ae5e..ddf6dc2 100644
> --- a/drivers/gpu/drm/rockchip/rockchip_drm_vop.c
> +++ b/drivers/gpu/drm/rockchip/rockchip_drm_vop.c
> @@ -1289,7 +1289,7 @@ static void vop_win_state_complete(struct
> vop_win *vop_win,
>  
>  if (state->event) {
>  spin_lock_irqsave(&drm->event_lock, flags);
> - drm_send_vblank_event(drm, -1, state->event);
> + drm_crtc_send_vblank_event(crtc, state->event);
>  spin_unlock_irqrestore(&drm->event_lock, flags);
>  }
>  

-- 
Sjoerd Simons
Collabora Ltd.

[PATCH 2/2] drm/rockchip: Send events for same-fb flips

[Sjoerd Simons]

On Mon, 2015-11-16 at 12:50 +, Daniel Stone wrote:
> Rockchip previously treated a pageflip to the same framebuffer as a
> no-op, discarding the event if one was requested. This breaks Weston,
> which, when idle, sends a no-op vblank event to discover vblank
> timings if the vblank query interface is not usable.
> 
> Silently dropping events is also quite a hostile thing to do to
> userspace in general.

Tested on a Radxa Rock 2 square board, the combination of this patch
and the previous in this series makes weston (drm compositor, pixman
renderer) work nicely on this hardware while before it would get stuck
right away for the reasons pointed out by daniels

Tested-by: Sjoerd Simons 

> Signed-off-by: Daniel Stone 
> Cc: Sjoerd Simons 
> Cc: Heiko Stuebner 
> ---
>  drivers/gpu/drm/rockchip/rockchip_drm_vop.c | 30 ++-
> --
>  1 file changed, 14 insertions(+), 16 deletions(-)
> 
> diff --git a/drivers/gpu/drm/rockchip/rockchip_drm_vop.c
> b/drivers/gpu/drm/rockchip/rockchip_drm_vop.c
> index ddf6dc2..dad607e 100644
> --- a/drivers/gpu/drm/rockchip/rockchip_drm_vop.c
> +++ b/drivers/gpu/drm/rockchip/rockchip_drm_vop.c
> @@ -916,25 +916,23 @@ static int vop_update_plane_event(struct
> drm_plane *plane,
>   * unreference any previous framebuffers.
>   */
>  mutex_lock(&vop->vsync_mutex);
> - if (fb != vop_win_last_pending_fb(vop_win)) {
> - ret = drm_vblank_get(plane->dev, vop->pipe);
> - if (ret) {
> - DRM_ERROR("failed to get vblank, %d\n",
> ret);
> - mutex_unlock(&vop->vsync_mutex);
> - return ret;
> - }
> + ret = drm_vblank_get(plane->dev, vop->pipe);
> + if (ret) {
> + DRM_ERROR("failed to get vblank, %d\n", ret);
> + mutex_unlock(&vop->vsync_mutex);
> + return ret;
> + }
>  
> - drm_framebuffer_reference(fb);
> + drm_framebuffer_reference(fb);
>  
> - ret = vop_win_queue_fb(vop_win, fb, yrgb_mst,
> event);
> - if (ret) {
> - drm_vblank_put(plane->dev, vop->pipe);
> - mutex_unlock(&vop->vsync_mutex);
> - return ret;
> - }
> -
> - vop->vsync_work_pending = true;
> + ret = vop_win_queue_fb(vop_win, fb, yrgb_mst, event);
> + if (ret) {
> + drm_vblank_put(plane->dev, vop->pipe);
> + mutex_unlock(&vop->vsync_mutex);
> + return ret;
>  }
> +
> + vop->vsync_work_pending = true;
>  mutex_unlock(&vop->vsync_mutex);
>  
>  spin_lock(&vop->reg_lock);

-- 
Sjoerd Simons
Collabora Ltd.

[PATCH V8 00/14] drm/exynos: few patches to enhance bridge chip support

[Sjoerd Simons]

On Mon, 2015-01-19 at 15:01 -0200, Gustavo Padovan wrote:
> 2015-01-19 Thierry Reding :
> 
> > On Mon, Jan 19, 2015 at 11:27:52AM +0100, Javier Martinez Canillas wrote:
> > > Hello Thierry,
> > > 
> > > On 01/05/2015 02:50 PM, Thierry Reding wrote:
> > > > On Fri, Jan 02, 2015 at 01:10:14PM +, Daniel Stone wrote:
> > > >> >
> > > >> > Ajay's series don't apply cleanly anymore because it has been a 
> > > >> > while since
> > > >> > he posted it but he can rebase on top of 3.19-rc1 once it is 
> > > >> > released and
> > > >> > re-resend.
> > > >> >
> > > >> 
> > > >> Do you have any plans to rebase this so it's ready for merging?
> > > >> 
> > > >> Thierry, Daniel, Dave - whose tree would this be best to merge through?
> > > > 
> > > > The plan is for me to take the bridge patches through the drm/panel
> > > > tree. I'm going to look at these patches again later this week but from
> > > > a very quick peek there don't seem to be any major issues left.
> > > >
> > > 
> > > I know you probably are very busy but it would be great if you can take a 
> > > look
> > > to this series to avoid another kernel release to be missed since we are 
> > > already
> > > at v3.19-rc5.
> > > 
> > > Only this version was posted 2 months ago and the first version of the 
> > > series
> > > was sent on May, 2014 so it has been on the list for almost a year now...
> > > 
> > > Tomi and Laurent had already agreed with the DT binging so I think that 
> > > we can
> > > already merge these and if there are any remaining issues, those can be 
> > > fixed
> > > during the 3.20 -rc cycle.
> > 
> > I see only a single Tested-by on this series and that seems to be with a
> > bunch of out-of-tree patches applied on top. Does this now actually work
> > applied on top of upstream? Also it seems like many people actually want
> > this to get merged but there's been no Reviewed-bys and only a single
> > Tested-by? Is that as good as it's going to get?
> 
> I've been using this series on top of exynos-drm for more than a month and it 
> works fine for me so..
> 
> Tested-by: Gustavo Padovan 

Same here, just test-booted on my snow with just these patch on top of
3.19-rc5 and got a nicely working display. 

Tested-by: Sjoerd Simons 

-- 
Sjoerd Simons 
Collabora Ltd.
-- next part --
A non-text attachment was scrubbed...
Name: smime.p7s
Type: application/x-pkcs7-signature
Size: 6170 bytes
Desc: not available
URL: 
<http://lists.freedesktop.org/archives/dri-devel/attachments/20150119/f0d3ea45/attachment.bin>

Re: [PATCH v6 6/6] arm64: dts: mediatek: add display support for mt8365-evk

[Sjoerd Simons]

On Thu, 2025-01-09 at 11:37 +0100, Alexandre Mergnat wrote:
> MIPI DSI:
> - Add "vsys_lcm_reg" regulator support and setup the
> "mt6357_vsim1_reg",
> to power the pannel plugged to the DSI connector.
> - Setup the Display Parallel Interface.
>   - Add the startek kd070fhfid015 pannel support.
> 
> HDMI:
> - Add HDMI connector support.
> - Add the "ite,it66121" HDMI bridge support, driven by I2C1.
> - Setup the Display Parallel Interface.
> 
> Signed-off-by: Alexandre Mergnat 
> ---
>  arch/arm64/boot/dts/mediatek/mt8365-evk.dts | 236
> 
>  1 file changed, 236 insertions(+)
> 
> diff --git a/arch/arm64/boot/dts/mediatek/mt8365-evk.dts
> b/arch/arm64/boot/dts/mediatek/mt8365-evk.dts
> index 7d90112a7e27..70bd49a9d02f 100644
> --- a/arch/arm64/boot/dts/mediatek/mt8365-evk.dts
> +++ b/arch/arm64/boot/dts/mediatek/mt8365-evk.dts
> [...]
> @@ -131,6 +156,88 @@ &cpu3 {
>   sram-supply = <&mt6357_vsram_proc_reg>;
>  };
>  
> +&dither0_out {
> + remote-endpoint = <&dsi0_in>;
> +};
> +
> +&dpi0 {
> + pinctrl-0 = <&dpi_default_pins>;
> + pinctrl-1 = <&dpi_idle_pins>;
> + pinctrl-names = "default", "sleep";

Would be good to document that these pins clash with ethernet support,
similar to what is done in the ethernet node.  Also some notes on what
nodes to enable/disable when selecting either ethernet or hdmi output
would be appreciated :)


-- 
Sjoerd Simons 
Collabora