On Tue, Sep 02, 2025 at 05:17:24PM +0300, Dmitry Baryshkov wrote: > On Tue, Sep 02, 2025 at 05:14:51PM +0300, Ville Syrjälä wrote: > > On Tue, Sep 02, 2025 at 04:25:12PM +0300, Dmitry Baryshkov wrote: > > > On Tue, Sep 02, 2025 at 11:35:35AM +0200, Maxime Ripard wrote: > > > > The drm_atomic_get_private_obj_state() function tries to find if a > > > > private_obj had already been allocated and was part of the given > > > > drm_atomic_state. If one is found, it returns the existing state > > > > pointer. > > > > > > > > At the point in time where drm_atomic_get_private_obj_state() can be > > > > called (ie, during atomic_check), the existing state is the new state > > > > and we can thus replace the hand-crafted logic by a call to > > > > drm_atomic_get_new_private_obj_state(). > > > > > > > > > This function is being used in e.g. this call stack: > > > ingenic_drm_crtc_atomic_enable -> ingenic_drm_get_priv_state -> > > > drm_atomic_get_private_obj_state(). Please correct me if I'm wrong, > > > doesn't it happen already after the state switch? > > > > Looks like it should just use the get_new_state() there. > > > > Hmm, I wonder if we should make the get_state() functions warn > > (and maybe return NULL) if they get called after the state has > > been swapped? > > Might be. > > Note: I just quickly git grepped the function and stumbled upon the > first questionable use. There might be uther users which call it after > swapping the state.
I tried to have a quick look at all the callers and didn't spot other obviously wrong uses, but I can't guarantee that I didn't miss some by accident. > > > > > > > > > > > > > > Signed-off-by: Maxime Ripard <mrip...@kernel.org> > > > > --- > > > > drivers/gpu/drm/drm_atomic.c | 8 ++++---- > > > > 1 file changed, 4 insertions(+), 4 deletions(-) > > > > > > > > > > -- > > > With best wishes > > > Dmitry > > > > -- > > Ville Syrjälä > > Intel > > -- > With best wishes > Dmitry -- Ville Syrjälä Intel
