Re: [PATCH v2 15/20] drm/drv: Call drm_mode_config_create_state() by default

Maxime Ripard
2026-04-22T06:45:30Z

[-- Attachment #1: Type: text/plain, Size: 3483 bytes --]

Hi Thomas,

On Tue, Apr 21, 2026 at 05:33:12PM +0200, Thomas Zimmermann wrote:
> Am 21.04.26 um 15:38 schrieb Thomas Zimmermann:
> > Hi
> >
> > Am 20.03.26 um 17:27 schrieb Maxime Ripard:
> > > Almost all drivers, and our documented skeleton, call
> > > drm_mode_config_reset() prior to calling drm_dev_register() to
> > > initialize its DRM object states.
> > >
> > > Now that we have drm_mode_config_create_state() to create that initial
> > > state if it doesn't exist, we can call it directly in
> > > drm_dev_register(). That way, we know that the initial atomic state will
> > > always be allocated without any boilerplate.
> > >
> > > Signed-off-by: Maxime Ripard <mripard@kernel.org>
> > > ---
> > >   drivers/gpu/drm/drm_drv.c | 4 ++++
> > >   1 file changed, 4 insertions(+)
> > >
> > > diff --git a/drivers/gpu/drm/drm_drv.c b/drivers/gpu/drm/drm_drv.c
> > > index 2915118436ce8a6640cfb0c59936031990727ed1..820106d56ab399a39cac56d98662b5ddbcae8ded
> > > 100644
> > > --- a/drivers/gpu/drm/drm_drv.c
> > > +++ b/drivers/gpu/drm/drm_drv.c
> > > @@ -1097,10 +1097,14 @@ int drm_dev_register(struct drm_device *dev,
> > > unsigned long flags)
> > >         if (drm_core_check_feature(dev, DRIVER_MODESET)) {
> > >           ret = drm_modeset_register_all(dev);
> > >           if (ret)
> > >               goto err_unload;
> > > +
> > > +        ret = drm_mode_config_create_state(dev);
> > > +        if (ret)
> > > +            goto err_unload;
> >
> > Way too late. Lets rather go through drivers and call this where they
> > currently call drm_mode_config_reset() for initialization. This can be a
> > single-patch mass conversion IMHO.

I think that was Ville's main objection too. He suggested to do it in
the object initialization instead, but I believe it would be too early.

> On a second thought, can't we modify the suspend code and leave the reset
> as-is for now?  I'd still be interested to use reset as a means of
> initializing the hardware or loading state on probe. So keeping the _reset()
> calls in place might be helpful for that.
>
> What's the long-term plan here?

So, the way I was thinking about this is reset is done for several
things right now: initial state creation and software reset, and
hardware reset.

The latter isn't really commonly used. Most drivers, basically all
drivers that use the reset helpers, will not perform the hardware reset
as part of drm_mode_config_reset but will do it in probe or similar.

This is also a concern for hardware state read-out, since you don't want
that reset to happen.

So, eventually, I wanted to have something like try a readout, and if it
fails for any reason (disabled, unsupported, or failing to perform the
readout), we fallback to allocating a pristine state + resetting the
hardware.

To do that, we need create_state introduced here both for the readout
and non-readout paths, but also a (possibly device wide?) hw_reset hook
that will *only* reset the device without affecting the software state.

So:
- probe with readout would be create_state + readout_state for all objects
- probe without readout would be create_state + hw_reset for all objects
- resume would be create_state + hw_reset for a limited number of objects

And then we don't need .reset at all anymore and / or can implement
drm_mode_config_reset() on top of that.

I wanted to work on that next when done with this series.

Maxime

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 273 bytes --]