Re: [PATCH v4 2/4] drm/rockchip: add an common abstracted PSR driver

On Thu, Jul 14, 2016 at 12:15:49PM +0800, Yakir Yang wrote:

The PSR driver have exported four symbols for specific device driver:

• rockchip_drm_psr_register()
• rockchip_drm_psr_unregister()
• rockchip_drm_psr_enable()
• rockchip_drm_psr_disable()
• rockchip_drm_psr_flush()

Encoder driver should call the register/unregister interfaces to hook itself into common PSR driver, encoder have implement the 'psr_set' callback which use the set PSR state in hardware side.

Crtc driver would call the enable/disable interfaces when vblank is enable/disable, after that the common PSR driver would call the encoder registered callback to set the PSR state.

Fb driver would call the flush interface in 'fb->dirty' callback, this helper function would force all PSR enabled encoders to exit from PSR for 3 seconds.

Signed-off-by: Yakir Yang ykk@rock-chips.com

I still don't think it's a good idea to pull this out into a separate PSR driver, but I suppose we can integrate it at a later time if it becomes cumbersome.

Reviewed-by: Sean Paul seanpaul@chromium.org

---

Changes in v4:

• Tuck the global "psr_list" & "psr_list_mutex" in struct rockchip_drm_private. (Sean)
• Move the access of "psr->state" under "psr->state_mutex"'s protect. (Sean)
• Let "psr->state = PSR_FLUSH" under "psr->state_mutex"'s protect. (Sean)
• Collect psr_enable() and psr_disable() into psr_set_state()
• s/5\ second/PSR_FLUSH_TIMEOUT/ (Sean)
• Flush the psr callback in vop_crtc_disable(). (Stéphane, reviewed in Google gerrit) [https://chromium-review.googlesource.com/#/c/349084/6/drivers/gpu/drm/rockch...]
• Add the missing file head with license. (Stéphane, reviewed in Google gerrit) [https://chromium-review.googlesource.com/#/c/357563/1/drivers/gpu/drm/rockch...]

Changes in v3:

• split the psr flow into an common abstracted PSR driver
• implement the 'fb->dirty' callback function (Daniel)
• avoid to use notify to acqiure for vact event (Daniel)
• remove psr_active() callback which introduce in v2

Changes in v2: None

drivers/gpu/drm/rockchip/Makefile | 2 +- drivers/gpu/drm/rockchip/rockchip_drm_drv.c | 4 + drivers/gpu/drm/rockchip/rockchip_drm_drv.h | 3 + drivers/gpu/drm/rockchip/rockchip_drm_fb.c | 12 ++ drivers/gpu/drm/rockchip/rockchip_drm_psr.c | 223 ++++++++++++++++++++++++++++ drivers/gpu/drm/rockchip/rockchip_drm_psr.h | 26 ++++ drivers/gpu/drm/rockchip/rockchip_drm_vop.c | 29 ++++ 7 files changed, 298 insertions(+), 1 deletion(-) create mode 100644 drivers/gpu/drm/rockchip/rockchip_drm_psr.c create mode 100644 drivers/gpu/drm/rockchip/rockchip_drm_psr.h

diff --git a/drivers/gpu/drm/rockchip/Makefile b/drivers/gpu/drm/rockchip/Makefile index 05d0713..9746365 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_psr.o rockchip_drm_vop.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/rockchip_drm_drv.c b/drivers/gpu/drm/rockchip/rockchip_drm_drv.c index d665fb0..26c12b3 100644 --- a/drivers/gpu/drm/rockchip/rockchip_drm_drv.c +++ b/drivers/gpu/drm/rockchip/rockchip_drm_drv.c @@ -156,6 +156,9 @@ static int rockchip_drm_bind(struct device *dev)

drm_dev->dev_private = private;

• INIT_LIST_HEAD(&private->psr_list);

• mutex_init(&private->psr_list_mutex);

• drm_mode_config_init(drm_dev);

  rockchip_drm_mode_config_init(drm_dev);

@@ -218,6 +221,7 @@ static int rockchip_drm_bind(struct device *dev)

if (is_support_iommu) arm_iommu_release_mapping(mapping);

• return 0;

err_fbdev_fini: rockchip_drm_fbdev_fini(drm_dev); diff --git a/drivers/gpu/drm/rockchip/rockchip_drm_drv.h b/drivers/gpu/drm/rockchip/rockchip_drm_drv.h index 239b830..9c34c9e 100644 --- a/drivers/gpu/drm/rockchip/rockchip_drm_drv.h +++ b/drivers/gpu/drm/rockchip/rockchip_drm_drv.h @@ -61,6 +61,9 @@ struct rockchip_drm_private { struct drm_gem_object *fbdev_bo; const struct rockchip_crtc_funcs *crtc_funcs[ROCKCHIP_MAX_CRTC]; struct drm_atomic_state *state;

• struct list_head psr_list;
• struct mutex psr_list_mutex;

};

int rockchip_register_crtc_funcs(struct drm_crtc *crtc, diff --git a/drivers/gpu/drm/rockchip/rockchip_drm_fb.c b/drivers/gpu/drm/rockchip/rockchip_drm_fb.c index 20f12bc..36afd9c 100644 --- a/drivers/gpu/drm/rockchip/rockchip_drm_fb.c +++ b/drivers/gpu/drm/rockchip/rockchip_drm_fb.c @@ -21,6 +21,7 @@

#include "rockchip_drm_drv.h" #include "rockchip_drm_gem.h" +#include "rockchip_drm_psr.h"

#define to_rockchip_fb(x) container_of(x, struct rockchip_drm_fb, fb)

@@ -66,9 +67,20 @@ static int rockchip_drm_fb_create_handle(struct drm_framebuffer *fb, rockchip_fb->obj[0], handle); }

+static int rockchip_drm_fb_dirty(struct drm_framebuffer *fb,

• 		 struct drm_file *file,
  

• 		 unsigned int flags, unsigned int color,
  

• 		 struct drm_clip_rect *clips,
  

• 		 unsigned int num_clips)
  

+{

• rockchip_drm_psr_flush(fb->dev);
• return 0;

+}

static const struct drm_framebuffer_funcs rockchip_drm_fb_funcs = { .destroy = rockchip_drm_fb_destroy, .create_handle = rockchip_drm_fb_create_handle,

• .dirty = rockchip_drm_fb_dirty,

};

static struct rockchip_drm_fb * diff --git a/drivers/gpu/drm/rockchip/rockchip_drm_psr.c b/drivers/gpu/drm/rockchip/rockchip_drm_psr.c new file mode 100644 index 0000000..e79ea18 --- /dev/null +++ b/drivers/gpu/drm/rockchip/rockchip_drm_psr.c @@ -0,0 +1,223 @@ +/*

• • Copyright (C) Fuzhou Rockchip Electronics Co.Ltd
• • Author: Yakir Yang ykk@rock-chips.com
• • 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 <drm/drmP.h> +#include <drm/drm_crtc_helper.h>

+#include "rockchip_drm_drv.h" +#include "rockchip_drm_psr.h"

+#define PSR_FLUSH_TIMEOUT msecs_to_jiffies(3000) /* 3 seconds */

+static LIST_HEAD(psr_list); +static DEFINE_MUTEX(psr_list_mutex);

+enum psr_state {

• PSR_FLUSH,
• PSR_ENABLE,
• PSR_DISABLE,

+};

+struct psr_drv {

• struct list_head list;
• enum psr_state state;
• struct mutex state_mutex;
• struct timer_list flush_timer;
• struct drm_encoder *encoder;
• void (*set)(struct drm_encoder *encoder, bool enable);

+};

+static struct psr_drv *find_psr_by_crtc(struct drm_crtc *crtc) +{

• struct rockchip_drm_private *drm_drv = crtc->dev->dev_private;
• struct psr_drv *psr;
• mutex_lock(&drm_drv->psr_list_mutex);
• list_for_each_entry(psr, &drm_drv->psr_list, list) {

• if (psr->encoder->crtc == crtc) {
  

• 	mutex_unlock(&drm_drv->psr_list_mutex);
  

• 	return psr;
  

• }
  

• }
• mutex_unlock(&drm_drv->psr_list_mutex);
• return ERR_PTR(-ENODEV);

+}

+static void psr_set_state(struct psr_drv *psr, enum psr_state state) +{

• mutex_lock(&psr->state_mutex);
• if (psr->state == state) {

• mutex_unlock(&psr->state_mutex);
  

• return;
  

• }
• psr->state = state;
• switch (state) {
• case PSR_ENABLE:

• psr->set(psr->encoder, true);
  

• break;
  

• case PSR_DISABLE:
• case PSR_FLUSH:

• psr->set(psr->encoder, false);
  

• break;
  

• };
• mutex_unlock(&psr->state_mutex);

+}

+static void psr_flush_handler(unsigned long data) +{

• struct psr_drv *psr = (struct psr_drv *)data;
• if (!psr || psr->state != PSR_FLUSH)

• return;
  

• psr_set_state(psr, PSR_ENABLE);

+}

+/**

• • rockchip_drm_psr_enable - enable the encoder PSR which bind to given CRTC
• • @crtc: CRTC to obtain the PSR encoder
• • Returns:
• • Zero on success, negative errno on failure.
• */

+int rockchip_drm_psr_enable(struct drm_crtc *crtc) +{

• struct psr_drv *psr = find_psr_by_crtc(crtc);
• if (IS_ERR(psr))

• return PTR_ERR(psr);
  

• psr_set_state(psr, PSR_ENABLE);
• return 0;

+} +EXPORT_SYMBOL(rockchip_drm_psr_enable);

+/**

• • rockchip_drm_psr_disable - disable the encoder PSR which bind to given CRTC
• • @crtc: CRTC to obtain the PSR encoder
• • Returns:
• • Zero on success, negative errno on failure.
• */

+int rockchip_drm_psr_disable(struct drm_crtc *crtc) +{

• struct psr_drv *psr = find_psr_by_crtc(crtc);
• if (IS_ERR(psr))

• return PTR_ERR(psr);
  

• psr_set_state(psr, PSR_DISABLE);
• return 0;

+} +EXPORT_SYMBOL(rockchip_drm_psr_disable);

+/**

• • rockchip_drm_psr_flush - force to flush all registered PSR encoders
• • @dev: drm device
• • Disable the PSR function for all registered encoders, and then enable the
• • PSR function back after PSR_FLUSH_TIMEOUT. If encoder PSR state have been
• • changed during flush time, then keep the state no change after flush
• • timeout.
• • Returns:
• • Zero on success, negative errno on failure.
• */

+void rockchip_drm_psr_flush(struct drm_device *dev) +{

• struct rockchip_drm_private *drm_drv = dev->dev_private;
• struct psr_drv *psr;
• mutex_lock(&drm_drv->psr_list_mutex);
• list_for_each_entry(psr, &drm_drv->psr_list, list) {

• if (psr->state == PSR_DISABLE)
  

• 	continue;
  

• mod_timer(&psr->flush_timer,
  

• 	  round_jiffies_up(jiffies + PSR_FLUSH_TIMEOUT));
  

• psr_set_state(psr, PSR_FLUSH);
  

• }
• mutex_unlock(&drm_drv->psr_list_mutex);

+} +EXPORT_SYMBOL(rockchip_drm_psr_flush);

+/**

• • rockchip_drm_psr_register - register encoder to psr driver
• • @encoder: encoder that obtain the PSR function
• • @psr_set: call back to set PSR state
• • Returns:
• • Zero on success, negative errno on failure.
• */

+int rockchip_drm_psr_register(struct drm_encoder *encoder,

• 	void (*psr_set)(struct drm_encoder *, bool enable))
  

+{

• struct rockchip_drm_private *drm_drv = encoder->dev->dev_private;
• struct psr_drv *psr;
• if (!encoder || !psr_set)

• return -EINVAL;
  

• psr = kzalloc(sizeof(struct psr_drv), GFP_KERNEL);
• if (!psr)

• return -ENOMEM;
  

• setup_timer(&psr->flush_timer, psr_flush_handler, (unsigned long)psr);
• mutex_init(&psr->state_mutex);
• psr->state = PSR_DISABLE;
• psr->encoder = encoder;
• psr->set = psr_set;
• mutex_lock(&drm_drv->psr_list_mutex);
• list_add_tail(&psr->list, &drm_drv->psr_list);
• mutex_unlock(&drm_drv->psr_list_mutex);
• return 0;

+} +EXPORT_SYMBOL(rockchip_drm_psr_register);

+/**

• • rockchip_drm_psr_unregister - unregister encoder to psr driver
• • @encoder: encoder that obtain the PSR function
• • @psr_set: call back to set PSR state
• • Returns:
• • Zero on success, negative errno on failure.
• */

+void rockchip_drm_psr_unregister(struct drm_encoder *encoder) +{

• struct rockchip_drm_private *drm_drv = encoder->dev->dev_private;
• struct psr_drv *psr;
• mutex_lock(&drm_drv->psr_list_mutex);
• list_for_each_entry(psr, &drm_drv->psr_list, list) {

• if (psr->encoder == encoder) {
  

• 	del_timer(&psr->flush_timer);
  

• 	list_del(&psr->list);
  

• 	kfree(psr);
  

• }
  

• }
• mutex_unlock(&drm_drv->psr_list_mutex);

+} +EXPORT_SYMBOL(rockchip_drm_psr_unregister); diff --git a/drivers/gpu/drm/rockchip/rockchip_drm_psr.h b/drivers/gpu/drm/rockchip/rockchip_drm_psr.h new file mode 100644 index 0000000..c35b688 --- /dev/null +++ b/drivers/gpu/drm/rockchip/rockchip_drm_psr.h @@ -0,0 +1,26 @@ +/*

• • Copyright (C) Fuzhou Rockchip Electronics Co.Ltd
• • Author: Yakir Yang ykk@rock-chips.com
• • 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_PSR___ +#define __ROCKCHIP_DRM_PSR___

+void rockchip_drm_psr_flush(struct drm_device *dev); +int rockchip_drm_psr_enable(struct drm_crtc *crtc); +int rockchip_drm_psr_disable(struct drm_crtc *crtc);

+int rockchip_drm_psr_register(struct drm_encoder *encoder,

• 	void (*psr_set)(struct drm_encoder *, bool enable));
  

+void rockchip_drm_psr_unregister(struct drm_encoder *encoder);

+#endif /* __ROCKCHIP_DRM_PSR__ */ diff --git a/drivers/gpu/drm/rockchip/rockchip_drm_vop.c b/drivers/gpu/drm/rockchip/rockchip_drm_vop.c index 69d32f1..e702fb3 100644 --- a/drivers/gpu/drm/rockchip/rockchip_drm_vop.c +++ b/drivers/gpu/drm/rockchip/rockchip_drm_vop.c @@ -34,6 +34,7 @@ #include "rockchip_drm_drv.h" #include "rockchip_drm_gem.h" #include "rockchip_drm_fb.h" +#include "rockchip_drm_psr.h" #include "rockchip_drm_vop.h"

#define __REG_SET_RELAXED(x, off, mask, shift, v, write_mask) \ @@ -87,6 +88,8 @@ #define to_vop_win(x) container_of(x, struct vop_win, base) #define to_vop_plane_state(x) container_of(x, struct vop_plane_state, base)

+#define VOP_PSR_SET_DELAY_TIME msecs_to_jiffies(10)

struct vop_plane_state { struct drm_plane_state base; int format; @@ -121,6 +124,9 @@ struct vop { /* protected by dev->event_lock */ struct drm_pending_vblank_event *event;

• bool psr_enabled;

• struct delayed_work psr_work;

• struct completion line_flag_completion;

  const struct vop_data *data;

@@ -629,6 +635,9 @@ static void vop_crtc_disable(struct drm_crtc *crtc)

crtc->state->event = NULL;

}

• vop->psr_enabled = false;
• INIT_DELAYED_WORK(&vop->psr_work, vop_psr_work);

}

static void vop_plane_destroy(struct drm_plane *plane) @@ -923,6 +932,16 @@ static const struct drm_plane_funcs vop_plane_funcs = { .atomic_destroy_state = vop_atomic_plane_destroy_state, };

+static void vop_psr_work(struct work_struct *work) +{

• struct vop *vop = container_of(work, typeof(*vop), psr_work.work);
• if (vop->psr_enabled)

• rockchip_drm_psr_enable(&vop->crtc);
  

• else

• rockchip_drm_psr_disable(&vop->crtc);
  

+}

static int vop_crtc_enable_vblank(struct drm_crtc *crtc) { struct vop *vop = to_vop(crtc); @@ -937,6 +956,9 @@ static int vop_crtc_enable_vblank(struct drm_crtc *crtc)

spin_unlock_irqrestore(&vop->irq_lock, flags);

• vop->psr_enabled = false;
• schedule_delayed_work(&vop->psr_work, VOP_PSR_SET_DELAY_TIME);
• return 0;

}

@@ -953,6 +975,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);

• vop->psr_enabled = true;
• schedule_delayed_work(&vop->psr_work, VOP_PSR_SET_DELAY_TIME);

}

static void vop_crtc_wait_for_update(struct drm_crtc *crtc) @@ -1597,6 +1622,10 @@ 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);
• return 0;

}

-- 1.9.1

---

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