| Message ID | 199d29db89a7953f59e1eb4e91a3421336e3ed2a.1505493461.git-series.kieran.bingham+renesas@ideasonboard.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
Hi Laurent, Thanks for your speedy review! On 15/09/17 18:02, Laurent Pinchart wrote: > Hi Kieran, > > Thank you for the patch. > > On Friday, 15 September 2017 19:42:06 EEST Kieran Bingham wrote: >> The pipeline needs to ensure that the hardware is idle for suspend and >> resume operations. > > I'm not sure to really understand this sentence. It makes sense to me ... :) - But I'm not the (only) target audience. How about re-wording it in a similar way to your suggestion in [1/3] """ To support system suspend operations we must ensure the hardware is stopped, and resumed explicitly from the suspend and resume handlers. Implement suspend and resume functions using the DRM atomic helper functions. """ > >> Implement suspend and resume functions using the DRM atomic helper >> functions. >> >> CC: dri-devel@lists.freedesktop.org >> >> Signed-off-by: Kieran Bingham <kieran.bingham+renesas@ideasonboard.com> > > The rest of the patch looks good to me. With the commit message clarified, > > Reviewed-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com> > >> --- >> drivers/gpu/drm/rcar-du/rcar_du_drv.c | 18 +++++++++++++++--- >> drivers/gpu/drm/rcar-du/rcar_du_drv.h | 1 + >> 2 files changed, 16 insertions(+), 3 deletions(-) >> >> diff --git a/drivers/gpu/drm/rcar-du/rcar_du_drv.c >> b/drivers/gpu/drm/rcar-du/rcar_du_drv.c index 09fbceade6b1..01b91d0c169c >> 100644 >> --- a/drivers/gpu/drm/rcar-du/rcar_du_drv.c >> +++ b/drivers/gpu/drm/rcar-du/rcar_du_drv.c >> @@ -22,6 +22,7 @@ >> #include <linux/wait.h> >> >> #include <drm/drmP.h> >> +#include <drm/drm_atomic_helper.h> >> #include <drm/drm_crtc_helper.h> >> #include <drm/drm_fb_cma_helper.h> >> #include <drm/drm_gem_cma_helper.h> >> @@ -267,9 +268,19 @@ static struct drm_driver rcar_du_driver = { >> static int rcar_du_pm_suspend(struct device *dev) >> { >> struct rcar_du_device *rcdu = dev_get_drvdata(dev); >> + struct drm_atomic_state *state; >> >> drm_kms_helper_poll_disable(rcdu->ddev); >> - /* TODO Suspend the CRTC */ >> + drm_fbdev_cma_set_suspend_unlocked(rcdu->fbdev, true); >> + >> + state = drm_atomic_helper_suspend(rcdu->ddev); >> + if (IS_ERR(state)) { >> + drm_fbdev_cma_set_suspend_unlocked(rcdu->fbdev, false); >> + drm_kms_helper_poll_enable(rcdu->ddev); >> + return PTR_ERR(state); >> + } >> + >> + rcdu->suspend_state = state; >> >> return 0; >> } >> @@ -278,9 +289,10 @@ static int rcar_du_pm_resume(struct device *dev) >> { >> struct rcar_du_device *rcdu = dev_get_drvdata(dev); >> >> - /* TODO Resume the CRTC */ >> - >> + drm_atomic_helper_resume(rcdu->ddev, rcdu->suspend_state); >> + drm_fbdev_cma_set_suspend_unlocked(rcdu->fbdev, false); >> drm_kms_helper_poll_enable(rcdu->ddev); >> + >> return 0; >> } >> #endif >> diff --git a/drivers/gpu/drm/rcar-du/rcar_du_drv.h >> b/drivers/gpu/drm/rcar-du/rcar_du_drv.h index f8cd79488ece..f400fde65a0c >> 100644 >> --- a/drivers/gpu/drm/rcar-du/rcar_du_drv.h >> +++ b/drivers/gpu/drm/rcar-du/rcar_du_drv.h >> @@ -81,6 +81,7 @@ struct rcar_du_device { >> >> struct drm_device *ddev; >> struct drm_fbdev_cma *fbdev; >> + struct drm_atomic_state *suspend_state; >> >> struct rcar_du_crtc crtcs[RCAR_DU_MAX_CRTCS]; >> unsigned int num_crtcs; > >
diff --git a/drivers/gpu/drm/rcar-du/rcar_du_drv.c b/drivers/gpu/drm/rcar-du/rcar_du_drv.c index 09fbceade6b1..01b91d0c169c 100644 --- a/drivers/gpu/drm/rcar-du/rcar_du_drv.c +++ b/drivers/gpu/drm/rcar-du/rcar_du_drv.c @@ -22,6 +22,7 @@ #include <linux/wait.h> #include <drm/drmP.h> +#include <drm/drm_atomic_helper.h> #include <drm/drm_crtc_helper.h> #include <drm/drm_fb_cma_helper.h> #include <drm/drm_gem_cma_helper.h> @@ -267,9 +268,19 @@ static struct drm_driver rcar_du_driver = { static int rcar_du_pm_suspend(struct device *dev) { struct rcar_du_device *rcdu = dev_get_drvdata(dev); + struct drm_atomic_state *state; drm_kms_helper_poll_disable(rcdu->ddev); - /* TODO Suspend the CRTC */ + drm_fbdev_cma_set_suspend_unlocked(rcdu->fbdev, true); + + state = drm_atomic_helper_suspend(rcdu->ddev); + if (IS_ERR(state)) { + drm_fbdev_cma_set_suspend_unlocked(rcdu->fbdev, false); + drm_kms_helper_poll_enable(rcdu->ddev); + return PTR_ERR(state); + } + + rcdu->suspend_state = state; return 0; } @@ -278,9 +289,10 @@ static int rcar_du_pm_resume(struct device *dev) { struct rcar_du_device *rcdu = dev_get_drvdata(dev); - /* TODO Resume the CRTC */ - + drm_atomic_helper_resume(rcdu->ddev, rcdu->suspend_state); + drm_fbdev_cma_set_suspend_unlocked(rcdu->fbdev, false); drm_kms_helper_poll_enable(rcdu->ddev); + return 0; } #endif diff --git a/drivers/gpu/drm/rcar-du/rcar_du_drv.h b/drivers/gpu/drm/rcar-du/rcar_du_drv.h index f8cd79488ece..f400fde65a0c 100644 --- a/drivers/gpu/drm/rcar-du/rcar_du_drv.h +++ b/drivers/gpu/drm/rcar-du/rcar_du_drv.h @@ -81,6 +81,7 @@ struct rcar_du_device { struct drm_device *ddev; struct drm_fbdev_cma *fbdev; + struct drm_atomic_state *suspend_state; struct rcar_du_crtc crtcs[RCAR_DU_MAX_CRTCS]; unsigned int num_crtcs;
The pipeline needs to ensure that the hardware is idle for suspend and resume operations. Implement suspend and resume functions using the DRM atomic helper functions. CC: dri-devel@lists.freedesktop.org Signed-off-by: Kieran Bingham <kieran.bingham+renesas@ideasonboard.com> --- drivers/gpu/drm/rcar-du/rcar_du_drv.c | 18 +++++++++++++++--- drivers/gpu/drm/rcar-du/rcar_du_drv.h | 1 + 2 files changed, 16 insertions(+), 3 deletions(-)