Commit a95acec1 authored by Liviu Dudau's avatar Liviu Dudau

drm: hdlcd: Revamp runtime power management

Because the HDLCD driver acts as a component master it can end
up enabling the runtime PM functionality before the encoders
are initialised. This can cause crashes if the component slave
never probes (missing module) or if the PM operations kick in
before the probe finishes.

Move the enabling of the runtime PM after the component master
has finished collecting the slave components and use the DRM
atomic helpers to suspend and resume the device.
Tested-by: default avatarRobin Murphy <Robin.Murphy@arm.com>
Signed-off-by: default avatarLiviu Dudau <Liviu.Dudau@arm.com>
parent 1a695a90
...@@ -33,8 +33,17 @@ ...@@ -33,8 +33,17 @@
* *
*/ */
static void hdlcd_crtc_cleanup(struct drm_crtc *crtc)
{
struct hdlcd_drm_private *hdlcd = crtc_to_hdlcd_priv(crtc);
/* stop the controller on cleanup */
hdlcd_write(hdlcd, HDLCD_REG_COMMAND, 0);
drm_crtc_cleanup(crtc);
}
static const struct drm_crtc_funcs hdlcd_crtc_funcs = { static const struct drm_crtc_funcs hdlcd_crtc_funcs = {
.destroy = drm_crtc_cleanup, .destroy = hdlcd_crtc_cleanup,
.set_config = drm_atomic_helper_set_config, .set_config = drm_atomic_helper_set_config,
.page_flip = drm_atomic_helper_page_flip, .page_flip = drm_atomic_helper_page_flip,
.reset = drm_atomic_helper_crtc_reset, .reset = drm_atomic_helper_crtc_reset,
...@@ -155,8 +164,8 @@ static void hdlcd_crtc_disable(struct drm_crtc *crtc) ...@@ -155,8 +164,8 @@ static void hdlcd_crtc_disable(struct drm_crtc *crtc)
if (!crtc->primary->fb) if (!crtc->primary->fb)
return; return;
clk_disable_unprepare(hdlcd->clk);
hdlcd_write(hdlcd, HDLCD_REG_COMMAND, 0); hdlcd_write(hdlcd, HDLCD_REG_COMMAND, 0);
clk_disable_unprepare(hdlcd->clk);
drm_crtc_vblank_off(crtc); drm_crtc_vblank_off(crtc);
} }
...@@ -294,16 +303,6 @@ static struct drm_plane *hdlcd_plane_init(struct drm_device *drm) ...@@ -294,16 +303,6 @@ static struct drm_plane *hdlcd_plane_init(struct drm_device *drm)
return plane; return plane;
} }
void hdlcd_crtc_suspend(struct drm_crtc *crtc)
{
hdlcd_crtc_disable(crtc);
}
void hdlcd_crtc_resume(struct drm_crtc *crtc)
{
hdlcd_crtc_enable(crtc);
}
int hdlcd_setup_crtc(struct drm_device *drm) int hdlcd_setup_crtc(struct drm_device *drm)
{ {
struct hdlcd_drm_private *hdlcd = drm->dev_private; struct hdlcd_drm_private *hdlcd = drm->dev_private;
......
...@@ -84,11 +84,7 @@ static int hdlcd_load(struct drm_device *drm, unsigned long flags) ...@@ -84,11 +84,7 @@ static int hdlcd_load(struct drm_device *drm, unsigned long flags)
goto setup_fail; goto setup_fail;
} }
pm_runtime_enable(drm->dev);
pm_runtime_get_sync(drm->dev);
ret = drm_irq_install(drm, platform_get_irq(pdev, 0)); ret = drm_irq_install(drm, platform_get_irq(pdev, 0));
pm_runtime_put_sync(drm->dev);
if (ret < 0) { if (ret < 0) {
DRM_ERROR("failed to install IRQ handler\n"); DRM_ERROR("failed to install IRQ handler\n");
goto irq_fail; goto irq_fail;
...@@ -357,6 +353,8 @@ static int hdlcd_drm_bind(struct device *dev) ...@@ -357,6 +353,8 @@ static int hdlcd_drm_bind(struct device *dev)
return -ENOMEM; return -ENOMEM;
drm->dev_private = hdlcd; drm->dev_private = hdlcd;
dev_set_drvdata(dev, drm);
hdlcd_setup_mode_config(drm); hdlcd_setup_mode_config(drm);
ret = hdlcd_load(drm, 0); ret = hdlcd_load(drm, 0);
if (ret) if (ret)
...@@ -366,14 +364,18 @@ static int hdlcd_drm_bind(struct device *dev) ...@@ -366,14 +364,18 @@ static int hdlcd_drm_bind(struct device *dev)
if (ret) if (ret)
goto err_unload; goto err_unload;
dev_set_drvdata(dev, drm);
ret = component_bind_all(dev, drm); ret = component_bind_all(dev, drm);
if (ret) { if (ret) {
DRM_ERROR("Failed to bind all components\n"); DRM_ERROR("Failed to bind all components\n");
goto err_unregister; goto err_unregister;
} }
ret = pm_runtime_set_active(dev);
if (ret)
goto err_pm_active;
pm_runtime_enable(dev);
ret = drm_vblank_init(drm, drm->mode_config.num_crtc); ret = drm_vblank_init(drm, drm->mode_config.num_crtc);
if (ret < 0) { if (ret < 0) {
DRM_ERROR("failed to initialise vblank\n"); DRM_ERROR("failed to initialise vblank\n");
...@@ -399,16 +401,16 @@ static int hdlcd_drm_bind(struct device *dev) ...@@ -399,16 +401,16 @@ static int hdlcd_drm_bind(struct device *dev)
drm_mode_config_cleanup(drm); drm_mode_config_cleanup(drm);
drm_vblank_cleanup(drm); drm_vblank_cleanup(drm);
err_vblank: err_vblank:
pm_runtime_disable(drm->dev);
err_pm_active:
component_unbind_all(dev, drm); component_unbind_all(dev, drm);
err_unregister: err_unregister:
drm_dev_unregister(drm); drm_dev_unregister(drm);
err_unload: err_unload:
pm_runtime_get_sync(drm->dev);
drm_irq_uninstall(drm); drm_irq_uninstall(drm);
pm_runtime_put_sync(drm->dev);
pm_runtime_disable(drm->dev);
of_reserved_mem_device_release(drm->dev); of_reserved_mem_device_release(drm->dev);
err_free: err_free:
dev_set_drvdata(dev, NULL);
drm_dev_unref(drm); drm_dev_unref(drm);
return ret; return ret;
...@@ -495,30 +497,34 @@ MODULE_DEVICE_TABLE(of, hdlcd_of_match); ...@@ -495,30 +497,34 @@ MODULE_DEVICE_TABLE(of, hdlcd_of_match);
static int __maybe_unused hdlcd_pm_suspend(struct device *dev) static int __maybe_unused hdlcd_pm_suspend(struct device *dev)
{ {
struct drm_device *drm = dev_get_drvdata(dev); struct drm_device *drm = dev_get_drvdata(dev);
struct drm_crtc *crtc; struct hdlcd_drm_private *hdlcd = drm ? drm->dev_private : NULL;
if (pm_runtime_suspended(dev)) if (!hdlcd)
return 0; return 0;
drm_modeset_lock_all(drm); drm_kms_helper_poll_disable(drm);
list_for_each_entry(crtc, &drm->mode_config.crtc_list, head)
hdlcd_crtc_suspend(crtc); hdlcd->state = drm_atomic_helper_suspend(drm);
drm_modeset_unlock_all(drm); if (IS_ERR(hdlcd->state)) {
drm_kms_helper_poll_enable(drm);
return PTR_ERR(hdlcd->state);
}
return 0; return 0;
} }
static int __maybe_unused hdlcd_pm_resume(struct device *dev) static int __maybe_unused hdlcd_pm_resume(struct device *dev)
{ {
struct drm_device *drm = dev_get_drvdata(dev); struct drm_device *drm = dev_get_drvdata(dev);
struct drm_crtc *crtc; struct hdlcd_drm_private *hdlcd = drm ? drm->dev_private : NULL;
if (!pm_runtime_suspended(dev)) if (!hdlcd)
return 0; return 0;
drm_modeset_lock_all(drm); drm_atomic_helper_resume(drm, hdlcd->state);
list_for_each_entry(crtc, &drm->mode_config.crtc_list, head) drm_kms_helper_poll_enable(drm);
hdlcd_crtc_resume(crtc); pm_runtime_set_active(dev);
drm_modeset_unlock_all(drm);
return 0; return 0;
} }
......
...@@ -13,6 +13,7 @@ struct hdlcd_drm_private { ...@@ -13,6 +13,7 @@ struct hdlcd_drm_private {
struct list_head event_list; struct list_head event_list;
struct drm_crtc crtc; struct drm_crtc crtc;
struct drm_plane *plane; struct drm_plane *plane;
struct drm_atomic_state *state;
#ifdef CONFIG_DEBUG_FS #ifdef CONFIG_DEBUG_FS
atomic_t buffer_underrun_count; atomic_t buffer_underrun_count;
atomic_t bus_error_count; atomic_t bus_error_count;
...@@ -36,7 +37,5 @@ static inline u32 hdlcd_read(struct hdlcd_drm_private *hdlcd, unsigned int reg) ...@@ -36,7 +37,5 @@ static inline u32 hdlcd_read(struct hdlcd_drm_private *hdlcd, unsigned int reg)
int hdlcd_setup_crtc(struct drm_device *dev); int hdlcd_setup_crtc(struct drm_device *dev);
void hdlcd_set_scanout(struct hdlcd_drm_private *hdlcd); void hdlcd_set_scanout(struct hdlcd_drm_private *hdlcd);
void hdlcd_crtc_suspend(struct drm_crtc *crtc);
void hdlcd_crtc_resume(struct drm_crtc *crtc);
#endif /* __HDLCD_DRV_H__ */ #endif /* __HDLCD_DRV_H__ */
Markdown is supported
0%
or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment