Re: [PATCH 04/29] drm/omap: Use atomic suspend/resume helpers

[Laurent Pinchart <laurent.pinchart@ideasonboard.com>]

Hi Sebastian,

On Sunday, 9 December 2018 23:53:09 EET Sebastian Reichel wrote:
> Hi,
> 
> On Wed, Dec 05, 2018 at 04:59:57PM +0200, Laurent Pinchart wrote:
> > Instead of rolling out custom suspend/resume implementations based on
> > state information stored in the driver's data structures, use the atomic
> > suspend/resume helpers that rely on a DRM atomic state object.
> > 
> > Signed-off-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>
> > ---
> 
> Documentation/gpu/todo.rst says, that this should be converted to
> drm_mode_config_helper_suspend, but at least it's no longer open
> coded after your change.

Good point. I'll fix this in the next version. Thank you for pointing it out.

> Reviewed-by: Sebastian Reichel <sebastian.reichel@collabora.com>
> 
> >  drivers/gpu/drm/omapdrm/dss/omapdss.h |  3 --
> >  drivers/gpu/drm/omapdrm/omap_drv.c    | 52 +++++----------------------
> >  drivers/gpu/drm/omapdrm/omap_drv.h    |  2 ++
> >  3 files changed, 11 insertions(+), 46 deletions(-)
> > 
> > diff --git a/drivers/gpu/drm/omapdrm/dss/omapdss.h
> > b/drivers/gpu/drm/omapdrm/dss/omapdss.h index d3b576e6edf5..149a0f09adbc
> > 100644
> > --- a/drivers/gpu/drm/omapdrm/dss/omapdss.h
> > +++ b/drivers/gpu/drm/omapdrm/dss/omapdss.h
> > @@ -432,9 +432,6 @@ struct omap_dss_device {
> > 
> >  	unsigned long ops_flags;
> >  	unsigned long bus_flags;
> > 
> > -	/* helper variable for driver suspend/resume */
> > -	bool activate_after_resume;
> > -
> > 
> >  	enum omap_display_caps caps;
> >  	
> >  	enum omap_dss_display_state state;
> > 
> > diff --git a/drivers/gpu/drm/omapdrm/omap_drv.c
> > b/drivers/gpu/drm/omapdrm/omap_drv.c index 5e67d58cbc28..66c4cedb5539
> > 100644
> > --- a/drivers/gpu/drm/omapdrm/omap_drv.c
> > +++ b/drivers/gpu/drm/omapdrm/omap_drv.c
> > @@ -685,52 +685,21 @@ static int pdev_remove(struct platform_device *pdev)
> > 
> >  }
> >  
> >  #ifdef CONFIG_PM_SLEEP
> > 
> > -static int omap_drm_suspend_all_displays(struct drm_device *ddev)
> > -{
> > -	struct omap_drm_private *priv = ddev->dev_private;
> > -	int i;
> > -
> > -	for (i = 0; i < priv->num_pipes; i++) {
> > -		struct omap_dss_device *display = priv->pipes[i].display;
> > -
> > -		if (display->state == OMAP_DSS_DISPLAY_ACTIVE) {
> > -			display->ops->disable(display);
> > -			display->activate_after_resume = true;
> > -		} else {
> > -			display->activate_after_resume = false;
> > -		}
> > -	}
> > -
> > -	return 0;
> > -}
> > -
> > -static int omap_drm_resume_all_displays(struct drm_device *ddev)
> > -{
> > -	struct omap_drm_private *priv = ddev->dev_private;
> > -	int i;
> > -
> > -	for (i = 0; i < priv->num_pipes; i++) {
> > -		struct omap_dss_device *display = priv->pipes[i].display;
> > -
> > -		if (display->activate_after_resume) {
> > -			display->ops->enable(display);
> > -			display->activate_after_resume = false;
> > -		}
> > -	}
> > -
> > -	return 0;
> > -}
> > -
> > 
> >  static int omap_drm_suspend(struct device *dev)
> >  {
> >  
> >  	struct omap_drm_private *priv = dev_get_drvdata(dev);
> >  	struct drm_device *drm_dev = priv->ddev;
> > 
> > +	struct drm_atomic_state *state;
> > 
> >  	drm_kms_helper_poll_disable(drm_dev);
> > 
> > -	drm_modeset_lock_all(drm_dev);
> > -	omap_drm_suspend_all_displays(drm_dev);
> > -	drm_modeset_unlock_all(drm_dev);
> > +	state = drm_atomic_helper_suspend(drm_dev);
> > +	if (IS_ERR(state)) {
> > +		drm_kms_helper_poll_enable(drm_dev);
> > +		return PTR_ERR(state);
> > +	}
> > +
> > +	priv->suspend_state = state;
> > 
> >  	return 0;
> >  
> >  }
> > 
> > @@ -740,10 +709,7 @@ static int omap_drm_resume(struct device *dev)
> > 
> >  	struct omap_drm_private *priv = dev_get_drvdata(dev);
> >  	struct drm_device *drm_dev = priv->ddev;
> > 
> > -	drm_modeset_lock_all(drm_dev);
> > -	omap_drm_resume_all_displays(drm_dev);
> > -	drm_modeset_unlock_all(drm_dev);
> > -
> > +	drm_atomic_helper_resume(drm_dev, priv->suspend_state);
> > 
> >  	drm_kms_helper_poll_enable(drm_dev);
> >  	
> >  	return omap_gem_resume(drm_dev);
> > 
> > diff --git a/drivers/gpu/drm/omapdrm/omap_drv.h
> > b/drivers/gpu/drm/omapdrm/omap_drv.h index bd7f2c227a25..788aa9f7e6df
> > 100644
> > --- a/drivers/gpu/drm/omapdrm/omap_drv.h
> > +++ b/drivers/gpu/drm/omapdrm/omap_drv.h
> > @@ -73,6 +73,8 @@ struct omap_drm_private {
> > 
> >  	struct workqueue_struct *wq;
> > 
> > +	struct drm_atomic_state *suspend_state;
> > +
> > 
> >  	/* lock for obj_list below */
> >  	struct mutex list_lock;

-- 
Regards,

Laurent Pinchart



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