From mboxrd@z Thu Jan 1 00:00:00 1970 From: Thierry Reding Subject: Re: [PATCH 2/9] drm/tegra: Assign conn_state->connector when allocating connector state. Date: Tue, 24 Nov 2015 11:51:01 +0100 Message-ID: <20151124105101.GF6149@ulmo> References: <1448357676-27837-1-git-send-email-maarten.lankhorst@linux.intel.com> <1448357676-27837-3-git-send-email-maarten.lankhorst@linux.intel.com> <20151124103747.GI17050@phenom.ffwll.local> Mime-Version: 1.0 Content-Type: multipart/mixed; boundary="===============0461020909==" Return-path: In-Reply-To: <20151124103747.GI17050@phenom.ffwll.local> List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Errors-To: intel-gfx-bounces@lists.freedesktop.org Sender: "Intel-gfx" To: Daniel Vetter Cc: intel-gfx@lists.freedesktop.org, dri-devel@lists.freedesktop.org List-Id: dri-devel@lists.freedesktop.org --===============0461020909== Content-Type: multipart/signed; micalg=pgp-sha256; protocol="application/pgp-signature"; boundary="7LkOrbQMr4cezO2T" Content-Disposition: inline --7LkOrbQMr4cezO2T Content-Type: text/plain; charset=us-ascii Content-Disposition: inline Content-Transfer-Encoding: quoted-printable On Tue, Nov 24, 2015 at 11:37:47AM +0100, Daniel Vetter wrote: > On Tue, Nov 24, 2015 at 10:34:29AM +0100, Maarten Lankhorst wrote: > > Signed-off-by: Maarten Lankhorst > > --- > > drivers/gpu/drm/tegra/dsi.c | 4 +++- > > 1 file changed, 3 insertions(+), 1 deletion(-) > >=20 > > diff --git a/drivers/gpu/drm/tegra/dsi.c b/drivers/gpu/drm/tegra/dsi.c > > index f0a138ef68ce..6b8ae3d08eeb 100644 > > --- a/drivers/gpu/drm/tegra/dsi.c > > +++ b/drivers/gpu/drm/tegra/dsi.c > > @@ -751,8 +751,10 @@ static void tegra_dsi_connector_reset(struct drm_c= onnector *connector) > > connector->state =3D NULL; > > =20 > > state =3D kzalloc(sizeof(*state), GFP_KERNEL); > > - if (state) > > + if (state) { > > + state->base.connector =3D connector; > > connector->state =3D &state->base; > > + } >=20 > Hm, we might want __ versions of all the _reset hooks if this becomes more > common. I do wonder a bit why it isn't since a lot of drivers overwrite > state structures by now, and then the provided reset functions aren't > sufficient really. We already have the __ versions for duplicate_state helpers, but the problem with reset helpers is that they need to know the allocation size... Actually, that's true of the duplicate_state helpers as well, and the __ variants don't actually allocate the memory but rather just copy the state from old to new. So we could do something just like that for the reset helpers: void __drm_atomic_helper_connector_reset(struct drm_connector *connector, struct drm_connector_state *state) { state->base.connector =3D connector; connector->state =3D state; } and use like this: static void tegra_dsi_connector_reset(struct drm_connector *connector) { struct tegra_dsi_state *state; ... state =3D kzalloc(sizeof(*state), GFP_KERNEL); if (state) __drm_atomic_helper_connector_reset(connector, &state->base); } I think back at the time when we did this for duplicate_state helpers we had a discussion about the usefulness of this, but this patchset clearly shows that this kind of change, which applies to a number of drivers, is going to happen again and again, so the only way to avoid mistakes or an extensive set of changes across all drivers is by putting this into a helper, even if it's only two assignments now. Thierry --7LkOrbQMr4cezO2T Content-Type: application/pgp-signature; name="signature.asc" -----BEGIN PGP SIGNATURE----- Version: GnuPG v2 iQIcBAABCAAGBQJWVEESAAoJEN0jrNd/PrOh+/gQALsrS4QgLjYiPNb8q+kiPif/ 1JDQGWCWj+zvlxYp1pzgIrKE5D/xxQM1R1cZvzQvo3L7e0qNXVb2QnY8ZXyStU0u SL2eKvepH/7gzb4k00oUWaj3Wqa88XdCWO7ifjpgLCMibKxzHPiUceVQABRTM1pZ w9tGNttH9UyhH5NoAeN8TM3k2zis43xT/Ftnxyx11E6CcEj9pKIbFakaQxil2azx HnB7KbmqLi6BBEO+cuEUhz7oXEtIISJaP589Caw7H1mYJr3TMiWiZdJ4JONdRdvC foepOPGbXedZCG8iXScaSdFzb2h6SGdlfMHPI1QEPm3jjAB5GBio+iNlYdtH+6U3 061I/DC7eBXZARoJIryHDgX+/trfJO61+8KSH9pshQD7qHQqkPr9C0M/EF4dd+84 53q794FjfBZBav0SWC17dPx2IFyrSsWgP3V/Kf08iwOoYafzs/DOnCO/0WdlGEHh YuXJl1ETVj+xusrPAcGE9xV/zDb0fqxZwLN5tjOMD7zDM6LeLQXkPgSLtCgyA0JE pniG9Z6Ma+T4FM/rcGFgVdvvMu827jd7FUuVb0Pt9axSAIEGXhIn/s8of14WfTlc KgEGmOaCP+agTwMePlUKkm6UKQUSifbdUpoU+Q+zFn3ViocmHYH2Ih6Q5MB3JgQm YHPzgAG8yM48SvElBPpZ =O0Df -----END PGP SIGNATURE----- --7LkOrbQMr4cezO2T-- --===============0461020909== Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: base64 Content-Disposition: inline X19fX19fX19fX19fX19fX19fX19fX19fX19fX19fX19fX19fX19fX19fX19fX18KSW50ZWwtZ2Z4 IG1haWxpbmcgbGlzdApJbnRlbC1nZnhAbGlzdHMuZnJlZWRlc2t0b3Aub3JnCmh0dHA6Ly9saXN0 cy5mcmVlZGVza3RvcC5vcmcvbWFpbG1hbi9saXN0aW5mby9pbnRlbC1nZngK --===============0461020909==--