Den 01.11.2017 09.47, skrev Daniel Vetter:
On Tue, Oct 31, 2017 at 05:37:23PM +0100, Noralf Trønnes wrote:
Den 30.10.2017 10.34, skrev Daniel Vetter:
Hi Noralf,
On Sun, Oct 22, 2017 at 06:52:41PM +0200, Noralf Trønnes wrote:
Hi,
I've spent some time in the fbdev emulation code and discovered a
recurring pattern around suspend/resume.
Should we add some more helpers :-)
You're maybe a bit too good at spotting these for your own good :-)
But yeah, a "suspend for dummies" is one of the things which would be nice
I think ... Especially since we now have the atomic suspend/resume
helpers.
struct drm_device {
/**
* @suspend_state:
*
* Atomic state when suspended.
* Set by drm_dev_suspend(), cleared by drm_dev_resume().
*/
struct drm_atomic_state *suspend_state;
};
Imo fits better when we put it into drm_mode_config.
int drm_dev_suspend(struct drm_device *dev)
{
struct drm_atomic_state *state;
if (!dev)
return 0;
drm_kms_helper_poll_disable(dev);
drm_fb_helper_set_suspend_unlocked(dev->fb_helper, 1);
state = drm_atomic_helper_suspend(dev);
if (IS_ERR(state)) {
drm_fb_helper_set_suspend_unlocked(dev->fb_helper, 0);
drm_kms_helper_poll_enable(dev);
return PTR_ERR(state);
}
dev->suspend_state = state;
return 0;
}
This is all about suspending the modeset side ... I'd give it a
drm_mode_config prefix instead of the drm_dev_ (that's maybe a bit too
generic), but then maybe type a general suspend/resume kernel-doc text in
the drm-internals.rst (maybe pulled in from drm_dev.c) which references
these 2 functions as the recommended way to suspend/resume the modeset
side of a driver. These won't suspend/resume a render part (if present),
so drm_dev_ seems a bit too much.
I just realised that this is pulling helpers (atomic, crtc, fbdev) into
the core which IIRC is something that you didn't want?
Ugh right. I think starting a new drm_mode_config_helper.c for top-level
helper stuff would be a reasonable solution. Or some other name if you
have a better one.
Does it fit in drm_modeset_helper.c ?
/**
* DOC: aux kms helpers
*
* This helper library contains various one-off functions which don't
really fit
* anywhere else in the DRM modeset helper library.
*/
Noralf.
-Daniel
diff --git a/drivers/gpu/drm/drm_mode_config.c
b/drivers/gpu/drm/drm_mode_config.c
index 74f6ff5df656..72f8fe1e83cb 100644
--- a/drivers/gpu/drm/drm_mode_config.c
+++ b/drivers/gpu/drm/drm_mode_config.c
@@ -20,7 +20,10 @@
* OF THIS SOFTWARE.
*/
+#include <drm/drm_atomic_helper.h>
+#include <drm/drm_crtc_helper.h>
#include <drm/drm_encoder.h>
+#include <drm/drm_fb_helper.h>
#include <drm/drm_mode_config.h>
#include <drm/drmP.h>
@@ -473,3 +476,49 @@ void drm_mode_config_cleanup(struct drm_device *dev)
drm_modeset_lock_fini(&dev->mode_config.connection_mutex);
}
EXPORT_SYMBOL(drm_mode_config_cleanup);
+
+/**
+ * drm_mode_config_suspend
+ * @dev: DRM device
+ *
+ */
+int drm_mode_config_suspend(struct drm_device *dev)
+{
+ struct drm_atomic_state *state;
+
+ if (!dev)
+ return 0;
+
+ drm_kms_helper_poll_disable(dev);
+ drm_fb_helper_set_suspend_unlocked(dev->fb_helper, 1);
+ state = drm_atomic_helper_suspend(dev);
+ if (IS_ERR(state)) {
+ drm_fb_helper_set_suspend_unlocked(dev->fb_helper, 0);
+ drm_kms_helper_poll_enable(dev);
+ return PTR_ERR(state);
+ }
+
+ dev->mode_config.suspend_state = state;
+
+ return 0;
+}
+EXPORT_SYMBOL(drm_mode_config_suspend);
+
+int drm_mode_config_resume(struct drm_device *dev)
+{
+ int ret;
+
+ if (!dev || WARN_ON(!dev->mode_config.suspend_state))
+ return 0;
+
+ ret = drm_atomic_helper_resume(dev, dev->mode_config.suspend_state);
+ if (ret)
+ DRM_ERROR("Failed to resume (%d)\n", ret);
+ dev->mode_config.suspend_state = NULL;
+
+ drm_fb_helper_set_suspend_unlocked(dev->fb_helper, 0);
+ drm_kms_helper_poll_enable(dev);
+
+ return 0;
+}
+EXPORT_SYMBOL(drm_mode_config_resume);
diff --git a/include/drm/drm_mode_config.h b/include/drm/drm_mode_config.h
index 1b37368416c8..86ef327a996d 100644
--- a/include/drm/drm_mode_config.h
+++ b/include/drm/drm_mode_config.h
@@ -766,11 +766,21 @@ struct drm_mode_config {
/* cursor size */
uint32_t cursor_width, cursor_height;
+ /**
+ * @suspend_state:
+ *
+ * Atomic state when suspended. Set by drm_mode_config_suspend(),
+ * cleared by drm_mode_config_resume().
+ */
+ struct drm_atomic_state *suspend_state;
+
const struct drm_mode_config_helper_funcs *helper_private;
};
void drm_mode_config_init(struct drm_device *dev);
void drm_mode_config_reset(struct drm_device *dev);
void drm_mode_config_cleanup(struct drm_device *dev);
+int drm_mode_config_suspend(struct drm_device *dev);
+int drm_mode_config_resume(struct drm_device *dev);
#endif
Noralf.
int drm_dev_resume(struct drm_device *dev)
{
int ret;
if (!dev || WARN_ON(!dev->suspend_state))
return 0;
ret = drm_atomic_helper_resume(dev, dev->suspend_state);
if (ret)
DRM_ERROR("Failed to resume (%d)\n", ret);
dev->suspend_state = NULL;
drm_fb_helper_set_suspend_unlocked(dev->fb_helper, 0);
drm_kms_helper_poll_enable(dev);
return 0;
}
The drm_kms_helper_poll_enable() docs states that it's not allowed to
call this function if polling support has been set up, but the code
allows it. drm_kms_helper_poll_disable() docs states that it's ok to
call it even if polling is not enabled.
Yeah need to fix that, but need to keep a note that if you want polling,
then you need to follow that order or it goes wrong. Perhaps explain that
this is meant to avoid special-cases in suspend/resume code?
Here are the suspend/resume code from the drivers that use
drm_atomic_helper_suspend/resume():
Yeah, looks like a really good idea.
Thanks, Daniel
static int __maybe_unused hdlcd_pm_suspend(struct device *dev)
{
struct drm_device *drm = dev_get_drvdata(dev);
struct hdlcd_drm_private *hdlcd = drm ? drm->dev_private : NULL;
if (!hdlcd)
return 0;
drm_kms_helper_poll_disable(drm);
hdlcd->state = drm_atomic_helper_suspend(drm);
if (IS_ERR(hdlcd->state)) {
drm_kms_helper_poll_enable(drm);
return PTR_ERR(hdlcd->state);
}
return 0;
}
static int __maybe_unused hdlcd_pm_resume(struct device *dev)
{
struct drm_device *drm = dev_get_drvdata(dev);
struct hdlcd_drm_private *hdlcd = drm ? drm->dev_private : NULL;
if (!hdlcd)
return 0;
drm_atomic_helper_resume(drm, hdlcd->state);
drm_kms_helper_poll_enable(drm);
pm_runtime_set_active(dev);
return 0;
}
static int __maybe_unused malidp_pm_suspend(struct device *dev)
{
struct drm_device *drm = dev_get_drvdata(dev);
struct malidp_drm *malidp = drm->dev_private;
drm_kms_helper_poll_disable(drm);
console_lock();
drm_fbdev_cma_set_suspend(malidp->fbdev, 1);
console_unlock();
malidp->pm_state = drm_atomic_helper_suspend(drm);
if (IS_ERR(malidp->pm_state)) {
console_lock();
drm_fbdev_cma_set_suspend(malidp->fbdev, 0);
console_unlock();
drm_kms_helper_poll_enable(drm);
return PTR_ERR(malidp->pm_state);
}
return 0;
}
static int __maybe_unused malidp_pm_resume(struct device *dev)
{
struct drm_device *drm = dev_get_drvdata(dev);
struct malidp_drm *malidp = drm->dev_private;
drm_atomic_helper_resume(drm, malidp->pm_state);
console_lock();
drm_fbdev_cma_set_suspend(malidp->fbdev, 0);
console_unlock();
drm_kms_helper_poll_enable(drm);
return 0;
}
static int atmel_hlcdc_dc_drm_suspend(struct device *dev)
{
struct drm_device *drm_dev = dev_get_drvdata(dev);
struct atmel_hlcdc_dc *dc = drm_dev->dev_private;
struct regmap *regmap = dc->hlcdc->regmap;
struct drm_atomic_state *state;
state = drm_atomic_helper_suspend(drm_dev);
if (IS_ERR(state))
return PTR_ERR(state);
dc->suspend.state = state;
regmap_read(regmap, ATMEL_HLCDC_IMR, &dc->suspend.imr);
regmap_write(regmap, ATMEL_HLCDC_IDR, dc->suspend.imr);
clk_disable_unprepare(dc->hlcdc->periph_clk);
return 0;
}
static int atmel_hlcdc_dc_drm_resume(struct device *dev)
{
struct drm_device *drm_dev = dev_get_drvdata(dev);
struct atmel_hlcdc_dc *dc = drm_dev->dev_private;
clk_prepare_enable(dc->hlcdc->periph_clk);
regmap_write(dc->hlcdc->regmap, ATMEL_HLCDC_IER, dc->suspend.imr);
return drm_atomic_helper_resume(drm_dev, dc->suspend.state);
}
static int exynos_drm_suspend(struct device *dev)
{
struct drm_device *drm_dev = dev_get_drvdata(dev);
struct exynos_drm_private *private = drm_dev->dev_private;
if (pm_runtime_suspended(dev) || !drm_dev)
return 0;
drm_kms_helper_poll_disable(drm_dev);
exynos_drm_fbdev_suspend(drm_dev);
private->suspend_state = drm_atomic_helper_suspend(drm_dev);
if (IS_ERR(private->suspend_state)) {
exynos_drm_fbdev_resume(drm_dev);
drm_kms_helper_poll_enable(drm_dev);
return PTR_ERR(private->suspend_state);
}
return 0;
}
static int exynos_drm_resume(struct device *dev)
{
struct drm_device *drm_dev = dev_get_drvdata(dev);
struct exynos_drm_private *private = drm_dev->dev_private;
if (pm_runtime_suspended(dev) || !drm_dev)
return 0;
drm_atomic_helper_resume(drm_dev, private->suspend_state);
exynos_drm_fbdev_resume(drm_dev);
drm_kms_helper_poll_enable(drm_dev);
return 0;
}
static int fsl_dcu_drm_pm_suspend(struct device *dev)
{
struct fsl_dcu_drm_device *fsl_dev = dev_get_drvdata(dev);
if (!fsl_dev)
return 0;
disable_irq(fsl_dev->irq);
drm_kms_helper_poll_disable(fsl_dev->drm);
console_lock();
drm_fbdev_cma_set_suspend(fsl_dev->fbdev, 1);
console_unlock();
fsl_dev->state = drm_atomic_helper_suspend(fsl_dev->drm);
if (IS_ERR(fsl_dev->state)) {
console_lock();
drm_fbdev_cma_set_suspend(fsl_dev->fbdev, 0);
console_unlock();
drm_kms_helper_poll_enable(fsl_dev->drm);
enable_irq(fsl_dev->irq);
return PTR_ERR(fsl_dev->state);
}
clk_disable_unprepare(fsl_dev->pix_clk);
clk_disable_unprepare(fsl_dev->clk);
return 0;
}
static int fsl_dcu_drm_pm_resume(struct device *dev)
{
struct fsl_dcu_drm_device *fsl_dev = dev_get_drvdata(dev);
int ret;
if (!fsl_dev)
return 0;
ret = clk_prepare_enable(fsl_dev->clk);
if (ret < 0) {
dev_err(dev, "failed to enable dcu clk\n");
return ret;
}
if (fsl_dev->tcon)
fsl_tcon_bypass_enable(fsl_dev->tcon);
fsl_dcu_drm_init_planes(fsl_dev->drm);
drm_atomic_helper_resume(fsl_dev->drm, fsl_dev->state);
console_lock();
drm_fbdev_cma_set_suspend(fsl_dev->fbdev, 0);
console_unlock();
drm_kms_helper_poll_enable(fsl_dev->drm);
enable_irq(fsl_dev->irq);
return 0;
}
static int __maybe_unused hibmc_pm_suspend(struct device *dev)
{
struct pci_dev *pdev = to_pci_dev(dev);
struct drm_device *drm_dev = pci_get_drvdata(pdev);
struct hibmc_drm_private *priv = drm_dev->dev_private;
drm_kms_helper_poll_disable(drm_dev);
priv->suspend_state = drm_atomic_helper_suspend(drm_dev);
if (IS_ERR(priv->suspend_state)) {
DRM_ERROR("drm_atomic_helper_suspend failed: %ld\n",
PTR_ERR(priv->suspend_state));
drm_kms_helper_poll_enable(drm_dev);
return PTR_ERR(priv->suspend_state);
}
return 0;
}
static int __maybe_unused hibmc_pm_resume(struct device *dev)
{
struct pci_dev *pdev = to_pci_dev(dev);
struct drm_device *drm_dev = pci_get_drvdata(pdev);
struct hibmc_drm_private *priv = drm_dev->dev_private;
drm_atomic_helper_resume(drm_dev, priv->suspend_state);
drm_kms_helper_poll_enable(drm_dev);
return 0;
}
i915 is excluded since it's rather complicated.
static int imx_drm_suspend(struct device *dev)
{
struct drm_device *drm_dev = dev_get_drvdata(dev);
struct imx_drm_device *imxdrm;
/* The drm_dev is NULL before .load hook is called */
if (drm_dev == NULL)
return 0;
drm_kms_helper_poll_disable(drm_dev);
imxdrm = drm_dev->dev_private;
imxdrm->state = drm_atomic_helper_suspend(drm_dev);
if (IS_ERR(imxdrm->state)) {
drm_kms_helper_poll_enable(drm_dev);
return PTR_ERR(imxdrm->state);
}
return 0;
}
static int imx_drm_resume(struct device *dev)
{
struct drm_device *drm_dev = dev_get_drvdata(dev);
struct imx_drm_device *imx_drm;
if (drm_dev == NULL)
return 0;
imx_drm = drm_dev->dev_private;
drm_atomic_helper_resume(drm_dev, imx_drm->state);
drm_kms_helper_poll_enable(drm_dev);
return 0;
}
static int mtk_drm_sys_suspend(struct device *dev)
{
struct mtk_drm_private *private = dev_get_drvdata(dev);
struct drm_device *drm = private->drm;
drm_kms_helper_poll_disable(drm);
private->suspend_state = drm_atomic_helper_suspend(drm);
if (IS_ERR(private->suspend_state)) {
drm_kms_helper_poll_enable(drm);
return PTR_ERR(private->suspend_state);
}
DRM_DEBUG_DRIVER("mtk_drm_sys_suspend\n");
return 0;
}
static int mtk_drm_sys_resume(struct device *dev)
{
struct mtk_drm_private *private = dev_get_drvdata(dev);
struct drm_device *drm = private->drm;
drm_atomic_helper_resume(drm, private->suspend_state);
drm_kms_helper_poll_enable(drm);
DRM_DEBUG_DRIVER("mtk_drm_sys_resume\n");
return 0;
}
nouveau is also rather complicated.
static int rockchip_drm_sys_suspend(struct device *dev)
{
struct drm_device *drm = dev_get_drvdata(dev);
struct rockchip_drm_private *priv;
if (!drm)
return 0;
drm_kms_helper_poll_disable(drm);
rockchip_drm_fb_suspend(drm);
priv = drm->dev_private;
priv->state = drm_atomic_helper_suspend(drm);
if (IS_ERR(priv->state)) {
rockchip_drm_fb_resume(drm);
drm_kms_helper_poll_enable(drm);
return PTR_ERR(priv->state);
}
return 0;
}
static int rockchip_drm_sys_resume(struct device *dev)
{
struct drm_device *drm = dev_get_drvdata(dev);
struct rockchip_drm_private *priv;
if (!drm)
return 0;
priv = drm->dev_private;
drm_atomic_helper_resume(drm, priv->state);
rockchip_drm_fb_resume(drm);
drm_kms_helper_poll_enable(drm);
return 0;
}
static int host1x_drm_suspend(struct device *dev)
{
struct drm_device *drm = dev_get_drvdata(dev);
struct tegra_drm *tegra = drm->dev_private;
drm_kms_helper_poll_disable(drm);
tegra_drm_fb_suspend(drm);
tegra->state = drm_atomic_helper_suspend(drm);
if (IS_ERR(tegra->state)) {
tegra_drm_fb_resume(drm);
drm_kms_helper_poll_enable(drm);
return PTR_ERR(tegra->state);
}
return 0;
}
static int host1x_drm_resume(struct device *dev)
{
struct drm_device *drm = dev_get_drvdata(dev);
struct tegra_drm *tegra = drm->dev_private;
drm_atomic_helper_resume(drm, tegra->state);
tegra_drm_fb_resume(drm);
drm_kms_helper_poll_enable(drm);
return 0;
}
static int tilcdc_pm_suspend(struct device *dev)
{
struct drm_device *ddev = dev_get_drvdata(dev);
struct tilcdc_drm_private *priv = ddev->dev_private;
priv->saved_state = drm_atomic_helper_suspend(ddev);
/* Select sleep pin state */
pinctrl_pm_select_sleep_state(dev);
return 0;
}
static int tilcdc_pm_resume(struct device *dev)
{
struct drm_device *ddev = dev_get_drvdata(dev);
struct tilcdc_drm_private *priv = ddev->dev_private;
int ret = 0;
/* Select default pin state */
pinctrl_pm_select_default_state(dev);
if (priv->saved_state)
ret = drm_atomic_helper_resume(ddev, priv->saved_state);
return ret;
}
int tinydrm_suspend(struct tinydrm_device *tdev)
{
struct drm_atomic_state *state;
if (tdev->suspend_state) {
DRM_ERROR("Failed to suspend: state already set\n");
return -EINVAL;
}
drm_fbdev_cma_set_suspend_unlocked(tdev->fbdev_cma, 1);
state = drm_atomic_helper_suspend(tdev->drm);
if (IS_ERR(state)) {
drm_fbdev_cma_set_suspend_unlocked(tdev->fbdev_cma, 0);
return PTR_ERR(state);
}
tdev->suspend_state = state;
return 0;
}
int tinydrm_resume(struct tinydrm_device *tdev)
{
struct drm_atomic_state *state = tdev->suspend_state;
int ret;
if (!state) {
DRM_ERROR("Failed to resume: state is not set\n");
return -EINVAL;
}
tdev->suspend_state = NULL;
ret = drm_atomic_helper_resume(tdev->drm, state);
if (ret) {
DRM_ERROR("Error resuming state: %d\n", ret);
return ret;
}
drm_fbdev_cma_set_suspend_unlocked(tdev->fbdev_cma, 0);
return 0;
}
Noralf.
_______________________________________________
dri-devel mailing list
dri-devel@xxxxxxxxxxxxxxxxxxxxx
https://lists.freedesktop.org/mailman/listinfo/dri-devel
