From mboxrd@z Thu Jan 1 00:00:00 1970 From: Laurent Pinchart Subject: Re: [Patch 1/1] drm/atomic: integrate private objects with suspend/resume helpers Date: Thu, 14 Mar 2019 20:19:07 +0200 Message-ID: <20190314181907.GH5455@pendragon.ideasonboard.com> References: <20190314134445.19260-1-bparrot@ti.com> Mime-Version: 1.0 Content-Type: text/plain; charset=utf-8 Return-path: Content-Disposition: inline In-Reply-To: <20190314134445.19260-1-bparrot@ti.com> Sender: linux-kernel-owner@vger.kernel.org To: Benoit Parrot Cc: Daniel Vetter , Rob Clark , dri-devel@lists.freedesktop.org, linux-kernel@vger.kernel.org, Boris Brezillon , Tomi Valkeinen , Peter Ujfalusi , Jyri Sarha List-Id: dri-devel@lists.freedesktop.org Hi Benoit, Thank you for the patch. On Thu, Mar 14, 2019 at 08:44:45AM -0500, Benoit Parrot wrote: > During a suspend cycle the atomic state is saved to be used during the > restore cycle. > > However the current state duplication logic does not duplicate private > objects. This leads to state inconsistencies at resume time. > > With private objects modeset lock now integrated, we can make sure that > private object state are properly saved and restored. > > Signed-off-by: Benoit Parrot This looks good to me. I was actually wondering if private object state was properly handled by the suspend/resume helpers no later than yesterday. Seems you read my mind :-) Reviewed-by: Laurent Pinchart > --- > drivers/gpu/drm/drm_atomic_helper.c | 16 ++++++++++++++++ > 1 file changed, 16 insertions(+) > > diff --git a/drivers/gpu/drm/drm_atomic_helper.c b/drivers/gpu/drm/drm_atomic_helper.c > index 540a77a2ade9..b108021cc092 100644 > --- a/drivers/gpu/drm/drm_atomic_helper.c > +++ b/drivers/gpu/drm/drm_atomic_helper.c > @@ -3189,6 +3189,7 @@ drm_atomic_helper_duplicate_state(struct drm_device *dev, > struct drm_connector_list_iter conn_iter; > struct drm_plane *plane; > struct drm_crtc *crtc; > + struct drm_private_obj *privobj; > int err = 0; > > state = drm_atomic_state_alloc(dev); > @@ -3218,6 +3219,16 @@ drm_atomic_helper_duplicate_state(struct drm_device *dev, > } > } > > + drm_for_each_privobj(privobj, dev) { > + struct drm_private_state *priv_state; > + > + priv_state = drm_atomic_get_private_obj_state(state, privobj); > + if (IS_ERR(priv_state)) { > + err = PTR_ERR(priv_state); > + goto free; > + } > + } > + > drm_connector_list_iter_begin(dev, &conn_iter); > drm_for_each_connector_iter(conn, &conn_iter) { > struct drm_connector_state *conn_state; > @@ -3325,12 +3336,17 @@ int drm_atomic_helper_commit_duplicated_state(struct drm_atomic_state *state, > struct drm_connector_state *new_conn_state; > struct drm_crtc *crtc; > struct drm_crtc_state *new_crtc_state; > + struct drm_private_obj *privobj; > + struct drm_private_state *new_priv_state; > > state->acquire_ctx = ctx; > > for_each_new_plane_in_state(state, plane, new_plane_state, i) > state->planes[i].old_state = plane->state; > > + for_each_new_private_obj_in_state(state, privobj, new_priv_state, i) > + state->private_objs[i].old_state = privobj->state; > + > for_each_new_crtc_in_state(state, crtc, new_crtc_state, i) > state->crtcs[i].old_state = crtc->state; > -- Regards, Laurent Pinchart