| Message ID | 1470799604-12877-9-git-send-email-hl@rock-chips.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
Hi Lin, Looks good to me about the devfreq/devfreq-event usage. [Usage of the devfreq/devfreq-event APIs] Reviewed-by: Chanwoo Choi <cw00.choi@samsung.com> Regards, Chanwoo Choi On 2016년 08월 10일 12:26, Lin Huang wrote: > 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. > > Signed-off-by: Lin Huang <hl@rock-chips.com> > --- > Changes in v5: > - improve some nits > > Changes in v4: > - register notifier to devfreq_register_notifier > - use DEVFREQ_PRECHANGE and DEVFREQ_POSTCHANGE to get dmc status > - when two vop enable, disable dmc > - when two vop back to one vop, enable dmc > > Changes in v3: > - when do vop eanble/disable, dmc will wait until it finish > > Changes in v2: > - None > > Changes in v1: > - use wait_event instead usleep > > drivers/gpu/drm/rockchip/rockchip_drm_vop.c | 128 +++++++++++++++++++++++++++- > 1 file changed, 125 insertions(+), 3 deletions(-) > > diff --git a/drivers/gpu/drm/rockchip/rockchip_drm_vop.c b/drivers/gpu/drm/rockchip/rockchip_drm_vop.c > index 31744fe..7ce3890 100644 > --- a/drivers/gpu/drm/rockchip/rockchip_drm_vop.c > +++ b/drivers/gpu/drm/rockchip/rockchip_drm_vop.c > @@ -12,6 +12,8 @@ > * GNU General Public License for more details. > */ > > +#include <linux/devfreq.h> > +#include <linux/devfreq-event.h> > #include <drm/drm.h> > #include <drm/drmP.h> > #include <drm/drm_atomic.h> > @@ -118,6 +120,13 @@ struct vop { > > const struct vop_data *data; > > + struct devfreq *devfreq; > + struct devfreq_event_dev *devfreq_event_dev; > + 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; > > @@ -428,21 +437,59 @@ static void vop_dsp_hold_valid_irq_disable(struct vop *vop) > 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 == DEVFREQ_PRECHANGE) { > + /* > + * check if vop in enable or disable process, > + * if yes, wait until it finishes, use 200ms as > + * timeout. > + */ > + if (!wait_event_timeout(vop->wait_vop_switch_queue, > + !vop->vop_switch_status, HZ / 5)) > + dev_warn(vop->dev, > + "Timeout waiting for vop swtich status\n"); > + vop->dmc_in_process = 1; > + } else if (event == DEVFREQ_POSTCHANGE) { > + 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); > + int num_enabled_crtc = 0; > int ret; > > + if (vop->is_enabled) > + return; > + > + /* > + * if in dmc scaling frequency process, wait until it finishes > + * use 100ms as timeout time. > + */ > + if (!wait_event_timeout(vop->wait_dmc_queue, > + !vop->dmc_in_process, HZ / 5)) > + dev_warn(vop->dev, > + "Timeout waiting for dmc when vop enable\n"); > + > + 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); > @@ -456,7 +503,6 @@ static void vop_enable(struct drm_crtc *crtc) > 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. > @@ -485,6 +531,21 @@ static void vop_enable(struct drm_crtc *crtc) > > drm_crtc_vblank_on(crtc); > > + vop->vop_switch_status = 0; > + wake_up(&vop->wait_vop_switch_queue); > + > + /* check how many vop we use now */ > + drm_for_each_crtc(crtc, vop->drm_dev) { > + if (crtc->state->enable) > + num_enabled_crtc++; > + } > + > + /* if enable two vop, need to disable dmc */ > + if ((num_enabled_crtc > 1) && vop->devfreq) { > + if (vop->devfreq_event_dev) > + devfreq_event_disable_edev(vop->devfreq_event_dev); > + devfreq_suspend_device(vop->devfreq); > + } > return; > > err_disable_aclk: > @@ -493,16 +554,32 @@ err_disable_dclk: > 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) > { > struct vop *vop = to_vop(crtc); > + int num_enabled_crtc = 0; > int i; > > WARN_ON(vop->event); > > /* > + * if in dmc scaling frequency process, wait until it finish > + * use 100ms as timeout time. > + */ > + if (!wait_event_timeout(vop->wait_dmc_queue, > + !vop->dmc_in_process, HZ / 5)) > + dev_warn(vop->dev, > + "Timeout waiting for dmc when vop disable\n"); > + > + vop->vop_switch_status = 1; > + > + /* > * We need to make sure that all windows are disabled before we > * disable that crtc. Otherwise we might try to scan from a destroyed > * buffer later. > @@ -559,6 +636,25 @@ static void vop_crtc_disable(struct drm_crtc *crtc) > > crtc->state->event = NULL; > } > + > + vop->vop_switch_status = 0; > + wake_up(&vop->wait_vop_switch_queue); > + > + /* check how many vop use now */ > + drm_for_each_crtc(crtc, vop->drm_dev) { > + if (crtc->state->enable) > + num_enabled_crtc++; > + } > + > + /* > + * if num_enabled_crtc = 1 now, it means 2 vop enabled > + * change to 1 vop enabled need to enable dmc again. > + */ > + if ((num_enabled_crtc == 1) && vop->devfreq) { > + if (vop->devfreq_event_dev) > + devfreq_event_enable_edev(vop->devfreq_event_dev); > + devfreq_resume_device(vop->devfreq); > + } > } > > static void vop_plane_destroy(struct drm_plane *plane) > @@ -1406,6 +1502,8 @@ static int vop_bind(struct device *dev, struct device *master, void *data) > struct drm_device *drm_dev = data; > struct vop *vop; > struct resource *res; > + struct devfreq *devfreq; > + struct devfreq_event_dev *event_dev; > size_t alloc_size; > int ret, irq; > > @@ -1467,6 +1565,30 @@ static int vop_bind(struct device *dev, struct device *master, void *data) > return ret; > > pm_runtime_enable(&pdev->dev); > + > + vop->psr_enabled = false; > + INIT_DELAYED_WORK(&vop->psr_work, vop_psr_work); > + > + 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; > + > + devfreq = devfreq_get_devfreq_by_phandle(dev, 0); > + if (IS_ERR(devfreq)) > + goto out; > + > + vop->devfreq = devfreq; > + vop->dmc_nb.notifier_call = dmc_notify; > + devfreq_register_notifier(vop->devfreq, &vop->dmc_nb, > + DEVFREQ_TRANSITION_NOTIFIER); > + > + event_dev = devfreq_event_get_edev_by_phandle(vop->devfreq->dev.parent, > + 0); > + if (IS_ERR(event_dev)) > + goto out; > + vop->devfreq_event_dev = event_dev; > +out: > return 0; > } > >
Hi Lin,
[auto build test ERROR on v4.8-rc1]
[also build test ERROR on next-20160809]
[cannot apply to rockchip/for-next devfreq/for-rafael linux/master]
[if your patch is applied to the wrong git tree, please drop us a note to help improve the system]
url: https://github.com/0day-ci/linux/commits/Lin-Huang/rk3399-support-ddr-frequency-scaling/20160810-114433
config: arm-multi_v7_defconfig (attached as .config)
compiler: arm-linux-gnueabi-gcc (Debian 5.4.0-6) 5.4.0 20160609
reproduce:
wget https://git.kernel.org/cgit/linux/kernel/git/wfg/lkp-tests.git/plain/sbin/make.cross -O ~/bin/make.cross
chmod +x ~/bin/make.cross
# save the attached .config to linux build tree
make.cross ARCH=arm
All error/warnings (new ones prefixed by >>):
In file included from drivers/gpu/drm/rockchip/rockchip_drm_vop.c:16:0:
>> include/linux/devfreq-event.h:190:21: error: redefinition of 'devfreq_event_get_drvdata'
static inline void *devfreq_event_get_drvdata(struct devfreq_event_dev *edev)
^
include/linux/devfreq-event.h:151:21: note: previous definition of 'devfreq_event_get_drvdata' was here
static inline void *devfreq_event_get_drvdata(struct devfreq_event_dev *edev)
^
drivers/gpu/drm/rockchip/rockchip_drm_vop.c: In function 'vop_bind':
>> drivers/gpu/drm/rockchip/rockchip_drm_vop.c:1586:5: error: 'struct vop' has no member named 'psr_enabled'
vop->psr_enabled = false;
^
In file included from include/linux/kobject.h:29:0,
from include/linux/device.h:17,
from include/linux/devfreq.h:16,
from drivers/gpu/drm/rockchip/rockchip_drm_vop.c:15:
>> drivers/gpu/drm/rockchip/rockchip_drm_vop.c:1587:24: error: 'struct vop' has no member named 'psr_work'
INIT_DELAYED_WORK(&vop->psr_work, vop_psr_work);
^
include/linux/workqueue.h:216:16: note: in definition of macro '__INIT_WORK'
__init_work((_work), _onstack); \
^
>> include/linux/workqueue.h:231:3: note: in expansion of macro 'INIT_WORK'
INIT_WORK(&(_work)->work, (_func)); \
^
>> include/linux/workqueue.h:247:2: note: in expansion of macro '__INIT_DELAYED_WORK'
__INIT_DELAYED_WORK(_work, _func, 0)
^
>> drivers/gpu/drm/rockchip/rockchip_drm_vop.c:1587:2: note: in expansion of macro 'INIT_DELAYED_WORK'
INIT_DELAYED_WORK(&vop->psr_work, vop_psr_work);
^
>> drivers/gpu/drm/rockchip/rockchip_drm_vop.c:1587:24: error: 'struct vop' has no member named 'psr_work'
INIT_DELAYED_WORK(&vop->psr_work, vop_psr_work);
^
include/linux/workqueue.h:217:4: note: in definition of macro '__INIT_WORK'
(_work)->data = (atomic_long_t) WORK_DATA_INIT(); \
^
>> include/linux/workqueue.h:231:3: note: in expansion of macro 'INIT_WORK'
INIT_WORK(&(_work)->work, (_func)); \
^
>> include/linux/workqueue.h:247:2: note: in expansion of macro '__INIT_DELAYED_WORK'
__INIT_DELAYED_WORK(_work, _func, 0)
^
>> drivers/gpu/drm/rockchip/rockchip_drm_vop.c:1587:2: note: in expansion of macro 'INIT_DELAYED_WORK'
INIT_DELAYED_WORK(&vop->psr_work, vop_psr_work);
^
>> drivers/gpu/drm/rockchip/rockchip_drm_vop.c:1587:24: error: 'struct vop' has no member named 'psr_work'
INIT_DELAYED_WORK(&vop->psr_work, vop_psr_work);
^
include/linux/workqueue.h:218:20: note: in definition of macro '__INIT_WORK'
INIT_LIST_HEAD(&(_work)->entry); \
^
>> include/linux/workqueue.h:231:3: note: in expansion of macro 'INIT_WORK'
INIT_WORK(&(_work)->work, (_func)); \
^
>> include/linux/workqueue.h:247:2: note: in expansion of macro '__INIT_DELAYED_WORK'
__INIT_DELAYED_WORK(_work, _func, 0)
^
>> drivers/gpu/drm/rockchip/rockchip_drm_vop.c:1587:2: note: in expansion of macro 'INIT_DELAYED_WORK'
INIT_DELAYED_WORK(&vop->psr_work, vop_psr_work);
^
>> drivers/gpu/drm/rockchip/rockchip_drm_vop.c:1587:24: error: 'struct vop' has no member named 'psr_work'
INIT_DELAYED_WORK(&vop->psr_work, vop_psr_work);
^
include/linux/workqueue.h:219:4: note: in definition of macro '__INIT_WORK'
(_work)->func = (_func); \
^
>> include/linux/workqueue.h:231:3: note: in expansion of macro 'INIT_WORK'
INIT_WORK(&(_work)->work, (_func)); \
^
>> include/linux/workqueue.h:247:2: note: in expansion of macro '__INIT_DELAYED_WORK'
__INIT_DELAYED_WORK(_work, _func, 0)
^
>> drivers/gpu/drm/rockchip/rockchip_drm_vop.c:1587:2: note: in expansion of macro 'INIT_DELAYED_WORK'
INIT_DELAYED_WORK(&vop->psr_work, vop_psr_work);
^
>> drivers/gpu/drm/rockchip/rockchip_drm_vop.c:1587:36: error: 'vop_psr_work' undeclared (first use in this function)
INIT_DELAYED_WORK(&vop->psr_work, vop_psr_work);
^
include/linux/workqueue.h:219:20: note: in definition of macro '__INIT_WORK'
(_work)->func = (_func); \
^
>> include/linux/workqueue.h:231:3: note: in expansion of macro 'INIT_WORK'
INIT_WORK(&(_work)->work, (_func)); \
^
vim +/devfreq_event_get_drvdata +190 include/linux/devfreq-event.h
f262f28c Chanwoo Choi 2015-01-26 184
f262f28c Chanwoo Choi 2015-01-26 185 static inline void devm_devfreq_event_remove_edev(struct device *dev,
f262f28c Chanwoo Choi 2015-01-26 186 struct devfreq_event_dev *edev)
f262f28c Chanwoo Choi 2015-01-26 187 {
f262f28c Chanwoo Choi 2015-01-26 188 }
f262f28c Chanwoo Choi 2015-01-26 189
f262f28c Chanwoo Choi 2015-01-26 @190 static inline void *devfreq_event_get_drvdata(struct devfreq_event_dev *edev)
f262f28c Chanwoo Choi 2015-01-26 191 {
f262f28c Chanwoo Choi 2015-01-26 192 return NULL;
f262f28c Chanwoo Choi 2015-01-26 193 }
:::::: The code at line 190 was first introduced by commit
:::::: f262f28c147051e7aa6daaf4fb5996833ffadff4 PM / devfreq: event: Add devfreq_event class
:::::: TO: Chanwoo Choi <cw00.choi@samsung.com>
:::::: CC: MyungJoo Ham <myungjoo.ham@samsung.com>
---
0-DAY kernel test infrastructure Open Source Technology Center
https://lists.01.org/pipermail/kbuild-all Intel Corporation
diff --git a/drivers/gpu/drm/rockchip/rockchip_drm_vop.c b/drivers/gpu/drm/rockchip/rockchip_drm_vop.c index 31744fe..7ce3890 100644 --- a/drivers/gpu/drm/rockchip/rockchip_drm_vop.c +++ b/drivers/gpu/drm/rockchip/rockchip_drm_vop.c @@ -12,6 +12,8 @@ * GNU General Public License for more details. */ +#include <linux/devfreq.h> +#include <linux/devfreq-event.h> #include <drm/drm.h> #include <drm/drmP.h> #include <drm/drm_atomic.h> @@ -118,6 +120,13 @@ struct vop { const struct vop_data *data; + struct devfreq *devfreq; + struct devfreq_event_dev *devfreq_event_dev; + 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; @@ -428,21 +437,59 @@ static void vop_dsp_hold_valid_irq_disable(struct vop *vop) 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 == DEVFREQ_PRECHANGE) { + /* + * check if vop in enable or disable process, + * if yes, wait until it finishes, use 200ms as + * timeout. + */ + if (!wait_event_timeout(vop->wait_vop_switch_queue, + !vop->vop_switch_status, HZ / 5)) + dev_warn(vop->dev, + "Timeout waiting for vop swtich status\n"); + vop->dmc_in_process = 1; + } else if (event == DEVFREQ_POSTCHANGE) { + 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); + int num_enabled_crtc = 0; int ret; + if (vop->is_enabled) + return; + + /* + * if in dmc scaling frequency process, wait until it finishes + * use 100ms as timeout time. + */ + if (!wait_event_timeout(vop->wait_dmc_queue, + !vop->dmc_in_process, HZ / 5)) + dev_warn(vop->dev, + "Timeout waiting for dmc when vop enable\n"); + + 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); @@ -456,7 +503,6 @@ static void vop_enable(struct drm_crtc *crtc) 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. @@ -485,6 +531,21 @@ static void vop_enable(struct drm_crtc *crtc) drm_crtc_vblank_on(crtc); + vop->vop_switch_status = 0; + wake_up(&vop->wait_vop_switch_queue); + + /* check how many vop we use now */ + drm_for_each_crtc(crtc, vop->drm_dev) { + if (crtc->state->enable) + num_enabled_crtc++; + } + + /* if enable two vop, need to disable dmc */ + if ((num_enabled_crtc > 1) && vop->devfreq) { + if (vop->devfreq_event_dev) + devfreq_event_disable_edev(vop->devfreq_event_dev); + devfreq_suspend_device(vop->devfreq); + } return; err_disable_aclk: @@ -493,16 +554,32 @@ err_disable_dclk: 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) { struct vop *vop = to_vop(crtc); + int num_enabled_crtc = 0; int i; WARN_ON(vop->event); /* + * if in dmc scaling frequency process, wait until it finish + * use 100ms as timeout time. + */ + if (!wait_event_timeout(vop->wait_dmc_queue, + !vop->dmc_in_process, HZ / 5)) + dev_warn(vop->dev, + "Timeout waiting for dmc when vop disable\n"); + + vop->vop_switch_status = 1; + + /* * We need to make sure that all windows are disabled before we * disable that crtc. Otherwise we might try to scan from a destroyed * buffer later. @@ -559,6 +636,25 @@ static void vop_crtc_disable(struct drm_crtc *crtc) crtc->state->event = NULL; } + + vop->vop_switch_status = 0; + wake_up(&vop->wait_vop_switch_queue); + + /* check how many vop use now */ + drm_for_each_crtc(crtc, vop->drm_dev) { + if (crtc->state->enable) + num_enabled_crtc++; + } + + /* + * if num_enabled_crtc = 1 now, it means 2 vop enabled + * change to 1 vop enabled need to enable dmc again. + */ + if ((num_enabled_crtc == 1) && vop->devfreq) { + if (vop->devfreq_event_dev) + devfreq_event_enable_edev(vop->devfreq_event_dev); + devfreq_resume_device(vop->devfreq); + } } static void vop_plane_destroy(struct drm_plane *plane) @@ -1406,6 +1502,8 @@ static int vop_bind(struct device *dev, struct device *master, void *data) struct drm_device *drm_dev = data; struct vop *vop; struct resource *res; + struct devfreq *devfreq; + struct devfreq_event_dev *event_dev; size_t alloc_size; int ret, irq; @@ -1467,6 +1565,30 @@ static int vop_bind(struct device *dev, struct device *master, void *data) return ret; pm_runtime_enable(&pdev->dev); + + vop->psr_enabled = false; + INIT_DELAYED_WORK(&vop->psr_work, vop_psr_work); + + 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; + + devfreq = devfreq_get_devfreq_by_phandle(dev, 0); + if (IS_ERR(devfreq)) + goto out; + + vop->devfreq = devfreq; + vop->dmc_nb.notifier_call = dmc_notify; + devfreq_register_notifier(vop->devfreq, &vop->dmc_nb, + DEVFREQ_TRANSITION_NOTIFIER); + + event_dev = devfreq_event_get_edev_by_phandle(vop->devfreq->dev.parent, + 0); + if (IS_ERR(event_dev)) + goto out; + vop->devfreq_event_dev = event_dev; +out: return 0; }
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. Signed-off-by: Lin Huang <hl@rock-chips.com> --- Changes in v5: - improve some nits Changes in v4: - register notifier to devfreq_register_notifier - use DEVFREQ_PRECHANGE and DEVFREQ_POSTCHANGE to get dmc status - when two vop enable, disable dmc - when two vop back to one vop, enable dmc Changes in v3: - when do vop eanble/disable, dmc will wait until it finish Changes in v2: - None Changes in v1: - use wait_event instead usleep drivers/gpu/drm/rockchip/rockchip_drm_vop.c | 128 +++++++++++++++++++++++++++- 1 file changed, 125 insertions(+), 3 deletions(-)