Received-SPF: pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 2620:137:e000::1:20 as permitted sender) client-ip=2620:137:e000::1:20;
Date:   Wed, 26 Jul 2023 15:54:34 +0200
From:   Maxime Ripard <mripard@kernel.org>
To:     Geert Uytterhoeven <geert@linux-m68k.org>
Cc:     Javier Martinez Canillas <javierm@redhat.com>,
        linux-kernel@vger.kernel.org,
        Thomas Zimmermann <tzimmermann@suse.de>,
        Daniel Vetter <daniel.vetter@ffwll.ch>,
        Daniel Vetter <daniel@ffwll.ch>,
        David Airlie <airlied@gmail.com>,
        dri-devel@lists.freedesktop.org
Subject: Re: [PATCH v4] drm/ssd130x: Allocate buffers in the plane's
 .atomic_check callback
Message-ID: <g6bu3b26evk464x4cn77xnzjiuotsq7pfvyakfvwnir5e3ihnk@2jh4dd56fsza>
References: <20230721070955.1170974-1-javierm@redhat.com>
 <CAMuHMdVxF80mdTaiXA6Y8Gd59Z7pxkoEphB4ohpVcK1q-+Yy=g@mail.gmail.com>
 <n4fmda4zw4vuezvwva35tgma3yqhdvb253q2tfdyauoxbfqshx@s6fsa7de6g5s>
 <CAMuHMdXtOozswqujA1h2spL8J86n65Q6=+z=5Jbb0nSXaBwqzA@mail.gmail.com>
 <874jlqlv5v.fsf@minerva.mail-host-address-is-not-set>
 <CAMuHMdX+J848ckG2JqsuDkRcWzRypw_Kv=0G+Hc329xstu_nqQ@mail.gmail.com>
MIME-Version: 1.0
Content-Type: multipart/signed; micalg=pgp-sha512;
        protocol="application/pgp-signature"; boundary="cczvehknair5dagk"
Content-Disposition: inline
In-Reply-To: <CAMuHMdX+J848ckG2JqsuDkRcWzRypw_Kv=0G+Hc329xstu_nqQ@mail.gmail.com>
Precedence: bulk


--cczvehknair5dagk
Content-Type: text/plain; charset=us-ascii
Content-Disposition: inline
Content-Transfer-Encoding: quoted-printable

On Wed, Jul 26, 2023 at 02:33:06PM +0200, Geert Uytterhoeven wrote:
> > >> Also, Javier pointed me to a discussion you had on IRC about using d=
evm
> > >> allocation here. We can't really do that. KMS devices are only freed
> > >> once the last userspace application closes its fd to the device file=
, so
> > >> you have an unbounded window during which the driver is still callab=
le
> > >> by userspace (and thus can still trigger an atomic commit) but the
> > >> buffer would have been freed for a while.
> > >
> > > It should still be safe for (at least) the data_array buffer. That
> > > buffer is only used to store pixels in hardware format, and immediate=
ly
> > > send them to the hardware.  If this can be called that late, it will
> > > fail horribly, as you can no longer talk to the hardware at that point
> > > (as ssd130x is an i2c driver, it might actually work; but a DRM driver
> > >  that calls devm_platform_ioremap_resource() will crash when writing
> > >  to its MMIO registers)?!?
> >
> > At the very least the SPI driver will fail since the GPIO that is used =
to
> > toggle the D/C pin is allocated with devm_gpiod_get_optional(), but also
> > the regulator, backlight device, etc.
> >
> > But in any case, as mentioned it is only relevant if the data_array buf=
fer
> > is allocated at probe time, and from Maxime's explanation is more corre=
ct
> > to do it in the .atomic_check handler.
>=20
> You need (at least) data_array for clear_screen, too, which is called
> from .atomic_disable().

I'm not sure I get what your concern is?

Even if we entirely disable the plane, the state will not have been
destroyed yet so you still have at least access to the data_array from
the old state.

Maxime

--cczvehknair5dagk
Content-Type: application/pgp-signature; name="signature.asc"

-----BEGIN PGP SIGNATURE-----

iHUEABYKAB0WIQRcEzekXsqa64kGDp7j7w1vZxhRxQUCZMElmgAKCRDj7w1vZxhR
xV3/AQDnwMW+Ru8NZqVM/6Uv+aypaEiFS7yM6LQxS3jkrDgCJwEA9l1H6lonPdO4
pWvLJ9iOIlUU+aoy6YCiZ8LSuDgdYA4=
=pQQv
-----END PGP SIGNATURE-----

--cczvehknair5dagk--