Switch mode-config initialization to drmm_mode_config_init() so that the
lifetime is tied to drm_device. Remove explicit drm_mode_config_cleanup()
from error and unbind paths since cleanup is now managed by DRM.

No functional change intended.

Signed-off-by: Hoyoung Lee <hy_fifty.lee@samsung.com>
---
 drivers/gpu/drm/exynos/exynos_drm_drv.c | 4 +---
 1 file changed, 1 insertion(+), 3 deletions(-)

diff --git a/drivers/gpu/drm/exynos/exynos_drm_drv.c b/drivers/gpu/drm/exynos/exynos_drm_drv.c
index 6cc7bf77bcac..1aea71778ab1 100644
--- a/drivers/gpu/drm/exynos/exynos_drm_drv.c
+++ b/drivers/gpu/drm/exynos/exynos_drm_drv.c
@@ -257,7 +257,7 @@ static int exynos_drm_bind(struct device *dev)
 	dev_set_drvdata(dev, drm);
 	drm->dev_private = (void *)private;
 
-	drm_mode_config_init(drm);
+	drmm_mode_config_init(drm);
 
 	exynos_drm_mode_config_init(drm);
 
@@ -297,7 +297,6 @@ static int exynos_drm_bind(struct device *dev)
 err_unbind_all:
 	component_unbind_all(drm->dev, drm);
 err_mode_config_cleanup:
-	drm_mode_config_cleanup(drm);
 	exynos_drm_cleanup_dma(drm);
 	kfree(private);
 	dev_set_drvdata(dev, NULL);
@@ -317,7 +316,6 @@ static void exynos_drm_unbind(struct device *dev)
 	drm_atomic_helper_shutdown(drm);
 
 	component_unbind_all(drm->dev, drm);
-	drm_mode_config_cleanup(drm);
 	exynos_drm_cleanup_dma(drm);
 
 	kfree(drm->dev_private);
-- 
2.34.1

2025년 9월 29일 (월) 오후 1:54, Hoyoung Lee <hy_fifty.lee@samsung.com>님이 작성:
>
> Switch mode-config initialization to drmm_mode_config_init() so that the
> lifetime is tied to drm_device. Remove explicit drm_mode_config_cleanup()
> from error and unbind paths since cleanup is now managed by DRM.
>
> No functional change intended.
>
> Signed-off-by: Hoyoung Lee <hy_fifty.lee@samsung.com>
> ---
>  drivers/gpu/drm/exynos/exynos_drm_drv.c | 4 +---
>  1 file changed, 1 insertion(+), 3 deletions(-)
>
> diff --git a/drivers/gpu/drm/exynos/exynos_drm_drv.c b/drivers/gpu/drm/exynos/exynos_drm_drv.c
> index 6cc7bf77bcac..1aea71778ab1 100644
> --- a/drivers/gpu/drm/exynos/exynos_drm_drv.c
> +++ b/drivers/gpu/drm/exynos/exynos_drm_drv.c
> @@ -257,7 +257,7 @@ static int exynos_drm_bind(struct device *dev)
>         dev_set_drvdata(dev, drm);
>         drm->dev_private = (void *)private;
>
> -       drm_mode_config_init(drm);
> +       drmm_mode_config_init(drm);
>
>         exynos_drm_mode_config_init(drm);
>
> @@ -297,7 +297,6 @@ static int exynos_drm_bind(struct device *dev)
>  err_unbind_all:
>         component_unbind_all(drm->dev, drm);
>  err_mode_config_cleanup:
> -       drm_mode_config_cleanup(drm);

In the current implementation, there is a potential dereference issue
because the private object may be freed before to_dma_dev(dev) is
called.
When drmm_mode_config_init() is invoked, it registers
drm_mode_config_cleanup() as a managed action. This means that the
cleanup function will be automatically executed later when
drm_dev_put() is called.

The problem arises when drm_dev_put() is called without explicitly
invoking drm_mode_config_cleanup() first, as in the original code. In
that case, the managed cleanup is performed later, which allows
to_dma_dev(dev) to be called after the private object has already been
released.

For reference, the following sequence may occur internally when
drm_mode_config_cleanup() is executed:
1. drm_mode_config_cleanup() is called.
2. During the cleanup of FBs, planes, CRTCs, encoders, and connectors,
framebuffers or GEM objects may be released.
3. At this point, Exynos-specific code could invoke to_dma_dev(dev).

Therefore, the private object must remain valid until
drm_mode_config_cleanup() completes.
It would be safer to adjust the code so that kfree(private) is
performed after drm_dev_put(drm) to ensure the private data remains
available during cleanup.

Thanks,
Inki Dae

>         exynos_drm_cleanup_dma(drm);
>         kfree(private);
>         dev_set_drvdata(dev, NULL);
> @@ -317,7 +316,6 @@ static void exynos_drm_unbind(struct device *dev)
>         drm_atomic_helper_shutdown(drm);
>
>         component_unbind_all(drm->dev, drm);
> -       drm_mode_config_cleanup(drm);

Ditto.

>         exynos_drm_cleanup_dma(drm);
>
>         kfree(drm->dev_private);
> --
> 2.34.1
>
>

> -----Original Message-----
> From: Inki Dae <daeinki@gmail.com>
> Sent: Monday, November 10, 2025 2:22 PM
> To: Hoyoung Lee <hy_fifty.lee@samsung.com>
> Cc: Seung-Woo Kim <sw0312.kim@samsung.com>; Kyungmin Park
> <kyungmin.park@samsung.com>; David Airlie <airlied@gmail.com>; Simona
> Vetter <simona@ffwll.ch>; Krzysztof Kozlowski <krzk@kernel.org>; Alim
> Akhtar <alim.akhtar@samsung.com>; dri-devel@lists.freedesktop.org; linux-
> arm-kernel@lists.infradead.org; linux-samsung-soc@vger.kernel.org; linux-
> kernel@vger.kernel.org
> Subject: Re: [PATCH 2/3] drm/exynos: Convert to drmm_mode_config_init()
> and drop manual cleanup
> 
> 2025년 9월 29일 (월) 오후 1:54, Hoyoung Lee <hy_fifty.lee@samsung.com>님이 작
> 성:
> >
> > Switch mode-config initialization to drmm_mode_config_init() so that
> > the lifetime is tied to drm_device. Remove explicit
> > drm_mode_config_cleanup() from error and unbind paths since cleanup is
> now managed by DRM.
> >
> > No functional change intended.
> >
> > Signed-off-by: Hoyoung Lee <hy_fifty.lee@samsung.com>
> > ---
> >  drivers/gpu/drm/exynos/exynos_drm_drv.c | 4 +---
> >  1 file changed, 1 insertion(+), 3 deletions(-)
> >
> > diff --git a/drivers/gpu/drm/exynos/exynos_drm_drv.c
> > b/drivers/gpu/drm/exynos/exynos_drm_drv.c
> > index 6cc7bf77bcac..1aea71778ab1 100644
> > --- a/drivers/gpu/drm/exynos/exynos_drm_drv.c
> > +++ b/drivers/gpu/drm/exynos/exynos_drm_drv.c
> > @@ -257,7 +257,7 @@ static int exynos_drm_bind(struct device *dev)
> >         dev_set_drvdata(dev, drm);
> >         drm->dev_private = (void *)private;
> >
> > -       drm_mode_config_init(drm);
> > +       drmm_mode_config_init(drm);
> >
> >         exynos_drm_mode_config_init(drm);
> >
> > @@ -297,7 +297,6 @@ static int exynos_drm_bind(struct device *dev)
> >  err_unbind_all:
> >         component_unbind_all(drm->dev, drm);
> >  err_mode_config_cleanup:
> > -       drm_mode_config_cleanup(drm);
> 
> In the current implementation, there is a potential dereference issue
> because the private object may be freed before to_dma_dev(dev) is called.
> When drmm_mode_config_init() is invoked, it registers
> drm_mode_config_cleanup() as a managed action. This means that the cleanup
> function will be automatically executed later when
> drm_dev_put() is called.
> 
> The problem arises when drm_dev_put() is called without explicitly
> invoking drm_mode_config_cleanup() first, as in the original code. In that
> case, the managed cleanup is performed later, which allows
> to_dma_dev(dev) to be called after the private object has already been
> released.
> 
> For reference, the following sequence may occur internally when
> drm_mode_config_cleanup() is executed:
> 1. drm_mode_config_cleanup() is called.
> 2. During the cleanup of FBs, planes, CRTCs, encoders, and connectors,
> framebuffers or GEM objects may be released.
> 3. At this point, Exynos-specific code could invoke to_dma_dev(dev).
> 
> Therefore, the private object must remain valid until
> drm_mode_config_cleanup() completes.
> It would be safer to adjust the code so that kfree(private) is performed
> after drm_dev_put(drm) to ensure the private data remains available during
> cleanup.
> 
> Thanks,
> Inki Dae
> 
> >         exynos_drm_cleanup_dma(drm);
> >         kfree(private);
> >         dev_set_drvdata(dev, NULL);
> > @@ -317,7 +316,6 @@ static void exynos_drm_unbind(struct device *dev)
> >         drm_atomic_helper_shutdown(drm);
> >
> >         component_unbind_all(drm->dev, drm);
> > -       drm_mode_config_cleanup(drm);
> 
> Ditto.
> 
> >         exynos_drm_cleanup_dma(drm);
> >
> >         kfree(drm->dev_private);
> > --
> > 2.34.1
> >
> >

Hi, Inki
Thanks for the review and for pointing out the to_dma_dev() path

If I understand you correctly, fine with using DRMM, but kfree(priv) should occur after drm_dev_put(drm)
That would mean releasing the drm_device first and freeing dev_private afterwards.
Of course, we will also need to adjust the probe() error-unwind (err_free) order accordingly.
Do you anticipate any side effects from this ordering change? I’d appreciate your thoughts.

BRs,
Hoyoung Lee

2025년 11월 12일 (수) 오후 12:03, <hy_fifty.lee@samsung.com>님이 작성:
>
> > -----Original Message-----
> > From: Inki Dae <daeinki@gmail.com>
> > Sent: Monday, November 10, 2025 2:22 PM
> > To: Hoyoung Lee <hy_fifty.lee@samsung.com>
> > Cc: Seung-Woo Kim <sw0312.kim@samsung.com>; Kyungmin Park
> > <kyungmin.park@samsung.com>; David Airlie <airlied@gmail.com>; Simona
> > Vetter <simona@ffwll.ch>; Krzysztof Kozlowski <krzk@kernel.org>; Alim
> > Akhtar <alim.akhtar@samsung.com>; dri-devel@lists.freedesktop.org; linux-
> > arm-kernel@lists.infradead.org; linux-samsung-soc@vger.kernel.org; linux-
> > kernel@vger.kernel.org
> > Subject: Re: [PATCH 2/3] drm/exynos: Convert to drmm_mode_config_init()
> > and drop manual cleanup
> >
> > 2025년 9월 29일 (월) 오후 1:54, Hoyoung Lee <hy_fifty.lee@samsung.com>님이 작
> > 성:
> > >
> > > Switch mode-config initialization to drmm_mode_config_init() so that
> > > the lifetime is tied to drm_device. Remove explicit
> > > drm_mode_config_cleanup() from error and unbind paths since cleanup is
> > now managed by DRM.
> > >
> > > No functional change intended.
> > >
> > > Signed-off-by: Hoyoung Lee <hy_fifty.lee@samsung.com>
> > > ---
> > >  drivers/gpu/drm/exynos/exynos_drm_drv.c | 4 +---
> > >  1 file changed, 1 insertion(+), 3 deletions(-)
> > >
> > > diff --git a/drivers/gpu/drm/exynos/exynos_drm_drv.c
> > > b/drivers/gpu/drm/exynos/exynos_drm_drv.c
> > > index 6cc7bf77bcac..1aea71778ab1 100644
> > > --- a/drivers/gpu/drm/exynos/exynos_drm_drv.c
> > > +++ b/drivers/gpu/drm/exynos/exynos_drm_drv.c
> > > @@ -257,7 +257,7 @@ static int exynos_drm_bind(struct device *dev)
> > >         dev_set_drvdata(dev, drm);
> > >         drm->dev_private = (void *)private;
> > >
> > > -       drm_mode_config_init(drm);
> > > +       drmm_mode_config_init(drm);
> > >
> > >         exynos_drm_mode_config_init(drm);
> > >
> > > @@ -297,7 +297,6 @@ static int exynos_drm_bind(struct device *dev)
> > >  err_unbind_all:
> > >         component_unbind_all(drm->dev, drm);
> > >  err_mode_config_cleanup:
> > > -       drm_mode_config_cleanup(drm);
> >
> > In the current implementation, there is a potential dereference issue
> > because the private object may be freed before to_dma_dev(dev) is called.
> > When drmm_mode_config_init() is invoked, it registers
> > drm_mode_config_cleanup() as a managed action. This means that the cleanup
> > function will be automatically executed later when
> > drm_dev_put() is called.
> >
> > The problem arises when drm_dev_put() is called without explicitly
> > invoking drm_mode_config_cleanup() first, as in the original code. In that
> > case, the managed cleanup is performed later, which allows
> > to_dma_dev(dev) to be called after the private object has already been
> > released.
> >
> > For reference, the following sequence may occur internally when
> > drm_mode_config_cleanup() is executed:
> > 1. drm_mode_config_cleanup() is called.
> > 2. During the cleanup of FBs, planes, CRTCs, encoders, and connectors,
> > framebuffers or GEM objects may be released.
> > 3. At this point, Exynos-specific code could invoke to_dma_dev(dev).
> >
> > Therefore, the private object must remain valid until
> > drm_mode_config_cleanup() completes.
> > It would be safer to adjust the code so that kfree(private) is performed
> > after drm_dev_put(drm) to ensure the private data remains available during
> > cleanup.
> >
> > Thanks,
> > Inki Dae
> >
> > >         exynos_drm_cleanup_dma(drm);
> > >         kfree(private);
> > >         dev_set_drvdata(dev, NULL);
> > > @@ -317,7 +316,6 @@ static void exynos_drm_unbind(struct device *dev)
> > >         drm_atomic_helper_shutdown(drm);
> > >
> > >         component_unbind_all(drm->dev, drm);
> > > -       drm_mode_config_cleanup(drm);
> >
> > Ditto.
> >
> > >         exynos_drm_cleanup_dma(drm);
> > >
> > >         kfree(drm->dev_private);
> > > --
> > > 2.34.1
> > >
> > >
>
> Hi, Inki
> Thanks for the review and for pointing out the to_dma_dev() path
>
> If I understand you correctly, fine with using DRMM, but kfree(priv) should occur after drm_dev_put(drm)
> That would mean releasing the drm_device first and freeing dev_private afterwards.
> Of course, we will also need to adjust the probe() error-unwind (err_free) order accordingly.
> Do you anticipate any side effects from this ordering change? I’d appreciate your thoughts.
>

Yes, that is correct. Also, changing the order of the cleanup steps
should not introduce any issues, because this simply restores the
original sequence that the code previously followed. In fact, the
current patch is, strictly speaking, altering the existing behavior I
think.

Thanks,
Inki Dae

> BRs,
> Hoyoung Lee
>
>