Re: [RFC PATCH v2 3/3] drm/rockchip: analogix_dp: add PSR support
From: Daniel Vetter
Date: Thu Jun 02 2016 - 09:18:58 EST
On Thu, Jun 02, 2016 at 08:57:38PM +0800, Yakir Yang wrote:
> Let VOP vblank status decide whether panle should enter into or
> exit from PSR status. Before eDP start to change PSR status, it
> need to wait for VOP vact_end event. In order to listen vact_end
> event, I create a new file about PSR notify between eDP and VOP.
>
> Signed-off-by: Yakir Yang <ykk@xxxxxxxxxxxxxx>
> ---
> Changes in v2:
> - Remove vblank notify out (Daniel)
> - Create a psr_active() callback in vop data struct.
Still contains a notifier. Still doesn't contain a proper fb->dirty
callback. Please don't just act on review without understanding the deeper
implications, since I didn't ask you to remove the vblank logic (you
probably still need that), I suggested to implement it differently
(withotu notifiers).
-Daniel
>
> drivers/gpu/drm/rockchip/Makefile | 2 +-
> drivers/gpu/drm/rockchip/analogix_dp-rockchip.c | 64 ++++++++++++++++++++++++-
> drivers/gpu/drm/rockchip/rockchip_drm_drv.h | 7 +++
> drivers/gpu/drm/rockchip/rockchip_drm_notify.c | 54 +++++++++++++++++++++
> drivers/gpu/drm/rockchip/rockchip_drm_notify.h | 23 +++++++++
> drivers/gpu/drm/rockchip/rockchip_drm_vop.c | 41 ++++++++++++++++
> 6 files changed, 189 insertions(+), 2 deletions(-)
> create mode 100644 drivers/gpu/drm/rockchip/rockchip_drm_notify.c
> create mode 100644 drivers/gpu/drm/rockchip/rockchip_drm_notify.h
>
> diff --git a/drivers/gpu/drm/rockchip/Makefile b/drivers/gpu/drm/rockchip/Makefile
> index 05d0713..49fee8c 100644
> --- a/drivers/gpu/drm/rockchip/Makefile
> +++ b/drivers/gpu/drm/rockchip/Makefile
> @@ -3,7 +3,7 @@
> # Direct Rendering Infrastructure (DRI) in XFree86 4.1.0 and higher.
>
> rockchipdrm-y := rockchip_drm_drv.o rockchip_drm_fb.o \
> - rockchip_drm_gem.o rockchip_drm_vop.o
> + rockchip_drm_gem.o rockchip_drm_vop.o rockchip_drm_notify.o
> rockchipdrm-$(CONFIG_DRM_FBDEV_EMULATION) += rockchip_drm_fbdev.o
>
> obj-$(CONFIG_ROCKCHIP_ANALOGIX_DP) += analogix_dp-rockchip.o
> diff --git a/drivers/gpu/drm/rockchip/analogix_dp-rockchip.c b/drivers/gpu/drm/rockchip/analogix_dp-rockchip.c
> index 4b64964..25fb687 100644
> --- a/drivers/gpu/drm/rockchip/analogix_dp-rockchip.c
> +++ b/drivers/gpu/drm/rockchip/analogix_dp-rockchip.c
> @@ -33,6 +33,7 @@
>
> #include "rockchip_drm_drv.h"
> #include "rockchip_drm_vop.h"
> +#include "rockchip_drm_notify.h"
>
> #define to_dp(nm) container_of(nm, struct rockchip_dp_device, nm)
>
> @@ -54,6 +55,10 @@ struct rockchip_dp_device {
> struct regmap *grf;
> struct reset_control *rst;
>
> + struct workqueue_struct *dp_workq;
> + struct work_struct psr_work;
> + unsigned int psr_state;
> +
> const struct rockchip_dp_chip_data *data;
>
> struct analogix_dp_plat_data plat_data;
> @@ -97,6 +102,42 @@ static int rockchip_dp_powerdown(struct analogix_dp_plat_data *plat_data)
> return 0;
> }
>
> +static int rockchip_dp_psr_active(enum psr_action action, void *priv)
> +{
> + struct rockchip_dp_device *dp = (struct rockchip_dp_device *)priv;
> +
> + switch (action) {
> + case PSR_INACTIVE:
> + dp->psr_state = 0;
> + break;
> + case PSR_ACTIVE:
> + dp->psr_state = EDP_VSC_PSR_STATE_ACTIVE;
> + break;
> + default:
> + return 0;
> + }
> +
> + queue_work(dp->dp_workq, &dp->psr_work);
> + return 0;
> +}
> +
> +static void dp_psr_work(struct work_struct *psr_work)
> +{
> + struct rockchip_dp_device *dp = to_dp(psr_work);
> + int psr_state = dp->psr_state;
> + int ret;
> +
> + if (psr_state == EDP_VSC_PSR_STATE_ACTIVE) {
> + ret = rockchip_psr_ready_wait();
> + if (ret == 0)
> + analogix_dp_actice_psr(dp->dev);
> + } else {
> + ret = rockchip_psr_ready_wait();
> + if (ret == 0)
> + analogix_dp_inactice_psr(dp->dev);
> + }
> +}
> +
> static enum drm_mode_status
> rockchip_dp_mode_valid(struct analogix_dp_plat_data *plat_data,
> struct drm_connector *connector,
> @@ -128,9 +169,18 @@ static void rockchip_dp_drm_encoder_mode_set(struct drm_encoder *encoder,
> struct drm_display_mode *mode,
> struct drm_display_mode *adjusted)
> {
> - /* do nothing */
> + struct rockchip_dp_device *dp = to_dp(encoder);
> + struct drm_crtc *crtc = encoder->crtc;
> + struct rockchip_crtc_state *s;
> +
> + if (crtc) {
> + s = to_rockchip_crtc_state(crtc->state);
> + s->psr_active = rockchip_dp_psr_active;
> + s->psr_priv = dp;
> + }
> }
>
> +
> static void rockchip_dp_drm_encoder_enable(struct drm_encoder *encoder)
> {
> struct rockchip_dp_device *dp = to_dp(encoder);
> @@ -198,6 +248,9 @@ rockchip_dp_drm_encoder_atomic_check(struct drm_encoder *encoder,
> break;
> }
>
> + s->psr_active = rockchip_dp_psr_active;
> + s->psr_priv = dp;
> +
> return 0;
> }
>
> @@ -320,6 +373,15 @@ static int rockchip_dp_bind(struct device *dev, struct device *master,
> dp->plat_data.power_off = rockchip_dp_powerdown;
> dp->plat_data.mode_valid = rockchip_dp_mode_valid;
>
> + dp->dp_workq = create_singlethread_workqueue("dp");
> + if (!dp->dp_workq) {
> + dev_err(dp->dev, "failed to create workqueue\n");
> + return PTR_ERR(dp->dp_workq);
> + }
> +
> + dp->psr_state = 0;
> + INIT_WORK(&dp->psr_work, dp_psr_work);
> +
> return analogix_dp_bind(dev, dp->drm_dev, &dp->plat_data);
> }
>
> diff --git a/drivers/gpu/drm/rockchip/rockchip_drm_drv.h b/drivers/gpu/drm/rockchip/rockchip_drm_drv.h
> index 56f43a3..f1ccc10 100644
> --- a/drivers/gpu/drm/rockchip/rockchip_drm_drv.h
> +++ b/drivers/gpu/drm/rockchip/rockchip_drm_drv.h
> @@ -31,6 +31,11 @@
> struct drm_device;
> struct drm_connector;
>
> +enum psr_action {
> + PSR_ACTIVE = 0,
> + PSR_INACTIVE,
> +};
> +
> /*
> * Rockchip drm private crtc funcs.
> * @enable_vblank: enable crtc vblank irq.
> @@ -54,6 +59,8 @@ struct rockchip_crtc_state {
> struct drm_crtc_state base;
> int output_type;
> int output_mode;
> + int (*psr_active)(enum psr_action action, void *priv);
> + void *psr_priv;
> };
> #define to_rockchip_crtc_state(s) \
> container_of(s, struct rockchip_crtc_state, base)
> diff --git a/drivers/gpu/drm/rockchip/rockchip_drm_notify.c b/drivers/gpu/drm/rockchip/rockchip_drm_notify.c
> new file mode 100644
> index 0000000..908e991
> --- /dev/null
> +++ b/drivers/gpu/drm/rockchip/rockchip_drm_notify.c
> @@ -0,0 +1,54 @@
> +/*
> + * Copyright (C) Fuzhou Rockchip Electronics Co.Ltd
> + * Author: Yakir Yang <ykk@xxxxxxxxxxxxxx>
> + *
> + * This software is licensed under the terms of the GNU General Public
> + * License version 2, as published by the Free Software Foundation, and
> + * may be copied, distributed, and modified under those terms.
> + *
> + * This program is distributed in the hope that it will be useful,
> + * but WITHOUT ANY WARRANTY; without even the implied warranty of
> + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the
> + * GNU General Public License for more details.
> + */
> +
> +#include "rockchip_drm_notify.h"
> +
> +static RAW_NOTIFIER_HEAD(psr_ready_chain);
> +static DEFINE_MUTEX(psr_ready_lock);
> +
> +int rockchip_drm_psr_ready_register_notifier(struct notifier_block *nb)
> +{
> + int ret = 0;
> +
> + if (!nb)
> + return -EINVAL;
> +
> + ret = raw_notifier_chain_register(&psr_ready_chain, nb);
> + return ret;
> +}
> +EXPORT_SYMBOL_GPL(rockchip_drm_psr_ready_register_notifier);
> +
> +int rockchip_drm_psr_ready_unregister_notifier(struct notifier_block *nb)
> +{
> + int ret = 0;
> +
> + if (!nb)
> + return -EINVAL;
> +
> + ret = raw_notifier_chain_unregister(&psr_ready_chain, nb);
> + return ret;
> +}
> +EXPORT_SYMBOL_GPL(rockchip_drm_psr_ready_unregister_notifier);
> +
> +int rockchip_psr_ready_wait(void)
> +{
> + int ret;
> +
> + ret = raw_notifier_call_chain(&psr_ready_chain, 0, 0);
> + if (ret == NOTIFY_BAD)
> + return -ETIMEDOUT;
> +
> + return 0;
> +}
> +EXPORT_SYMBOL_GPL(rockchip_psr_ready_wait);
> diff --git a/drivers/gpu/drm/rockchip/rockchip_drm_notify.h b/drivers/gpu/drm/rockchip/rockchip_drm_notify.h
> new file mode 100644
> index 0000000..1b190e5
> --- /dev/null
> +++ b/drivers/gpu/drm/rockchip/rockchip_drm_notify.h
> @@ -0,0 +1,23 @@
> +/*
> + * Copyright (C) 2014 Google, Inc.
> + *
> + * This software is licensed under the terms of the GNU General Public
> + * License version 2, as published by the Free Software Foundation, and
> + * may be copied, distributed, and modified under those terms.
> + *
> + * This program is distributed in the hope that it will be useful,
> + * but WITHOUT ANY WARRANTY; without even the implied warranty of
> + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the
> + * GNU General Public License for more details.
> + */
> +
> +#ifndef __ROCKCHIP_DRM_NOTIFY_H
> +#define __ROCKCHIP_DRM_NOTIFY_H
> +
> +#include <linux/notifier.h>
> +
> +int rockchip_psr_ready_wait(void);
> +int rockchip_drm_psr_ready_register_notifier(struct notifier_block *nb);
> +int rockchip_drm_psr_ready_unregister_notifier(struct notifier_block *nb);
> +
> +#endif /* __ROCKCHIP_DRM_NOTIFY_H */
> diff --git a/drivers/gpu/drm/rockchip/rockchip_drm_vop.c b/drivers/gpu/drm/rockchip/rockchip_drm_vop.c
> index b2a36db..b5a015b 100644
> --- a/drivers/gpu/drm/rockchip/rockchip_drm_vop.c
> +++ b/drivers/gpu/drm/rockchip/rockchip_drm_vop.c
> @@ -35,6 +35,7 @@
> #include "rockchip_drm_gem.h"
> #include "rockchip_drm_fb.h"
> #include "rockchip_drm_vop.h"
> +#include "rockchip_drm_notify.h"
>
> #define __REG_SET_RELAXED(x, off, mask, shift, v, write_mask) \
> vop_mask_write(x, off, mask, shift, v, write_mask, true)
> @@ -117,6 +118,10 @@ struct vop {
> struct completion wait_update_complete;
> struct drm_pending_vblank_event *event;
>
> + /* eDP PSR callback */
> + int (*psr_active)(enum psr_action action, void *priv);
> + void *psr_priv;
> + struct notifier_block psr_prepare_nb;
> struct completion line_flag_completion;
>
> const struct vop_data *data;
> @@ -906,6 +911,9 @@ static int vop_crtc_enable_vblank(struct drm_crtc *crtc)
>
> spin_unlock_irqrestore(&vop->irq_lock, flags);
>
> + if (vop->psr_active)
> + vop->psr_active(PSR_INACTIVE, vop->psr_priv);
> +
> return 0;
> }
>
> @@ -922,6 +930,9 @@ static void vop_crtc_disable_vblank(struct drm_crtc *crtc)
> VOP_INTR_SET_TYPE(vop, enable, FS_INTR, 0);
>
> spin_unlock_irqrestore(&vop->irq_lock, flags);
> +
> + if (vop->psr_active)
> + vop->psr_active(PSR_ACTIVE, vop->psr_priv);
> }
>
> static void vop_crtc_wait_for_update(struct drm_crtc *crtc)
> @@ -1066,6 +1077,9 @@ static void vop_crtc_enable(struct drm_crtc *crtc)
> clk_set_rate(vop->dclk, adjusted_mode->clock * 1000);
>
> VOP_CTRL_SET(vop, standby, 0);
> +
> + vop->psr_active = s->psr_active;
> + vop->psr_priv = s->psr_priv;
> }
>
> static void vop_crtc_atomic_flush(struct drm_crtc *crtc,
> @@ -1178,6 +1192,30 @@ static void vop_handle_vblank(struct vop *vop)
> complete(&vop->wait_update_complete);
> }
>
> +static int psr_prepare_notify(struct notifier_block *psr_prepare_nb,
> + unsigned long action, void *data)
> +{
> + struct vop *vop = container_of(psr_prepare_nb, struct vop,
> + psr_prepare_nb);
> + struct drm_display_mode *mode = &vop->crtc.mode;
> + int vact_end = mode->vtotal - mode->vsync_start + mode->vdisplay;
> + unsigned long jiffies_left;
> +
> + reinit_completion(&vop->line_flag_completion);
> + vop_line_flag_irq_enable(vop, vact_end);
> +
> + jiffies_left = wait_for_completion_timeout(&vop->line_flag_completion,
> + msecs_to_jiffies(200));
> + vop_line_flag_irq_disable(vop);
> +
> + if (jiffies_left == 0) {
> + dev_err(vop->dev, "Timeout waiting for IRQ\n");
> + return NOTIFY_BAD;
> + }
> +
> + return NOTIFY_STOP;
> +}
> +
> static irqreturn_t vop_isr(int irq, void *data)
> {
> struct vop *vop = data;
> @@ -1312,6 +1350,9 @@ static int vop_create_crtc(struct vop *vop)
> goto err_cleanup_crtc;
> }
>
> + vop->psr_prepare_nb.notifier_call = psr_prepare_notify;
> + rockchip_drm_psr_ready_register_notifier(&vop->psr_prepare_nb);
> +
> init_completion(&vop->dsp_hold_completion);
> init_completion(&vop->wait_update_complete);
> init_completion(&vop->line_flag_completion);
> --
> 1.9.1
>
>
> _______________________________________________
> dri-devel mailing list
> dri-devel@xxxxxxxxxxxxxxxxxxxxx
> https://lists.freedesktop.org/mailman/listinfo/dri-devel
--
Daniel Vetter
Software Engineer, Intel Corporation
http://blog.ffwll.ch