drm/rockchip: Add dmc notifier in vop driver [chromiumos/third_party/kernel : chromeos-4.4]

19 views
Skip to first unread message

Lin Huang (Gerrit)

unread,
Jul 24, 2016, 3:33:20 AM7/24/16
to chromium-...@chromium.org
Lin Huang has uploaded a new change for review.

https://chromium-review.googlesource.com/362907

Change subject: drm/rockchip: Add dmc notifier in vop driver
......................................................................

drm/rockchip: Add dmc notifier in vop driver

when in ddr frequency scaling process, vop can not do
enable or disable operate, since dcf will base on vop vblank
time to do frequency scaling and need to get vop irq if there
have vop enabled. So need register to dmc notifier, and we can
get the dmc status.

Change-Id: I690ae1c0d2436b957fc823585c1a32b48cfcf3fd
Signed-off-by: Lin Huang <h...@rock-chips.com>
---
M drivers/gpu/drm/rockchip/rockchip_drm_vop.c
1 file changed, 66 insertions(+), 6 deletions(-)



diff --git a/drivers/gpu/drm/rockchip/rockchip_drm_vop.c
b/drivers/gpu/drm/rockchip/rockchip_drm_vop.c
index c179933..b92bfa1 100644
--- a/drivers/gpu/drm/rockchip/rockchip_drm_vop.c
+++ b/drivers/gpu/drm/rockchip/rockchip_drm_vop.c
@@ -31,6 +31,8 @@
#include <linux/reset.h>
#include <linux/delay.h>

+#include <soc/rockchip/rockchip_dmc.h>
+
#include "rockchip_drm_drv.h"
#include "rockchip_drm_gem.h"
#include "rockchip_drm_fb.h"
@@ -127,6 +129,11 @@

const struct vop_data *data;

+ struct notifier_block dmc_nb;
+ int dmc_in_process;
+ int vop_switch_status;
+ wait_queue_head_t wait_dmc_queue;
+ wait_queue_head_t wait_vop_switch_queue;
uint32_t *regsbak;
void __iomem *regs;

@@ -500,6 +507,29 @@
spin_unlock_irqrestore(&vop->irq_lock, flags);
}

+static int dmc_notify(struct notifier_block *nb, unsigned long event,
+ void *data)
+{
+ struct vop *vop = container_of(nb, struct vop, dmc_nb);
+
+ if (event == DMCFREQ_ADJUST) {
+
+ /*
+ * check if vop in enable or disable process,
+ * if yes, wait until it finish, use 200ms as
+ * timeout.
+ */
+ wait_event_timeout(vop->wait_vop_switch_queue,
+ !vop->vop_switch_status, HZ / 5);
+ vop->dmc_in_process = 1;
+ } else if (event == DMCFREQ_FINISH) {
+ vop->dmc_in_process = 0;
+ wake_up(&vop->wait_dmc_queue);
+ }
+
+ return NOTIFY_OK;
+}
+
static void vop_enable(struct drm_crtc *crtc)
{
struct vop *vop = to_vop(crtc);
@@ -508,16 +538,24 @@
if (vop->is_enabled)
return;

+ /*
+ * if in dmc scaling frequency process, wait until it finish
+ * use 100ms as timeout time.
+ */
+ wait_event_timeout(vop->wait_dmc_queue,
+ !vop->dmc_in_process, HZ / 5);
+
+ vop->vop_switch_status = 1;
ret = pm_runtime_get_sync(vop->dev);
if (ret < 0) {
dev_err(vop->dev, "failed to get pm runtime: %d\n", ret);
- return;
+ goto err;
}

ret = clk_enable(vop->hclk);
if (ret < 0) {
dev_err(vop->dev, "failed to enable hclk - %d\n", ret);
- return;
+ goto err;
}

ret = clk_enable(vop->dclk);
@@ -531,7 +569,6 @@
dev_err(vop->dev, "failed to enable aclk - %d\n", ret);
goto err_disable_dclk;
}
-
/*
* Slave iommu shares power, irq and clock with vop. It was associated
* automatically with this master device via common driver code.
@@ -559,7 +596,9 @@
enable_irq(vop->irq);

drm_crtc_vblank_on(crtc);
-
+ vop->vop_switch_status = 0;
+ wake_up(&vop->wait_vop_switch_queue);
+ rockchip_dmc_get(&vop->dmc_nb);
return;

err_disable_aclk:
@@ -568,6 +607,10 @@
clk_disable(vop->dclk);
err_disable_hclk:
clk_disable(vop->hclk);
+err:
+ vop->vop_switch_status = 0;
+ wake_up(&vop->wait_vop_switch_queue);
+ return;
}

static void vop_crtc_disable(struct drm_crtc *crtc)
@@ -577,6 +620,15 @@

if (!vop->is_enabled)
return;
+
+ /*
+ * if in dmc scaling frequency process, wait until it finish
+ * use 100ms as timeout time.
+ */
+ wait_event_timeout(vop->wait_dmc_queue,
+ !vop->dmc_in_process, HZ / 5);
+
+ vop->vop_switch_status = 1;

/*
* We need to make sure that all windows are disabled before we
@@ -591,7 +643,6 @@
VOP_WIN_SET(vop, win, enable, 0);
spin_unlock(&vop->reg_lock);
}
-
drm_crtc_vblank_off(crtc);

/*
@@ -622,7 +673,6 @@
* vop standby complete, so iommu detach is safe.
*/
rockchip_drm_dma_detach_device(vop->drm_dev, vop->dev);
-
clk_disable(vop->dclk);
clk_disable(vop->aclk);
clk_disable(vop->hclk);
@@ -632,6 +682,10 @@
vop->psr_enabled = false;
schedule_delayed_work(&vop->psr_work, VOP_PSR_SET_DELAY_TIME);
}
+
+ vop->vop_switch_status = 0;
+ wake_up(&vop->wait_vop_switch_queue);
+ rockchip_dmc_put(&vop->dmc_nb);
}

static void vop_plane_destroy(struct drm_plane *plane)
@@ -1361,6 +1415,7 @@
goto err_cleanup_crtc;
}

+ vop->dmc_nb.notifier_call = dmc_notify;
init_completion(&vop->dsp_hold_completion);
init_completion(&vop->wait_update_complete);
init_completion(&vop->line_flag_completion);
@@ -1639,6 +1694,11 @@
/* IRQ is initially disabled; it gets enabled in power_on */
disable_irq(vop->irq);

+ init_waitqueue_head(&vop->wait_vop_switch_queue);
+ vop->vop_switch_status = 0;
+ init_waitqueue_head(&vop->wait_dmc_queue);
+ vop->dmc_in_process = 0;
+
ret = vop_create_crtc(vop);
if (ret)
return ret;

--
To view, visit https://chromium-review.googlesource.com/362907
To unsubscribe, visit https://chromium-review.googlesource.com/settings

Gerrit-MessageType: newchange
Gerrit-Change-Id: I690ae1c0d2436b957fc823585c1a32b48cfcf3fd
Gerrit-PatchSet: 1
Gerrit-Project: chromiumos/third_party/kernel
Gerrit-Branch: chromeos-4.4
Gerrit-Owner: Lin Huang <h...@rock-chips.com>

Stéphane Marchesin (Gerrit)

unread,
Jul 25, 2016, 4:49:52 PM7/25/16
to Lin Huang, Nick Sanders, Douglas Anderson, Derek Basehore
Stéphane Marchesin has posted comments on this change. (
https://chromium-review.googlesource.com/362907 )

Change subject: drm/rockchip: Add dmc notifier in vop driver
......................................................................


Patch Set 1:

(6 comments)

https://chromium-review.googlesource.com/#/c/362907/1//COMMIT_MSG
Commit Message:

Line 10: enable or disable operate, since dcf will base on vop vblank
s/operate/operations/

s/will base on/uses/


https://chromium-review.googlesource.com/#/c/362907/1/drivers/gpu/drm/rockchip/rockchip_drm_vop.c
File drivers/gpu/drm/rockchip/rockchip_drm_vop.c:

Line 136: wait_queue_head_t wait_vop_switch_queue;
why use tabs for these 2?


Line 516:
remove dead blank line


Line 542: * if in dmc scaling frequency process, wait until it finish
"until it finishes"


Line 546: !vop->dmc_in_process, HZ / 5);
a timeout here should print an error message


Line 629: !vop->dmc_in_process, HZ / 5);
same here, print a message in case of timeout
Gerrit-MessageType: comment
Gerrit-Change-Id: I690ae1c0d2436b957fc823585c1a32b48cfcf3fd
Gerrit-PatchSet: 1
Gerrit-Project: chromiumos/third_party/kernel
Gerrit-Branch: chromeos-4.4
Gerrit-Owner: Lin Huang <h...@rock-chips.com>
Gerrit-Reviewer: Derek Basehore <dbas...@chromium.org>
Gerrit-Reviewer: Douglas Anderson <dian...@chromium.org>
Gerrit-Reviewer: Nick Sanders <nsan...@chromium.org>
Gerrit-Reviewer: Stéphane Marchesin <mar...@chromium.org>
Gerrit-HasComments: Yes

Lin Huang (Gerrit)

unread,
Jul 29, 2016, 4:39:59 AM7/29/16
to Douglas Anderson, Stéphane Marchesin, Nick Sanders, Derek Basehore
Lin Huang has uploaded a new patch set (#2). (
https://chromium-review.googlesource.com/362907 )

Change subject: FROMLIST: drm/rockchip: Add dmc notifier in vop driver
......................................................................

FROMLIST: drm/rockchip: Add dmc notifier in vop driver

when in ddr frequency scaling process, vop can not do
enable or disable operation, since dcf will base on vop vblank
time to do frequency scaling and need to get vop irq if there
have vop enabled. So need register to devfreq notifier, and we can
get the dmc status. Also, when there have two vop enabled, we need
to disable dmc, since dcf only base on one vop vblank time, so the
other panel will flicker when do ddr frequency scaling.

BUG=chrome-os-partner:54651
TEST=Boot from kevin

Change-Id: I690ae1c0d2436b957fc823585c1a32b48cfcf3fd
Signed-off-by: Lin Huang <h...@rock-chips.com>
(am from https://patchwork.kernel.org/patch/9252207)
---
M drivers/gpu/drm/rockchip/rockchip_drm_vop.c
1 file changed, 119 insertions(+), 5 deletions(-)
Gerrit-MessageType: newpatchset
Gerrit-Change-Id: I690ae1c0d2436b957fc823585c1a32b48cfcf3fd
Gerrit-PatchSet: 2
Gerrit-Project: chromiumos/third_party/kernel
Gerrit-Branch: chromeos-4.4
Gerrit-Owner: Lin Huang <h...@rock-chips.com>

Stéphane Marchesin (Gerrit)

unread,
Aug 4, 2016, 2:23:07 AM8/4/16
to Lin Huang, Nick Sanders, Douglas Anderson, Derek Basehore
Stéphane Marchesin has posted comments on this change. (
https://chromium-review.googlesource.com/362907 )

Change subject: FROMLIST: drm/rockchip: Add dmc notifier in vop driver
......................................................................


Patch Set 4:

(2 comments)

https://chromium-review.googlesource.com/#/c/362907/4/drivers/gpu/drm/rockchip/rockchip_drm_vop.c
File drivers/gpu/drm/rockchip/rockchip_drm_vop.c:

Line 518:
nit: this blank line isn't needed


Line 521: * if yes, wait until it finish, use 200ms as
"it finishes"
Gerrit-MessageType: comment
Gerrit-Change-Id: I690ae1c0d2436b957fc823585c1a32b48cfcf3fd
Gerrit-PatchSet: 4
Gerrit-Project: chromiumos/third_party/kernel
Gerrit-Branch: chromeos-4.4
Gerrit-Owner: Lin Huang <h...@rock-chips.com>
Gerrit-Reviewer: Derek Basehore <dbas...@chromium.org>
Gerrit-Reviewer: Douglas Anderson <dian...@chromium.org>
Gerrit-Reviewer: Nick Sanders <nsan...@chromium.org>
Gerrit-Reviewer: Stéphane Marchesin <mar...@chromium.org>
Gerrit-HasComments: Yes

Lin Huang (Gerrit)

unread,
Aug 10, 2016, 8:56:27 PM8/10/16
to Douglas Anderson, Stéphane Marchesin, Nick Sanders, Derek Basehore
Lin Huang has uploaded a new patch set (#5). (
https://chromium-review.googlesource.com/362907 )

Change subject: FROMLIST: drm/rockchip: Add dmc notifier in vop driver
......................................................................

FROMLIST: drm/rockchip: Add dmc notifier in vop driver

when in ddr frequency scaling process, vop can not do
enable or disable operation, since dcf will base on vop vblank
time to do frequency scaling and need to get vop irq if there
have vop enabled. So need register to devfreq notifier, and we can
get the dmc status. Also, when there have two vop enabled, we need
to disable dmc, since dcf only base on one vop vblank time, so the
other panel will flicker when do ddr frequency scaling.

BUG=chrome-os-partner:54651
TEST=Boot from kevin

Change-Id: I690ae1c0d2436b957fc823585c1a32b48cfcf3fd
Signed-off-by: Lin Huang <h...@rock-chips.com>
Reviewed-by: Chanwoo Choi <cw00...@samsung.com>
(am from https://patchwork.kernel.org/patch/9272567)
---
M drivers/gpu/drm/rockchip/rockchip_drm_vop.c
1 file changed, 118 insertions(+), 5 deletions(-)
Gerrit-MessageType: newpatchset
Gerrit-Change-Id: I690ae1c0d2436b957fc823585c1a32b48cfcf3fd
Gerrit-PatchSet: 5

Lin Huang (Gerrit)

unread,
Aug 16, 2016, 8:06:15 PM8/16/16
to Douglas Anderson, Stéphane Marchesin, Nick Sanders, Derek Basehore
Lin Huang has uploaded a new patch set (#7). (
https://chromium-review.googlesource.com/362907 )

Change subject: FROMLIST: drm/rockchip: Add dmc notifier in vop driver
......................................................................

FROMLIST: drm/rockchip: Add dmc notifier in vop driver

when in ddr frequency scaling process, vop can not do
enable or disable operation, since dcf will base on vop vblank
time to do frequency scaling and need to get vop irq if there
have vop enabled. So need register to devfreq notifier, and we can
get the dmc status. Also, when there have two vop enabled, we need
to disable dmc, since dcf only base on one vop vblank time, so the
other panel will flicker when do ddr frequency scaling.

BUG=chrome-os-partner:54651
TEST=Boot from kevin

Change-Id: I690ae1c0d2436b957fc823585c1a32b48cfcf3fd
Signed-off-by: Lin Huang <h...@rock-chips.com>
Reviewed-by: Chanwoo Choi <cw00...@samsung.com>
(am from https://patchwork.kernel.org/patch/9284903)
---
M drivers/gpu/drm/rockchip/rockchip_drm_vop.c
1 file changed, 118 insertions(+), 5 deletions(-)


--
To view, visit https://chromium-review.googlesource.com/362907
To unsubscribe, visit https://chromium-review.googlesource.com/settings

Gerrit-MessageType: newpatchset
Gerrit-Change-Id: I690ae1c0d2436b957fc823585c1a32b48cfcf3fd
Gerrit-PatchSet: 7

Lin Huang (Gerrit)

unread,
Aug 21, 2016, 11:55:18 PM8/21/16
to Douglas Anderson, Stéphane Marchesin, Nick Sanders, Derek Basehore
Lin Huang has uploaded a new patch set (#10). (
https://chromium-review.googlesource.com/362907 )

Change subject: FROMLIST: drm/rockchip: Add dmc notifier in vop driver
......................................................................

FROMLIST: drm/rockchip: Add dmc notifier in vop driver

when in ddr frequency scaling process, vop can not do enable or
disable operation, since in dcf we check vop clock to see whether
vop work. If vop work, dcf do ddr frequency scaling when vop
in vblank status, and we need to read vop register to check whether
vop go into vblank status. If vop not work, dcf can do ddr frequency
any time. So when do ddr frequency scaling, you disabled or enable
vop, there may two bad thing happen: 1, the panel flicker(when vop from
disable status change to enable). 2, kernel hang (when vop from enable
status change to disable, dcf need to read vblank status, but if you disable
vop clock, it can not get the status, it will lead soc dead) So we need
register to devfreq notifier, and we can get the dmc status. Also, when
there have two vop enabled, we need to disable dmc, since dcf only base
on one vop vblank time, so the other panel will flicker when do ddr
frequency scaling.

BUG=chrome-os-partner:54651
TEST=Boot from kevin

Change-Id: I690ae1c0d2436b957fc823585c1a32b48cfcf3fd
Signed-off-by: Lin Huang <h...@rock-chips.com>
Reviewed-by: Chanwoo Choi <cw00...@samsung.com>
(am from https://patchwork.kernel.org/patch/9292843)
---
M drivers/gpu/drm/rockchip/rockchip_drm_vop.c
1 file changed, 118 insertions(+), 5 deletions(-)


--
To view, visit https://chromium-review.googlesource.com/362907
To unsubscribe, visit https://chromium-review.googlesource.com/settings

Gerrit-MessageType: newpatchset
Gerrit-Change-Id: I690ae1c0d2436b957fc823585c1a32b48cfcf3fd
Gerrit-PatchSet: 10

Stéphane Marchesin (Gerrit)

unread,
Aug 24, 2016, 1:49:19 AM8/24/16
to Lin Huang, Nick Sanders, Douglas Anderson, Derek Basehore
Stéphane Marchesin has posted comments on this change. (
https://chromium-review.googlesource.com/362907 )

Change subject: FROMLIST: drm/rockchip: Add dmc notifier in vop driver
......................................................................


Patch Set 11:

(7 comments)

https://chromium-review.googlesource.com/#/c/362907/11/drivers/gpu/drm/rockchip/rockchip_drm_vop.c
File drivers/gpu/drm/rockchip/rockchip_drm_vop.c:

Line 16: #include <linux/devfreq-event.h>
why not put these with the other linux/ includes below?


Line 526: "Timeout waiting for vop swtich status\n");
shouldn't you error out in this case?


Line 547: * use 100ms as timeout time.
this isn't 100ms, is it?


Line 646: * use 100ms as timeout time.
same


Line 711: /* check how many vop use now */
"count how many VOPs are in use"


Line 712: drm_for_each_crtc(crtc, vop->drm_dev) {
kernel style doesn't require braces here


Line 720: */
it doesn't necessarily mean a 2->1 change (you could be re-disabling the
same one which is 1->1).

But it doesn't matter since we want to enable DMC if we have exactly 1 VOP
enabled and it's the internal one. So we should change this to if
(num_enabled_crtc == 1 && is_little_vop)
Gerrit-MessageType: comment
Gerrit-Change-Id: I690ae1c0d2436b957fc823585c1a32b48cfcf3fd
Gerrit-PatchSet: 11
Gerrit-Project: chromiumos/third_party/kernel
Gerrit-Branch: chromeos-4.4
Gerrit-Owner: Lin Huang <h...@rock-chips.com>
Gerrit-Reviewer: Derek Basehore <dbas...@chromium.org>
Gerrit-Reviewer: Douglas Anderson <dian...@chromium.org>
Gerrit-Reviewer: Nick Sanders <nsan...@chromium.org>
Gerrit-Reviewer: Stéphane Marchesin <mar...@chromium.org>
Gerrit-HasComments: Yes

Lin Huang (Gerrit)

unread,
Aug 24, 2016, 12:22:07 PM8/24/16
to Sean Paul, Stéphane Marchesin, Nick Sanders, Douglas Anderson, Derek Basehore
Lin Huang has posted comments on this change. (
https://chromium-review.googlesource.com/362907 )

Change subject: FROMLIST: drm/rockchip: Add dmc notifier in vop driver
......................................................................


Patch Set 11:

(4 comments)

https://chromium-review.googlesource.com/#/c/362907/11/drivers/gpu/drm/rockchip/rockchip_drm_vop.c
File drivers/gpu/drm/rockchip/rockchip_drm_vop.c:

Line 16: #include <linux/devfreq-event.h>
> why not put these with the other linux/ includes below?
it's upstream .h file, and it has been there, so....


Line 526: "Timeout waiting for vop swtich status\n");
> shouldn't you error out in this case?
i think we can keep do ddr frequency scaling even it timeout.


Line 547: * use 100ms as timeout time.
> this isn't 100ms, is it?
yes, will fix next version


Line 720: */
> it doesn't necessarily mean a 2->1 change (you could be re-disabling the
> sa
is it internal panel alway use little_vop? And some device may only have
external display, like chromebit or box.


--
To view, visit https://chromium-review.googlesource.com/362907
To unsubscribe, visit https://chromium-review.googlesource.com/settings

Gerrit-MessageType: comment
Gerrit-Change-Id: I690ae1c0d2436b957fc823585c1a32b48cfcf3fd
Gerrit-PatchSet: 11
Gerrit-Project: chromiumos/third_party/kernel
Gerrit-Branch: chromeos-4.4
Gerrit-Owner: Lin Huang <h...@rock-chips.com>
Gerrit-Reviewer: Derek Basehore <dbas...@chromium.org>
Gerrit-Reviewer: Douglas Anderson <dian...@chromium.org>
Gerrit-Reviewer: Lin Huang <h...@rock-chips.com>
Gerrit-Reviewer: Nick Sanders <nsan...@chromium.org>
Gerrit-Reviewer: Sean Paul <sean...@chromium.org>

Stéphane Marchesin (Gerrit)

unread,
Aug 24, 2016, 8:05:04 PM8/24/16
to Lin Huang, Sean Paul, Nick Sanders, Douglas Anderson, Derek Basehore
Stéphane Marchesin has posted comments on this change. (
https://chromium-review.googlesource.com/362907 )

Change subject: FROMLIST: drm/rockchip: Add dmc notifier in vop driver
......................................................................


Patch Set 11:

(2 comments)

https://chromium-review.googlesource.com/#/c/362907/11/drivers/gpu/drm/rockchip/rockchip_drm_vop.c
File drivers/gpu/drm/rockchip/rockchip_drm_vop.c:

Line 16: #include <linux/devfreq-event.h>
> it's upstream .h file, and it has been there, so....
if upstream gets it wrong, doesn't mean we also should :)


Line 720: */
> is it internal panel alway use little_vop? And some device may only have
> ex
Right now it is always using the little vop, but sadly we don't want to
keep doing this forever (we want to switch dynamically). It is probably
better to look at the connector and check if its type is eDP.

Yakir Yang (Gerrit)

unread,
Aug 25, 2016, 12:18:41 AM8/25/16
to Lin Huang, Sean Paul, Stéphane Marchesin, Douglas Anderson, Nick Sanders, Derek Basehore
Hello Sean Paul, Lin Huang, Stéphane Marchesin,

I'd like you to reexamine a change. Please visit

https://chromium-review.googlesource.com/362907

to look at the new patch set (#12).

Change subject: FROMLIST: drm/rockchip: Add dmc notifier in vop driver
......................................................................

FROMLIST: drm/rockchip: Add dmc notifier in vop driver

when in ddr frequency scaling process, vop can not do enable or
disable operation, since in dcf we check vop clock to see whether
vop work. If vop work, dcf do ddr frequency scaling when vop
in vblank status, and we need to read vop register to check whether
vop go into vblank status. If vop not work, dcf can do ddr frequency
any time. So when do ddr frequency scaling, you disabled or enable
vop, there may two bad thing happen: 1, the panel flicker(when vop from
disable status change to enable). 2, kernel hang (when vop from enable
status change to disable, dcf need to read vblank status, but if you disable
vop clock, it can not get the status, it will lead soc dead) So we need
register to devfreq notifier, and we can get the dmc status. Also, when
there have two vop enabled, we need to disable dmc, since dcf only base
on one vop vblank time, so the other panel will flicker when do ddr
frequency scaling.

BUG=chrome-os-partner:54651
TEST=Boot from kevin

Change-Id: I690ae1c0d2436b957fc823585c1a32b48cfcf3fd
Signed-off-by: Lin Huang <h...@rock-chips.com>
Reviewed-by: Chanwoo Choi <cw00...@samsung.com>
(am from https://patchwork.kernel.org/patch/9292843)
---
M drivers/gpu/drm/rockchip/rockchip_drm_vop.c
1 file changed, 118 insertions(+), 0 deletions(-)
Gerrit-MessageType: newpatchset
Gerrit-Change-Id: I690ae1c0d2436b957fc823585c1a32b48cfcf3fd
Gerrit-PatchSet: 12
Gerrit-Project: chromiumos/third_party/kernel
Gerrit-Branch: chromeos-4.4
Gerrit-Owner: Lin Huang <h...@rock-chips.com>
Gerrit-Reviewer: Derek Basehore <dbas...@chromium.org>
Gerrit-Reviewer: Douglas Anderson <dian...@chromium.org>
Gerrit-Reviewer: Lin Huang <h...@rock-chips.com>
Gerrit-Reviewer: Nick Sanders <nsan...@chromium.org>
Gerrit-Reviewer: Sean Paul <sean...@chromium.org>
Gerrit-Reviewer: Stéphane Marchesin <mar...@chromium.org>
Gerrit-Reviewer: Yakir Yang <y...@rock-chips.com>

Derek Basehore (Gerrit)

unread,
Aug 31, 2016, 8:03:49 PM8/31/16
to Lin Huang, Yakir Yang, Stéphane Marchesin, Sean Paul, Nick Sanders, Douglas Anderson
Derek Basehore has posted comments on this change. (
https://chromium-review.googlesource.com/362907 )

Change subject: FROMLIST: drm/rockchip: Add dmc notifier in vop driver
......................................................................


Patch Set 13:

This doesn't cleanly apply anymore.
Gerrit-MessageType: comment
Gerrit-Change-Id: I690ae1c0d2436b957fc823585c1a32b48cfcf3fd
Gerrit-PatchSet: 13
Gerrit-Project: chromiumos/third_party/kernel
Gerrit-Branch: chromeos-4.4
Gerrit-Owner: Lin Huang <h...@rock-chips.com>
Gerrit-Reviewer: Derek Basehore <dbas...@chromium.org>
Gerrit-Reviewer: Douglas Anderson <dian...@chromium.org>
Gerrit-Reviewer: Lin Huang <h...@rock-chips.com>
Gerrit-Reviewer: Nick Sanders <nsan...@chromium.org>
Gerrit-Reviewer: Sean Paul <sean...@chromium.org>
Gerrit-Reviewer: Stéphane Marchesin <mar...@chromium.org>
Gerrit-Reviewer: Yakir Yang <y...@rock-chips.com>
Gerrit-HasComments: No

Lin Huang (Gerrit)

unread,
Aug 31, 2016, 10:54:00 PM8/31/16
to Sean Paul, Stéphane Marchesin, Yakir Yang, Douglas Anderson, Nick Sanders, Derek Basehore
Hello Sean Paul, Stéphane Marchesin, Yakir Yang,

I'd like you to reexamine a change. Please visit

https://chromium-review.googlesource.com/362907

to look at the new patch set (#14).

Change subject: FROMLIST: drm/rockchip: Add dmc notifier in vop driver
......................................................................

FROMLIST: drm/rockchip: Add dmc notifier in vop driver

when in ddr frequency scaling process, vop can not do enable or
disable operation, since in dcf we check vop clock to see whether
vop work. If vop work, dcf do ddr frequency scaling when vop
in vblank status, and we need to read vop register to check whether
vop go into vblank status. If vop not work, dcf can do ddr frequency
any time. So when do ddr frequency scaling, you disabled or enable
vop, there may two bad thing happen: 1, the panel flicker(when vop from
disable status change to enable). 2, kernel hang (when vop from enable
status change to disable, dcf need to read vblank status, but if you disable
vop clock, it can not get the status, it will lead soc dead) So we need
register to devfreq notifier, and we can get the dmc status. Also, when
there have two vop enabled, we need to disable dmc, since dcf only base
on one vop vblank time, so the other panel will flicker when do ddr
frequency scaling.

BUG=chrome-os-partner:54651
TEST=Boot from kevin

Change-Id: I690ae1c0d2436b957fc823585c1a32b48cfcf3fd
Signed-off-by: Lin Huang <h...@rock-chips.com>
Reviewed-by: Chanwoo Choi <cw00...@samsung.com>
(am from https://patchwork.kernel.org/patch/9292843)
---
M drivers/gpu/drm/rockchip/rockchip_drm_vop.c
1 file changed, 115 insertions(+), 0 deletions(-)
Gerrit-MessageType: newpatchset
Gerrit-Change-Id: I690ae1c0d2436b957fc823585c1a32b48cfcf3fd
Gerrit-PatchSet: 14

Lin Huang (Gerrit)

unread,
Sep 1, 2016, 6:50:51 PM9/1/16
to Sean Paul, Stéphane Marchesin, Yakir Yang, Douglas Anderson, Nick Sanders, Derek Basehore
Hello Sean Paul, Stéphane Marchesin, Yakir Yang,

I'd like you to reexamine a change. Please visit

https://chromium-review.googlesource.com/362907

to look at the new patch set (#15).

Change subject: FROMLIST: drm/rockchip: Add dmc notifier in vop driver
......................................................................

FROMLIST: drm/rockchip: Add dmc notifier in vop driver

when in ddr frequency scaling process, vop can not do enable or
disable operation, since in dcf we check vop clock to see whether
vop work. If vop work, dcf do ddr frequency scaling when vop
in vblank status, and we need to read vop register to check whether
vop go into vblank status. If vop not work, dcf can do ddr frequency
any time. So when do ddr frequency scaling, you disabled or enable
vop, there may two bad thing happen: 1, the panel flicker(when vop from
disable status change to enable). 2, kernel hang (when vop from enable
status change to disable, dcf need to read vblank status, but if you disable
vop clock, it can not get the status, it will lead soc dead) So we need
register to devfreq notifier, and we can get the dmc status. Also, when
there have two vop enabled, we need to disable dmc, since dcf only base
on one vop vblank time, so the other panel will flicker when do ddr
frequency scaling.

BUG=chrome-os-partner:54651
TEST=Boot from kevin

Change-Id: I690ae1c0d2436b957fc823585c1a32b48cfcf3fd
Signed-off-by: Lin Huang <h...@rock-chips.com>
Reviewed-by: Chanwoo Choi <cw00...@samsung.com>
(am from https://patchwork.kernel.org/patch/9309975)
---
M drivers/gpu/drm/rockchip/rockchip_drm_vop.c
1 file changed, 115 insertions(+), 0 deletions(-)


--
To view, visit https://chromium-review.googlesource.com/362907
To unsubscribe, visit https://chromium-review.googlesource.com/settings

Gerrit-MessageType: newpatchset
Gerrit-Change-Id: I690ae1c0d2436b957fc823585c1a32b48cfcf3fd
Gerrit-PatchSet: 15

Lin Huang (Gerrit)

unread,
Sep 6, 2016, 2:03:44 PM9/6/16
to Sean Paul, Stéphane Marchesin, Yakir Yang, Douglas Anderson, Nick Sanders, Derek Basehore
Hello Sean Paul, Stéphane Marchesin, Yakir Yang,

I'd like you to reexamine a change. Please visit

https://chromium-review.googlesource.com/362907

to look at the new patch set (#16).

Change subject: FROMLIST: drm/rockchip: Add dmc notifier in vop driver
......................................................................

FROMLIST: drm/rockchip: Add dmc notifier in vop driver

when in ddr frequency scaling process, vop can not do enable or
disable operation, since in dcf we check vop clock to see whether
vop work. If vop work, dcf do ddr frequency scaling when vop
in vblank status, and we need to read vop register to check whether
vop go into vblank status. If vop not work, dcf can do ddr frequency
any time. So when do ddr frequency scaling, you disabled or enable
vop, there may two bad thing happen: 1, the panel flicker(when vop from
disable status change to enable). 2, kernel hang (when vop from enable
status change to disable, dcf need to read vblank status, but if you disable
vop clock, it can not get the status, it will lead soc dead) So we need
register to devfreq notifier, and we can get the dmc status. Also, when
there have two vop enabled, we need to disable dmc, since dcf only base
on one vop vblank time, so the other panel will flicker when do ddr
frequency scaling.

BUG=chrome-os-partner:54651
TEST=Boot from kevin

Change-Id: I690ae1c0d2436b957fc823585c1a32b48cfcf3fd
Signed-off-by: Lin Huang <h...@rock-chips.com>
Reviewed-by: Chanwoo Choi <cw00...@samsung.com>
Reviewed-by: MyungJoo Ham <myungj...@samsung.com>
(am from https://patchwork.kernel.org/patch/9309975)
---
M drivers/gpu/drm/rockchip/rockchip_drm_vop.c
1 file changed, 115 insertions(+), 0 deletions(-)


--
To view, visit https://chromium-review.googlesource.com/362907
To unsubscribe, visit https://chromium-review.googlesource.com/settings

Gerrit-MessageType: newpatchset
Gerrit-Change-Id: I690ae1c0d2436b957fc823585c1a32b48cfcf3fd
Gerrit-PatchSet: 16

Lin Huang (Gerrit)

unread,
Sep 6, 2016, 6:32:47 PM9/6/16
to Yakir Yang, Stéphane Marchesin, Sean Paul, Nick Sanders, Douglas Anderson, Derek Basehore
Lin Huang has posted comments on this change. (
https://chromium-review.googlesource.com/362907 )

Change subject: FROMLIST: drm/rockchip: Add dmc notifier in vop driver
......................................................................


Patch Set 16:

Sean, can we move this patch disscussion to here:
now when dmc_in_progress = 1, vop can not do vop_enable/disable,
when vop_switch_status = 1, dmc can not do ddr dvfs. And we can not do the
vop_disable when vop_enable do not finish, right? Also, in this code, we
have
if (vop->is_enabled)
return 0;
in vop_enable(), and
if (!vop->is_enabled)
return 0;
in vop_crtc_disable(),
so i think it can work.
Gerrit-MessageType: comment
Gerrit-Change-Id: I690ae1c0d2436b957fc823585c1a32b48cfcf3fd
Gerrit-PatchSet: 16
Gerrit-Project: chromiumos/third_party/kernel
Gerrit-Branch: chromeos-4.4
Gerrit-Owner: Lin Huang <h...@rock-chips.com>
Gerrit-Reviewer: Derek Basehore <dbas...@chromium.org>
Gerrit-Reviewer: Douglas Anderson <dian...@chromium.org>
Gerrit-Reviewer: Lin Huang <h...@rock-chips.com>
Gerrit-Reviewer: Nick Sanders <nsan...@chromium.org>
Gerrit-Reviewer: Sean Paul <sean...@chromium.org>
Gerrit-Reviewer: Stéphane Marchesin <mar...@chromium.org>
Gerrit-Reviewer: Yakir Yang <y...@rock-chips.com>
Gerrit-HasComments: No

Derek Basehore (Gerrit)

unread,
Sep 7, 2016, 3:58:11 AM9/7/16
to Lin Huang, Caesar Wang, Yakir Yang, Stéphane Marchesin, Sean Paul, Nick Sanders, Douglas Anderson
Derek Basehore has posted comments on this change. (
https://chromium-review.googlesource.com/362907 )

Change subject: FROMLIST: drm/rockchip: Add dmc notifier in vop driver
......................................................................


Patch Set 11:

Lin, can you link to the discussion you were having with Sean?

--
To view, visit https://chromium-review.googlesource.com/362907
To unsubscribe, visit https://chromium-review.googlesource.com/settings

Gerrit-MessageType: comment
Gerrit-Change-Id: I690ae1c0d2436b957fc823585c1a32b48cfcf3fd
Gerrit-PatchSet: 11
Gerrit-Project: chromiumos/third_party/kernel
Gerrit-Branch: chromeos-4.4
Gerrit-Owner: Lin Huang <h...@rock-chips.com>
Gerrit-Reviewer: Caesar Wang <w...@rock-chips.com>

Lin Huang (Gerrit)

unread,
Sep 7, 2016, 12:46:26 PM9/7/16
to Caesar Wang, Yakir Yang, Stéphane Marchesin, Sean Paul, Nick Sanders, Douglas Anderson, Derek Basehore
Lin Huang has posted comments on this change. (
https://chromium-review.googlesource.com/362907 )

Change subject: FROMLIST: drm/rockchip: Add dmc notifier in vop driver
......................................................................


Patch Set 17:

> Lin, can you link to the discussion you were having with Sean?

https://patchwork.kernel.org/patch/9313045/
--
To view, visit https://chromium-review.googlesource.com/362907
To unsubscribe, visit https://chromium-review.googlesource.com/settings

Gerrit-MessageType: comment
Gerrit-Change-Id: I690ae1c0d2436b957fc823585c1a32b48cfcf3fd
Gerrit-PatchSet: 17

Sean Paul (Gerrit)

unread,
Sep 7, 2016, 12:53:00 PM9/7/16
to Caesar Wang, Lin Huang, Sean Paul, Yakir Yang, Stéphane Marchesin, Nick Sanders, Douglas Anderson, Derek Basehore
Sean Paul has posted comments on this change. (
https://chromium-review.googlesource.com/362907 )

Change subject: FROMLIST: drm/rockchip: Add dmc notifier in vop driver
......................................................................


Patch Set 17:

Why are we moving this discussion away from upstream? If you hope to land
this patch upstream, it's probably best to defend it there, rather than
here.

Derek Basehore (Gerrit)

unread,
Sep 7, 2016, 3:39:32 PM9/7/16
to Caesar Wang, Lin Huang, Sean Paul, Yakir Yang, Stéphane Marchesin, Nick Sanders, Douglas Anderson
Derek Basehore has posted comments on this change. (
https://chromium-review.googlesource.com/362907 )

Change subject: FROMLIST: drm/rockchip: Add dmc notifier in vop driver
......................................................................


Patch Set 17:

Based on the discussion upstream, this patch will be getting some changes.

Sean Paul (Gerrit)

unread,
Sep 16, 2016, 2:30:36 PM9/16/16
to Sean Paul, Lin Huang, Caesar Wang, Stéphane Marchesin, Yakir Yang, Douglas Anderson, Nick Sanders, Derek Basehore
Hello Caesar Wang, Lin Huang, Stéphane Marchesin, Yakir Yang,

I'd like you to reexamine a change. Please visit

https://chromium-review.googlesource.com/362907

to look at the new patch set (#19).

Change subject: CHROMIUM: drm/rockchip: Synchronize enable/disable with dmc
......................................................................

CHROMIUM: drm/rockchip: Synchronize enable/disable with dmc

Add a dmc notifier to the vop driver to ensure that we're not in
the middle of a devfreq change while enabling/disabling. Likewise,
ensure that we don't initiate a devfreq change during enable/disable.

Originally based on code from Lin Huang <h...@rock-chips.com>, seanpaul
added proper synchronization

BUG=chrome-os-partner:54651
TEST=kevin compiles/boots

Signed-off-by: Sean Paul <sean...@chromium.org>
Change-Id: I690ae1c0d2436b957fc823585c1a32b48cfcf3fd
---
M drivers/gpu/drm/rockchip/rockchip_drm_vop.c
1 file changed, 97 insertions(+), 6 deletions(-)
Gerrit-MessageType: newpatchset
Gerrit-Change-Id: I690ae1c0d2436b957fc823585c1a32b48cfcf3fd
Gerrit-PatchSet: 19

Derek Basehore (Gerrit)

unread,
Sep 16, 2016, 3:57:04 PM9/16/16
to Sean Paul, Lin Huang, Caesar Wang, Yakir Yang, Stéphane Marchesin, Nick Sanders, Douglas Anderson
Derek Basehore has posted comments on this change. (
https://chromium-review.googlesource.com/362907 )

Change subject: CHROMIUM: drm/rockchip: Synchronize enable/disable with dmc
......................................................................


Patch Set 19:

(2 comments)

https://chromium-review.googlesource.com/#/c/362907/19/drivers/gpu/drm/rockchip/rockchip_drm_vop.c
File drivers/gpu/drm/rockchip/rockchip_drm_vop.c:

Line 1578: DEVFREQ_TRANSITION_NOTIFIER);
You should be able to do this after calling
devfreq_event_get_edev_by_phandle. This way you don't need to unregister in
the cleanup code below.


Line 1671: return vop_create_crtc(vop);
Do you not need to undo the disable_irq anymore on failure?

You also need to undo registration of the devfreq notifiers. It looks like
we don't do that in unbind either.
Gerrit-MessageType: comment
Gerrit-Change-Id: I690ae1c0d2436b957fc823585c1a32b48cfcf3fd
Gerrit-PatchSet: 19
Gerrit-Project: chromiumos/third_party/kernel
Gerrit-Branch: chromeos-4.4
Gerrit-Owner: Lin Huang <h...@rock-chips.com>
Gerrit-Reviewer: Caesar Wang <w...@rock-chips.com>
Gerrit-Reviewer: Derek Basehore <dbas...@chromium.org>
Gerrit-Reviewer: Douglas Anderson <dian...@chromium.org>
Gerrit-Reviewer: Lin Huang <h...@rock-chips.com>
Gerrit-Reviewer: Nick Sanders <nsan...@chromium.org>
Gerrit-Reviewer: Sean Paul <sean...@chromium.org>
Gerrit-Reviewer: Stéphane Marchesin <mar...@chromium.org>
Gerrit-Reviewer: Yakir Yang <y...@rock-chips.com>
Gerrit-HasComments: Yes

Sean Paul (Gerrit)

unread,
Sep 16, 2016, 4:05:39 PM9/16/16
to Sean Paul, Lin Huang, Caesar Wang, Yakir Yang, Stéphane Marchesin, Nick Sanders, Douglas Anderson, Derek Basehore
Sean Paul has posted comments on this change. (
https://chromium-review.googlesource.com/362907 )

Change subject: CHROMIUM: drm/rockchip: Synchronize enable/disable with dmc
......................................................................


Patch Set 19:

(2 comments)

https://chromium-review.googlesource.com/#/c/362907/19/drivers/gpu/drm/rockchip/rockchip_drm_vop.c
File drivers/gpu/drm/rockchip/rockchip_drm_vop.c:

Line 1578: DEVFREQ_TRANSITION_NOTIFIER);
> You should be able to do this after calling
> devfreq_event_get_edev_by_phand
Yeah, I had that initially and it resulted in a system hang.

I can give it another shot, perhaps it was a fluke


Line 1671: return vop_create_crtc(vop);
> Do you not need to undo the disable_irq anymore on failure?
Ah, yeah, brain cramp.

Derek Basehore (Gerrit)

unread,
Sep 16, 2016, 4:10:28 PM9/16/16
to Sean Paul, Lin Huang, Caesar Wang, Yakir Yang, Stéphane Marchesin, Nick Sanders, Douglas Anderson
Derek Basehore has posted comments on this change. (
https://chromium-review.googlesource.com/362907 )

Change subject: CHROMIUM: drm/rockchip: Synchronize enable/disable with dmc
......................................................................


Patch Set 19:

(1 comment)

https://chromium-review.googlesource.com/#/c/362907/19/drivers/gpu/drm/rockchip/rockchip_drm_vop.c
File drivers/gpu/drm/rockchip/rockchip_drm_vop.c:

Line 1578: DEVFREQ_TRANSITION_NOTIFIER);
> Yeah, I had that initially and it resulted in a system hang.
There were some issues that popped up with the firmware breaking ddrfreq
completely. If you have firmware from last week or earlier, then you won't
have the issue.

If you do, revert "rockchip: rk3399: improve write leveling flow" in
coreboot. This issue is in the middle of being resolved.

Sean Paul (Gerrit)

unread,
Sep 16, 2016, 4:32:54 PM9/16/16
to Sean Paul, Lin Huang, Caesar Wang, Stéphane Marchesin, Yakir Yang, Douglas Anderson, Nick Sanders, Derek Basehore
Hello Caesar Wang, Lin Huang, Stéphane Marchesin, Yakir Yang,

I'd like you to reexamine a change. Please visit

https://chromium-review.googlesource.com/362907

to look at the new patch set (#20).

Change subject: CHROMIUM: drm/rockchip: Synchronize enable/disable with dmc
......................................................................

CHROMIUM: drm/rockchip: Synchronize enable/disable with dmc

Add a dmc notifier to the vop driver to ensure that we're not in
the middle of a devfreq change while enabling/disabling. Likewise,
ensure that we don't initiate a devfreq change during enable/disable.

Originally based on code from Lin Huang <h...@rock-chips.com>, seanpaul
added proper synchronization

BUG=chrome-os-partner:54651
TEST=kevin compiles/boots

Signed-off-by: Sean Paul <sean...@chromium.org>
Change-Id: I690ae1c0d2436b957fc823585c1a32b48cfcf3fd
---
M drivers/gpu/drm/rockchip/rockchip_drm_vop.c
1 file changed, 96 insertions(+), 4 deletions(-)
Gerrit-MessageType: newpatchset
Gerrit-Change-Id: I690ae1c0d2436b957fc823585c1a32b48cfcf3fd
Gerrit-PatchSet: 20

Derek Basehore (Gerrit)

unread,
Sep 16, 2016, 4:37:11 PM9/16/16
to Sean Paul, Lin Huang, Caesar Wang, Yakir Yang, Stéphane Marchesin, Nick Sanders, Douglas Anderson
Derek Basehore has posted comments on this change. (
https://chromium-review.googlesource.com/362907 )

Change subject: CHROMIUM: drm/rockchip: Synchronize enable/disable with dmc
......................................................................


Patch Set 20:

(1 comment)

https://chromium-review.googlesource.com/#/c/362907/20/drivers/gpu/drm/rockchip/rockchip_drm_vop.c
File drivers/gpu/drm/rockchip/rockchip_drm_vop.c:

Line 1669: if (ret)
Should remove the notifiers in this case. Should also remove the notifiers
in unbind.
Gerrit-MessageType: comment
Gerrit-Change-Id: I690ae1c0d2436b957fc823585c1a32b48cfcf3fd
Gerrit-PatchSet: 20
Gerrit-Project: chromiumos/third_party/kernel
Gerrit-Branch: chromeos-4.4
Gerrit-Owner: Lin Huang <h...@rock-chips.com>
Gerrit-Reviewer: Caesar Wang <w...@rock-chips.com>
Gerrit-Reviewer: Derek Basehore <dbas...@chromium.org>
Gerrit-Reviewer: Douglas Anderson <dian...@chromium.org>
Gerrit-Reviewer: Lin Huang <h...@rock-chips.com>
Gerrit-Reviewer: Nick Sanders <nsan...@chromium.org>
Gerrit-Reviewer: Sean Paul <sean...@chromium.org>
Gerrit-Reviewer: Stéphane Marchesin <mar...@chromium.org>
Gerrit-Reviewer: Yakir Yang <y...@rock-chips.com>
Gerrit-HasComments: Yes

Sean Paul (Gerrit)

unread,
Sep 16, 2016, 4:40:20 PM9/16/16
to Sean Paul, Lin Huang, Caesar Wang, Yakir Yang, Stéphane Marchesin, Nick Sanders, Douglas Anderson, Derek Basehore
Sean Paul has posted comments on this change. (
https://chromium-review.googlesource.com/362907 )

Change subject: CHROMIUM: drm/rockchip: Synchronize enable/disable with dmc
......................................................................


Patch Set 20:

(1 comment)

https://chromium-review.googlesource.com/#/c/362907/20/drivers/gpu/drm/rockchip/rockchip_drm_vop.c
File drivers/gpu/drm/rockchip/rockchip_drm_vop.c:

Line 1669: if (ret)
> Should remove the notifiers in this case. Should also remove the notifiers
Aren't they device managed?

Derek Basehore (Gerrit)

unread,
Sep 16, 2016, 4:42:56 PM9/16/16
to Sean Paul, Lin Huang, Caesar Wang, Yakir Yang, Stéphane Marchesin, Nick Sanders, Douglas Anderson
Derek Basehore has posted comments on this change. (
https://chromium-review.googlesource.com/362907 )

Change subject: CHROMIUM: drm/rockchip: Synchronize enable/disable with dmc
......................................................................


Patch Set 20: Code-Review+2

(1 comment)

https://chromium-review.googlesource.com/#/c/362907/20/drivers/gpu/drm/rockchip/rockchip_drm_vop.c
File drivers/gpu/drm/rockchip/rockchip_drm_vop.c:

Line 1669: if (ret)
> Aren't they device managed?
Oops, I missed the devm_*

Everything looks fine in this patch then.

Lin Huang (Gerrit)

unread,
Sep 18, 2016, 6:45:32 AM9/18/16
to Sean Paul, Caesar Wang, Stéphane Marchesin, Derek Basehore, Yakir Yang, Douglas Anderson, Nick Sanders
Hello Sean Paul, Caesar Wang, Stéphane Marchesin, Derek Basehore, Yakir
Yang,

I'd like you to reexamine a change. Please visit

https://chromium-review.googlesource.com/362907

to look at the new patch set (#21).

Change subject: FROMLIST: drm/rockchip: Add dmc notifier in vop driver
......................................................................

FROMLIST: drm/rockchip: Add dmc notifier in vop driver

when in ddr frequency scaling process, vop can not do enable or
disable operation, since in dcf we check vop clock to see whether
vop work. If vop work, dcf do ddr frequency scaling when vop
in vblank status, and we need to read vop register to check whether
vop go into vblank status. If vop not work, dcf can do ddr frequency
any time. So when do ddr frequency scaling, you disabled or enable
vop, there may two bad thing happen: 1, the panel flicker(when vop from
disable status change to enable). 2, kernel hang (when vop from enable
status change to disable, dcf need to read vblank status, but if you disable
vop clock, it can not get the status, it will lead soc dead) So we need
register to devfreq notifier, and we can get the dmc status. Also, when
there have two vop enabled, we need to disable dmc, since dcf only base
on one vop vblank time, so the other panel will flicker when do ddr
frequency scaling.

BUG=chrome-os-partner:54651
TEST=Boot from kevin

Change-Id: I690ae1c0d2436b957fc823585c1a32b48cfcf3fd
Signed-off-by: Lin Huang <h...@rock-chips.com>
Reviewed-by: Chanwoo Choi <cw00...@samsung.com>
Reviewed-by: MyungJoo Ham <myungj...@samsung.com>
(am from https://patchwork.kernel.org/patch/9309975)
---
M drivers/gpu/drm/rockchip/rockchip_drm_vop.c
1 file changed, 115 insertions(+), 0 deletions(-)
Gerrit-MessageType: newpatchset
Gerrit-Change-Id: I690ae1c0d2436b957fc823585c1a32b48cfcf3fd
Gerrit-PatchSet: 21

Sean Paul (Gerrit)

unread,
Sep 19, 2016, 12:51:30 PM9/19/16
to Sean Paul, Lin Huang, Caesar Wang, Stéphane Marchesin, Derek Basehore, Yakir Yang, Douglas Anderson, Nick Sanders
Hello Caesar Wang, Lin Huang, Stéphane Marchesin, Derek Basehore, Yakir
Yang,

I'd like you to reexamine a change. Please visit

https://chromium-review.googlesource.com/362907

to look at the new patch set (#22).

Change subject: CHROMIUM: drm/rockchip: Toggle devfreq on/off
......................................................................

CHROMIUM: drm/rockchip: Toggle devfreq on/off

This patch adds devfreq to the rockchip drm driver. It will activate
when there is one crtc active, and disable if more than one becomes
active (to avoid flickering on one of the screens).

BUG=chrome-os-partner:54651
TEST=kevin compiles/boots

Signed-off-by: Sean Paul <sean...@chromium.org>
Change-Id: I690ae1c0d2436b957fc823585c1a32b48cfcf3fd
---
M drivers/gpu/drm/rockchip/rockchip_drm_drv.c
M drivers/gpu/drm/rockchip/rockchip_drm_drv.h
M drivers/gpu/drm/rockchip/rockchip_drm_fb.c
3 files changed, 136 insertions(+), 0 deletions(-)


--
To view, visit https://chromium-review.googlesource.com/362907
To unsubscribe, visit https://chromium-review.googlesource.com/settings

Gerrit-MessageType: newpatchset
Gerrit-Change-Id: I690ae1c0d2436b957fc823585c1a32b48cfcf3fd
Gerrit-PatchSet: 22

Derek Basehore (Gerrit)

unread,
Sep 26, 2016, 11:17:32 PM9/26/16
to Sean Paul, Lin Huang, Caesar Wang, Yakir Yang, Stéphane Marchesin, Nick Sanders, Douglas Anderson
Derek Basehore has posted comments on this change. (
https://chromium-review.googlesource.com/362907 )

Change subject: CHROMIUM: drm/rockchip: Toggle devfreq on/off
......................................................................


Patch Set 23:

This appears to cause a hung task:
[ 240.111722] INFO: task kworker/5:1:99 blocked for more than 120 seconds.
[ 240.111736] Not tainted 4.4.14 #77
[ 240.111741] "echo 0 > /proc/sys/kernel/hung_task_timeout_secs" disables
this message.
[ 240.111745] kworker/5:1 D ffffffc000204fd8 0 99 2
0x00000000
[ 240.111771] Workqueue: events rockchip_drm_atomic_work
[ 240.111778] Call trace:
[ 240.111789] [<ffffffc000204fd8>] __switch_to+0x9c/0xa8
[ 240.111798] [<ffffffc0009023ac>] __schedule+0x440/0x6d8
[ 240.111802] [<ffffffc0009026dc>] schedule+0x98/0xb8
[ 240.111807] [<ffffffc000905814>] schedule_timeout+0x44/0x27c
[ 240.111813] [<ffffffc000603888>] fence_default_wait+0x1ac/0x2ac
[ 240.111819] [<ffffffc000602df8>] fence_wait_timeout+0x10c/0x1f8
[ 240.111823] [<ffffffc00059aacc>]
rockchip_atomic_commit_complete+0x16c/0x238
[ 240.111827] [<ffffffc00059aca4>] rockchip_drm_atomic_work+0x20/0x2c
[ 240.111834] [<ffffffc0002392e8>] process_one_work+0x240/0x424
[ 240.111839] [<ffffffc000239d90>] worker_thread+0x304/0x424
[ 240.111844] [<ffffffc00023f0b8>] kthread+0x10c/0x114
[ 240.111850] [<ffffffc000203c50>] ret_from_fork+0x10/0x40
[ 240.111853] task PC stack pid father
[ 240.111859] kworker/5:1 D ffffffc000204fd8 0 99 2
0x00000000
[ 240.111869] Workqueue: events rockchip_drm_atomic_work
[ 240.111873] Call trace:
[ 240.111878] [<ffffffc000204fd8>] __switch_to+0x9c/0xa8
[ 240.111882] [<ffffffc0009023ac>] __schedule+0x440/0x6d8
[ 240.111887] [<ffffffc0009026dc>] schedule+0x98/0xb8
[ 240.111891] [<ffffffc000905814>] schedule_timeout+0x44/0x27c
[ 240.111896] [<ffffffc000603888>] fence_default_wait+0x1ac/0x2ac
[ 240.111900] [<ffffffc000602df8>] fence_wait_timeout+0x10c/0x1f8
[ 240.111904] [<ffffffc00059aacc>]
rockchip_atomic_commit_complete+0x16c/0x238
[ 240.111908] [<ffffffc00059aca4>] rockchip_drm_atomic_work+0x20/0x2c
[ 240.111913] [<ffffffc0002392e8>] process_one_work+0x240/0x424
[ 240.111917] [<ffffffc000239d90>] worker_thread+0x304/0x424
[ 240.111921] [<ffffffc00023f0b8>] kthread+0x10c/0x114
[ 240.111925] [<ffffffc000203c50>] ret_from_fork+0x10/0x40
[ 240.111969] chrome D ffffffc000204fd8 0 1961 1
0x00400805
[ 240.111979] Call trace:
[ 240.111983] [<ffffffc000204fd8>] __switch_to+0x9c/0xa8
[ 240.111988] [<ffffffc0009023ac>] __schedule+0x440/0x6d8
[ 240.111992] [<ffffffc0009026dc>] schedule+0x98/0xb8
[ 240.111997] [<ffffffc000905814>] schedule_timeout+0x44/0x27c
[ 240.112001] [<ffffffc000903334>] wait_for_common+0xf8/0x198
[ 240.112005] [<ffffffc0009033fc>] wait_for_completion+0x28/0x34
[ 240.112010] [<ffffffc000238d68>] flush_work+0x16c/0x1a8
[ 240.112017] [<ffffffc000588660>] drm_fb_release+0xec/0x110
[ 240.112025] [<ffffffc000576dd4>] drm_release+0x2bc/0x444
[ 240.112031] [<ffffffc00034d304>] __fput+0x10c/0x1d4
[ 240.112036] [<ffffffc00034d440>] ____fput+0x20/0x2c
[ 240.112040] [<ffffffc00023d280>] task_work_run+0xa4/0xd4
[ 240.112046] [<ffffffc0002225ac>] do_exit+0x474/0x900
[ 240.112051] [<ffffffc000223b88>] do_group_exit+0x50/0xb0
[ 240.112058] [<ffffffc00022f27c>] get_signal+0x544/0x5c4
[ 240.112063] [<ffffffc000207af8>] do_signal+0xb0/0x554
[ 240.112068] [<ffffffc0002081d4>] do_notify_resume+0x28/0x6c
[ 240.112072] [<ffffffc000203ba8>] work_pending+0x1c/0x20
[ 240.112077] kworker/5:2 D ffffffc000204fd8 0 2048 2
0x00000000
[ 240.112088] Workqueue: events drm_mode_rmfb_work_fn
[ 240.112093] Call trace:
[ 240.112098] [<ffffffc000204fd8>] __switch_to+0x9c/0xa8
[ 240.112102] [<ffffffc0009023ac>] __schedule+0x440/0x6d8
[ 240.112107] [<ffffffc0009026dc>] schedule+0x98/0xb8
[ 240.112111] [<ffffffc000905814>] schedule_timeout+0x44/0x27c
[ 240.112115] [<ffffffc000903334>] wait_for_common+0xf8/0x198
[ 240.112120] [<ffffffc0009033fc>] wait_for_completion+0x28/0x34
[ 240.112124] [<ffffffc000238d68>] flush_work+0x16c/0x1a8
[ 240.112128] [<ffffffc00059abec>] rockchip_drm_atomic_commit+0x54/0xac
[ 240.112135] [<ffffffc0005948d4>] drm_atomic_commit+0x78/0x90
[ 240.112140] [<ffffffc00056feec>] drm_atomic_helper_set_config+0x5c/0xa8
[ 240.112145] [<ffffffc000582cd4>] drm_mode_set_config_internal+0x68/0xe4
[ 240.112149] [<ffffffc000582e94>] drm_framebuffer_remove+0xb8/0x160
[ 240.112154] [<ffffffc000582f84>] drm_mode_rmfb_work_fn+0x48/0x58
[ 240.112159] [<ffffffc0002392e8>] process_one_work+0x240/0x424
[ 240.112163] [<ffffffc000239df0>] worker_thread+0x364/0x424
[ 240.112168] [<ffffffc00023f0b8>] kthread+0x10c/0x114
[ 240.112172] [<ffffffc000203c50>] ret_from_fork+0x10/0x40
[ 240.112202] chrome D ffffffc000204fd8 0 7297 1
0x00400001
[ 240.112210] Call trace:
[ 240.112214] [<ffffffc000204fd8>] __switch_to+0x9c/0xa8
[ 240.112219] [<ffffffc0009023ac>] __schedule+0x440/0x6d8
[ 240.112222] [<ffffffc0009026dc>] schedule+0x98/0xb8
[ 240.112226] [<ffffffc000902a60>] schedule_preempt_disabled+0x28/0x44
[ 240.112231] [<ffffffc000904504>] __mutex_lock_slowpath+0x120/0x1ac
[ 240.112236] [<ffffffc0009045dc>] mutex_lock+0x4c/0x68
[ 240.112241] [<ffffffc00057d2c8>] drm_stub_open+0x5c/0x110
[ 240.112246] [<ffffffc0003501bc>] chrdev_open+0x154/0x19c
[ 240.112252] [<ffffffc0003491a8>] do_dentry_open+0x1d8/0x2f4
[ 240.112256] [<ffffffc00034a4e8>] vfs_open+0x74/0x84
[ 240.112262] [<ffffffc00035a100>] path_openat+0xaf8/0xdb0
[ 240.112266] [<ffffffc00035b41c>] do_filp_open+0x58/0xbc
[ 240.112271] [<ffffffc00034a908>] do_sys_open+0x184/0x2d8
[ 240.112277] [<ffffffc00039c050>] compat_SyS_open+0x38/0x48
[ 240.112281] [<ffffffc000203cb4>] el0_svc_naked+0x24/0x28
Gerrit-MessageType: comment
Gerrit-Change-Id: I690ae1c0d2436b957fc823585c1a32b48cfcf3fd
Gerrit-PatchSet: 23
Gerrit-Project: chromiumos/third_party/kernel
Gerrit-Branch: chromeos-4.4
Gerrit-Owner: Lin Huang <h...@rock-chips.com>
Gerrit-Reviewer: Caesar Wang <w...@rock-chips.com>
Gerrit-Reviewer: Derek Basehore <dbas...@chromium.org>
Gerrit-Reviewer: Douglas Anderson <dian...@chromium.org>
Gerrit-Reviewer: Lin Huang <h...@rock-chips.com>
Gerrit-Reviewer: Nick Sanders <nsan...@chromium.org>
Gerrit-Reviewer: Sean Paul <sean...@chromium.org>
Gerrit-Reviewer: Stéphane Marchesin <mar...@chromium.org>
Gerrit-Reviewer: Yakir Yang <y...@rock-chips.com>
Gerrit-HasComments: No

Sean Paul (Gerrit)

unread,
Sep 27, 2016, 9:55:16 AM9/27/16
to Sean Paul, Lin Huang, Derek Basehore, Caesar Wang, Yakir Yang, Stéphane Marchesin, Nick Sanders, Douglas Anderson
Sean Paul has posted comments on this change. (
https://chromium-review.googlesource.com/362907 )

Change subject: CHROMIUM: drm/rockchip: Toggle devfreq on/off
......................................................................


Patch Set 23:

That trace looks an awful lot like crosbug.com/p/57562, how do you repro it?

Derek Basehore (Gerrit)

unread,
Sep 27, 2016, 3:21:34 PM9/27/16
to Sean Paul, Lin Huang, Caesar Wang, Yakir Yang, Stéphane Marchesin, Nick Sanders, Douglas Anderson
Derek Basehore has posted comments on this change. (
https://chromium-review.googlesource.com/362907 )

Change subject: CHROMIUM: drm/rockchip: Toggle devfreq on/off
......................................................................


Patch Set 23:

I log in as guest to repro.

Lin Huang (Gerrit)

unread,
Oct 1, 2016, 1:17:16 AM10/1/16
to Sean Paul, Derek Basehore, Caesar Wang, Yakir Yang, Stéphane Marchesin, Nick Sanders, Douglas Anderson
Lin Huang has posted comments on this change. (
https://chromium-review.googlesource.com/362907 )

Change subject: CHROMIUM: drm/rockchip: Toggle devfreq on/off
......................................................................


Patch Set 24:

(1 comment)

https://chromium-review.googlesource.com/#/c/362907/24/drivers/gpu/drm/rockchip/rockchip_drm_fb.c
File drivers/gpu/drm/rockchip/rockchip_drm_fb.c:

Line 303: rockchip_drm_enable_dmc(priv);
after use
https://chromium-review.googlesource.com/#/c/391746/1
this patch, it seem will trigger dead lock:
Call trace:
[ 240.140680] [<ffffffc000204fd8>] __switch_to+0x9c/0xa8
[ 240.145821] [<ffffffc000902fec>] __schedule+0x440/0x6d8
[ 240.151051] [<ffffffc000903318>] schedule+0x94/0xb4
[ 240.155927] [<ffffffc00090369c>] schedule_preempt_disabled+0x28/0x44
[ 240.162285] [<ffffffc000905150>] __mutex_lock_slowpath+0x120/0x1ac
[ 240.168462] [<ffffffc000905228>] mutex_lock+0x4c/0x68
[ 240.173517] [<ffffffc000599938>] dmc_notify+0x30/0x7c
[ 240.178575] [<ffffffc0002402e4>] notifier_call_chain+0x58/0x8c
[ 240.184411] [<ffffffc000240938>] __srcu_notifier_call_chain+0xac/0xd0
[ 240.190855] [<ffffffc000240994>] srcu_notifier_call_chain+0x38/0x44
[ 240.197126] [<ffffffc00077eb78>] update_devfreq+0xf0/0x1b0
[ 240.202612] [<ffffffc000780af4>] ddrmon_thread_isr+0x3c/0x58
[ 240.208277] [<ffffffc000271fb8>] irq_thread_fn+0x30/0x70
[ 240.213596] [<ffffffc000272214>] irq_thread+0x134/0x230
[ 240.218825] [<ffffffc00023f00c>] kthread+0x10c/0x114
[ 240.223792] [<ffffffc000203c50>] ret_from_fork+0x10/0x40
[ 240.229108] task PC stack pid father
[ 240.235045] irq/58-ff630000 D ffffffc000204fd8 0 137 2
0x00000000
[ 240.242134] Call trace:
[ 240.244590] [<ffffffc000204fd8>] __switch_to+0x9c/0xa8
[ 240.249727] [<ffffffc000902fec>] __schedule+0x440/0x6d8
[ 240.254954] [<ffffffc000903318>] schedule+0x94/0xb4
[ 240.259831] [<ffffffc00090369c>] schedule_preempt_disabled+0x28/0x44
[ 240.266185] [<ffffffc000905150>] __mutex_lock_slowpath+0x120/0x1ac
[ 240.272363] [<ffffffc000905228>] mutex_lock+0x4c/0x68
[ 240.277416] [<ffffffc000599938>] dmc_notify+0x30/0x7c
[ 240.282467] [<ffffffc0002402e4>] notifier_call_chain+0x58/0x8c
[ 240.288300] [<ffffffc000240938>] __srcu_notifier_call_chain+0xac/0xd0
[ 240.294736] [<ffffffc000240994>] srcu_notifier_call_chain+0x38/0x44
[ 240.301002] [<ffffffc00077eb78>] update_devfreq+0xf0/0x1b0
[ 240.306487] [<ffffffc000780af4>] ddrmon_thread_isr+0x3c/0x58
[ 240.312154] [<ffffffc000271fb8>] irq_thread_fn+0x30/0x70
[ 240.317464] [<ffffffc000272214>] irq_thread+0x134/0x230
[ 240.322692] [<ffffffc00023f00c>] kthread+0x10c/0x114
[ 240.327655] [<ffffffc000203c50>] ret_from_fork+0x10/0x40
[ 240.333012] DrmThread D ffffffc000204fd8 0 2007 1955
0x00400808
[ 240.340093] Call trace:
[ 240.342547] [<ffffffc000204fd8>] __switch_to+0x9c/0xa8
[ 240.347683] [<ffffffc000902fec>] __schedule+0x440/0x6d8
[ 240.352910] [<ffffffc000903318>] schedule+0x94/0xb4
[ 240.357787] [<ffffffc00090369c>] schedule_preempt_disabled+0x28/0x44
[ 240.364139] [<ffffffc000905150>] __mutex_lock_slowpath+0x120/0x1ac
[ 240.370318] [<ffffffc000905228>] mutex_lock+0x4c/0x68
[ 240.375372] [<ffffffc00077d2b8>] devfreq_monitor_resume+0x34/0xd4
[ 240.381465] [<ffffffc00077fbfc>]
devfreq_simple_ondemand_handler+0x80/0x90
[ 240.388338] [<ffffffc00077d194>] devfreq_resume_device+0x40/0x4c
[ 240.394343] [<ffffffc00059970c>] rockchip_drm_enable_dmc+0x40/0x4c
[ 240.400523] [<ffffffc00059aab8>]
rockchip_atomic_commit_complete+0x214/0x250
[ 240.407565] [<ffffffc00059ab80>] rockchip_drm_atomic_commit+0x8c/0xac
[ 240.414007] [<ffffffc000594858>] drm_atomic_commit+0x70/0x7c
[ 240.419667] [<ffffffc00056fe64>] drm_atomic_helper_set_config+0x5c/0xac
[ 240.426282] [<ffffffc000582c2c>] drm_mode_set_config_internal+0x68/0xe4
[ 240.432896] [<ffffffc000587e38>] drm_mode_setcrtc+0x408/0x49c
[ 240.438645] [<ffffffc000578874>] drm_ioctl+0x2ac/0x428
[ 240.443785] [<ffffffc000595eac>] drm_compat_ioctl+0x60/0x70
[ 240.449359] [<ffffffc00039d520>] compat_SyS_ioctl+0x134/0x10b8
[ 240.455194] [<ffffffc000203d10>] __sys_trace_return+0x0/0x4

the devfreq have been enable when probe, but here always call
devfreq_resume_device(), even haven't call devfreq_suspend_device().



--
To view, visit https://chromium-review.googlesource.com/362907
To unsubscribe, visit https://chromium-review.googlesource.com/settings

Gerrit-MessageType: comment
Gerrit-Change-Id: I690ae1c0d2436b957fc823585c1a32b48cfcf3fd
Gerrit-PatchSet: 24
Gerrit-Project: chromiumos/third_party/kernel
Gerrit-Branch: chromeos-4.4
Gerrit-Owner: Lin Huang <h...@rock-chips.com>
Gerrit-Reviewer: Caesar Wang <w...@rock-chips.com>
Gerrit-Reviewer: Derek Basehore <dbas...@chromium.org>
Gerrit-Reviewer: Douglas Anderson <dian...@chromium.org>
Gerrit-Reviewer: Lin Huang <h...@rock-chips.com>
Gerrit-Reviewer: Nick Sanders <nsan...@chromium.org>
Gerrit-Reviewer: Sean Paul <sean...@chromium.org>
Gerrit-Reviewer: Stéphane Marchesin <mar...@chromium.org>
Gerrit-Reviewer: Yakir Yang <y...@rock-chips.com>
Gerrit-HasComments: Yes

Sean Paul (Gerrit)

unread,
Oct 3, 2016, 3:36:22 PM10/3/16
to Sean Paul, Lin Huang, Xing Zheng, Derek Basehore, Caesar Wang, Yakir Yang, Stéphane Marchesin, Nick Sanders, Douglas Anderson
Sean Paul has posted comments on this change. (
https://chromium-review.googlesource.com/362907 )

Change subject: CHROMIUM: drm/rockchip: Toggle devfreq on/off
......................................................................


Patch Set 24:

re: deadlock, I don't seem to be getting the DEVFREQ_POSTCHANGE event, just
the PRECHANGE event.
--
To view, visit https://chromium-review.googlesource.com/362907
To unsubscribe, visit https://chromium-review.googlesource.com/settings

Gerrit-MessageType: comment
Gerrit-Change-Id: I690ae1c0d2436b957fc823585c1a32b48cfcf3fd
Gerrit-PatchSet: 24
Gerrit-Project: chromiumos/third_party/kernel
Gerrit-Branch: chromeos-4.4
Gerrit-Owner: Lin Huang <h...@rock-chips.com>
Gerrit-Reviewer: Caesar Wang <w...@rock-chips.com>
Gerrit-Reviewer: Derek Basehore <dbas...@chromium.org>
Gerrit-Reviewer: Douglas Anderson <dian...@chromium.org>
Gerrit-Reviewer: Lin Huang <h...@rock-chips.com>
Gerrit-Reviewer: Nick Sanders <nsan...@chromium.org>
Gerrit-Reviewer: Sean Paul <sean...@chromium.org>
Gerrit-Reviewer: Stéphane Marchesin <mar...@chromium.org>
Gerrit-Reviewer: Xing Zheng <zhen...@rock-chips.com>
Gerrit-Reviewer: Yakir Yang <y...@rock-chips.com>
Gerrit-HasComments: No

Lin Huang (Gerrit)

unread,
Oct 3, 2016, 4:41:24 PM10/3/16
to Sean Paul, Xing Zheng, Derek Basehore, Caesar Wang, Yakir Yang, Stéphane Marchesin, Nick Sanders, Douglas Anderson
Lin Huang has posted comments on this change. (
https://chromium-review.googlesource.com/362907 )

Change subject: CHROMIUM: drm/rockchip: Toggle devfreq on/off
......................................................................


Patch Set 24:

> re: deadlock, I don't seem to be getting the DEVFREQ_POSTCHANGE
> event, just the PRECHANGE event.

there is a problem may easy reproduce: i just checkout
https://chromium-review.googlesource.com/#/c/364450/26, and use following
command:
cd /sys/bus/platform/drivers/rk3399-dmc-freq/dmc/devfreq/dmc
echo userspace > governor
while true; do echo 200000000 > userspace/set_freq; echo 800000000 >
userspace/set_freq; done &
to ddr dvfs very fast, then try to login in guest, i will happen:
do not call blocking ops when !TASK_RUNNING; state=2 set at
[<ffffffc0002612ec>] prepare_to_wait_event+0xb0/0x11c
[ 45.852859] ------------[ cut here ]------------
[ 45.857520] WARNING: at
/mnt/host/source/src/third_party/kernel/v4.4/kernel/sched/core.c:7736
[ 45.866047] Modules linked in: rfcomm btusb btrtl btbcm btintel uinput
uvcvideo videobuf2_vmalloc bluetooth ipt_MASQUERADE nf_nat_masquerade_ipv4
iptable_nat nf_nat_ipv4 nf_nat xt_mark bridge stp llc mwifiex_pcie mwifiex
fuse zram cfg80211 ip6table_filter asix usbnet mii n
[ 45.899866]
[ 45.901380] CPU: 2 PID: 9813 Comm: DrmThread Not tainted 4.4.14 #93
[ 45.907652] Hardware name: Google Kevin (DT)
[ 45.911957] task: ffffffc0c5bf9900 ti: ffffffc0d3244000 task.ti:
ffffffc0d3244000
[ 45.919454] PC is at __might_sleep+0x64/0x90
[ 45.923735] LR is at __might_sleep+0x64/0x90
[ 45.928013] pc : [<ffffffc000248140>] lr : [<ffffffc000248140>] pstate:
60000145
[ 45.935444] sp : ffffffc0d3247a70
[ 45.938780] x29: ffffffc0d3247a70 x28: 0000000000000001
[ 45.944135] x27: ffffffc0ecedd028 x26: ffffffc0ed0dd290
[ 45.949487] x25: ffffffc0ed0dd250 x24: ffffffc001088000
[ 45.954847] x23: 0000000000000001 x22: 0000000000000000
[ 45.960198] x21: 0000000000000061 x20: ffffffc000be224c
[ 45.965554] x19: ffffffc00113e83d x18: 0000000000000000
[ 45.970934] x17: 0000000000000000 x16: ffffffc00039d3ec
[ 45.976285] x15: 0000000000000000 x14: 63313178302f3062
[ 45.981635] x13: 78302b746e657665 x12: 5f746961775f6f74
[ 45.987016] x11: 5f65726170657270 x10: 00000000000008a0
[ 45.992373] x9 : ffffffc0d3247820 x8 : ffffffc0c5bfa200
[ 45.997757] x7 : ffffffc0010c73c0 x6 : 0000000000000000
[ 46.003177] x5 : 0000000000000000 x4 : 000000000009ba00
[ 46.008529] x3 : 0000000000000002 x2 : cb88537fdc8ba60e
[ 46.013879] x1 : cb88537fdc8ba60e x0 : 0000000000000071
[ 46.019231]
[ 46.019231] PC: 0xffffffc0002480c0:
[ 46.024200] 80c0 913b3c00 9402bddf d53b4220 9409a8b3 a94153f3 a8c37bfd
d65f03c0 a9bd7bfd
[ 46.032572] 80e0 910003fd a90153f3 a9025bf5 aa0003f4 aa1e03e0 2a0103f5
2a0203f6 d503201f
[ 46.040988] 8100 910003e0 9272c400 f9400800 f9400001 b40001e1 f9444802
b40001a2 d00077b3
[ 46.049370] 8120 9120f673 39400661 35000121 f9400001 90004cc0 aa0203e3
913bec00 9402bdc1
[ 46.057708] 8140 d4210000 52800020 39000660 aa1403e0 2a1503e1 2a1603e2
97ffff8e a94153f3
[ 46.066050] 8160 a9425bf5 a8c37bfd d65f03c0 a9b67bfd 910003fd a90153f3
a9025bf5 a90363f7
[ 46.074433] 8180 a9046bf9 a90573fb aa0003f6 aa1e03e0 2a0203f3 f9004ba1
d0007217 d503201f
[ 46.082782] 81a0 913022e0 b0007114 90000019 910e029a aa1603f5 9132b339
f873d800 9292f1fb
[ 46.091129]
[ 46.091129] LR: 0xffffffc0002480c0:
[ 46.096101] 80c0 913b3c00 9402bddf d53b4220 9409a8b3 a94153f3 a8c37bfd
d65f03c0 a9bd7bfd
[ 46.104531] 80e0 910003fd a90153f3 a9025bf5 aa0003f4 aa1e03e0 2a0103f5
2a0203f6 d503201f
[ 46.112963] 8100 910003e0 9272c400 f9400800 f9400001 b40001e1 f9444802
b40001a2 d00077b3
[ 46.121360] 8120 9120f673 39400661 35000121 f9400001 90004cc0 aa0203e3
913bec00 9402bdc1
[ 46.129707] 8140 d4210000 52800020 39000660 aa1403e0 2a1503e1 2a1603e2
97ffff8e a94153f3
[ 46.138098] 8160 a9425bf5 a8c37bfd d65f03c0 a9b67bfd 910003fd a90153f3
a9025bf5 a90363f7
[ 46.146442] 8180 a9046bf9 a90573fb aa0003f6 aa1e03e0 2a0203f3 f9004ba1
d0007217 d503201f
[ 46.154845] 81a0 913022e0 b0007114 90000019 910e029a aa1603f5 9132b339
f873d800 9292f1fb
[ 46.163204]
[ 46.163204] SP: 0xffffffc0d32479f0:
[ 46.168176] 79f0 00be224c ffffffc0 00000061 00000000 00000000 00000000
00000001 00000000
[ 46.176592] 7a10 01088000 ffffffc0 ed0dd250 ffffffc0 ed0dd290 ffffffc0
ecedd028 ffffffc0
[ 46.184932] 7a30 00000001 00000000 d3247a70 ffffffc0 00248140 ffffffc0
d3247a70 ffffffc0
[ 46.193272] 7a50 00248140 ffffffc0 60000145 00000000 cfcd8c80 ffffffc0
00000000 00000000
[ 46.201648] 7a70 d3247aa0 ffffffc0 00905000 ffffffc0 ed0dd250 ffffffc0
ed0dd028 ffffffc0
[ 46.209990] 7a90 ed003000 ffffffc0 00000000 00000000 d3247ac0 ffffffc0
0059aa3c ffffffc0
[ 46.218376] 7ab0 c5ba4f80 ffffffc0 dc8ba60e cb88537f d3247b40 ffffffc0
0059ac18 ffffffc0
[ 46.226768] 7ad0 ed0dd110 ffffffc0 ed0dd0e0 ffffffc0 ed003000 ffffffc0
c5ba4f80 ffffffc0
[ 46.235119]
[ 46.235119] X7: 0xffffffc0010c7340:
[ 46.240092] 7340 ee1b2900 ffffffc0 00000000 00000000 ee1b2b00 ffffffc0
f7edc000 00000000
[ 46.248494] 7360 00100000 00000000 00000000 00000000 00020000 00000000
00020000 00000000
[ 46.256874] 7380 00000000 00000000 00020000 00000000 00000001 00000000
00000000 00000000
[ 46.265211] 73a0 00000000 00000006 00000000 00000006 00000002 00000001
00000001 00000000
[ 46.273567] 73c0 00000000 00000000 00bffbc0 ffffffc0 043f043f dead4ead
ffffffff 00000000
[ 46.281929] 73e0 ffffffff ffffffff ed720000 ffffffc0 00020000 00000000
00000001 00000000
[ 46.290302] 7400 00000000 dead4ead ffffffff 00000000 ffffffff ffffffff
010c7418 ffffffc0
[ 46.298691] 7420 010c7418 ffffffc0 00000000 00000000 00000000 00000000
00000000 00000000
[ 46.307058]
[ 46.307058] X8: 0xffffffc0c5bfa180:
[ 46.312051] a180 00000000 00000000 00000000 00000000 002612ec ffffffc0
00000001 00000000
[ 46.320375] a1a0 ecf1cb00 ffffffc0 c5bf9900 ffffffc0 ecf1cb00 ffffffc0
cdba5040 ffffffc0
[ 46.328710] a1c0 00903158 ffffffc0 01069000 ffffffc0 f7b40380 ffffffc0
00000001 00000000
[ 46.337066] a1e0 c5bf9e80 ffffffc0 00000000 00000000 d3247820 ffffffc0
d3247820 ffffffc0
[ 46.345449] a200 00204fd8 ffffffc0 f238b900 00000000 00000000 00000000
00000000 00000000
[ 46.353756] a220 09900960 0a0409b0 06400000 064d0643 000006c5 0000003c
0000000a 00000048
[ 46.362109] a240 30303432 30363178 00000030 00000000 00000000 00000000
00000000 00000000
[ 46.370430] a260 00000000 00000000 00000000 00000000 00000000 00000000
00000000 00000000
[ 46.378759]
[ 46.378759] X9: 0xffffffc0d32477a0:
[ 46.383724] 77a0 d3247950 ffffffc0 0000003d 00000000 ed0dd250 ffffffc0
00340000 ffffff80
[ 46.392077] 77c0 00003958 00000000 dc8ba60e cb88537f d32477f0 ffffffc0
004b1dc0 ffffffc0
[ 46.400425] 77e0 00c7b810 ffffffc0 dc8ba60e cb88537f d3247830 ffffffc0
00208b00 ffffffc0
[ 46.408817] 7800 d3247950 ffffffc0 00000001 00000000 f2000800 00000000
cdba5040 ffffffc0
[ 46.417174] 7820 00001e38 00000000 00bdd2b2 ffffffc0 d3247850 ffffffc0
00201a1c ffffffc0
[ 46.425533] 7840 00208acc ffffffc0 d3247950 ffffffc0 d3247880 ffffffc0
002004d4 ffffffc0
[ 46.433865] 7860 01088000 ffffffc0 ac96f14c 00000000 011358b0 ffffffc0
00000000 00000000
[ 46.442188] 7880 d3247a70 ffffffc0 002034b4 ffffffc0 0113e83d ffffffc0
00be224c ffffffc0
[ 46.450560]
[ 46.450560] X16: 0xffffffc00039d36c:
[ 46.455606] d36c d360fc20 eb1f001f 5a9f0021 d503201f 52800000 9100b262
b9000041 d503201f
[ 46.463926] d38c 35fff840 d503201f 9100c261 7940f3a2 79000022 d503201f
35fff780 d503201f
[ 46.472244] d3ac 9100d273 b9407fa1 b9000261 d503201f 35fff6c0 2a1403e0
f94047a2 f94506a1
[ 46.480560] d3cc eb01005f 54000040 97fa0b54 a94153f3 a9425bf5 f9401bf7
a8c97bfd d65f03c0
[ 46.488872] d3ec a9b57bfd 910003fd a90153f3 a9025bf5 a90363f7 a9046bf9
a90573fb f0006759
[ 46.497175] d40c aa0003f5 aa1e03e0 aa0103f6 f9003fa2 12800114 d503201f
f9450720 f9403fa2
[ 46.505563] d42c f90057a0 2a1503e0 92407c57 97ff2bc9 f9003bb9 f27ef413
aa0003f8 54008120
[ 46.513876] d44c aa1303e0 2a1603e1 aa1703e2 9402b65c 2a0003f4 35008000
528a8a40 6b0002df
[ 46.522182]
[ 46.522182] X19: 0xffffffc00113e7bd:
[ 46.527235] e7bc ffffffc0 00812104 ffffffc0 002dd93c ffffffc0 00e36e5c
ffffffc0 00000000
[ 46.535555] e7dc 00000000 ed8f7210 ffffffc0 ed8f71b0 ffffffc0 002dd68c
ffffffc0 00000000
[ 46.543869] e7fc 00000000 00000000 00000000 00000000 00000000 00000000
00000000 00000000
[ 46.552169] e81c 00000000 00000000 00000000 00000000 00000000 00000000
00000000 00000000
[ 46.560477] e83c 00000000 00000000 00000000 00000000 00000000 00000000
00000000 00000000
[ 46.568784] e85c 00000000 00000000 00000000 00000000 00000000 00000000
00000000 00000000
[ 46.577129] e87c 00000000 00000000 00000000 00000000 00000000 00000000
00000000 00000000
[ 46.585452] e89c 00000000 00000000 00000000 00000000 00000000 00000000
00000000 00000000
[ 46.593760] e8bc 00000000 00000000 00000000 00000000 00000000 00000000
00000000 00000000
[ 46.602132]
[ 46.602132] X20: 0xffffffc000be21cc:
[ 46.607184] 21cc 75735f6c 6573006d 6776612e 616f6c2e 76615f64 65730067
6776612e 6974752e
[ 46.615557] 21ec 76615f6c 65730067 6776612e 73616c2e 70755f74 65746164
6d69745f 6c630065
[ 46.623863] 220c 2d6b636f 746c6564 6f6c0061 69726167 63696d74 6e696c00
00726165 25207325
[ 46.632174] 222c 0a646c6c 26282600 6b636f6c 61773e2d 6c5f7469 296b636f
6c723e2d 006b636f
[ 46.640555] 224c 746e6d2f 736f682f 6f732f74 65637275 6372732f 6968742f
705f6472 79747261
[ 46.648858] 226c 72656b2f 2f6c656e 342e3476 72656b2f 2f6c656e 6b636f6c
2f676e69 6574756d
[ 46.657151] 228c 00632e78 746e6d2f 736f682f 6f732f74 65637275 6372732f
6968742f 705f6472
[ 46.665448] 22ac 79747261 72656b2f 2f6c656e 342e3476 72656b2f 2f6c656e
6b636f6c 2f676e69
[ 46.673824]
[ 46.673824] X24: 0xffffffc001087f80:
[ 46.678873] 7f80 ffffffff 00000000 ffffffff ffffffff 01e001e0 dead4ead
ffffffff 00000000
[ 46.687202] 7fa0 ffffffff ffffffff 01e201e2 dead4ead ffffffff 00000000
ffffffff ffffffff
[ 46.695495] 7fc0 01e001e0 dead4ead ffffffff 00000000 ffffffff ffffffff
01e001e0 dead4ead
[ 46.703841] 7fe0 ffffffff 00000000 ffffffff ffffffff 01e001e0 dead4ead
ffffffff 00000000
[ 46.712141] 8000 ffffffff ffffffff 01e001e0 dead4ead ffffffff 00000000
ffffffff ffffffff
[ 46.720454] 8020 01e001e0 dead4ead ffffffff 00000000 ffffffff ffffffff
01e001e0 dead4ead
[ 46.728742] 8040 ffffffff 00000000 ffffffff ffffffff 01e001e0 dead4ead
ffffffff 00000000
[ 46.737060] 8060 ffffffff ffffffff 01e201e2 dead4ead ffffffff 00000000
ffffffff ffffffff
[ 46.745379]
[ 46.745379] X25: 0xffffffc0ed0dd1d0:
[ 46.750432] d1d0 00000000 00000000 00000000 00000000 00000000 00000000
00000000 00000000
[ 46.758751] d1f0 00000000 00000000 00000000 00000000 ed169e00 ffffffc0
ed169e00 ffffffc0
[ 46.767043] d210 01230123 dead4ead ffffffff 00000000 ffffffff ffffffff
ed002800 ffffffc0
[ 46.775339] d230 ed0c4800 ffffffc0 005999b8 ffffffc0 00000000 00000000
00000000 00000000
[ 46.783628] d250 00000001 00000000 00000000 dead4ead ffffffff 00000000
ffffffff ffffffff
[ 46.791997] d270 ed0dd270 ffffffc0 ed0dd270 ffffffc0 00000000 00000000
00000000 00000000
[ 46.800287] d290 1cc61cc6 dead4ead ffffffff 00000000 ffffffff ffffffff
ed0dd2a8 ffffffc0
[ 46.808578] d2b0 ed0dd2a8 ffffffc0 00000001 00000000 00000000 00000000
00000000 00000000
[ 46.816872]
[ 46.816872] X26: 0xffffffc0ed0dd210:
[ 46.821922] d210 01230123 dead4ead ffffffff 00000000 ffffffff ffffffff
ed002800 ffffffc0
[ 46.830240] d230 ed0c4800 ffffffc0 005999b8 ffffffc0 00000000 00000000
00000000 00000000
[ 46.838555] d250 00000001 00000000 00000000 dead4ead ffffffff 00000000
ffffffff ffffffff
[ 46.846864] d270 ed0dd270 ffffffc0 ed0dd270 ffffffc0 00000000 00000000
00000000 00000000
[ 46.855153] d290 1d321d32 dead4ead ffffffff 00000000 ffffffff ffffffff
ed0dd2a8 ffffffc0
[ 46.863445] d2b0 ed0dd2a8 ffffffc0 00000001 00000000 00000000 00000000
00000000 00000000
[ 46.871744] d2d0 00000000 00000000 00000000 00000000 00000000 00000000
00000000 00000000
[ 46.880032] d2f0 00000000 00000000 00000000 00000000 00000000 00000000
00000000 00000000
[ 46.888334]
[ 46.888334] X27: 0xffffffc0ecedcfa8:
[ 46.893380] cfa8 00000000 00000000 00000000 00000000 00000000 00000000
00000000 00000000
[ 46.901743] cfc8 00000000 00000000 00000000 00000000 00000000 00000000
00000000 00000000
[ 46.910034] cfe8 00000000 00000000 00000000 00000000 00000000 00000000
ed1c1900 ffffffc0
[ 46.918331] d008 ed1c1880 ffffffc0 005d7798 ffffffc0 00c2ab7f ffffffc0
00000a40 00000000
[ 46.926672] d028 ed003000 ffffffc0 f7bab028 ffffffc0 ecede038 ffffffc0
ed0034b8 ffffffc0
[ 46.934959] d048 ed1c3300 ffffffc0 ffffffff 00000000 032d032d dead4ead
ffffffff 00000000
[ 46.943248] d068 ffffffff ffffffff ecedd070 ffffffc0 ecedd070 ffffffc0
c5bf9900 ffffffc0
[ 46.951557] d088 00000000 00000000 ea0fb500 ffffffc0 ed003338 ffffffc0
ecede098 ffffffc0
[ 46.959857]
[ 46.959857] X29: 0xffffffc0d32479f0:
[ 46.964905] 79f0 00be224c ffffffc0 00000061 00000000 00000000 00000000
00000001 00000000
[ 46.973237] 7a10 01088000 ffffffc0 ed0dd250 ffffffc0 ed0dd290 ffffffc0
ecedd028 ffffffc0
[ 46.981554] 7a30 00000001 00000000 d3247a70 ffffffc0 00248140 ffffffc0
d3247a70 ffffffc0
[ 46.989854] 7a50 00248140 ffffffc0 60000145 00000000 cfcd8c80 ffffffc0
00000000 00000000
[ 46.998142] 7a70 d3247aa0 ffffffc0 00905000 ffffffc0 ed0dd250 ffffffc0
ed0dd028 ffffffc0
[ 47.006443] 7a90 ed003000 ffffffc0 00000000 00000000 d3247ac0 ffffffc0
0059aa3c ffffffc0
[ 47.014743] 7ab0 c5ba4f80 ffffffc0 dc8ba60e cb88537f d3247b40 ffffffc0
0059ac18 ffffffc0
[ 47.023030] 7ad0 ed0dd110 ffffffc0 ed0dd0e0 ffffffc0 ed003000 ffffffc0
c5ba4f80 ffffffc0
[ 47.031330]
[ 47.032824] ---[ end trace da3669b4420c5f4a ]---
[ 47.037439] Call trace:
[ 47.039901] [<ffffffc000248140>] __might_sleep+0x64/0x90
[ 47.045218] [<ffffffc000905000>] mutex_lock+0x2c/0x68
[ 47.050280] [<ffffffc00059aa3c>]
rockchip_atomic_commit_complete+0xe8/0x238
[ 47.057242] [<ffffffc00059ac18>] rockchip_drm_atomic_commit+0x8c/0xac
[ 47.063686] [<ffffffc000594908>] drm_atomic_commit+0x70/0x7c
[ 47.069347] [<ffffffc00056ff14>] drm_atomic_helper_set_config+0x5c/0xac
[ 47.075963] [<ffffffc000582cdc>] drm_mode_set_config_internal+0x68/0xe4
[ 47.082574] [<ffffffc000587ee8>] drm_mode_setcrtc+0x408/0x49c
[ 47.088321] [<ffffffc000578924>] drm_ioctl+0x2ac/0x428
[ 47.093464] [<ffffffc000595f5c>] drm_compat_ioctl+0x60/0x70
[ 47.099038] [<ffffffc00039d520>] compat_SyS_ioctl+0x134/0x10b8
[ 47.104872] [<ffffffc000203d10>] __sys_trace_return+0x0/0x4
almost every time

Lin Huang (Gerrit)

unread,
Oct 3, 2016, 4:58:32 PM10/3/16
to Sean Paul, Xing Zheng, Derek Basehore, Caesar Wang, Yakir Yang, Stéphane Marchesin, Nick Sanders, Douglas Anderson
Lin Huang has posted comments on this change. (
https://chromium-review.googlesource.com/362907 )

Change subject: CHROMIUM: drm/rockchip: Toggle devfreq on/off
......................................................................


Patch Set 24:

and i try to use wait_event_interruptible/wait_up_interrupt, it will happen:
[ 240.117222] INFO: task kworker/1:1:117 blocked for more than 120 seconds.
[ 240.124021] Not tainted 4.4.14 #94
[ 240.128046] "echo 0 > /proc/sys/kernel/hung_task_timeout_secs" disables
this message.
[ 240.135876] kworker/1:1 D ffffffc000204fd8 0 117 2
0x00000000
[ 240.142994] Workqueue: events rockchip_drm_atomic_work
[ 240.148161] Call trace:
[ 240.150641] [<ffffffc000204fd8>] __switch_to+0x9c/0xa8
[ 240.155779] [<ffffffc000902df4>] __schedule+0x440/0x6d8
[ 240.161038] [<ffffffc000903120>] schedule+0x94/0xb4
[ 240.165922] [<ffffffc000906270>] schedule_timeout+0x44/0x27c
[ 240.171617] [<ffffffc000603900>] fence_default_wait+0x1ac/0x2ac
[ 240.177532] [<ffffffc000602e68>] fence_wait_timeout+0x10c/0x1f8
[ 240.183473] [<ffffffc00059aacc>]
rockchip_atomic_commit_complete+0x178/0x248
[ 240.190518] [<ffffffc00059aca8>] rockchip_drm_atomic_work+0x20/0x2c
[ 240.196818] [<ffffffc000239200>] process_one_work+0x240/0x424
[ 240.202564] [<ffffffc000239c9c>] worker_thread+0x2fc/0x424
[ 240.208072] [<ffffffc00023f00c>] kthread+0x10c/0x114
[ 240.213035] [<ffffffc000203c50>] ret_from_fork+0x10/0x40
[ 240.218366] task PC stack pid father
[ 240.224297] kworker/1:1 D ffffffc000204fd8 0 117 2
0x00000000
[ 240.231409] Workqueue: events rockchip_drm_atomic_work
[ 240.236584] Call trace:
[ 240.239061] [<ffffffc000204fd8>] __switch_to+0x9c/0xa8
[ 240.244197] [<ffffffc000902df4>] __schedule+0x440/0x6d8
[ 240.249442] [<ffffffc000903120>] schedule+0x94/0xb4
[ 240.254322] [<ffffffc000906270>] schedule_timeout+0x44/0x27c
[ 240.260016] [<ffffffc000603900>] fence_default_wait+0x1ac/0x2ac
[ 240.265934] [<ffffffc000602e68>] fence_wait_timeout+0x10c/0x1f8
[ 240.271880] [<ffffffc00059aacc>]
rockchip_atomic_commit_complete+0x178/0x248
[ 240.278924] [<ffffffc00059aca8>] rockchip_drm_atomic_work+0x20/0x2c
[ 240.285217] [<ffffffc000239200>] process_one_work+0x240/0x424
[ 240.290959] [<ffffffc000239c9c>] worker_thread+0x2fc/0x424
[ 240.296471] [<ffffffc00023f00c>] kthread+0x10c/0x114
[ 240.301437] [<ffffffc000203c50>] ret_from_fork+0x10/0x40
[ 240.306792] Chrome_ChildIOT D ffffffc000204fd8 0 2010 1519
0x0040080d

Derek Basehore (Gerrit)

unread,
Oct 4, 2016, 5:31:09 PM10/4/16
to Sean Paul, Lin Huang, Xing Zheng, Caesar Wang, Yakir Yang, Stéphane Marchesin, Nick Sanders, Douglas Anderson
Derek Basehore has posted comments on this change. (
https://chromium-review.googlesource.com/362907 )

Change subject: CHROMIUM: drm/rockchip: Toggle devfreq on/off
......................................................................


Patch Set 24:

Line 271: rockchip_drm_disable_dmc(priv);
Okay, we seem to have a mutex inversion here. We are holding the dmc_mutex
when we call this, which will lock the devfreq mutex.

The devfreq device will lock the devfreq mutex then lock the dmc_mutex when
it calls the notifier call chain which will call into dmc_notify.

Do we need to hold the dmc_mutex when we call
rockchip_drm_disable/enable_dmc?



--
To view, visit https://chromium-review.googlesource.com/362907
To unsubscribe, visit https://chromium-review.googlesource.com/settings

Gerrit-MessageType: comment
Gerrit-Change-Id: I690ae1c0d2436b957fc823585c1a32b48cfcf3fd
Gerrit-PatchSet: 24
Gerrit-Project: chromiumos/third_party/kernel
Gerrit-Branch: chromeos-4.4
Gerrit-Owner: Lin Huang <h...@rock-chips.com>
Gerrit-Reviewer: Caesar Wang <w...@rock-chips.com>
Gerrit-Reviewer: Derek Basehore <dbas...@chromium.org>
Gerrit-Reviewer: Douglas Anderson <dian...@chromium.org>
Gerrit-Reviewer: Lin Huang <h...@rock-chips.com>
Gerrit-Reviewer: Nick Sanders <nsan...@chromium.org>
Gerrit-Reviewer: Sean Paul <sean...@chromium.org>
Gerrit-Reviewer: Stéphane Marchesin <mar...@chromium.org>
Gerrit-Reviewer: Xing Zheng <zhen...@rock-chips.com>
Gerrit-Reviewer: Yakir Yang <y...@rock-chips.com>
Gerrit-HasComments: Yes

Sean Paul (Gerrit)

unread,
Oct 5, 2016, 12:25:19 PM10/5/16
to Sean Paul, Lin Huang, Derek Basehore, Xing Zheng, Caesar Wang, Yakir Yang, Stéphane Marchesin, Nick Sanders, Douglas Anderson
Sean Paul has posted comments on this change. (
https://chromium-review.googlesource.com/362907 )

Change subject: CHROMIUM: drm/rockchip: Toggle devfreq on/off
......................................................................


Patch Set 24:

(1 comment)

https://chromium-review.googlesource.com/#/c/362907/24/drivers/gpu/drm/rockchip/rockchip_drm_fb.c
File drivers/gpu/drm/rockchip/rockchip_drm_fb.c:

Line 271: rockchip_drm_disable_dmc(priv);
> Okay, we seem to have a mutex inversion here. We are holding the dmc_mutex
Yes, since that ensures the scaling doesn't kick in during disable/enable.

Lin Huang (Gerrit)

unread,
Oct 5, 2016, 1:25:47 PM10/5/16
to Sean Paul, Derek Basehore, Xing Zheng, Caesar Wang, Yakir Yang, Stéphane Marchesin, Nick Sanders, Douglas Anderson
Lin Huang has posted comments on this change. (
https://chromium-review.googlesource.com/362907 )

Change subject: CHROMIUM: drm/rockchip: Toggle devfreq on/off
......................................................................


Patch Set 25:

so for may test, there may have 3 problem:
1.
if (active_changed) {
if (num_active_crtcs == 1)
rockchip_drm_enable_dmc(priv);
mutex_unlock(&priv->dmc_mutex);
}
may hold the update_devfreq() thread.
2.
wait_event(priv->wait_dmc_queue,
!rockchip_test_dmc_and_mutex_lock(priv));
may trigger:
"do not call blocking ops when !TASK_RUNNING; state=2 set at
[<ffffffc0002612ec>] prepare_to_wait_event+0xb0/0x11c", like the log i
paste before.
3. if i add the https://chromium-review.googlesource.com/#/c/391670,
it may trigger the dead lock when we operate vop pd.
Sean, i don't know much about drm, do you have any idea about to fix it.
--
To view, visit https://chromium-review.googlesource.com/362907
To unsubscribe, visit https://chromium-review.googlesource.com/settings

Gerrit-MessageType: comment
Gerrit-Change-Id: I690ae1c0d2436b957fc823585c1a32b48cfcf3fd
Gerrit-PatchSet: 25
Gerrit-Project: chromiumos/third_party/kernel
Gerrit-Branch: chromeos-4.4
Gerrit-Owner: Lin Huang <h...@rock-chips.com>
Gerrit-Reviewer: Caesar Wang <w...@rock-chips.com>
Gerrit-Reviewer: Derek Basehore <dbas...@chromium.org>
Gerrit-Reviewer: Douglas Anderson <dian...@chromium.org>
Gerrit-Reviewer: Lin Huang <h...@rock-chips.com>
Gerrit-Reviewer: Nick Sanders <nsan...@chromium.org>
Gerrit-Reviewer: Sean Paul <sean...@chromium.org>
Gerrit-Reviewer: Stéphane Marchesin <mar...@chromium.org>
Gerrit-Reviewer: Xing Zheng <zhen...@rock-chips.com>
Gerrit-Reviewer: Yakir Yang <y...@rock-chips.com>
Gerrit-HasComments: No

Sean Paul (Gerrit)

unread,
Oct 5, 2016, 1:39:12 PM10/5/16
to Sean Paul, Lin Huang, Derek Basehore, Xing Zheng, Caesar Wang, Yakir Yang, Stéphane Marchesin, Nick Sanders, Douglas Anderson
Sean Paul has posted comments on this change. (
https://chromium-review.googlesource.com/362907 )

Change subject: CHROMIUM: drm/rockchip: Toggle devfreq on/off
......................................................................


Patch Set 25:

> 1
This is by design. You stated in your original patch that we shouldn't
transition the freq while vop is being enabled/disabled

> 2
This seems to be caused by the inversion dbaseshore mentioned in the
previous comment. I'm not receiving the postchange event, so the wait_event
will never trigger.

> 3
I didn't follow, can you please explain?

Lin Huang (Gerrit)

unread,
Oct 5, 2016, 1:57:16 PM10/5/16
to Sean Paul, Derek Basehore, Xing Zheng, Caesar Wang, Yakir Yang, Stéphane Marchesin, Nick Sanders, Douglas Anderson
Lin Huang has posted comments on this change. (
https://chromium-review.googlesource.com/362907 )

Change subject: CHROMIUM: drm/rockchip: Toggle devfreq on/off
......................................................................


Patch Set 25:

> > 1
> This is by design. You stated in your original patch that we
> shouldn't transition the freq while vop is being enabled/disabled
>
i think we can modify like:
mutex_unlock(&priv->dmc_mutex);
if (num_active_crtcs == 1)
rockchip_drm_enable_dmc(priv);
since we can not do the frequency transition when vop enable/disabel,
but here we have finished it. and if we do like this, we can fix this
problem.

> > 2
> This seems to be caused by the inversion dbaseshore mentioned in
> the previous comment. I'm not receiving the postchange event, so
> the wait_event will never trigger.
Derek comment is about the NO.1 issuse, you can check my comment "OCT3
1:41PM", i think it may be cause by when we call the waitevent(), the wake
up have been executed. and the workqueue may never wake up.
>
> > 3
> I didn't follow, can you please explain?
since when we do the ddr dvfs, the power domain can not operate, so for now
i add a mutex to handle, but when the vop enable/disable, it will
disable/enable the power domain, they will all try to get the dmc_mutex and
pd_mutex, it may cause dead lock.

Sean Paul (Gerrit)

unread,
Oct 5, 2016, 2:04:07 PM10/5/16
to Sean Paul, Lin Huang, Derek Basehore, Xing Zheng, Caesar Wang, Yakir Yang, Stéphane Marchesin, Nick Sanders, Douglas Anderson
Sean Paul has posted comments on this change. (
https://chromium-review.googlesource.com/362907 )

Change subject: CHROMIUM: drm/rockchip: Toggle devfreq on/off
......................................................................


Patch Set 25:

> but here we have finished it. and if we do like this, we can fix this
> problem.

Doesn't the same problem exist for the disable_dmc case above?

Lin Huang (Gerrit)

unread,
Oct 5, 2016, 2:13:42 PM10/5/16
to Sean Paul, Derek Basehore, Xing Zheng, Caesar Wang, Yakir Yang, Stéphane Marchesin, Nick Sanders, Douglas Anderson
Lin Huang has posted comments on this change. (
https://chromium-review.googlesource.com/362907 )

Change subject: CHROMIUM: drm/rockchip: Toggle devfreq on/off
......................................................................


Patch Set 25:

(1 comment)

https://chromium-review.googlesource.com/#/c/362907/25/drivers/gpu/drm/rockchip/rockchip_drm_fb.c
File drivers/gpu/drm/rockchip/rockchip_drm_fb.c:

Line 306: }
so can we modify code like that:
if (active_changed) {
mutex_unlock(&priv->dmc_mutex);
if (num_active_crtcs == 1)
rockchip_drm_enable_dmc(priv);
else if (num_active_crtcs > 1)
rockchip_drm_disable_dmc(priv);

}



--
To view, visit https://chromium-review.googlesource.com/362907
To unsubscribe, visit https://chromium-review.googlesource.com/settings

Gerrit-MessageType: comment
Gerrit-Change-Id: I690ae1c0d2436b957fc823585c1a32b48cfcf3fd
Gerrit-PatchSet: 25
Gerrit-Project: chromiumos/third_party/kernel
Gerrit-Branch: chromeos-4.4
Gerrit-Owner: Lin Huang <h...@rock-chips.com>
Gerrit-Reviewer: Caesar Wang <w...@rock-chips.com>
Gerrit-Reviewer: Derek Basehore <dbas...@chromium.org>
Gerrit-Reviewer: Douglas Anderson <dian...@chromium.org>
Gerrit-Reviewer: Lin Huang <h...@rock-chips.com>
Gerrit-Reviewer: Nick Sanders <nsan...@chromium.org>
Gerrit-Reviewer: Sean Paul <sean...@chromium.org>
Gerrit-Reviewer: Stéphane Marchesin <mar...@chromium.org>
Gerrit-Reviewer: Xing Zheng <zhen...@rock-chips.com>
Gerrit-Reviewer: Yakir Yang <y...@rock-chips.com>
Gerrit-HasComments: Yes

Lin Huang (Gerrit)

unread,
Oct 5, 2016, 2:17:14 PM10/5/16
to Sean Paul, Derek Basehore, Xing Zheng, Caesar Wang, Yakir Yang, Stéphane Marchesin, Nick Sanders, Douglas Anderson
Lin Huang has posted comments on this change. (
https://chromium-review.googlesource.com/362907 )

Change subject: CHROMIUM: drm/rockchip: Toggle devfreq on/off
......................................................................


Patch Set 25:

(2 comments)

https://chromium-review.googlesource.com/#/c/362907/25/drivers/gpu/drm/rockchip/rockchip_drm_drv.c
File drivers/gpu/drm/rockchip/rockchip_drm_drv.c:

Line 103: {
for problem 2, i think we also need to modify here:
mutex_lock(&priv->dmc_mutex);
if (event == DEVFREQ_PRECHANGE) {
priv->dmc_in_process = 1;
} else if (event == DEVFREQ_POSTCHANGE) {
priv->dmc_in_process = 0;
}
mutex_unlock(&priv->dmc_mutex);
if (event == DEVFREQ_POSTCHANGE)
wake_up(&priv->wait_dmc_queue);


https://chromium-review.googlesource.com/#/c/362907/25/drivers/gpu/drm/rockchip/rockchip_drm_fb.c
File drivers/gpu/drm/rockchip/rockchip_drm_fb.c:

Line 239: mutex_lock(&priv->dmc_mutex);
and about the problem 2, i think we can modify it to:
{
if (!mutex_trylock(&priv->dmc_mutex))
return 1;

if (!priv->dmc_in_process)
return 0;

mutex_unlock(&priv->dmc_mutex);
return 1;
}
what do you think about it?

Derek Basehore (Gerrit)

unread,
Oct 5, 2016, 6:36:08 PM10/5/16
to Sean Paul, Lin Huang, Xing Zheng, Caesar Wang, Yakir Yang, Stéphane Marchesin, Nick Sanders, Douglas Anderson
Derek Basehore has posted comments on this change. (
https://chromium-review.googlesource.com/362907 )

Change subject: CHROMIUM: drm/rockchip: Toggle devfreq on/off
......................................................................


Patch Set 24:

(1 comment)

https://chromium-review.googlesource.com/#/c/362907/24/drivers/gpu/drm/rockchip/rockchip_drm_fb.c
File drivers/gpu/drm/rockchip/rockchip_drm_fb.c:

Line 271: rockchip_drm_disable_dmc(priv);
> Yes, since that ensures the scaling doesn't kick in during disable/enable.
We will probably want to change the frequency during disable.

Also, this is a deadlock, you should never have those by design. You won't
perform scaling, but this thread won't be doing much else either.



--
To view, visit https://chromium-review.googlesource.com/362907
To unsubscribe, visit https://chromium-review.googlesource.com/settings

Gerrit-MessageType: comment
Gerrit-Change-Id: I690ae1c0d2436b957fc823585c1a32b48cfcf3fd
Gerrit-PatchSet: 24

Lin Huang (Gerrit)

unread,
Oct 5, 2016, 9:35:20 PM10/5/16
to Sean Paul, Derek Basehore, Xing Zheng, Caesar Wang, Yakir Yang, Stéphane Marchesin, Nick Sanders, Douglas Anderson
Lin Huang has posted comments on this change. (
https://chromium-review.googlesource.com/362907 )

Change subject: CHROMIUM: drm/rockchip: Toggle devfreq on/off
......................................................................


Patch Set 25:

(1 comment)

https://chromium-review.googlesource.com/#/c/362907/25/drivers/gpu/drm/rockchip/rockchip_drm_drv.c
File drivers/gpu/drm/rockchip/rockchip_drm_drv.c:

Line 151: priv->dmc_nb.notifier_call = dmc_notify;
it seems if i add "priv->dmc_nb.priority = 1;" here, we can workaround the
problem 3. Since the deadlock casue by the devfreq got the pd_lock and try
to get the dmc_lock, but vop got the dmc_lock first, and try to get the
pd_lock, so dead lock happen. with this change, the devfreq and vop will
alway to get the dmc lock first, so can avoid it. But this may be a
workaround way, hope somebody can figure out a better way to fix it.



--
To view, visit https://chromium-review.googlesource.com/362907
To unsubscribe, visit https://chromium-review.googlesource.com/settings

Gerrit-MessageType: comment
Gerrit-Change-Id: I690ae1c0d2436b957fc823585c1a32b48cfcf3fd
Gerrit-PatchSet: 25

Stéphane Marchesin (Gerrit)

unread,
Oct 6, 2016, 7:57:34 PM10/6/16
to Sean Paul, Lin Huang, Derek Basehore, Xing Zheng, Caesar Wang, Yakir Yang, Nick Sanders, Douglas Anderson
Stéphane Marchesin has posted comments on this change. (
https://chromium-review.googlesource.com/362907 )

Change subject: CHROMIUM: drm/rockchip: Toggle devfreq on/off
......................................................................


Patch Set 25:

(1 comment)

https://chromium-review.googlesource.com/#/c/362907/25/drivers/gpu/drm/rockchip/rockchip_drm_fb.c
File drivers/gpu/drm/rockchip/rockchip_drm_fb.c:

Line 239: mutex_lock(&priv->dmc_mutex);
> and about the problem 2, i think we can modify it to:
why not simply put all this in the caller? It would be simpler than having
two different sites lock/unlock. It would also avoid some of this
complicated logic.

Lin Huang (Gerrit)

unread,
Nov 2, 2016, 2:21:03 AM11/2/16
to Sean Paul, Stéphane Marchesin, Derek Basehore, Xing Zheng, Caesar Wang, Yakir Yang, Nick Sanders, Douglas Anderson

Lin Huang posted comments on CHROMIUM: drm/rockchip: Toggle devfreq on/off.

View Change

Patch Set 27:

(1 comment)

To view, visit this change. To unsubscribe, visit settings.

Gerrit-MessageType: comment
Gerrit-Change-Id: I690ae1c0d2436b957fc823585c1a32b48cfcf3fd
Gerrit-PatchSet: 27


Gerrit-Project: chromiumos/third_party/kernel
Gerrit-Branch: chromeos-4.4
Gerrit-Owner: Lin Huang <h...@rock-chips.com>

Gerrit-HasComments: Yes

智情 姚 (Gerrit)

unread,
Nov 3, 2016, 5:17:46 AM11/3/16
to Sean Paul, Lin Huang, Stéphane Marchesin, Derek Basehore, Xing Zheng, Caesar Wang, Yakir Yang, Nick Sanders, Douglas Anderson

智情 姚 posted comments on CHROMIUM: drm/rockchip: Toggle devfreq on/off.

View Change

Patch Set 27:

(1 comment)
  • File drivers/gpu/drm/rockchip/rockchip_drm_fb.c:

    • Patch Set #27, Line 258:

      for_each_crtc_in_state(state, crtc, crtc_state, i) {
      		if (crtc_state->active)
      			num_active_crtcs++;
      		if (crtc_state->active_changed)
      			active_changed = true;
      	}
      

      if only one crtc status change, use for_each_crtc_in_state only can found one crtc state. rockchip_atomic_commit_complete is after drm_atomic_helper_swap_state, so crtc->state is new state. So I think we can change like that: drm_for_each_crtc(crtc,dev) { if (crtc->state->active) num_active_crtcs++; if (crtc->state->active_changed) active_changed = true; }

Stéphane Marchesin (Gerrit)

unread,
Nov 3, 2016, 10:07:09 PM11/3/16
to Sean Paul, Lin Huang, 智情 姚, Derek Basehore, Xing Zheng, Caesar Wang, Yakir Yang, Nick Sanders, Douglas Anderson

Stéphane Marchesin posted comments on CHROMIUM: drm/rockchip: Toggle devfreq on/off.

View Change

Patch Set 27:

(1 comment)

Lin Huang (Gerrit)

unread,
Nov 3, 2016, 11:51:01 PM11/3/16
to Sean Paul, Stéphane Marchesin, 智情 姚, Derek Basehore, Xing Zheng, Caesar Wang, Yakir Yang, Nick Sanders, Douglas Anderson

Lin Huang posted comments on CHROMIUM: drm/rockchip: Toggle devfreq on/off.

View Change

Patch Set 27:

(1 comment)
    • for_each_crtc_in_state(state, crtc, crtc_state, i) {
      		if (crtc_state->active)
      			num_active_crtcs++;
      		if (crtc_state->active_changed)
      			active_changed = true;
      	}
      

      if only one crtc status change, use for_each_crtc_in_state only can found o

    • maybe we should use: drm_for_each_crtc(crtc,dev) { if (crtc->state->active) num_active_crtcs++; if (crtc->state->active_changed) active_changed = true; }

Lin Huang (Gerrit)

unread,
Nov 7, 2016, 2:46:07 AM11/7/16
to Sean Paul, Caesar Wang, Xing Zheng, Douglas Anderson, Stéphane Marchesin, Derek Basehore, Yakir Yang, Nick Sanders, 智情 姚

Lin Huang uploaded patch set #28 to CHROMIUM: drm/rockchip: Toggle devfreq on/off.

View Change

CHROMIUM: drm/rockchip: Toggle devfreq on/off

This patch adds devfreq to the rockchip drm driver. It will activate
when there is one crtc active, and disable if more than one becomes
active (to avoid flickering on one of the screens).

BUG=chrome-os-partner:54651
TEST=kevin compiles/boots

Signed-off-by: Sean Paul <sean...@chromium.org>
Change-Id: I690ae1c0d2436b957fc823585c1a32b48cfcf3fd
---
M drivers/gpu/drm/rockchip/rockchip_drm_drv.c
M drivers/gpu/drm/rockchip/rockchip_drm_drv.h
M drivers/gpu/drm/rockchip/rockchip_drm_fb.c
3 files changed, 138 insertions(+), 0 deletions(-)

To view, visit this change. To unsubscribe, visit settings.

Gerrit-MessageType: newpatchset
Gerrit-Change-Id: I690ae1c0d2436b957fc823585c1a32b48cfcf3fd
Gerrit-PatchSet: 28

Stéphane Marchesin (Gerrit)

unread,
Nov 7, 2016, 7:30:30 PM11/7/16
to Sean Paul, Lin Huang, 智情 姚, Derek Basehore, Xing Zheng, Caesar Wang, Yakir Yang, Nick Sanders, Douglas Anderson

Stéphane Marchesin posted comments on CHROMIUM: drm/rockchip: Toggle devfreq on/off.

View Change

Patch Set 28: Code-Review-2

instead of repeating my comment I'm going to mark -2...

    To view, visit this change. To unsubscribe, visit settings.

    Gerrit-MessageType: comment


    Gerrit-Change-Id: I690ae1c0d2436b957fc823585c1a32b48cfcf3fd
    Gerrit-PatchSet: 28
    Gerrit-Project: chromiumos/third_party/kernel
    Gerrit-Branch: chromeos-4.4
    Gerrit-Owner: Lin Huang <h...@rock-chips.com>

    Gerrit-HasComments: No

    Lin Huang (Gerrit)

    unread,
    Nov 9, 2016, 2:08:28 AM11/9/16
    to Sean Paul, Caesar Wang, Xing Zheng, Douglas Anderson, Stéphane Marchesin, Derek Basehore, Yakir Yang, Nick Sanders, 智情 姚

    Lin Huang uploaded patch set #29 to CHROMIUM: drm/rockchip: Toggle devfreq on/off.

    View Change

    CHROMIUM: drm/rockchip: Toggle devfreq on/off
    
    This patch adds devfreq to the rockchip drm driver. It will activate
    when there is one crtc active, and disable if more than one becomes
    active (to avoid flickering on one of the screens).
    
    BUG=chrome-os-partner:54651
    TEST=kevin compiles/boots
    
    Change-Id: I690ae1c0d2436b957fc823585c1a32b48cfcf3fd
    Signed-off-by: Sean Paul <sean...@chromium.org>
    Signed-off-by: Lin Huang <h...@rock-chips.com>
    ---
    M drivers/gpu/drm/rockchip/rockchip_drm_drv.c
    M drivers/gpu/drm/rockchip/rockchip_drm_drv.h
    M drivers/gpu/drm/rockchip/rockchip_drm_fb.c
    3 files changed, 142 insertions(+), 0 deletions(-)
    
    

    To view, visit this change. To unsubscribe, visit settings.

    Gerrit-MessageType: newpatchset
    Gerrit-Change-Id: I690ae1c0d2436b957fc823585c1a32b48cfcf3fd
    Gerrit-PatchSet: 29

    Lin Huang (Gerrit)

    unread,
    Nov 9, 2016, 2:12:02 AM11/9/16
    to Sean Paul, Stéphane Marchesin, 智情 姚, Derek Basehore, Xing Zheng, Caesar Wang, Yakir Yang, Nick Sanders, Douglas Anderson

    Lin Huang posted comments on CHROMIUM: drm/rockchip: Toggle devfreq on/off.

    View Change

    Patch Set 27:
    
    (1 comment)
      • same comment as before you should inline this to make locking simpler to un

      • sorry miss your comment before, in your before comment, you said "why not simply put all this in the caller?", i am sorry not got what you mean, can you explain in detail?

    To view, visit this change. To unsubscribe, visit settings.

    Gerrit-MessageType: comment
    Gerrit-Change-Id: I690ae1c0d2436b957fc823585c1a32b48cfcf3fd
    Gerrit-PatchSet: 27


    Gerrit-Project: chromiumos/third_party/kernel
    Gerrit-Branch: chromeos-4.4
    Gerrit-Owner: Lin Huang <h...@rock-chips.com>

    Gerrit-HasComments: Yes

    Derek Basehore (Gerrit)

    unread,
    Nov 11, 2016, 7:25:56 PM11/11/16
    to Sean Paul, Lin Huang, Stéphane Marchesin, 智情 姚, Xing Zheng, Caesar Wang, Yakir Yang, Nick Sanders, Douglas Anderson

    Derek Basehore posted comments on CHROMIUM: drm/rockchip: Toggle devfreq on/off.

    View Change

    Patch Set 25: (1 comment)
      • why not simply put all this in the caller? It would be simpler than having

      • Sean wrote this code, but it seems we need to hold the mutex while checking the condition !priv->dmc_in_progress. wait_event just requires this as a wrapper so it will lock the mutex whenever it checks the condition.

    To view, visit this change. To unsubscribe, visit settings.

    Gerrit-MessageType: comment
    Gerrit-Change-Id: I690ae1c0d2436b957fc823585c1a32b48cfcf3fd
    Gerrit-PatchSet: 25
    Gerrit-Project: chromiumos/third_party/kernel
    Gerrit-Branch: chromeos-4.4
    Gerrit-Owner: Lin Huang <h...@rock-chips.com>Gerrit-Reviewer: Caesar Wang <w...@rock-chips.com>Gerrit-Reviewer: Derek Basehore <dbas...@chromium.org>Gerrit-Reviewer: Douglas Anderson <dian...@chromium.org>Gerrit-Reviewer: Lin Huang <h...@rock-chips.com>Gerrit-Reviewer: Nick Sanders <nsan...@chromium.org>Gerrit-Reviewer: Sean Paul <sean...@chromium.org>Gerrit-Reviewer: Stéphane Marchesin <mar...@chromium.org>Gerrit-Reviewer: Xing Zheng <zhen...@rock-chips.com>Gerrit-Reviewer: Yakir Yang <y...@rock-chips.com>Gerrit-Reviewer: 智情 姚 <mark...@rock-chips.com>

    Gerrit-HasComments: Yes

    Stéphane Marchesin (Gerrit)

    unread,
    Nov 16, 2016, 8:54:50 PM11/16/16
    to Sean Paul, Lin Huang, Derek Basehore, 智情 姚, Xing Zheng, Caesar Wang, Yakir Yang, Nick Sanders, Douglas Anderson

    Stéphane Marchesin posted comments on CHROMIUM: drm/rockchip: Toggle devfreq on/off.

    View Change

    Patch Set 27: (1 comment)
      • sorry miss your comment before,

      • Put all this code in the only caller. That way, the locking/unlocking happens in the same function which is much easier to read/understand.

    To view, visit this change. To unsubscribe, visit settings.

    Gerrit-MessageType: comment
    Gerrit-Change-Id: I690ae1c0d2436b957fc823585c1a32b48cfcf3fd
    Gerrit-PatchSet: 27


    Gerrit-Project: chromiumos/third_party/kernel
    Gerrit-Branch: chromeos-4.4
    Gerrit-Owner: Lin Huang <h...@rock-chips.com>Gerrit-Reviewer: Caesar Wang <w...@rock-chips.com>Gerrit-Reviewer: Derek Basehore <dbas...@chromium.org>Gerrit-Reviewer: Douglas Anderson <dian...@chromium.org>Gerrit-Reviewer: Lin Huang <h...@rock-chips.com>Gerrit-Reviewer: Nick Sanders <nsan...@chromium.org>Gerrit-Reviewer: Sean Paul <sean...@chromium.org>Gerrit-Reviewer: Stéphane Marchesin <mar...@chromium.org>Gerrit-Reviewer: Xing Zheng <zhen...@rock-chips.com>Gerrit-Reviewer: Yakir Yang <y...@rock-chips.com>Gerrit-Reviewer: 智情 姚 <mark...@rock-chips.com>

    Gerrit-HasComments: Yes

    Lin Huang (Gerrit)

    unread,
    Nov 16, 2016, 9:13:29 PM11/16/16
    to Sean Paul, Stéphane Marchesin, Derek Basehore, 智情 姚, Xing Zheng, Caesar Wang, Yakir Yang, Nick Sanders, Douglas Anderson

    Lin Huang posted comments on CHROMIUM: drm/rockchip: Toggle devfreq on/off.

    View Change

    Patch Set 27: (1 comment)
      • Put all this code in the only caller. That way, the locking/unlocking happe

        but the caller is a wait_event condition check, and we need to check the lock, it seem hard to put then into the caller.

    Derek Basehore (Gerrit)

    unread,
    Nov 16, 2016, 9:24:24 PM11/16/16
    to Sean Paul, Lin Huang, Stéphane Marchesin, 智情 姚, Xing Zheng, Caesar Wang, Yakir Yang, Nick Sanders, Douglas Anderson

    Derek Basehore posted comments on CHROMIUM: drm/rockchip: Toggle devfreq on/off.

    View Change

    Patch Set 27: (1 comment)
      • but the caller is a wait_event condition check, and we need to check the lo

        You can technically do this with a code block, but that's a non-standard C extension. The other option is to rewrite the code to something like it was in the earlier versions.

    Lin Huang (Gerrit)

    unread,
    Nov 16, 2016, 9:53:44 PM11/16/16
    to Sean Paul, Derek Basehore, Stéphane Marchesin, 智情 姚, Xing Zheng, Caesar Wang, Yakir Yang, Nick Sanders, Douglas Anderson

    Lin Huang posted comments on CHROMIUM: drm/rockchip: Toggle devfreq on/off.

    View Change

    Patch Set 27: (1 comment)
      • You can technically do this with a code block, but that's a non-standard C

        it very early versions, and Sean have refactor it, it hard to go back now.

    Derek Basehore (Gerrit)

    unread,
    Nov 16, 2016, 11:21:25 PM11/16/16
    to Sean Paul, Lin Huang, Stéphane Marchesin, 智情 姚, Xing Zheng, Caesar Wang, Yakir Yang, Nick Sanders, Douglas Anderson

    Derek Basehore posted comments on CHROMIUM: drm/rockchip: Toggle devfreq on/off.

    View Change

    Patch Set 27: (1 comment)
      • I think we could just switch to a mutex/semaphore by itself.

    Derek Basehore (Gerrit)

    unread,
    Nov 23, 2016, 12:30:13 AM11/23/16
    to Sean Paul, Lin Huang, Caesar Wang, Xing Zheng, Douglas Anderson, Stéphane Marchesin, Yakir Yang, Nick Sanders, 智情 姚

    Derek Basehore uploaded patch set #31 to this change.

    View Change

    CHROMIUM: drm/rockchip: Toggle devfreq on/off
    
    This patch adds devfreq to the rockchip drm driver. It will activate
    when there is one crtc active, and disable if more than one becomes
    active (to avoid flickering on one of the screens).
    
    BUG=chrome-os-partner:54651
    TEST=kevin compiles/boots
    
    Change-Id: I690ae1c0d2436b957fc823585c1a32b48cfcf3fd
    Signed-off-by: Sean Paul <sean...@chromium.org>
    Signed-off-by: Lin Huang <h...@rock-chips.com
    >
    ---
    M drivers/gpu/drm/rockchip/rockchip_drm_drv.c
    M drivers/gpu/drm/rockchip/rockchip_drm_drv.h
    M drivers/gpu/drm/rockchip/rockchip_drm_fb.c
    3 files changed, 117 insertions(+), 0 deletions(-)
    
    

    To view, visit this change. To unsubscribe, visit settings.

    Gerrit-MessageType: newpatchset
    Gerrit-Change-Id: I690ae1c0d2436b957fc823585c1a32b48cfcf3fd
    Gerrit-Change-Number: 362907
    Gerrit-PatchSet: 31

    Derek Basehore (Gerrit)

    unread,
    Nov 23, 2016, 12:32:24 AM11/23/16
    to Sean Paul, Lin Huang, Stéphane Marchesin, 智情 姚, Xing Zheng, Caesar Wang, Yakir Yang, Nick Sanders, Douglas Anderson

    Derek Basehore posted comments on this change.

    View Change

    Patch Set 31: (2 comments)
      • I think we could just switch to a mutex/semaphore by itself.

        Okay, I did what I suggested. This might cause a race condition later on, but that will have to be fixed separately. See other comment about race condition.

    • File drivers/gpu/drm/rockchip/rockchip_drm_fb.c:

      • Patch Set #31, Line 298: priv->dmc_disable_flag = true;

        I'm not sure if these have race conditions. Is this function protected by a mutex (or something) that will prevent multiple threads from accessing these values?

    To view, visit this change. To unsubscribe, visit settings.

    Gerrit-MessageType: comment


    Gerrit-Change-Id: I690ae1c0d2436b957fc823585c1a32b48cfcf3fd
    Gerrit-Change-Number: 362907
    Gerrit-PatchSet: 31
    Gerrit-Project: chromiumos/third_party/kernel
    Gerrit-Branch: chromeos-4.4
    Gerrit-Owner: Lin Huang <h...@rock-chips.com>Gerrit-Reviewer: Caesar Wang <w...@rock-chips.com>Gerrit-Reviewer: Derek Basehore <dbas...@chromium.org>Gerrit-Reviewer: Douglas Anderson <dian...@chromium.org>Gerrit-Reviewer: Lin Huang <h...@rock-chips.com>Gerrit-Reviewer: Nick Sanders <nsan...@chromium.org>Gerrit-Reviewer: Sean Paul <sean...@chromium.org>Gerrit-Reviewer: Stéphane Marchesin <mar...@chromium.org>Gerrit-Reviewer: Xing Zheng <zhen...@rock-chips.com>Gerrit-Reviewer: Yakir Yang <y...@rock-chips.com>Gerrit-Reviewer: 智情 姚 <mark...@rock-chips.com>

    Gerrit-HasComments: Yes

    Lin Huang (Gerrit)

    unread,
    Nov 23, 2016, 5:06:41 AM11/23/16
    to Sean Paul, Derek Basehore, Stéphane Marchesin, 智情 姚, Xing Zheng, Caesar Wang, Yakir Yang, Nick Sanders, Douglas Anderson

    Lin Huang posted comments on this change.

    View Change

    Patch Set 31: (1 comment)
      • Patch Set #31, Line 298: priv->dmc_disable_flag = true;

        I'm not sure if these have race conditions. Is this function protected by a

      • i think this function will protect by the commit->lock

    Lin Huang (Gerrit)

    unread,
    Nov 23, 2016, 7:59:25 AM11/23/16
    to Sean Paul, Caesar Wang, Xing Zheng, Douglas Anderson, Stéphane Marchesin, Derek Basehore, Yakir Yang, Nick Sanders, 智情 姚

    Lin Huang uploaded patch set #33 to this change.

    View Change

    CHROMIUM: drm/rockchip: Toggle devfreq on/off
    
    This patch adds devfreq to the rockchip drm driver. It will activate
    when there is one crtc active, and disable if more than one becomes
    active (to avoid flickering on one of the screens).
    
    BUG=chrome-os-partner:54651
    TEST=kevin compiles/boots
    
    Change-Id: I690ae1c0d2436b957fc823585c1a32b48cfcf3fd
    Signed-off-by: Sean Paul <sean...@chromium.org>
    Signed-off-by: Lin Huang <h...@rock-chips.com>
    ---
    M drivers/gpu/drm/rockchip/rockchip_drm_drv.c
    M drivers/gpu/drm/rockchip/rockchip_drm_drv.h
    M drivers/gpu/drm/rockchip/rockchip_drm_fb.c
    3 files changed, 117 insertions(+), 0 deletions(-)
    
    

    To view, visit this change. To unsubscribe, visit settings.

    Gerrit-MessageType: newpatchset
    Gerrit-Change-Id: I690ae1c0d2436b957fc823585c1a32b48cfcf3fd
    Gerrit-Change-Number: 362907
    Gerrit-PatchSet: 33

    Derek Basehore (Gerrit)

    unread,
    Nov 30, 2016, 7:26:57 PM11/30/16
    to Sean Paul, Lin Huang, Stéphane Marchesin, 智情 姚, Xing Zheng, Caesar Wang, Yakir Yang, Nick Sanders, Douglas Anderson

    Derek Basehore posted comments on this change.

    View Change

    Patch Set 31: (1 comment)
      • i think this function will protect by the commit->lock

        Is that lock shared by all of the displays? Also, is priv shared by all of the displays?

    To view, visit this change. To unsubscribe, visit settings.

    Gerrit-MessageType: comment
    Gerrit-Change-Id: I690ae1c0d2436b957fc823585c1a32b48cfcf3fd
    Gerrit-Change-Number: 362907
    Gerrit-PatchSet: 31


    Gerrit-Project: chromiumos/third_party/kernel
    Gerrit-Branch: chromeos-4.4
    Gerrit-Owner: Lin Huang <h...@rock-chips.com>Gerrit-Reviewer: Caesar Wang <w...@rock-chips.com>Gerrit-Reviewer: Derek Basehore <dbas...@chromium.org>Gerrit-Reviewer: Douglas Anderson <dian...@chromium.org>Gerrit-Reviewer: Lin Huang <h...@rock-chips.com>Gerrit-Reviewer: Nick Sanders <nsan...@chromium.org>Gerrit-Reviewer: Sean Paul <sean...@chromium.org>Gerrit-Reviewer: Stéphane Marchesin <mar...@chromium.org>Gerrit-Reviewer: Xing Zheng <zhen...@rock-chips.com>Gerrit-Reviewer: Yakir Yang <y...@rock-chips.com>Gerrit-Reviewer: 智情 姚 <mark...@rock-chips.com>

    Gerrit-Comment-Date: Thu, 01 Dec 2016 00:26:54 +0000Gerrit-HasComments: Yes

    Stéphane Marchesin (Gerrit)

    unread,
    Nov 30, 2016, 9:08:12 PM11/30/16
    to Sean Paul, Lin Huang, Derek Basehore, 智情 姚, Xing Zheng, Caesar Wang, Yakir Yang, Nick Sanders, Douglas Anderson

    Stéphane Marchesin posted comments on this change.

    View Change

    Patch Set 33: (1 comment)

    To view, visit this change. To unsubscribe, visit settings.

    Gerrit-MessageType: comment
    Gerrit-Change-Id: I690ae1c0d2436b957fc823585c1a32b48cfcf3fd
    Gerrit-Change-Number: 362907

    Gerrit-PatchSet: 33


    Gerrit-Project: chromiumos/third_party/kernel
    Gerrit-Branch: chromeos-4.4
    Gerrit-Owner: Lin Huang <h...@rock-chips.com>Gerrit-Reviewer: Caesar Wang <w...@rock-chips.com>Gerrit-Reviewer: Derek Basehore <dbas...@chromium.org>Gerrit-Reviewer: Douglas Anderson <dian...@chromium.org>Gerrit-Reviewer: Lin Huang <h...@rock-chips.com>Gerrit-Reviewer: Nick Sanders <nsan...@chromium.org>Gerrit-Reviewer: Sean Paul <sean...@chromium.org>Gerrit-Reviewer: Stéphane Marchesin <mar...@chromium.org>Gerrit-Reviewer: Xing Zheng <zhen...@rock-chips.com>Gerrit-Reviewer: Yakir Yang <y...@rock-chips.com>Gerrit-Reviewer: 智情 姚 <mark...@rock-chips.com>

    Gerrit-Comment-Date: Thu, 01 Dec 2016 02:08:06 +0000Gerrit-HasComments: Yes

    Lin Huang (Gerrit)

    unread,
    Nov 30, 2016, 9:46:44 PM11/30/16
    to Sean Paul, Stéphane Marchesin, Derek Basehore, 智情 姚, Xing Zheng, Caesar Wang, Yakir Yang, Nick Sanders, Douglas Anderson

    Lin Huang posted comments on this change.

    View Change

    Patch Set 33: (2 comments)
      • Patch Set #33, Line 295: priv->dmc_disable_flag = false;

        if we have only one display but that's an external (say hdmi) display, shou

      • we need to enable dmc when we have one display(even it is external), the enable/disable dmc only happen the display 2->1 or 1->2.

    To view, visit this change. To unsubscribe, visit settings.

    Gerrit-MessageType: comment
    Gerrit-Change-Id: I690ae1c0d2436b957fc823585c1a32b48cfcf3fd
    Gerrit-Change-Number: 362907
    Gerrit-PatchSet: 33
    Gerrit-Project: chromiumos/third_party/kernel
    Gerrit-Branch: chromeos-4.4
    Gerrit-Owner: Lin Huang <h...@rock-chips.com>Gerrit-Reviewer: Caesar Wang <w...@rock-chips.com>Gerrit-Reviewer: Derek Basehore <dbas...@chromium.org>Gerrit-Reviewer: Douglas Anderson <dian...@chromium.org>Gerrit-Reviewer: Lin Huang <h...@rock-chips.com>Gerrit-Reviewer: Nick Sanders <nsan...@chromium.org>Gerrit-Reviewer: Sean Paul <sean...@chromium.org>Gerrit-Reviewer: Stéphane Marchesin <mar...@chromium.org>Gerrit-Reviewer: Xing Zheng <zhen...@rock-chips.com>Gerrit-Reviewer: Yakir Yang <y...@rock-chips.com>Gerrit-Reviewer: 智情 姚 <mark...@rock-chips.com>

    Gerrit-Comment-Date: Thu, 01 Dec 2016 02:46:36 +0000Gerrit-HasComments: Yes

    Derek Basehore (Gerrit)

    unread,
    Dec 1, 2016, 4:44:00 PM12/1/16
    to Sean Paul, Lin Huang, Stéphane Marchesin, 智情 姚, Xing Zheng, Caesar Wang, Yakir Yang, Nick Sanders, Douglas Anderson

    Derek Basehore posted comments on this change.

    View Change

    Patch Set 33: (1 comment)
      • Patch Set #33, Line 295: priv->dmc_disable_flag = false;

        if we have only one display but that's an external (say hdmi) display, shou

      • We should check the vblank time against a set value. 300us is a large enough limit with plenty of room. It takes around 200us to change the rate. It doesn't have much variance either.

    To view, visit this change. To unsubscribe, visit settings.

    Gerrit-MessageType: comment
    Gerrit-Change-Id: I690ae1c0d2436b957fc823585c1a32b48cfcf3fd
    Gerrit-Change-Number: 362907
    Gerrit-PatchSet: 33
    Gerrit-Project: chromiumos/third_party/kernel
    Gerrit-Branch: chromeos-4.4
    Gerrit-Owner: Lin Huang <h...@rock-chips.com>Gerrit-Reviewer: Caesar Wang <w...@rock-chips.com>Gerrit-Reviewer: Derek Basehore <dbas...@chromium.org>Gerrit-Reviewer: Douglas Anderson <dian...@chromium.org>Gerrit-Reviewer: Lin Huang <h...@rock-chips.com>Gerrit-Reviewer: Nick Sanders <nsan...@chromium.org>Gerrit-Reviewer: Sean Paul <sean...@chromium.org>Gerrit-Reviewer: Stéphane Marchesin <mar...@chromium.org>Gerrit-Reviewer: Xing Zheng <zhen...@rock-chips.com>Gerrit-Reviewer: Yakir Yang <y...@rock-chips.com>Gerrit-Reviewer: 智情 姚 <mark...@rock-chips.com>

    Gerrit-Comment-Date: Thu, 01 Dec 2016 21:43:55 +0000Gerrit-HasComments: Yes

    Lin Huang (Gerrit)

    unread,
    Dec 1, 2016, 7:57:28 PM12/1/16
    to Sean Paul, Derek Basehore, Stéphane Marchesin, 智情 姚, Xing Zheng, Caesar Wang, Yakir Yang, Nick Sanders, Douglas Anderson

    Lin Huang posted comments on this change.

    View Change

    Patch Set 33: (1 comment)
      • We should check the vblank time against a set value. 300us is a large enoug

        i have measure the change rate time(use gpio), and we only care the time between idele_port and deidle_port(the flow in the M0 code), since when in this time we can not access the ddr, it take 90us now.

    To view, visit this change. To unsubscribe, visit settings.

    Gerrit-MessageType: comment
    Gerrit-Change-Id: I690ae1c0d2436b957fc823585c1a32b48cfcf3fd
    Gerrit-Change-Number: 362907
    Gerrit-PatchSet: 33
    Gerrit-Project: chromiumos/third_party/kernel
    Gerrit-Branch: chromeos-4.4
    Gerrit-Owner: Lin Huang <h...@rock-chips.com>Gerrit-Reviewer: Caesar Wang <w...@rock-chips.com>Gerrit-Reviewer: Derek Basehore <dbas...@chromium.org>Gerrit-Reviewer: Douglas Anderson <dian...@chromium.org>Gerrit-Reviewer: Lin Huang <h...@rock-chips.com>Gerrit-Reviewer: Nick Sanders <nsan...@chromium.org>Gerrit-Reviewer: Sean Paul <sean...@chromium.org>Gerrit-Reviewer: Stéphane Marchesin <mar...@chromium.org>Gerrit-Reviewer: Xing Zheng <zhen...@rock-chips.com>Gerrit-Reviewer: Yakir Yang <y...@rock-chips.com>Gerrit-Reviewer: 智情 姚 <mark...@rock-chips.com>

    Gerrit-Comment-Date: Fri, 02 Dec 2016 00:57:21 +0000Gerrit-HasComments: Yes

    Derek Basehore (Gerrit)

    unread,
    Dec 2, 2016, 5:14:25 PM12/2/16
    to Sean Paul, Lin Huang, Stéphane Marchesin, 智情 姚, Xing Zheng, Caesar Wang, Yakir Yang, Nick Sanders, Douglas Anderson

    Derek Basehore posted comments on this change.

    View Change

    Patch Set 33:

    (1 comment)

    We can probably get rid of the mutex altogether and just rely on the devfreq function calls to act as synchronization barriers.

      • i have measure the change rate time(use gpio), and we only care the time be

        Stephane, are there any displays that would have trouble with that?

    To view, visit this change. To unsubscribe, visit settings.

    Gerrit-MessageType: comment
    Gerrit-Change-Id: I690ae1c0d2436b957fc823585c1a32b48cfcf3fd
    Gerrit-Change-Number: 362907
    Gerrit-PatchSet: 33
    Gerrit-Project: chromiumos/third_party/kernel
    Gerrit-Branch: chromeos-4.4
    Gerrit-Owner: Lin Huang <h...@rock-chips.com>Gerrit-Reviewer: Caesar Wang <w...@rock-chips.com>Gerrit-Reviewer: Derek Basehore <dbas...@chromium.org>Gerrit-Reviewer: Douglas Anderson <dian...@chromium.org>Gerrit-Reviewer: Lin Huang <h...@rock-chips.com>Gerrit-Reviewer: Nick Sanders <nsan...@chromium.org>Gerrit-Reviewer: Sean Paul <sean...@chromium.org>Gerrit-Reviewer: Stéphane Marchesin <mar...@chromium.org>Gerrit-Reviewer: Xing Zheng <zhen...@rock-chips.com>Gerrit-Reviewer: Yakir Yang <y...@rock-chips.com>Gerrit-Reviewer: 智情 姚 <mark...@rock-chips.com>

    Gerrit-Comment-Date: Fri, 02 Dec 2016 22:14:20 +0000Gerrit-HasComments: Yes

    Stéphane Marchesin (Gerrit)

    unread,
    Dec 6, 2016, 7:19:58 PM12/6/16
    to Sean Paul, Lin Huang, Derek Basehore, 智情 姚, Xing Zheng, Caesar Wang, Yakir Yang, Nick Sanders, Douglas Anderson

    Stéphane Marchesin posted comments on this change.

    View Change

    Patch Set 33:

    (1 comment)

      • Stephane, are there any displays that would have trouble with that?

        display timings are flexible, so you cannot guarantee anything (the display brings its own timing through EDID). So you have to compare the display timing to the timing you need to figure out it if will fit.

    To view, visit this change. To unsubscribe, visit settings.

    Gerrit-MessageType: comment
    Gerrit-Change-Id: I690ae1c0d2436b957fc823585c1a32b48cfcf3fd
    Gerrit-Change-Number: 362907
    Gerrit-PatchSet: 33
    Gerrit-Project: chromiumos/third_party/kernel
    Gerrit-Branch: chromeos-4.4
    Gerrit-Owner: Lin Huang <h...@rock-chips.com>Gerrit-Reviewer: Caesar Wang <w...@rock-chips.com>Gerrit-Reviewer: Derek Basehore <dbas...@chromium.org>Gerrit-Reviewer: Douglas Anderson <dian...@chromium.org>Gerrit-Reviewer: Lin Huang <h...@rock-chips.com>Gerrit-Reviewer: Nick Sanders <nsan...@chromium.org>Gerrit-Reviewer: Sean Paul <sean...@chromium.org>Gerrit-Reviewer: Stéphane Marchesin <mar...@chromium.org>Gerrit-Reviewer: Xing Zheng <zhen...@rock-chips.com>Gerrit-Reviewer: Yakir Yang <y...@rock-chips.com>Gerrit-Reviewer: 智情 姚 <mark...@rock-chips.com>

    Gerrit-Comment-Date: Wed, 07 Dec 2016 00:18:49 +0000Gerrit-HasComments: Yes

    Derek Basehore (Gerrit)

    unread,
    Dec 6, 2016, 7:22:13 PM12/6/16
    to Sean Paul, Lin Huang, Stéphane Marchesin, 智情 姚, Xing Zheng, Caesar Wang, Yakir Yang, Nick Sanders, Douglas Anderson

    Derek Basehore posted comments on this change.

    View Change

    Patch Set 33:

    (1 comment)

      • display timings are flexible, so you cannot guarantee anything (the display

        Yes, I think we just need to calculate the vblank time in crtc_enable in rockchip_drm_vop.c. That seems to have all of the information needed to calculate vblank time.

    To view, visit this change. To unsubscribe, visit settings.

    Gerrit-MessageType: comment
    Gerrit-Change-Id: I690ae1c0d2436b957fc823585c1a32b48cfcf3fd
    Gerrit-Change-Number: 362907
    Gerrit-PatchSet: 33
    Gerrit-Project: chromiumos/third_party/kernel
    Gerrit-Branch: chromeos-4.4
    Gerrit-Owner: Lin Huang <h...@rock-chips.com>Gerrit-Reviewer: Caesar Wang <w...@rock-chips.com>Gerrit-Reviewer: Derek Basehore <dbas...@chromium.org>Gerrit-Reviewer: Douglas Anderson <dian...@chromium.org>Gerrit-Reviewer: Lin Huang <h...@rock-chips.com>Gerrit-Reviewer: Nick Sanders <nsan...@chromium.org>Gerrit-Reviewer: Sean Paul <sean...@chromium.org>Gerrit-Reviewer: Stéphane Marchesin <mar...@chromium.org>Gerrit-Reviewer: Xing Zheng <zhen...@rock-chips.com>Gerrit-Reviewer: Yakir Yang <y...@rock-chips.com>Gerrit-Reviewer: 智情 姚 <mark...@rock-chips.com>

    Gerrit-Comment-Date: Wed, 07 Dec 2016 00:21:13 +0000Gerrit-HasComments: Yes

    Derek Basehore (Gerrit)

    unread,
    Dec 9, 2016, 3:50:12 AM12/9/16
    to Sean Paul, Lin Huang, Stéphane Marchesin, 智情 姚, Xing Zheng, Caesar Wang, Yakir Yang, Nick Sanders, Douglas Anderson

    Derek Basehore posted comments on this change.

    View Change

    Patch Set 33:

    (1 comment)

      • Yes, I think we just need to calculate the vblank time in crtc_enable in ro

        Okay, I think that everything is addressed in the latest patch.

    To view, visit this change. To unsubscribe, visit settings.

    Gerrit-MessageType: comment
    Gerrit-Change-Id: I690ae1c0d2436b957fc823585c1a32b48cfcf3fd
    Gerrit-Change-Number: 362907
    Gerrit-PatchSet: 33
    Gerrit-Project: chromiumos/third_party/kernel
    Gerrit-Branch: chromeos-4.4
    Gerrit-Owner: Lin Huang <h...@rock-chips.com>Gerrit-Reviewer: Caesar Wang <w...@rock-chips.com>Gerrit-Reviewer: Derek Basehore <dbas...@chromium.org>Gerrit-Reviewer: Douglas Anderson <dian...@chromium.org>Gerrit-Reviewer: Lin Huang <h...@rock-chips.com>Gerrit-Reviewer: Nick Sanders <nsan...@chromium.org>Gerrit-Reviewer: Sean Paul <sean...@chromium.org>Gerrit-Reviewer: Stéphane Marchesin <mar...@chromium.org>Gerrit-Reviewer: Xing Zheng <zhen...@rock-chips.com>Gerrit-Reviewer: Yakir Yang <y...@rock-chips.com>Gerrit-Reviewer: 智情 姚 <mark...@rock-chips.com>

    Gerrit-Comment-Date: Fri, 09 Dec 2016 08:49:10 +0000Gerrit-HasComments: Yes

    Derek Basehore (Gerrit)

    unread,
    Dec 9, 2016, 3:50:14 AM12/9/16
    to Sean Paul, Lin Huang, Caesar Wang, Xing Zheng, Douglas Anderson, Stéphane Marchesin, Yakir Yang, Nick Sanders, 智情 姚

    Derek Basehore uploaded patch set #34 to this change.

    View Change

    CHROMIUM: drm/rockchip: Toggle devfreq on/off
    
    This patch adds devfreq to the rockchip drm driver. It will activate
    when there is one crtc active, and disable if more than one becomes
    active (to avoid flickering on one of the screens).
    
    BUG=chrome-os-partner:54651
    TEST=kevin compiles/boots
    
    Change-Id: I690ae1c0d2436b957fc823585c1a32b48cfcf3fd
    Signed-off-by: Sean Paul <sean...@chromium.org>
    Signed-off-by: Lin Huang <h...@rock-chips.com
    >
    ---
    M drivers/gpu/drm/rockchip/rockchip_drm_drv.c
    M drivers/gpu/drm/rockchip/rockchip_drm_drv.h
    M drivers/gpu/drm/rockchip/rockchip_drm_fb.c
    3 files changed, 136 insertions(+), 0 deletions(-)
    
    

    To view, visit this change. To unsubscribe, visit settings.

    Gerrit-MessageType: newpatchset
    Gerrit-Change-Id: I690ae1c0d2436b957fc823585c1a32b48cfcf3fd
    Gerrit-Change-Number: 362907
    Gerrit-PatchSet: 34

    Derek Basehore (Gerrit)

    unread,
    Dec 9, 2016, 7:14:20 PM12/9/16
    to Sean Paul, Lin Huang, Caesar Wang, Xing Zheng, Douglas Anderson, Stéphane Marchesin, Yakir Yang, Nick Sanders, 智情 姚

    Derek Basehore uploaded patch set #35 to this change.

    View Change

    CHROMIUM: drm/rockchip: Toggle devfreq on/off
    
    This patch adds devfreq to the rockchip drm driver. It will activate
    when there is one crtc active, and disable if more than one becomes
    active (to avoid flickering on one of the screens).
    
    BUG=chrome-os-partner:54651
    TEST=kevin compiles/boots
    
    Change-Id: I690ae1c0d2436b957fc823585c1a32b48cfcf3fd
    Signed-off-by: Sean Paul <sean...@chromium.org>
    Signed-off-by: Lin Huang <h...@rock-chips.com
    >
    ---
    M drivers/gpu/drm/rockchip/rockchip_drm_drv.c
    M drivers/gpu/drm/rockchip/rockchip_drm_drv.h
    M drivers/gpu/drm/rockchip/rockchip_drm_fb.c
    M include/soc/rockchip/rk3399_dmc.h
    4 files changed, 137 insertions(+), 0 deletions(-)
    
    

    To view, visit this change. To unsubscribe, visit settings.

    Gerrit-MessageType: newpatchset
    Gerrit-Change-Id: I690ae1c0d2436b957fc823585c1a32b48cfcf3fd
    Gerrit-Change-Number: 362907

    Gerrit-PatchSet: 35

    Derek Basehore (Gerrit)

    unread,
    Dec 9, 2016, 8:11:54 PM12/9/16
    to Sean Paul, Lin Huang, Caesar Wang, Xing Zheng, Douglas Anderson, Stéphane Marchesin, Yakir Yang, Nick Sanders, 智情 姚

    Derek Basehore uploaded patch set #36 to this change.

    View Change

    CHROMIUM: drm/rockchip: Toggle devfreq on/off
    
    This patch adds devfreq to the rockchip drm driver. It will activate
    when there is one crtc active, and disable if more than one becomes
    active (to avoid flickering on one of the screens).
    
    BUG=chrome-os-partner:54651
    TEST=kevin compiles/boots
    
    Change-Id: I690ae1c0d2436b957fc823585c1a32b48cfcf3fd
    Signed-off-by: Sean Paul <sean...@chromium.org>
    Signed-off-by: Lin Huang <h...@rock-chips.com
    >
    ---
    M drivers/gpu/drm/rockchip/rockchip_drm_drv.c
    M drivers/gpu/drm/rockchip/rockchip_drm_drv.h
    M drivers/gpu/drm/rockchip/rockchip_drm_fb.c
    M include/soc/rockchip/rk3399_dmc.h
    4 files changed, 140 insertions(+), 0 deletions(-)
    
    

    To view, visit this change. To unsubscribe, visit settings.

    Gerrit-MessageType: newpatchset
    Gerrit-Change-Id: I690ae1c0d2436b957fc823585c1a32b48cfcf3fd
    Gerrit-Change-Number: 362907

    Gerrit-PatchSet: 36

    Stéphane Marchesin (Gerrit)

    unread,
    Dec 10, 2016, 12:37:56 AM12/10/16
    to Sean Paul, Lin Huang, Derek Basehore, 智情 姚, Xing Zheng, Caesar Wang, Yakir Yang, Nick Sanders, Douglas Anderson

    Stéphane Marchesin posted comments on this change.

    View Change

    Patch Set 36:

    (2 comments)

    To view, visit this change. To unsubscribe, visit settings.

    Gerrit-MessageType: comment


    Gerrit-Change-Id: I690ae1c0d2436b957fc823585c1a32b48cfcf3fd
    Gerrit-Change-Number: 362907
    Gerrit-PatchSet: 36
    Gerrit-Project: chromiumos/third_party/kernel
    Gerrit-Branch: chromeos-4.4
    Gerrit-Owner: Lin Huang <h...@rock-chips.com>Gerrit-Reviewer: Caesar Wang <w...@rock-chips.com>Gerrit-Reviewer: Derek Basehore <dbas...@chromium.org>Gerrit-Reviewer: Douglas Anderson <dian...@chromium.org>Gerrit-Reviewer: Lin Huang <h...@rock-chips.com>Gerrit-Reviewer: Nick Sanders <nsan...@chromium.org>Gerrit-Reviewer: Sean Paul <sean...@chromium.org>Gerrit-Reviewer: Stéphane Marchesin <mar...@chromium.org>Gerrit-Reviewer: Xing Zheng <zhen...@rock-chips.com>Gerrit-Reviewer: Yakir Yang <y...@rock-chips.com>Gerrit-Reviewer: 智情 姚 <mark...@rock-chips.com>

    Gerrit-Comment-Date: Sat, 10 Dec 2016 05:37:05 +0000Gerrit-HasComments: Yes

    Derek Basehore (Gerrit)

    unread,
    Dec 10, 2016, 5:47:56 AM12/10/16
    to Sean Paul, Lin Huang, Stéphane Marchesin, 智情 姚, Xing Zheng, Caesar Wang, Yakir Yang, Nick Sanders, Douglas Anderson

    Derek Basehore posted comments on this change.

    View Change

    Patch Set 36:

    (2 comments)

      • Patch Set #36, Line 258: drm_for_each_crtc(crtc, dev) {

        if you're operating on crtcs, you need to own modeset locks I think

      • Okay, I'll fix this tomorrow. I'll be locking the drm_crtc:mutex (which is a struct drm_modeset_lock). If that's the wrong lock, please say so.

      • Patch Set #36, Line 281: mutex_lock(&priv->dmc_mutex);

        why do you lock this mutex even for cases where you don't need it (for exam

      • We might not need to lock the mutex at all, either. I need to think about it. Disabling dmc should force us into the suspend frequency (the higher frequency). At this point, it's either fine for the dmc to change frequency, or we won't be changing frequency at all until another display state change happens.

    To view, visit this change. To unsubscribe, visit settings.

    Gerrit-MessageType: comment
    Gerrit-Change-Id: I690ae1c0d2436b957fc823585c1a32b48cfcf3fd
    Gerrit-Change-Number: 362907
    Gerrit-PatchSet: 36
    Gerrit-Project: chromiumos/third_party/kernel
    Gerrit-Branch: chromeos-4.4
    Gerrit-Owner: Lin Huang <h...@rock-chips.com>Gerrit-Reviewer: Caesar Wang <w...@rock-chips.com>Gerrit-Reviewer: Derek Basehore <dbas...@chromium.org>Gerrit-Reviewer: Douglas Anderson <dian...@chromium.org>Gerrit-Reviewer: Lin Huang <h...@rock-chips.com>Gerrit-Reviewer: Nick Sanders <nsan...@chromium.org>Gerrit-Reviewer: Sean Paul <sean...@chromium.org>Gerrit-Reviewer: Stéphane Marchesin <mar...@chromium.org>Gerrit-Reviewer: Xing Zheng <zhen...@rock-chips.com>Gerrit-Reviewer: Yakir Yang <y...@rock-chips.com>Gerrit-Reviewer: 智情 姚 <mark...@rock-chips.com>

    Gerrit-Comment-Date: Sat, 10 Dec 2016 10:46:58 +0000Gerrit-HasComments: Yes

    Derek Basehore (Gerrit)

    unread,
    Dec 11, 2016, 1:48:15 AM12/11/16
    to Sean Paul, Lin Huang, Stéphane Marchesin, 智情 姚, Xing Zheng, Caesar Wang, Yakir Yang, Nick Sanders, Douglas Anderson

    Derek Basehore posted comments on this change.

    View Change

    Patch Set 36:

    (1 comment)

      • Okay, I'll fix this tomorrow. I'll be locking the drm_crtc:mutex (which is

        If I hold all of the modeset locks via drm_modeset_lock_all, it seems to deadlock reliably on boot. I'm not sure, but is there an existing order between those locks and the commit lock which is held when this function is called?

    To view, visit this change. To unsubscribe, visit settings.

    Gerrit-MessageType: comment
    Gerrit-Change-Id: I690ae1c0d2436b957fc823585c1a32b48cfcf3fd
    Gerrit-Change-Number: 362907
    Gerrit-PatchSet: 36
    Gerrit-Project: chromiumos/third_party/kernel
    Gerrit-Branch: chromeos-4.4
    Gerrit-Owner: Lin Huang <h...@rock-chips.com>Gerrit-Reviewer: Caesar Wang <w...@rock-chips.com>Gerrit-Reviewer: Derek Basehore <dbas...@chromium.org>Gerrit-Reviewer: Douglas Anderson <dian...@chromium.org>Gerrit-Reviewer: Lin Huang <h...@rock-chips.com>Gerrit-Reviewer: Nick Sanders <nsan...@chromium.org>Gerrit-Reviewer: Sean Paul <sean...@chromium.org>Gerrit-Reviewer: Stéphane Marchesin <mar...@chromium.org>Gerrit-Reviewer: Xing Zheng <zhen...@rock-chips.com>Gerrit-Reviewer: Yakir Yang <y...@rock-chips.com>Gerrit-Reviewer: 智情 姚 <mark...@rock-chips.com>

    Gerrit-Comment-Date: Sun, 11 Dec 2016 06:47:19 +0000Gerrit-HasComments: Yes

    Derek Basehore (Gerrit)

    unread,
    Dec 11, 2016, 1:49:04 AM12/11/16
    to Sean Paul, Lin Huang, Stéphane Marchesin, 智情 姚, Xing Zheng, Caesar Wang, Yakir Yang, Nick Sanders, Douglas Anderson

    Derek Basehore posted comments on this change.

    View Change

    Patch Set 36:

    (1 comment)

      • We might not need to lock the mutex at all, either. I need to think about i

      • Done

    To view, visit this change. To unsubscribe, visit settings.

    Gerrit-MessageType: comment
    Gerrit-Change-Id: I690ae1c0d2436b957fc823585c1a32b48cfcf3fd
    Gerrit-Change-Number: 362907
    Gerrit-PatchSet: 36
    Gerrit-Project: chromiumos/third_party/kernel
    Gerrit-Branch: chromeos-4.4
    Gerrit-Owner: Lin Huang <h...@rock-chips.com>Gerrit-Reviewer: Caesar Wang <w...@rock-chips.com>Gerrit-Reviewer: Derek Basehore <dbas...@chromium.org>Gerrit-Reviewer: Douglas Anderson <dian...@chromium.org>Gerrit-Reviewer: Lin Huang <h...@rock-chips.com>Gerrit-Reviewer: Nick Sanders <nsan...@chromium.org>Gerrit-Reviewer: Sean Paul <sean...@chromium.org>Gerrit-Reviewer: Stéphane Marchesin <mar...@chromium.org>Gerrit-Reviewer: Xing Zheng <zhen...@rock-chips.com>Gerrit-Reviewer: Yakir Yang <y...@rock-chips.com>Gerrit-Reviewer: 智情 姚 <mark...@rock-chips.com>

    Gerrit-Comment-Date: Sun, 11 Dec 2016 06:47:42 +0000Gerrit-HasComments: Yes

    Derek Basehore (Gerrit)

    unread,
    Dec 11, 2016, 1:49:47 AM12/11/16
    to Sean Paul, Lin Huang, Caesar Wang, Xing Zheng, Douglas Anderson, Stéphane Marchesin, Yakir Yang, Nick Sanders, 智情 姚

    Derek Basehore uploaded patch set #37 to this change.

    View Change

    CHROMIUM: drm/rockchip: Toggle devfreq on/off
    
    This patch adds devfreq to the rockchip drm driver. It will activate
    when there is one crtc active, and disable if more than one becomes
    active (to avoid flickering on one of the screens).
    
    BUG=chrome-os-partner:54651
    TEST=kevin compiles/boots
    
    Change-Id: I690ae1c0d2436b957fc823585c1a32b48cfcf3fd
    Signed-off-by: Sean Paul <sean...@chromium.org>
    Signed-off-by: Lin Huang <h...@rock-chips.com
    >
    ---
    M drivers/gpu/drm/rockchip/rockchip_drm_drv.c
    M drivers/gpu/drm/rockchip/rockchip_drm_drv.h
    M drivers/gpu/drm/rockchip/rockchip_drm_fb.c
    M include/soc/rockchip/rk3399_dmc.h
    4 files changed, 116 insertions(+), 0 deletions(-)
    
    

    To view, visit this change. To unsubscribe, visit settings.

    Gerrit-MessageType: newpatchset
    Gerrit-Change-Id: I690ae1c0d2436b957fc823585c1a32b48cfcf3fd
    Gerrit-Change-Number: 362907
    Gerrit-PatchSet: 37

    Derek Basehore (Gerrit)

    unread,
    Dec 12, 2016, 3:40:32 PM12/12/16
    to Sean Paul, Lin Huang, Stéphane Marchesin, 智情 姚, Xing Zheng, Caesar Wang, Yakir Yang, Nick Sanders, Douglas Anderson

    Derek Basehore posted comments on this change.

    View Change

    Patch Set 36:

    (1 comment)

      • If I hold all of the modeset locks via drm_modeset_lock_all, it seems to de

      • The drm_atomic_commit function make have the modeset locks acquired. We could make it such that the async functionality will have the modeset locks, though.

    To view, visit this change. To unsubscribe, visit settings.

    Gerrit-MessageType: comment
    Gerrit-Change-Id: I690ae1c0d2436b957fc823585c1a32b48cfcf3fd
    Gerrit-Change-Number: 362907
    Gerrit-PatchSet: 36


    Gerrit-Project: chromiumos/third_party/kernel
    Gerrit-Branch: chromeos-4.4
    Gerrit-Owner: Lin Huang <h...@rock-chips.com>Gerrit-Reviewer: Caesar Wang <w...@rock-chips.com>Gerrit-Reviewer: Derek Basehore <dbas...@chromium.org>Gerrit-Reviewer: Douglas Anderson <dian...@chromium.org>Gerrit-Reviewer: Lin Huang <h...@rock-chips.com>Gerrit-Reviewer: Nick Sanders <nsan...@chromium.org>Gerrit-Reviewer: Sean Paul <sean...@chromium.org>Gerrit-Reviewer: Stéphane Marchesin <mar...@chromium.org>Gerrit-Reviewer: Xing Zheng <zhen...@rock-chips.com>Gerrit-Reviewer: Yakir Yang <y...@rock-chips.com>Gerrit-Reviewer: 智情 姚 <mark...@rock-chips.com>

    Gerrit-Comment-Date: Mon, 12 Dec 2016 20:39:12 +0000Gerrit-HasComments: Yes

    Derek Basehore (Gerrit)

    unread,
    Dec 12, 2016, 6:56:21 PM12/12/16
    to Sean Paul, Lin Huang, Stéphane Marchesin, 智情 姚, Xing Zheng, Caesar Wang, Yakir Yang, Nick Sanders, Douglas Anderson

    Derek Basehore posted comments on this change.

    View Change

    Patch Set 36:

    (1 comment)

      • The drm_atomic_commit function make have the modeset locks acquired. We cou

        Are the crtcs going to be changed outside of this function?

    To view, visit this change. To unsubscribe, visit settings.

    Gerrit-MessageType: comment
    Gerrit-Change-Id: I690ae1c0d2436b957fc823585c1a32b48cfcf3fd
    Gerrit-Change-Number: 362907
    Gerrit-PatchSet: 36
    Gerrit-Project: chromiumos/third_party/kernel
    Gerrit-Branch: chromeos-4.4
    Gerrit-Owner: Lin Huang <h...@rock-chips.com>Gerrit-Reviewer: Caesar Wang <w...@rock-chips.com>Gerrit-Reviewer: Derek Basehore <dbas...@chromium.org>Gerrit-Reviewer: Douglas Anderson <dian...@chromium.org>Gerrit-Reviewer: Lin Huang <h...@rock-chips.com>Gerrit-Reviewer: Nick Sanders <nsan...@chromium.org>Gerrit-Reviewer: Sean Paul <sean...@chromium.org>Gerrit-Reviewer: Stéphane Marchesin <mar...@chromium.org>Gerrit-Reviewer: Xing Zheng <zhen...@rock-chips.com>Gerrit-Reviewer: Yakir Yang <y...@rock-chips.com>Gerrit-Reviewer: 智情 姚 <mark...@rock-chips.com>

    Gerrit-Comment-Date: Mon, 12 Dec 2016 23:55:29 +0000Gerrit-HasComments: Yes

    Derek Basehore (Gerrit)

    unread,
    Dec 14, 2016, 4:38:24 PM12/14/16
    to Sean Paul, Lin Huang, Caesar Wang, Xing Zheng, Douglas Anderson, Stéphane Marchesin, Yakir Yang, Nick Sanders, 智情 姚

    Derek Basehore uploaded patch set #38 to this change.

    View Change

    CHROMIUM: drm/rockchip: Toggle devfreq on/off
    
    This patch adds devfreq to the rockchip drm driver. It will activate
    when there is one crtc active, and disable if more than one becomes
    active (to avoid flickering on one of the screens).
    
    BUG=chrome-os-partner:54651
    TEST=kevin compiles/boots
    
    Change-Id: I690ae1c0d2436b957fc823585c1a32b48cfcf3fd
    Signed-off-by: Sean Paul <sean...@chromium.org>
    Signed-off-by: Lin Huang <h...@rock-chips.com
    >
    Signed-off-by: Derek Basehore <dbas...@chromium.org>
    ---
    M drivers/gpu/drm/rockchip/rockchip_drm_drv.c
    M drivers/gpu/drm/rockchip/rockchip_drm_drv.h
    M drivers/gpu/drm/rockchip/rockchip_drm_fb.c
    M include/soc/rockchip/rk3399_dmc.h
    4 files changed, 120 insertions(+), 0 deletions(-)
    
    

    To view, visit this change. To unsubscribe, visit settings.

    Gerrit-MessageType: newpatchset
    Gerrit-Change-Id: I690ae1c0d2436b957fc823585c1a32b48cfcf3fd
    Gerrit-Change-Number: 362907
    Gerrit-PatchSet: 38

    Derek Basehore (Gerrit)

    unread,
    Dec 14, 2016, 8:42:20 PM12/14/16
    to Sean Paul, Lin Huang, Stéphane Marchesin, 智情 姚, Xing Zheng, Caesar Wang, Yakir Yang, Nick Sanders, Douglas Anderson

    Derek Basehore posted comments on this change.

    View Change

    Patch Set 38:

    (1 comment)

      • Are the crtcs going to be changed outside of this function?

        Done

    To view, visit this change. To unsubscribe, visit settings.

    Gerrit-MessageType: comment


    Gerrit-Change-Id: I690ae1c0d2436b957fc823585c1a32b48cfcf3fd
    Gerrit-Change-Number: 362907
    Gerrit-PatchSet: 38
    Gerrit-Project: chromiumos/third_party/kernel
    Gerrit-Branch: chromeos-4.4
    Gerrit-Owner: Lin Huang <h...@rock-chips.com>Gerrit-Reviewer: Caesar Wang <w...@rock-chips.com>Gerrit-Reviewer: Derek Basehore <dbas...@chromium.org>Gerrit-Reviewer: Douglas Anderson <dian...@chromium.org>Gerrit-Reviewer: Lin Huang <h...@rock-chips.com>Gerrit-Reviewer: Nick Sanders <nsan...@chromium.org>Gerrit-Reviewer: Sean Paul <sean...@chromium.org>Gerrit-Reviewer: Stéphane Marchesin <mar...@chromium.org>Gerrit-Reviewer: Xing Zheng <zhen...@rock-chips.com>Gerrit-Reviewer: Yakir Yang <y...@rock-chips.com>Gerrit-Reviewer: 智情 姚 <mark...@rock-chips.com>

    Gerrit-Comment-Date: Thu, 15 Dec 2016 01:41:18 +0000Gerrit-HasComments: Yes

    Stéphane Marchesin (Gerrit)

    unread,
    Dec 15, 2016, 7:47:14 PM12/15/16
    to Sean Paul, Lin Huang, Derek Basehore, 智情 姚, Xing Zheng, Caesar Wang, Yakir Yang, Nick Sanders, Douglas Anderson

    Stéphane Marchesin posted comments on this change.

    View Change

    Patch Set 38:

    (1 comment)

    • File drivers/gpu/drm/rockchip/rockchip_drm_fb.c:

      • Patch Set #38, Line 265: }

        what about the following scenario:

        • we have 1 monitor total
        • initially we run with a video mode where vblank is large enough to enable dmc
        • enable dmc
        • we reconfigure the monitor to another mode where the vblank isn't large enough for dmc
        • since the # of monitors didn't change, active_changed is false and we are not touching dmc
        • we end up in a case where dmc is enabled but shouldn't be

    To view, visit this change. To unsubscribe, visit settings.

    Gerrit-MessageType: comment
    Gerrit-Change-Id: I690ae1c0d2436b957fc823585c1a32b48cfcf3fd
    Gerrit-Change-Number: 362907
    Gerrit-PatchSet: 38
    Gerrit-Project: chromiumos/third_party/kernel
    Gerrit-Branch: chromeos-4.4
    Gerrit-Owner: Lin Huang <h...@rock-chips.com>Gerrit-Reviewer: Caesar Wang <w...@rock-chips.com>Gerrit-Reviewer: Derek Basehore <dbas...@chromium.org>Gerrit-Reviewer: Douglas Anderson <dian...@chromium.org>Gerrit-Reviewer: Lin Huang <h...@rock-chips.com>Gerrit-Reviewer: Nick Sanders <nsan...@chromium.org>Gerrit-Reviewer: Sean Paul <sean...@chromium.org>Gerrit-Reviewer: Stéphane Marchesin <mar...@chromium.org>Gerrit-Reviewer: Xing Zheng <zhen...@rock-chips.com>Gerrit-Reviewer: Yakir Yang <y...@rock-chips.com>Gerrit-Reviewer: 智情 姚 <mark...@rock-chips.com>

    Gerrit-Comment-Date: Fri, 16 Dec 2016 00:46:07 +0000Gerrit-HasComments: Yes

    Derek Basehore (Gerrit)

    unread,
    Dec 15, 2016, 9:22:10 PM12/15/16
    to Sean Paul, Lin Huang, Stéphane Marchesin, 智情 姚, Xing Zheng, Caesar Wang, Yakir Yang, Nick Sanders, Douglas Anderson

    Derek Basehore posted comments on this change.

    View Change

    Patch Set 38:

    (1 comment)

      • I guess we should get rid of active_changed.

    To view, visit this change. To unsubscribe, visit settings.

    Gerrit-MessageType: comment
    Gerrit-Change-Id: I690ae1c0d2436b957fc823585c1a32b48cfcf3fd
    Gerrit-Change-Number: 362907
    Gerrit-PatchSet: 38
    Gerrit-Project: chromiumos/third_party/kernel
    Gerrit-Branch: chromeos-4.4
    Gerrit-Owner: Lin Huang <h...@rock-chips.com>Gerrit-Reviewer: Caesar Wang <w...@rock-chips.com>Gerrit-Reviewer: Derek Basehore <dbas...@chromium.org>Gerrit-Reviewer: Douglas Anderson <dian...@chromium.org>Gerrit-Reviewer: Lin Huang <h...@rock-chips.com>Gerrit-Reviewer: Nick Sanders <nsan...@chromium.org>Gerrit-Reviewer: Sean Paul <sean...@chromium.org>Gerrit-Reviewer: Stéphane Marchesin <mar...@chromium.org>Gerrit-Reviewer: Xing Zheng <zhen...@rock-chips.com>Gerrit-Reviewer: Yakir Yang <y...@rock-chips.com>Gerrit-Reviewer: 智情 姚 <mark...@rock-chips.com>

    Gerrit-Comment-Date: Fri, 16 Dec 2016 02:20:58 +0000Gerrit-HasComments: Yes

    Stéphane Marchesin (Gerrit)

    unread,
    Dec 15, 2016, 11:59:52 PM12/15/16
    to Sean Paul, Lin Huang, Derek Basehore, 智情 姚, Xing Zheng, Caesar Wang, Yakir Yang, Nick Sanders, Douglas Anderson

    Stéphane Marchesin posted comments on this change.

    View Change

    Patch Set 38:

    (1 comment)

      • I guess we should get rid of active_changed.

        as long as you keep dmc out of the fast path (where there are no mode changes)

    To view, visit this change. To unsubscribe, visit settings.

    Gerrit-MessageType: comment
    Gerrit-Change-Id: I690ae1c0d2436b957fc823585c1a32b48cfcf3fd
    Gerrit-Change-Number: 362907
    Gerrit-PatchSet: 38
    Gerrit-Project: chromiumos/third_party/kernel
    Gerrit-Branch: chromeos-4.4
    Gerrit-Owner: Lin Huang <h...@rock-chips.com>Gerrit-Reviewer: Caesar Wang <w...@rock-chips.com>Gerrit-Reviewer: Derek Basehore <dbas...@chromium.org>Gerrit-Reviewer: Douglas Anderson <dian...@chromium.org>Gerrit-Reviewer: Lin Huang <h...@rock-chips.com>Gerrit-Reviewer: Nick Sanders <nsan...@chromium.org>Gerrit-Reviewer: Sean Paul <sean...@chromium.org>Gerrit-Reviewer: Stéphane Marchesin <mar...@chromium.org>Gerrit-Reviewer: Xing Zheng <zhen...@rock-chips.com>Gerrit-Reviewer: Yakir Yang <y...@rock-chips.com>Gerrit-Reviewer: 智情 姚 <mark...@rock-chips.com>

    Gerrit-Comment-Date: Fri, 16 Dec 2016 04:57:01 +0000Gerrit-HasComments: Yes

    Derek Basehore (Gerrit)

    unread,
    Dec 16, 2016, 6:54:29 PM12/16/16
    to Sean Paul, Lin Huang, Caesar Wang, Xing Zheng, Douglas Anderson, Stéphane Marchesin, Yakir Yang, Nick Sanders, 智情 姚

    Derek Basehore uploaded patch set #39 to this change.

    View Change

    CHROMIUM: drm/rockchip: Toggle devfreq on/off
    
    This patch adds devfreq to the rockchip drm driver. It will activate
    when there is one crtc active, and disable if more than one becomes
    active (to avoid flickering on one of the screens).
    
    BUG=chrome-os-partner:54651
    TEST=kevin compiles/boots
    
    Change-Id: I690ae1c0d2436b957fc823585c1a32b48cfcf3fd
    Signed-off-by: Sean Paul <sean...@chromium.org>
    Signed-off-by: Lin Huang <h...@rock-chips.com>
    Signed-off-by: Derek Basehore <dbas...@chromium.org
    >
    ---
    M drivers/gpu/drm/rockchip/rockchip_drm_drv.c
    M drivers/gpu/drm/rockchip/rockchip_drm_drv.h
    M drivers/gpu/drm/rockchip/rockchip_drm_fb.c
    M include/soc/rockchip/rk3399_dmc.h
    4 files changed, 104 insertions(+), 0 deletions(-)
    
    

    To view, visit this change. To unsubscribe, visit settings.

    Gerrit-MessageType: newpatchset
    Gerrit-Change-Id: I690ae1c0d2436b957fc823585c1a32b48cfcf3fd
    Gerrit-Change-Number: 362907
    Gerrit-PatchSet: 39

    Stéphane Marchesin (Gerrit)

    unread,
    Dec 16, 2016, 11:23:24 PM12/16/16
    to Sean Paul, Lin Huang, Derek Basehore, 智情 姚, Xing Zheng, Caesar Wang, Yakir Yang, Nick Sanders, Douglas Anderson

    Stéphane Marchesin posted comments on this change.

    View Change

    Patch Set 39: Code-Review+2

      To view, visit this change. To unsubscribe, visit settings.

      Gerrit-MessageType: comment


      Gerrit-Change-Id: I690ae1c0d2436b957fc823585c1a32b48cfcf3fd
      Gerrit-Change-Number: 362907
      Gerrit-PatchSet: 39
      Gerrit-Project: chromiumos/third_party/kernel
      Gerrit-Branch: chromeos-4.4
      Gerrit-Owner: Lin Huang <h...@rock-chips.com>Gerrit-Reviewer: Caesar Wang <w...@rock-chips.com>Gerrit-Reviewer: Derek Basehore <dbas...@chromium.org>Gerrit-Reviewer: Douglas Anderson <dian...@chromium.org>Gerrit-Reviewer: Lin Huang <h...@rock-chips.com>Gerrit-Reviewer: Nick Sanders <nsan...@chromium.org>Gerrit-Reviewer: Sean Paul <sean...@chromium.org>Gerrit-Reviewer: Stéphane Marchesin <mar...@chromium.org>Gerrit-Reviewer: Xing Zheng <zhen...@rock-chips.com>Gerrit-Reviewer: Yakir Yang <y...@rock-chips.com>Gerrit-Reviewer: 智情 姚 <mark...@rock-chips.com>

      Gerrit-Comment-Date: Sat, 17 Dec 2016 04:22:27 +0000Gerrit-HasComments: No

      Derek Basehore (Gerrit)

      unread,
      Dec 20, 2016, 12:26:22 AM12/20/16
      to Sean Paul, Lin Huang, Caesar Wang, Xing Zheng, Douglas Anderson, Stéphane Marchesin, Yakir Yang, Nick Sanders, 智情 姚

      Derek Basehore uploaded patch set #40 to this change.

      View Change

      CHROMIUM: drm/rockchip: Toggle devfreq on/off
      
      This patch adds devfreq to the rockchip drm driver. It will activate
      when there is one crtc active, and disable if more than one becomes
      active (to avoid flickering on one of the screens).
      
      BUG=chrome-os-partner:54651
      TEST=kevin compiles/boots
      
      Change-Id: I690ae1c0d2436b957fc823585c1a32b48cfcf3fd
      Signed-off-by: Sean Paul <sean...@chromium.org>
      Signed-off-by: Lin Huang <h...@rock-chips.com>
      Signed-off-by: Derek Basehore <dbas...@chromium.org>
      ---
      M drivers/gpu/drm/rockchip/rockchip_drm_drv.c
      M drivers/gpu/drm/rockchip/rockchip_drm_drv.h
      M drivers/gpu/drm/rockchip/rockchip_drm_fb.c
      M include/soc/rockchip/rk3399_dmc.h
      4 files changed, 104 insertions(+), 0 deletions(-)
      
      

      To view, visit this change. To unsubscribe, visit settings.

      Gerrit-MessageType: newpatchset
      Gerrit-Change-Id: I690ae1c0d2436b957fc823585c1a32b48cfcf3fd
      Gerrit-Change-Number: 362907
      Gerrit-PatchSet: 40

      Derek Basehore (Gerrit)

      unread,
      Dec 20, 2016, 12:30:51 AM12/20/16
      to Sean Paul, Lin Huang, Stéphane Marchesin, 智情 姚, Xing Zheng, Caesar Wang, Yakir Yang, Nick Sanders, Douglas Anderson

      Derek Basehore posted comments on this change.

      View Change

      Patch Set 40: Verified+1 Code-Review+2 Commit-Queue+1 Trybot-Ready+1

      Just differences from rebasing. Carrying forward the +2 from Stephane.

        To view, visit this change. To unsubscribe, visit settings.

        Gerrit-MessageType: comment


        Gerrit-Change-Id: I690ae1c0d2436b957fc823585c1a32b48cfcf3fd
        Gerrit-Change-Number: 362907
        Gerrit-PatchSet: 40
        Gerrit-Project: chromiumos/third_party/kernel
        Gerrit-Branch: chromeos-4.4
        Gerrit-Owner: Lin Huang <h...@rock-chips.com>Gerrit-Reviewer: Caesar Wang <w...@rock-chips.com>Gerrit-Reviewer: Derek Basehore <dbas...@chromium.org>Gerrit-Reviewer: Douglas Anderson <dian...@chromium.org>Gerrit-Reviewer: Lin Huang <h...@rock-chips.com>Gerrit-Reviewer: Nick Sanders <nsan...@chromium.org>Gerrit-Reviewer: Sean Paul <sean...@chromium.org>Gerrit-Reviewer: Stéphane Marchesin <mar...@chromium.org>Gerrit-Reviewer: Xing Zheng <zhen...@rock-chips.com>Gerrit-Reviewer: Yakir Yang <y...@rock-chips.com>Gerrit-Reviewer: 智情 姚 <mark...@rock-chips.com>

        Gerrit-Comment-Date: Tue, 20 Dec 2016 05:29:06 +0000Gerrit-HasComments: No

        ChromeOS Commit Bot (Gerrit)

        unread,
        Dec 20, 2016, 6:41:32 AM12/20/16
        to Sean Paul, Lin Huang, Caesar Wang, Xing Zheng, Douglas Anderson, Stéphane Marchesin, Derek Basehore, Yakir Yang, Nick Sanders, 智情 姚

        ChromeOS Commit Bot uploaded patch set #41 to this change.

        View Change

        CHROMIUM: drm/rockchip: Toggle devfreq on/off
        
        This patch adds devfreq to the rockchip drm driver. It will activate
        when there is one crtc active, and disable if more than one becomes
        active (to avoid flickering on one of the screens).
        
        BUG=chrome-os-partner:54651
        TEST=kevin compiles/boots
        
        Change-Id: I690ae1c0d2436b957fc823585c1a32b48cfcf3fd
        Signed-off-by: Sean Paul <sean...@chromium.org>
        Signed-off-by: Lin Huang <h...@rock-chips.com>
        Signed-off-by: Derek Basehore <dbas...@chromium.org
        >
        Reviewed-on: https://chromium-review.googlesource.com/362907
        ---
        M drivers/gpu/drm/rockchip/rockchip_drm_drv.c
        M drivers/gpu/drm/rockchip/rockchip_drm_drv.h
        M drivers/gpu/drm/rockchip/rockchip_drm_fb.c
        M include/soc/rockchip/rk3399_dmc.h
        4 files changed, 104 insertions(+), 0 deletions(-)
        
        

        To view, visit this change. To unsubscribe, visit settings.

        Gerrit-MessageType: newpatchset
        Gerrit-Change-Id: I690ae1c0d2436b957fc823585c1a32b48cfcf3fd
        Gerrit-Change-Number: 362907
        Gerrit-PatchSet: 41
        Gerrit-Project: chromiumos/third_party/kernel
        Gerrit-Branch: chromeos-4.4
        Gerrit-Owner: Lin Huang <h...@rock-chips.com>Gerrit-Reviewer: Caesar Wang <w...@rock-chips.com>Gerrit-Reviewer: Derek Basehore <dbas...@chromium.org>Gerrit-Reviewer: Douglas Anderson <dian...@chromium.org>Gerrit-Reviewer: Lin Huang <h...@rock-chips.com>Gerrit-Reviewer: Nick Sanders <nsan...@chromium.org>Gerrit-Reviewer: Sean Paul <sean...@chromium.org>Gerrit-Reviewer: Stéphane Marchesin <mar...@chromium.org>Gerrit-Reviewer: Xing Zheng <zhen...@rock-chips.com>Gerrit-Reviewer: Yakir Yang <y...@rock-chips.com>Gerrit-Reviewer: 智情 姚 <mark...@rock-chips.com>Gerrit-CC: ChromeOS Commit Bot <chromeos-...@chromium.org>

        ChromeOS Commit Bot (Gerrit)

        unread,
        Dec 20, 2016, 6:43:24 AM12/20/16
        to Sean Paul, Lin Huang, Derek Basehore, Stéphane Marchesin, 智情 姚, Xing Zheng, Caesar Wang, Yakir Yang, Nick Sanders, Douglas Anderson

        ChromeOS Commit Bot merged this change.

        View Change

        Approvals: Derek Basehore: Looks good to me, approved
        CHROMIUM: drm/rockchip: Toggle devfreq on/off
        
        This patch adds devfreq to the rockchip drm driver. It will activate
        when there is one crtc active, and disable if more than one becomes
        active (to avoid flickering on one of the screens).
        
        BUG=chrome-os-partner:54651
        TEST=kevin compiles/boots
        
        Change-Id: I690ae1c0d2436b957fc823585c1a32b48cfcf3fd
        Signed-off-by: Sean Paul <sean...@chromium.org>
        Signed-off-by: Lin Huang <h...@rock-chips.com>
        Signed-off-by: Derek Basehore <dbas...@chromium.org>
        Reviewed-on: https://chromium-review.googlesource.com/362907
        ---
        M drivers/gpu/drm/rockchip/rockchip_drm_drv.c
        M drivers/gpu/drm/rockchip/rockchip_drm_drv.h
        M drivers/gpu/drm/rockchip/rockchip_drm_fb.c
        M include/soc/rockchip/rk3399_dmc.h
        4 files changed, 104 insertions(+), 0 deletions(-)
        
        
        diff --git a/drivers/gpu/drm/rockchip/rockchip_drm_drv.c b/drivers/gpu/drm/rockchip/rockchip_drm_drv.c
        index 5ca782d..4d90111 100644
        --- a/drivers/gpu/drm/rockchip/rockchip_drm_drv.c
        +++ b/drivers/gpu/drm/rockchip/rockchip_drm_drv.c
        @@ -17,6 +17,8 @@
         #include <drm/drmP.h>
         #include <drm/drm_crtc_helper.h>
         #include <drm/drm_fb_helper.h>
        +#include <linux/devfreq.h>
        +#include <linux/devfreq-event.h>
         #include <linux/dma-mapping.h>
         #include <linux/dma-iommu.h>
         #include <linux/pm_runtime.h>
        @@ -74,6 +76,56 @@
         		return;
         
         	iommu_detach_device(domain, dev);
        +}
        +
        +void rockchip_drm_enable_dmc(struct rockchip_drm_private *priv)
        +{
        +	if (priv->devfreq_event_dev)
        +		devfreq_event_enable_edev(priv->devfreq_event_dev);
        +	if (priv->devfreq)
        +		devfreq_resume_device(priv->devfreq);
        +}
        +EXPORT_SYMBOL(rockchip_drm_enable_dmc);
        +
        +void rockchip_drm_disable_dmc(struct rockchip_drm_private *priv)
        +{
        +	if (priv->devfreq_event_dev)
        +		devfreq_event_disable_edev(priv->devfreq_event_dev);
        +	if (priv->devfreq)
        +		devfreq_suspend_device(priv->devfreq);
        +}
        +EXPORT_SYMBOL(rockchip_drm_disable_dmc);
        +
        +static int rockchip_initialize_devfreq(struct device *dev,
        +				       struct rockchip_drm_private *priv)
        +{
        +	struct devfreq *devfreq;
        +	struct devfreq_event_dev *edev;
        +	int ret;
        +
        +	devfreq = devfreq_get_devfreq_by_phandle(dev, 0);
        +	if (IS_ERR(devfreq)) {
        +		ret = PTR_ERR(devfreq);
        +		if (ret == -ENODEV) {
        +			DRM_DEV_INFO(dev, "devfreq missing, skip\n");
        +			return 0;
        +		}
        +		return ret;
        +	}
        +
        +	edev = devfreq_event_get_edev_by_phandle(devfreq->dev.parent, 0);
        +	if (IS_ERR(edev)) {
        +		ret = PTR_ERR(edev);
        +		if (ret == -ENODEV) {
        +			DRM_DEV_INFO(dev, "devfreq edev missing, skip\n");
        +			return 0;
        +		}
        +		return ret;
        +	}
        +
        +	priv->devfreq = devfreq;
        +	priv->devfreq_event_dev = edev;
        +	return 0;
         }
         
         int rockchip_register_crtc_funcs(struct drm_crtc *crtc,
        @@ -200,6 +252,10 @@
         	if (!private)
         		return -ENOMEM;
         
        +	ret = rockchip_initialize_devfreq(dev, private);
        +	if (ret)
        +		return ret;
        +
         	ret = rockchip_initialize_kthread(&private->commit);
         	if (ret)
         		goto err_config_cleanup;
        diff --git a/drivers/gpu/drm/rockchip/rockchip_drm_drv.h b/drivers/gpu/drm/rockchip/rockchip_drm_drv.h
        index a88fe8a..ba0ff0c 100644
        --- a/drivers/gpu/drm/rockchip/rockchip_drm_drv.h
        +++ b/drivers/gpu/drm/rockchip/rockchip_drm_drv.h
        @@ -85,6 +85,9 @@
         	struct list_head psr_list;
         	struct mutex psr_list_lock;
         
        +	struct devfreq *devfreq;
        +	struct devfreq_event_dev *devfreq_event_dev;
        +	bool dmc_disable_flag;
         	struct drm_atomic_state *state;
         };
         
        @@ -99,4 +102,7 @@
         int rockchip_drm_wait_line_flag(struct drm_crtc *crtc, unsigned int line_num,
         				unsigned int mstimeout);
         
        +void rockchip_drm_enable_dmc(struct rockchip_drm_private *priv);
        +void rockchip_drm_disable_dmc(struct rockchip_drm_private *priv);
        +
         #endif /* _ROCKCHIP_DRM_DRV_H_ */
        diff --git a/drivers/gpu/drm/rockchip/rockchip_drm_fb.c b/drivers/gpu/drm/rockchip/rockchip_drm_fb.c
        index 1fecaa3..0b3febe 100644
        --- a/drivers/gpu/drm/rockchip/rockchip_drm_fb.c
        +++ b/drivers/gpu/drm/rockchip/rockchip_drm_fb.c
        @@ -19,6 +19,7 @@
         #include <drm/drm_atomic.h>
         #include <drm/drm_fb_helper.h>
         #include <drm/drm_crtc_helper.h>
        +#include <soc/rockchip/rk3399_dmc.h>
         
         #include "rockchip_drm_drv.h"
         #include "rockchip_drm_gem.h"
        @@ -233,13 +234,43 @@
         	}
         }
         
        +static uint32_t get_vblank_time_us(struct drm_display_mode *mode)
        +{
        +	uint64_t vblank_time = mode->vtotal - mode->vdisplay;
        +
        +	vblank_time *= (uint64_t)USEC_PER_SEC * mode->htotal;
        +	do_div(vblank_time, mode->clock * 1000);
        +
        +	return vblank_time;
        +}
        +
         static void
         rockchip_atomic_commit_complete(struct rockchip_atomic_commit *commit)
         {
         	struct drm_atomic_state *state = commit->state;
         	struct drm_device *dev = commit->dev;
        +	struct rockchip_drm_private *priv = dev->dev_private;
        +	struct drm_crtc *crtc;
        +	int num_active_crtcs = 0;
        +	bool force_dmc_off = false;
        +
        +	drm_for_each_crtc(crtc, dev) {
        +		if (crtc->state->active) {
        +			num_active_crtcs++;
        +			if (get_vblank_time_us(&crtc->state->adjusted_mode) <
        +			    ROCKCHIP_DMC_MIN_VBLANK_US)
        +				force_dmc_off = true;
        +		}
        +	}
         
         	wait_for_fences(dev, state);
        +
        +	/* If disabling dmc, disable it before committing mode set changes. */
        +	if ((force_dmc_off || num_active_crtcs > 1) &&
        +	    !priv->dmc_disable_flag) {
        +		rockchip_drm_disable_dmc(priv);
        +		priv->dmc_disable_flag = true;
        +	}
         
         	/*
         	 * Rockchip crtc support runtime PM, can't update display planes
        @@ -271,6 +302,11 @@
         
         	drm_atomic_helper_cleanup_planes(dev, state);
         
        +	if (!force_dmc_off && num_active_crtcs <= 1 && priv->dmc_disable_flag) {
        +		rockchip_drm_enable_dmc(priv);
        +		priv->dmc_disable_flag = false;
        +	}
        +
         	drm_atomic_state_free(state);
         }
         
        @@ -279,7 +315,11 @@
         	struct rockchip_atomic_commit *commit = container_of(work,
         					struct rockchip_atomic_commit, work);
         
        +	drm_modeset_lock_all(commit->dev);
        +	mutex_lock(&commit->lock);
         	rockchip_atomic_commit_complete(commit);
        +	mutex_unlock(&commit->lock);
        +	drm_modeset_unlock_all(commit->dev);
         }
         
         int rockchip_drm_atomic_commit(struct drm_device *dev,
        diff --git a/include/soc/rockchip/rk3399_dmc.h b/include/soc/rockchip/rk3399_dmc.h
        index f0b6b41..4c0574f 100644
        --- a/include/soc/rockchip/rk3399_dmc.h
        +++ b/include/soc/rockchip/rk3399_dmc.h
        @@ -17,6 +17,8 @@
         
         #include <linux/devfreq.h>
         
        +#define ROCKCHIP_DMC_MIN_VBLANK_US 300
        +
         struct rk3399_dmcfreq {
         	struct device *dev;
         	struct devfreq *devfreq;
        

        To view, visit this change. To unsubscribe, visit settings.

        Gerrit-MessageType: merged

        Reply all
        Reply to author
        Forward
        0 new messages