From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S941301AbcHJTTA (ORCPT ); Wed, 10 Aug 2016 15:19:00 -0400 Received: from mailout2.samsung.com ([203.254.224.25]:44863 "EHLO mailout2.samsung.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S934345AbcHJTSd (ORCPT ); Wed, 10 Aug 2016 15:18:33 -0400 MIME-version: 1.0 Content-type: text/plain; charset=UTF-8 X-AuditID: cbfee68d-f79286d000007a9a-96-57aab69ca56e Content-transfer-encoding: 8BIT Message-id: <57AAB69B.2000102@samsung.com> Date: Wed, 10 Aug 2016 14:07:39 +0900 From: Chanwoo Choi Organization: Samsung Electronics User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:31.0) Gecko/20100101 Thunderbird/31.6.0 To: Lin Huang , heiko@sntech.de Cc: myungjoo.ham@samsung.com, mark.yao@rock-chips.com, airlied@linux.ie, mturquette@baylibre.com, dbasehore@chromium.org, sboyd@codeaurora.org, linux-kernel@vger.kernel.org, dri-devel@lists.freedesktop.org, dianders@chromium.org, linux-rockchip@lists.infradead.org, kyungmin.park@samsung.com, linux-arm-kernel@lists.infradead.org, tixy@linaro.org, typ@rock-chips.com, sudeep.holla@arm.com, mark.rutland@arm.com, linux-pm@vger.kernel.org Subject: Re: [PATCH v5 8/8] drm/rockchip: Add dmc notifier in vop driver References: <1470799604-12877-1-git-send-email-hl@rock-chips.com> <1470799604-12877-9-git-send-email-hl@rock-chips.com> In-reply-to: <1470799604-12877-9-git-send-email-hl@rock-chips.com> X-Brightmail-Tracker: H4sIAAAAAAAAA02Sa0hTYRzGeXfOzo5D83Vqvk67aFgipc5br5RSgnTIDwl+MIrQZQcV75ta huWSAlNmppkxZdPoIjYwp6WEMRXBS87bysTbpoKZGUbepWVuFvntz/M88P898NCE4D5XSCem ZrKSVHGyO8Un1Q4BGSeq3tZF++onIJb393DwQmMrhXUv2in8cXWJwtsz37h443UvgXX5izys mR3hYv27KgovyzsB/mncJvDzz0Mc3PKojYOHesPx+J1aCm/0FZH4ZW8LiWtUBh6ufHX7jD2j VqoBszR6j8dUyoZIRl8s5zATI60U07xm5DKGoi4O0/gsjzEptCRT3FQHmOYRJcEsaw5GWl/i n77GJidmsxKf0Fh+wkBFPZn+OOTG1hMTkIE5n0JgRSMYgPSLBu7uvR8NTtVThYBPC2AtQPmm Pupf6M303b+GAqB6ZTHPbNhAO7RRNkUWApom4CHUOZxklgl4DJUqnxLmWwCNABVPXtmNe6H8 8k1gjpPQA8kGLQzUjqydH7W8soVu6NPGrCXiCC+iou4cs+wA/ZBuRcUxExCwjUCqrQcWAnvI INP2HNhF0wFkkDVYDCsYhppM3VyzgeA8jcoHpjlmg4QQrZV1WJgRPIA0bcRuR2fUXjtKlgAn xZ5miv/NFHuaVQOiDjiy6XHp0qvxEpG3VJwizUqN945LS9GAnYV8+D0nbwHjbac6AKSBu7UN E1IXLeCKs6U5KR0gcAfiISF0jEvbGVVqZozIP8gPBwYE+vudDA5yd7JxE25GCWC8OJNNYtl0 VhIjyUpmpR2AQ1sJZUBvVcPXJlaftfsuKhCtl9nbFvgaaWHUjNs5T51Hhazyq6r9smtV8GzY mDHKFd06fHOsoVSedNSWHBiKjTifkDHZJbwOer7EVYW6BOS+71sdCFdFZue7NC0khO0LMtSs /cotOa5W85z7I7S1WlJ5YXqlYpg9IlrXxI7pPIPzfriT0gSxyIuQSMV/AKKgFaYcAwAA X-Brightmail-Tracker: H4sIAAAAAAAAA+NgFrrGKsWRmVeSWpSXmKPExsVy+t9jQd0521aFG/z9yGHRe+4kk8WrzXvY LM4uO8hmceXrezaL/49es1r82HCK2eJs0xt2i02Pr7FaXN41h83ic+8RRotPD/4zWyy9fpHJ YseUA0wWF0+5WtxuXMFm8eNMN4vF8lM7WCwWzr/PbjF7dZ2DsMeaeWsYPd7faGX3mN1wkcXj cl8vk8eda3vYPLZ/e8Dqcb/7OJPH5iX1Hn9n7Wfx6NuyitFj+7V5zB6fN8kF8EQ1MNpkpCam pBYppOYl56dk5qXbKnkHxzvHm5oZGOoaWlqYKynkJeam2iq5+AToumXmAP2opFCWmFMKFApI LC5W0rfDNCE0xE3XAqYxQtc3JAiux8gADSSsYcw4P309S8E024pfM/4yNjA+0+9i5OSQEDCR 2PqwhQ3CFpO4cG89kM3FISQwi1Fi/bw+dpAEr4CgxI/J91i6GDk4mAXkJY5cygYJMwuoS0ya t4gZxBYSeMAo0Xc3FqJcS6Jp6k9GkHIWAVWJhgtgq9iAwvtf3ABbxS+gKHH1x2OwElGBCInu E5UgYREBI4mzX+YzgVzALHCAWWL+r36wC4QFPCT+/n/GCHHaWUaJ+w0bwRKcAk4SW/6eYJ3A KDgLyaWzEC6dheTSBYzMqxglUguSC4qT0nMN81LL9YoTc4tL89L1kvNzNzGC09QzqR2MB3e5 H2IU4GBU4uGN8F4VLsSaWFZcmXuIUYKDWUmEt24rUIg3JbGyKrUoP76oNCe1+BCjKdCvE5ml RJPzgSk0ryTe0NjEzMjSyNzQwsjYXEmc9/H/dWFCAumJJanZqakFqUUwfUwcnFINjHwX/Da0 hSy/nvpdLvHFtueb0lk5/Ow32ob9zLRubZwSaz3zKduRn6eOHNLI47zsmfZetObHb72GLM8U fWGlfRvD7t/1/1sV1af5MP23aoN25j5N6zlzPq7a9N3qwp3HPoE+Mr/nHbDkjY/Yupn/XG+Q TKK5OuPlw8vsPWp7/8YebN8imVvVpcRSnJFoqMVcVJwIAE7/QbBpAwAA DLP-Filter: Pass X-MTR: 20000000000000000@CPGS X-CFilter-Loop: Reflected Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org Hi Lin, Looks good to me about the devfreq/devfreq-event usage. [Usage of the devfreq/devfreq-event APIs] Reviewed-by: Chanwoo Choi 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 > --- > 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 > +#include > #include > #include > #include > @@ -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; > } > >