Re: [PATCH 10/29] drm/atomic: Add atomic_state_readout infrastructure

[Simona Vetter <simona.vetter@ffwll.ch>]

On Tue, Sep 23, 2025 at 01:30:43PM +0300, Dmitry Baryshkov wrote:
> On Tue, Sep 23, 2025 at 11:45:49AM +0200, Maxime Ripard wrote:
> > On Mon, Sep 15, 2025 at 09:40:57PM +0300, Dmitry Baryshkov wrote:
> > > On Tue, Sep 02, 2025 at 10:32:38AM +0200, Maxime Ripard wrote:
> > > > In order to enable drivers to fill their initial state from the hardware
> > > > state, we need to provide an alternative atomic_reset helper.
> > > > 
> > > > This helper relies on each state having its own atomic_state_readout()
> > > > hooks. Each component will thus be able to fill the initial state based
> > > > on what they can figure out from the hardware.
> > > > 
> > > > It also allocates a dummy drm_atomic_state to glue the whole thing
> > > > together so atomic_state_readout implementations can still figure out
> > > > the state of other related entities.
> > > > 
> > > > Link: https://lore.kernel.org/dri-devel/CAKMK7uHtqHy_oz4W7F+hmp9iqp7W5Ra8CxPvJ=9BwmvfU-O0gg@mail.gmail.com/
> > > > Signed-off-by: Maxime Ripard <mripard@kernel.org>
> > > > ---
> > > >  drivers/gpu/drm/drm_atomic_helper.c | 382 ++++++++++++++++++++++++++++++++++++
> > > >  drivers/gpu/drm/drm_mode_config.c   |   1 +
> > > >  include/drm/drm_atomic_helper.h     |   1 +
> > > >  include/drm/drm_bridge.h            |  21 ++
> > > >  include/drm/drm_connector.h         |  26 +++
> > > >  include/drm/drm_crtc.h              |  19 ++
> > > >  include/drm/drm_plane.h             |  27 +++
> > > >  7 files changed, 477 insertions(+)
> > > > 
> > > > +	drm_for_each_encoder(encoder, dev) {
> > > > +		struct drm_connector_state *enc_conn_state;
> > > > +		struct drm_crtc_state *enc_crtc_state;
> > > > +		struct drm_bridge *bridge;
> > > > +
> > > > +		/*
> > > > +		 * It works a bit differently for bridges. Because they are
> > > > +		 * using a drm_private_state, and because
> > > > +		 * drm_atomic_private_obj_init() asks for its initial state when
> > > > +		 * initializing, instead of doing it later on through a reset
> > > > +		 * call like the other entities, we can't have reset xor
> > > > +		 * readout.
> > > 
> > > Would it make sense to unify the way the bridges / priv_obj handle the
> > > state with the rest of the object types?
> > 
> > I would be all for it, but I think this is pretty much the same
> > conversation we had in my recent bridge improvement series. Aren't
> > bridges not assumed to have atomic support and thus we can't really do
> > something better here?
> > 
> > Or should we move all bridges to be atomic?
> 
> I think I had something smaller on my mind: make drm_private_obj /
> drm_bride provide the initial state during the reset call, like all
> other entities.

Yeah something like that crossed my mind too.
-Sima
-- 
Simona Vetter
Software Engineer
http://blog.ffwll.ch