[<prev] [next>] [<thread-prev] [day] [month] [year] [list]
Message-ID: <CAAQKjZMDe-nXN14qQQTHR9HMwqCcbTospjMg_P98G0e=XAO5NQ@mail.gmail.com>
Date: Mon, 10 Nov 2025 15:44:46 +0900
From: Inki Dae <daeinki@...il.com>
To: Hoyoung Lee <hy_fifty.lee@...sung.com>
Cc: Seung-Woo Kim <sw0312.kim@...sung.com>, Kyungmin Park <kyungmin.park@...sung.com>,
David Airlie <airlied@...il.com>, Simona Vetter <simona@...ll.ch>,
Krzysztof Kozlowski <krzk@...nel.org>, Alim Akhtar <alim.akhtar@...sung.com>,
dri-devel@...ts.freedesktop.org, linux-arm-kernel@...ts.infradead.org,
linux-samsung-soc@...r.kernel.org, linux-kernel@...r.kernel.org
Subject: Re: [PATCH 3/3] drm/exynos: Move mode_config setup from fb.c to drv.c
2025년 9월 29일 (월) 오후 1:29, Hoyoung Lee <hy_fifty.lee@...sung.com>님이 작성:
>
> Relocate exynos_drm_mode_config_init() and the mode_config funcs/helpers
> from exynos_drm_fb.c to exynos_drm_drv.c, and invoke
> drm_mode_config_init() from inside exynos_drm_mode_config_init().
>
> Rationale: resolve the historical fb.c placement, align with common DRM
> layering (mode_config is device-wide policy that belongs in the core
> driver), and make initialization order explicit before creating KMS
> objects and binding components.
>
> No functional change intended.
This patch looks fine to me.
However, since the second patch should be applied first, please repost
them as v2.
Thanks,
Inki Dae
>
> Signed-off-by: Hoyoung Lee <hy_fifty.lee@...sung.com>
> ---
> drivers/gpu/drm/exynos/exynos_drm_drv.c | 47 ++++++++++++++++++++++---
> drivers/gpu/drm/exynos/exynos_drm_fb.c | 34 ++----------------
> drivers/gpu/drm/exynos/exynos_drm_fb.h | 7 ++--
> 3 files changed, 49 insertions(+), 39 deletions(-)
>
> diff --git a/drivers/gpu/drm/exynos/exynos_drm_drv.c b/drivers/gpu/drm/exynos/exynos_drm_drv.c
> index 1aea71778ab1..6362cd417a4e 100644
> --- a/drivers/gpu/drm/exynos/exynos_drm_drv.c
> +++ b/drivers/gpu/drm/exynos/exynos_drm_drv.c
> @@ -233,6 +233,43 @@ static struct component_match *exynos_drm_match_add(struct device *dev)
> return match ?: ERR_PTR(-ENODEV);
> }
>
> +static struct drm_mode_config_helper_funcs exynos_drm_mode_config_helpers = {
> + .atomic_commit_tail = drm_atomic_helper_commit_tail_rpm,
> +};
> +
> +static const struct drm_mode_config_funcs exynos_drm_mode_config_funcs = {
> + .fb_create = exynos_user_fb_create,
> + .atomic_check = drm_atomic_helper_check,
> + .atomic_commit = drm_atomic_helper_commit,
> +};
> +
> +static int exynos_drm_mode_config_init(struct drm_device *dev)
> +{
> + int ret;
> +
> + ret = drmm_mode_config_init(dev);
> + if (ret)
> + return ret;
> +
> + dev->mode_config.min_width = 0;
> + dev->mode_config.min_height = 0;
> +
> + /*
> + * set max width and height as default value(4096x4096).
> + * this value would be used to check framebuffer size limitation
> + * at drm_mode_addfb().
> + */
> + dev->mode_config.max_width = 4096;
> + dev->mode_config.max_height = 4096;
> +
> + dev->mode_config.funcs = &exynos_drm_mode_config_funcs;
> + dev->mode_config.helper_private = &exynos_drm_mode_config_helpers;
> +
> + dev->mode_config.normalize_zpos = true;
> +
> + return 0;
> +}
> +
> static int exynos_drm_bind(struct device *dev)
> {
> struct exynos_drm_private *private;
> @@ -257,9 +294,9 @@ static int exynos_drm_bind(struct device *dev)
> dev_set_drvdata(dev, drm);
> drm->dev_private = (void *)private;
>
> - drmm_mode_config_init(drm);
> -
> - exynos_drm_mode_config_init(drm);
> + ret = exynos_drm_mode_config_init(drm);
> + if (ret)
> + goto err_free_private;
>
> /* setup possible_clones. */
> clone_mask = 0;
> @@ -272,7 +309,7 @@ static int exynos_drm_bind(struct device *dev)
> /* Try to bind all sub drivers. */
> ret = component_bind_all(drm->dev, drm);
> if (ret)
> - goto err_mode_config_cleanup;
> + goto err_free_private;
>
> ret = drm_vblank_init(drm, drm->mode_config.num_crtc);
> if (ret)
> @@ -296,7 +333,7 @@ static int exynos_drm_bind(struct device *dev)
> drm_kms_helper_poll_fini(drm);
> err_unbind_all:
> component_unbind_all(drm->dev, drm);
> -err_mode_config_cleanup:
> +err_free_private:
> exynos_drm_cleanup_dma(drm);
> kfree(private);
> dev_set_drvdata(dev, NULL);
> diff --git a/drivers/gpu/drm/exynos/exynos_drm_fb.c b/drivers/gpu/drm/exynos/exynos_drm_fb.c
> index ddd73e7f26a3..c118a079d308 100644
> --- a/drivers/gpu/drm/exynos/exynos_drm_fb.c
> +++ b/drivers/gpu/drm/exynos/exynos_drm_fb.c
> @@ -8,8 +8,7 @@
> * Seung-Woo Kim <sw0312.kim@...sung.com>
> */
>
> -#include <drm/drm_atomic.h>
> -#include <drm/drm_atomic_helper.h>
> +#include <drm/drm_modeset_helper.h>
> #include <drm/drm_crtc.h>
> #include <drm/drm_framebuffer.h>
> #include <drm/drm_fourcc.h>
> @@ -93,7 +92,7 @@ exynos_drm_framebuffer_init(struct drm_device *dev,
> return ERR_PTR(ret);
> }
>
> -static struct drm_framebuffer *
> +struct drm_framebuffer *
> exynos_user_fb_create(struct drm_device *dev, struct drm_file *file_priv,
> const struct drm_format_info *info,
> const struct drm_mode_fb_cmd2 *mode_cmd)
> @@ -150,32 +149,3 @@ dma_addr_t exynos_drm_fb_dma_addr(struct drm_framebuffer *fb, int index)
> exynos_gem = to_exynos_gem(fb->obj[index]);
> return exynos_gem->dma_addr + fb->offsets[index];
> }
> -
> -static struct drm_mode_config_helper_funcs exynos_drm_mode_config_helpers = {
> - .atomic_commit_tail = drm_atomic_helper_commit_tail_rpm,
> -};
> -
> -static const struct drm_mode_config_funcs exynos_drm_mode_config_funcs = {
> - .fb_create = exynos_user_fb_create,
> - .atomic_check = drm_atomic_helper_check,
> - .atomic_commit = drm_atomic_helper_commit,
> -};
> -
> -void exynos_drm_mode_config_init(struct drm_device *dev)
> -{
> - dev->mode_config.min_width = 0;
> - dev->mode_config.min_height = 0;
> -
> - /*
> - * set max width and height as default value(4096x4096).
> - * this value would be used to check framebuffer size limitation
> - * at drm_mode_addfb().
> - */
> - dev->mode_config.max_width = 4096;
> - dev->mode_config.max_height = 4096;
> -
> - dev->mode_config.funcs = &exynos_drm_mode_config_funcs;
> - dev->mode_config.helper_private = &exynos_drm_mode_config_helpers;
> -
> - dev->mode_config.normalize_zpos = true;
> -}
> diff --git a/drivers/gpu/drm/exynos/exynos_drm_fb.h b/drivers/gpu/drm/exynos/exynos_drm_fb.h
> index fdc6cb40cc9c..0c79ce5d4a8d 100644
> --- a/drivers/gpu/drm/exynos/exynos_drm_fb.h
> +++ b/drivers/gpu/drm/exynos/exynos_drm_fb.h
> @@ -19,8 +19,11 @@ exynos_drm_framebuffer_init(struct drm_device *dev,
> struct exynos_drm_gem **exynos_gem,
> int count);
>
> -dma_addr_t exynos_drm_fb_dma_addr(struct drm_framebuffer *fb, int index);
> +struct drm_framebuffer *
> +exynos_user_fb_create(struct drm_device *dev, struct drm_file *file_priv,
> + const struct drm_format_info *info,
> + const struct drm_mode_fb_cmd2 *mode_cmd);
>
> -void exynos_drm_mode_config_init(struct drm_device *dev);
> +dma_addr_t exynos_drm_fb_dma_addr(struct drm_framebuffer *fb, int index);
>
> #endif
> --
> 2.34.1
>
>
Powered by blists - more mailing lists