Hi Am 22.09.22 um 18:12 schrieb Ruhl, Michael J:
-----Original Message----- From: dri-devel <dri-devel-bounces@xxxxxxxxxxxxxxxxxxxxx> On Behalf Of Thomas Zimmermann Sent: Thursday, September 22, 2022 9:10 AM To: javierm@xxxxxxxxxx; airlied@xxxxxxxx; daniel@xxxxxxxx Cc: Thomas Zimmermann <tzimmermann@xxxxxxx>; dri- devel@xxxxxxxxxxxxxxxxxxxxx Subject: [PATCH 2/5] drm/simpledrm: Use drm_atomic_get_new_plane_state() Lookup the plane's state in atomic_update with the helper drm_atomic_get_new_plane_state(). Also rename the helpers' state arguments. No functional changes. Signed-off-by: Thomas Zimmermann <tzimmermann@xxxxxxx> --- drivers/gpu/drm/tiny/simpledrm.c | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/drivers/gpu/drm/tiny/simpledrm.c b/drivers/gpu/drm/tiny/simpledrm.c index 51d01e34d5eb..14782a50f816 100644 --- a/drivers/gpu/drm/tiny/simpledrm.c +++ b/drivers/gpu/drm/tiny/simpledrm.c @@ -470,10 +470,10 @@ static const uint64_t simpledrm_primary_plane_format_modifiers[] = { }; static void simpledrm_primary_plane_helper_atomic_update(struct drm_plane *plane, - struct drm_atomic_state *old_state) + struct drm_atomic_state *state) { - struct drm_plane_state *plane_state = plane->state; - struct drm_plane_state *old_plane_state = drm_atomic_get_old_plane_state(old_state, plane); + struct drm_plane_state *plane_state = drm_atomic_get_new_plane_state(state, plane);Going from plane->state to drm_atomic_get_new_plane_state seems like a slight function change. If this is the equivalent and the "right" way to do this, should the ->state part of the data structure be pruned?
The plane->state field is still relevant. I recently learned that the state-lookup helpers are supposed to be used in all atomic_check/commit functions. Ville gave a description here:
https://lore.kernel.org/dri-devel/Yx9pij4LmFHrq81V@xxxxxxxxx/
The comment for drm_atomic_get_new_plane_state also says that it can return NULL. would plane->state be NULL in this case?
I don't think so. In atomic_update, we know that we're supposed to change the plane. That requires the state. Maybe it's different in the plane's atomic_disable helper, we don't access the state there.
Best regards Thomas
Thanks, M+ struct drm_plane_state *old_plane_state = drm_atomic_get_old_plane_state(state, plane); struct drm_shadow_plane_state *shadow_plane_state = to_drm_shadow_plane_state(plane_state); struct drm_framebuffer *fb = plane_state->fb; struct drm_device *dev = plane->dev; @@ -503,7 +503,7 @@ static void simpledrm_primary_plane_helper_atomic_update(struct drm_plane *plane } static void simpledrm_primary_plane_helper_atomic_disable(struct drm_plane *plane, - struct drm_atomic_state *old_state) + struct drm_atomic_state *state) { struct drm_device *dev = plane->dev; struct simpledrm_device *sdev = simpledrm_device_of_dev(dev); -- 2.37.3
-- Thomas Zimmermann Graphics Driver Developer SUSE Software Solutions Germany GmbH Maxfeldstr. 5, 90409 Nürnberg, Germany (HRB 36809, AG Nürnberg) Geschäftsführer: Ivo Totev
Attachment:
OpenPGP_signature
Description: OpenPGP digital signature
