From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: X-Spam-Checker-Version: SpamAssassin 3.4.0 (2014-02-07) on aws-us-west-2-korg-lkml-1.web.codeaurora.org X-Spam-Level: X-Spam-Status: No, score=-8.3 required=3.0 tests=HEADER_FROM_DIFFERENT_DOMAINS, INCLUDES_PATCH,MAILING_LIST_MULTI,SIGNED_OFF_BY,SPF_HELO_NONE,SPF_PASS, USER_AGENT_SANE_1 autolearn=ham autolearn_force=no version=3.4.0 Received: from mail.kernel.org (mail.kernel.org [198.145.29.99]) by smtp.lore.kernel.org (Postfix) with ESMTP id CE7EBC3F2D2 for ; Fri, 28 Feb 2020 20:26:38 +0000 (UTC) Received: from gabe.freedesktop.org (gabe.freedesktop.org [131.252.210.177]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by mail.kernel.org (Postfix) with ESMTPS id AAF50246A2 for ; Fri, 28 Feb 2020 20:26:38 +0000 (UTC) DMARC-Filter: OpenDMARC Filter v1.3.2 mail.kernel.org AAF50246A2 Authentication-Results: mail.kernel.org; dmarc=none (p=none dis=none) header.from=ravnborg.org Authentication-Results: mail.kernel.org; spf=none smtp.mailfrom=dri-devel-bounces@lists.freedesktop.org Received: from gabe.freedesktop.org (localhost [127.0.0.1]) by gabe.freedesktop.org (Postfix) with ESMTP id DE9F36E0FE; Fri, 28 Feb 2020 20:26:37 +0000 (UTC) Received: from asavdk4.altibox.net (asavdk4.altibox.net [109.247.116.15]) by gabe.freedesktop.org (Postfix) with ESMTPS id 856036E0FE; Fri, 28 Feb 2020 20:26:36 +0000 (UTC) Received: from ravnborg.org (unknown [158.248.194.18]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by asavdk4.altibox.net (Postfix) with ESMTPS id CAC7F8046E; Fri, 28 Feb 2020 21:26:33 +0100 (CET) Date: Fri, 28 Feb 2020 21:26:32 +0100 From: Sam Ravnborg To: Daniel Vetter Subject: Re: [PATCH 26/51] drm: Manage drm_mode_config_init with drmm_ Message-ID: <20200228202632.GB22966@ravnborg.org> References: <20200227181522.2711142-1-daniel.vetter@ffwll.ch> <20200227181522.2711142-27-daniel.vetter@ffwll.ch> MIME-Version: 1.0 Content-Disposition: inline In-Reply-To: <20200227181522.2711142-27-daniel.vetter@ffwll.ch> User-Agent: Mutt/1.10.1 (2018-07-13) X-CMAE-Score: 0 X-CMAE-Analysis: v=2.3 cv=XpTUx2N9 c=1 sm=1 tr=0 a=UWs3HLbX/2nnQ3s7vZ42gw==:117 a=UWs3HLbX/2nnQ3s7vZ42gw==:17 a=jpOVt7BSZ2e4Z31A5e1TngXxSK0=:19 a=8nJEP1OIZ-IA:10 a=7gkXJVJtAAAA:8 a=P1BnusSwAAAA:8 a=SJz97ENfAAAA:8 a=QyXUC8HyAAAA:8 a=9vy5vZw2cxVn3W-tDF8A:9 a=wPNLvfGTeEIA:10 a=E9Po1WZjFZOl8hwRPBS3:22 a=D0XLA9XvdZm18NrgonBM:22 a=vFet0B0WnEQeilDPIY6i:22 X-BeenThere: dri-devel@lists.freedesktop.org X-Mailman-Version: 2.1.29 Precedence: list List-Id: Direct Rendering Infrastructure - Development List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Cc: Intel Graphics Development , m.felsch@pengutronix.de, DRI Development , Laurent Pinchart , Thomas Zimmermann , Daniel Vetter Content-Type: text/plain; charset="iso-8859-1" Content-Transfer-Encoding: quoted-printable Errors-To: dri-devel-bounces@lists.freedesktop.org Sender: "dri-devel" Hi Daniel. Some bikeshedding in the following. with or with addressing (IMHO valid points) consider the patch: Reviewed-by: Sam Ravnborg Sam On Thu, Feb 27, 2020 at 07:14:57PM +0100, Daniel Vetter wrote: > drm_mode_config_cleanup is idempotent, so no harm in calling this > twice. This allows us to gradually switch drivers over by removing > explicit drm_mode_config_cleanup calls. > > With this step it's not also possible that (at least for simple > drivers) automatic resource cleanup can be done correctly without a > drm_driver->release hook. Therefore allow this now in > devm_drm_dev_init(). I am not really sure what you try to explain here? Should the "not" be deleted? > = > Also with drmm_ explicit drm_driver->release hooks are kinda not the > best option, so deprecate that hook to discourage future users. The ->release hooks has others uses until everything is moved over to drmm_, or so I think. So deprecation seems a lttle too soon. > = > v2: Fixup the example in the kerneldoc too. > = > v3: > - For paranoia, double check that minor->dev =3D=3D dev in the release > hook, because I botched the pointer math in the drmm library. > - Call drm_mode_config_cleanup when drmm_add_action fails, we'd be > missing some mutex_destroy and ida_cleanup otherwise (Laurent) > = > v4: Add a drmm_add_action_or_reset (like devm_ has) to encapsulate this > pattern (Noralf). > = > v5: Fix oversight in the new add_action_or_reset macro (Noralf) ^ drmm_add_action_or_reset > = > Cc: Laurent Pinchart > Cc: "Noralf Tr=F8nnes" > Cc: Sam Ravnborg > Cc: Thomas Zimmermann > Acked-by: Noralf Tr=F8nnes > Signed-off-by: Daniel Vetter > --- > drivers/gpu/drm/drm_drv.c | 23 +++++++---------------- > drivers/gpu/drm/drm_managed.c | 14 ++++++++++++++ > drivers/gpu/drm/drm_mode_config.c | 13 ++++++++++++- > include/drm/drm_managed.h | 7 +++++++ > include/drm/drm_mode_config.h | 2 +- > 5 files changed, 41 insertions(+), 18 deletions(-) > = > diff --git a/drivers/gpu/drm/drm_drv.c b/drivers/gpu/drm/drm_drv.c > index 3cf40864d4a6..bb326b9bcde0 100644 > --- a/drivers/gpu/drm/drm_drv.c > +++ b/drivers/gpu/drm/drm_drv.c > @@ -98,6 +98,8 @@ static void drm_minor_alloc_release(struct drm_device *= dev, void *data) > struct drm_minor *minor =3D data; > unsigned long flags; > = > + WARN_ON(dev !=3D minor->dev); > + > put_device(minor->kdev); > = > spin_lock_irqsave(&drm_minor_lock, flags); > @@ -267,8 +269,7 @@ void drm_minor_release(struct drm_minor *minor) > * > * The following example shows a typical structure of a DRM display driv= er. > * The example focus on the probe() function and the other functions tha= t is > - * almost always present and serves as a demonstration of devm_drm_dev_i= nit() > - * usage with its accompanying drm_driver->release callback. > + * almost always present and serves as a demonstration of devm_drm_dev_i= nit(). > * > * .. code-block:: c > * > @@ -278,16 +279,8 @@ void drm_minor_release(struct drm_minor *minor) > * struct clk *pclk; > * }; > * > - * static void driver_drm_release(struct drm_device *drm) > - * { > - * struct driver_device *priv =3D container_of(...); > - * > - * drm_mode_config_cleanup(drm); > - * } > - * > * static struct drm_driver driver_drm_driver =3D { > * [...] > - * .release =3D driver_drm_release, > * }; > * > * static int driver_probe(struct platform_device *pdev) > @@ -312,7 +305,9 @@ void drm_minor_release(struct drm_minor *minor) > * } > * drmm_add_final_kfree(drm, priv); > * > - * drm_mode_config_init(drm); > + * ret =3D drm_mode_config_init(drm); > + * if (ret) > + * return ret; We do not print anything in drm_mode_config_init() - so should we do it here? Otherwise we only get the more generic error from the driver core. > * > * priv->userspace_facing =3D drmm_kzalloc(..., GFP_KERNEL); > * if (!priv->userspace_facing) > @@ -710,8 +705,7 @@ static void devm_drm_dev_init_release(void *data) > * @driver: DRM driver > * > * Managed drm_dev_init(). The DRM device initialized with this function= is > - * automatically put on driver detach using drm_dev_put(). You must supp= ly a > - * &drm_driver.release callback to control the finalization explicitly. > + * automatically put on driver detach using drm_dev_put(). > * > * RETURNS: > * 0 on success, or error code on failure. > @@ -722,9 +716,6 @@ int devm_drm_dev_init(struct device *parent, > { > int ret; > = > - if (WARN_ON(!driver->release)) > - return -EINVAL; > - > ret =3D drm_dev_init(dev, driver, parent); > if (ret) > return ret; > diff --git a/drivers/gpu/drm/drm_managed.c b/drivers/gpu/drm/drm_managed.c > index 626656369f0b..6376be01bbc8 100644 > --- a/drivers/gpu/drm/drm_managed.c > +++ b/drivers/gpu/drm/drm_managed.c > @@ -134,6 +134,20 @@ int __drmm_add_action(struct drm_device *dev, > } > EXPORT_SYMBOL(__drmm_add_action); > = > +int __drmm_add_action_or_reset(struct drm_device *dev, > + drmres_release_t action, > + void *data, const char *name) > +{ > + int ret; > + > + ret =3D __drmm_add_action(dev, action, data, name); > + if (ret) > + action(dev, data); > + > + return ret; > +} > +EXPORT_SYMBOL(__drmm_add_action_or_reset); Bikeshedding - but why oh why prefixing the function with two underscores? - It makes it less readable - It says "internal", at least to me - but it is exported - It makes the casual reader wonder why, removing focus from other more relevant things - It makes me writing several lines of rant drmm_add_action_or_reset_named(...) would do the trick. Same rant above goes for __drmm_add_action()... > + > void drmm_remove_action(struct drm_device *dev, > drmres_release_t action, > void *data) > diff --git a/drivers/gpu/drm/drm_mode_config.c b/drivers/gpu/drm/drm_mode= _config.c > index 08e6eff6a179..6f7005bc597f 100644 > --- a/drivers/gpu/drm/drm_mode_config.c > +++ b/drivers/gpu/drm/drm_mode_config.c > @@ -25,6 +25,7 @@ > #include > #include > #include > +#include > #include > #include > #include > @@ -373,6 +374,11 @@ static int drm_mode_create_standard_properties(struc= t drm_device *dev) > return 0; > } > = > +static void drm_mode_config_init_release(struct drm_device *dev, void *p= tr) > +{ > + drm_mode_config_cleanup(dev); > +} > + > /** > * drm_mode_config_init - initialize DRM mode_configuration structure > * @dev: DRM device > @@ -384,8 +390,10 @@ static int drm_mode_create_standard_properties(struc= t drm_device *dev) > * problem, since this should happen single threaded at init time. It is= the > * driver's problem to ensure this guarantee. > * > + * Cleanup is automatically handled through registering drm_mode_config_= cleanup > + * with drmm_add_action(). > */ > -void drm_mode_config_init(struct drm_device *dev) > +int drm_mode_config_init(struct drm_device *dev) > { > mutex_init(&dev->mode_config.mutex); > drm_modeset_lock_init(&dev->mode_config.connection_mutex); > @@ -443,6 +451,9 @@ void drm_mode_config_init(struct drm_device *dev) > drm_modeset_acquire_fini(&modeset_ctx); > dma_resv_fini(&resv); > } > + > + return drmm_add_action_or_reset(dev, drm_mode_config_init_release, > + NULL); > } > EXPORT_SYMBOL(drm_mode_config_init); As this is now a drmm_ managed function it should be named such. Maybe add a small drm_mode_config_init() wrapper in the header file for those that has not migrated yet. It is confusing if we are not consistent in naming and everywhere else the drm managed functions are named drmm_ > = > diff --git a/include/drm/drm_managed.h b/include/drm/drm_managed.h > index 2b1ba2ad5582..1e6291407586 100644 > --- a/include/drm/drm_managed.h > +++ b/include/drm/drm_managed.h > @@ -18,6 +18,13 @@ int __must_check __drmm_add_action(struct drm_device *= dev, > drmres_release_t action, > void *data, const char *name); > = > +#define drmm_add_action_or_reset(dev, action, data) \ > + __drmm_add_action_or_reset(dev, action, data, #action) > + > +int __must_check __drmm_add_action_or_reset(struct drm_device *dev, > + drmres_release_t action, > + void *data, const char *name); > + > void drmm_remove_action(struct drm_device *dev, > drmres_release_t action, > void *data); > diff --git a/include/drm/drm_mode_config.h b/include/drm/drm_mode_config.h > index 3bcbe30339f0..160a3e4b51c3 100644 > --- a/include/drm/drm_mode_config.h > +++ b/include/drm/drm_mode_config.h > @@ -929,7 +929,7 @@ struct drm_mode_config { > const struct drm_mode_config_helper_funcs *helper_private; > }; > = > -void drm_mode_config_init(struct drm_device *dev); > +int drm_mode_config_init(struct drm_device *dev); > void drm_mode_config_reset(struct drm_device *dev); > void drm_mode_config_cleanup(struct drm_device *dev); > = > -- = > 2.24.1 _______________________________________________ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: X-Spam-Checker-Version: SpamAssassin 3.4.0 (2014-02-07) on aws-us-west-2-korg-lkml-1.web.codeaurora.org X-Spam-Level: X-Spam-Status: No, score=-8.3 required=3.0 tests=HEADER_FROM_DIFFERENT_DOMAINS, INCLUDES_PATCH,MAILING_LIST_MULTI,SIGNED_OFF_BY,SPF_HELO_NONE,SPF_PASS, USER_AGENT_SANE_1 autolearn=unavailable autolearn_force=no version=3.4.0 Received: from mail.kernel.org (mail.kernel.org [198.145.29.99]) by smtp.lore.kernel.org (Postfix) with ESMTP id 0B454C3F2CD for ; Fri, 28 Feb 2020 20:26:43 +0000 (UTC) Received: from gabe.freedesktop.org (gabe.freedesktop.org [131.252.210.177]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by mail.kernel.org (Postfix) with ESMTPS id DBCA0246A2 for ; Fri, 28 Feb 2020 20:26:42 +0000 (UTC) DMARC-Filter: OpenDMARC Filter v1.3.2 mail.kernel.org DBCA0246A2 Authentication-Results: mail.kernel.org; dmarc=none (p=none dis=none) header.from=ravnborg.org Authentication-Results: mail.kernel.org; spf=none smtp.mailfrom=intel-gfx-bounces@lists.freedesktop.org Received: from gabe.freedesktop.org (localhost [127.0.0.1]) by gabe.freedesktop.org (Postfix) with ESMTP id 3D1296F4BC; Fri, 28 Feb 2020 20:26:38 +0000 (UTC) Received: from asavdk4.altibox.net (asavdk4.altibox.net [109.247.116.15]) by gabe.freedesktop.org (Postfix) with ESMTPS id 856036E0FE; Fri, 28 Feb 2020 20:26:36 +0000 (UTC) Received: from ravnborg.org (unknown [158.248.194.18]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by asavdk4.altibox.net (Postfix) with ESMTPS id CAC7F8046E; Fri, 28 Feb 2020 21:26:33 +0100 (CET) Date: Fri, 28 Feb 2020 21:26:32 +0100 From: Sam Ravnborg To: Daniel Vetter Message-ID: <20200228202632.GB22966@ravnborg.org> References: <20200227181522.2711142-1-daniel.vetter@ffwll.ch> <20200227181522.2711142-27-daniel.vetter@ffwll.ch> MIME-Version: 1.0 Content-Disposition: inline In-Reply-To: <20200227181522.2711142-27-daniel.vetter@ffwll.ch> User-Agent: Mutt/1.10.1 (2018-07-13) X-CMAE-Score: 0 X-CMAE-Analysis: v=2.3 cv=XpTUx2N9 c=1 sm=1 tr=0 a=UWs3HLbX/2nnQ3s7vZ42gw==:117 a=UWs3HLbX/2nnQ3s7vZ42gw==:17 a=jpOVt7BSZ2e4Z31A5e1TngXxSK0=:19 a=8nJEP1OIZ-IA:10 a=7gkXJVJtAAAA:8 a=P1BnusSwAAAA:8 a=SJz97ENfAAAA:8 a=QyXUC8HyAAAA:8 a=9vy5vZw2cxVn3W-tDF8A:9 a=wPNLvfGTeEIA:10 a=E9Po1WZjFZOl8hwRPBS3:22 a=D0XLA9XvdZm18NrgonBM:22 a=vFet0B0WnEQeilDPIY6i:22 Subject: Re: [Intel-gfx] [PATCH 26/51] drm: Manage drm_mode_config_init with drmm_ X-BeenThere: intel-gfx@lists.freedesktop.org X-Mailman-Version: 2.1.29 Precedence: list List-Id: Intel graphics driver community testing & development List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Cc: Intel Graphics Development , m.felsch@pengutronix.de, DRI Development , Noralf =?iso-8859-1?Q?Tr=F8nnes?= , Laurent Pinchart , Thomas Zimmermann , Daniel Vetter , l.stach@pengutronix.de Content-Type: text/plain; charset="iso-8859-1" Content-Transfer-Encoding: quoted-printable Errors-To: intel-gfx-bounces@lists.freedesktop.org Sender: "Intel-gfx" Hi Daniel. Some bikeshedding in the following. with or with addressing (IMHO valid points) consider the patch: Reviewed-by: Sam Ravnborg Sam On Thu, Feb 27, 2020 at 07:14:57PM +0100, Daniel Vetter wrote: > drm_mode_config_cleanup is idempotent, so no harm in calling this > twice. This allows us to gradually switch drivers over by removing > explicit drm_mode_config_cleanup calls. > > With this step it's not also possible that (at least for simple > drivers) automatic resource cleanup can be done correctly without a > drm_driver->release hook. Therefore allow this now in > devm_drm_dev_init(). I am not really sure what you try to explain here? Should the "not" be deleted? > = > Also with drmm_ explicit drm_driver->release hooks are kinda not the > best option, so deprecate that hook to discourage future users. The ->release hooks has others uses until everything is moved over to drmm_, or so I think. So deprecation seems a lttle too soon. > = > v2: Fixup the example in the kerneldoc too. > = > v3: > - For paranoia, double check that minor->dev =3D=3D dev in the release > hook, because I botched the pointer math in the drmm library. > - Call drm_mode_config_cleanup when drmm_add_action fails, we'd be > missing some mutex_destroy and ida_cleanup otherwise (Laurent) > = > v4: Add a drmm_add_action_or_reset (like devm_ has) to encapsulate this > pattern (Noralf). > = > v5: Fix oversight in the new add_action_or_reset macro (Noralf) ^ drmm_add_action_or_reset > = > Cc: Laurent Pinchart > Cc: "Noralf Tr=F8nnes" > Cc: Sam Ravnborg > Cc: Thomas Zimmermann > Acked-by: Noralf Tr=F8nnes > Signed-off-by: Daniel Vetter > --- > drivers/gpu/drm/drm_drv.c | 23 +++++++---------------- > drivers/gpu/drm/drm_managed.c | 14 ++++++++++++++ > drivers/gpu/drm/drm_mode_config.c | 13 ++++++++++++- > include/drm/drm_managed.h | 7 +++++++ > include/drm/drm_mode_config.h | 2 +- > 5 files changed, 41 insertions(+), 18 deletions(-) > = > diff --git a/drivers/gpu/drm/drm_drv.c b/drivers/gpu/drm/drm_drv.c > index 3cf40864d4a6..bb326b9bcde0 100644 > --- a/drivers/gpu/drm/drm_drv.c > +++ b/drivers/gpu/drm/drm_drv.c > @@ -98,6 +98,8 @@ static void drm_minor_alloc_release(struct drm_device *= dev, void *data) > struct drm_minor *minor =3D data; > unsigned long flags; > = > + WARN_ON(dev !=3D minor->dev); > + > put_device(minor->kdev); > = > spin_lock_irqsave(&drm_minor_lock, flags); > @@ -267,8 +269,7 @@ void drm_minor_release(struct drm_minor *minor) > * > * The following example shows a typical structure of a DRM display driv= er. > * The example focus on the probe() function and the other functions tha= t is > - * almost always present and serves as a demonstration of devm_drm_dev_i= nit() > - * usage with its accompanying drm_driver->release callback. > + * almost always present and serves as a demonstration of devm_drm_dev_i= nit(). > * > * .. code-block:: c > * > @@ -278,16 +279,8 @@ void drm_minor_release(struct drm_minor *minor) > * struct clk *pclk; > * }; > * > - * static void driver_drm_release(struct drm_device *drm) > - * { > - * struct driver_device *priv =3D container_of(...); > - * > - * drm_mode_config_cleanup(drm); > - * } > - * > * static struct drm_driver driver_drm_driver =3D { > * [...] > - * .release =3D driver_drm_release, > * }; > * > * static int driver_probe(struct platform_device *pdev) > @@ -312,7 +305,9 @@ void drm_minor_release(struct drm_minor *minor) > * } > * drmm_add_final_kfree(drm, priv); > * > - * drm_mode_config_init(drm); > + * ret =3D drm_mode_config_init(drm); > + * if (ret) > + * return ret; We do not print anything in drm_mode_config_init() - so should we do it here? Otherwise we only get the more generic error from the driver core. > * > * priv->userspace_facing =3D drmm_kzalloc(..., GFP_KERNEL); > * if (!priv->userspace_facing) > @@ -710,8 +705,7 @@ static void devm_drm_dev_init_release(void *data) > * @driver: DRM driver > * > * Managed drm_dev_init(). The DRM device initialized with this function= is > - * automatically put on driver detach using drm_dev_put(). You must supp= ly a > - * &drm_driver.release callback to control the finalization explicitly. > + * automatically put on driver detach using drm_dev_put(). > * > * RETURNS: > * 0 on success, or error code on failure. > @@ -722,9 +716,6 @@ int devm_drm_dev_init(struct device *parent, > { > int ret; > = > - if (WARN_ON(!driver->release)) > - return -EINVAL; > - > ret =3D drm_dev_init(dev, driver, parent); > if (ret) > return ret; > diff --git a/drivers/gpu/drm/drm_managed.c b/drivers/gpu/drm/drm_managed.c > index 626656369f0b..6376be01bbc8 100644 > --- a/drivers/gpu/drm/drm_managed.c > +++ b/drivers/gpu/drm/drm_managed.c > @@ -134,6 +134,20 @@ int __drmm_add_action(struct drm_device *dev, > } > EXPORT_SYMBOL(__drmm_add_action); > = > +int __drmm_add_action_or_reset(struct drm_device *dev, > + drmres_release_t action, > + void *data, const char *name) > +{ > + int ret; > + > + ret =3D __drmm_add_action(dev, action, data, name); > + if (ret) > + action(dev, data); > + > + return ret; > +} > +EXPORT_SYMBOL(__drmm_add_action_or_reset); Bikeshedding - but why oh why prefixing the function with two underscores? - It makes it less readable - It says "internal", at least to me - but it is exported - It makes the casual reader wonder why, removing focus from other more relevant things - It makes me writing several lines of rant drmm_add_action_or_reset_named(...) would do the trick. Same rant above goes for __drmm_add_action()... > + > void drmm_remove_action(struct drm_device *dev, > drmres_release_t action, > void *data) > diff --git a/drivers/gpu/drm/drm_mode_config.c b/drivers/gpu/drm/drm_mode= _config.c > index 08e6eff6a179..6f7005bc597f 100644 > --- a/drivers/gpu/drm/drm_mode_config.c > +++ b/drivers/gpu/drm/drm_mode_config.c > @@ -25,6 +25,7 @@ > #include > #include > #include > +#include > #include > #include > #include > @@ -373,6 +374,11 @@ static int drm_mode_create_standard_properties(struc= t drm_device *dev) > return 0; > } > = > +static void drm_mode_config_init_release(struct drm_device *dev, void *p= tr) > +{ > + drm_mode_config_cleanup(dev); > +} > + > /** > * drm_mode_config_init - initialize DRM mode_configuration structure > * @dev: DRM device > @@ -384,8 +390,10 @@ static int drm_mode_create_standard_properties(struc= t drm_device *dev) > * problem, since this should happen single threaded at init time. It is= the > * driver's problem to ensure this guarantee. > * > + * Cleanup is automatically handled through registering drm_mode_config_= cleanup > + * with drmm_add_action(). > */ > -void drm_mode_config_init(struct drm_device *dev) > +int drm_mode_config_init(struct drm_device *dev) > { > mutex_init(&dev->mode_config.mutex); > drm_modeset_lock_init(&dev->mode_config.connection_mutex); > @@ -443,6 +451,9 @@ void drm_mode_config_init(struct drm_device *dev) > drm_modeset_acquire_fini(&modeset_ctx); > dma_resv_fini(&resv); > } > + > + return drmm_add_action_or_reset(dev, drm_mode_config_init_release, > + NULL); > } > EXPORT_SYMBOL(drm_mode_config_init); As this is now a drmm_ managed function it should be named such. Maybe add a small drm_mode_config_init() wrapper in the header file for those that has not migrated yet. It is confusing if we are not consistent in naming and everywhere else the drm managed functions are named drmm_ > = > diff --git a/include/drm/drm_managed.h b/include/drm/drm_managed.h > index 2b1ba2ad5582..1e6291407586 100644 > --- a/include/drm/drm_managed.h > +++ b/include/drm/drm_managed.h > @@ -18,6 +18,13 @@ int __must_check __drmm_add_action(struct drm_device *= dev, > drmres_release_t action, > void *data, const char *name); > = > +#define drmm_add_action_or_reset(dev, action, data) \ > + __drmm_add_action_or_reset(dev, action, data, #action) > + > +int __must_check __drmm_add_action_or_reset(struct drm_device *dev, > + drmres_release_t action, > + void *data, const char *name); > + > void drmm_remove_action(struct drm_device *dev, > drmres_release_t action, > void *data); > diff --git a/include/drm/drm_mode_config.h b/include/drm/drm_mode_config.h > index 3bcbe30339f0..160a3e4b51c3 100644 > --- a/include/drm/drm_mode_config.h > +++ b/include/drm/drm_mode_config.h > @@ -929,7 +929,7 @@ struct drm_mode_config { > const struct drm_mode_config_helper_funcs *helper_private; > }; > = > -void drm_mode_config_init(struct drm_device *dev); > +int drm_mode_config_init(struct drm_device *dev); > void drm_mode_config_reset(struct drm_device *dev); > void drm_mode_config_cleanup(struct drm_device *dev); > = > -- = > 2.24.1 _______________________________________________ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/intel-gfx