* Re: [PATCH 4/6] drm/cma-helper: Support DRM_MODE_DUMB_KERNEL_MAP flag
2019-10-21 21:45 ` [PATCH 4/6] drm/cma-helper: Support DRM_MODE_DUMB_KERNEL_MAP flag Rob Herring
@ 2019-10-22 6:13 ` james qian wang (Arm Technology China)
2019-10-22 7:50 ` Neil Armstrong
2019-10-22 11:30 ` Laurent Pinchart
2 siblings, 0 replies; 8+ messages in thread
From: james qian wang (Arm Technology China) @ 2019-10-22 6:13 UTC (permalink / raw)
To: Rob Herring
Cc: Heiko Stübner, Neil Armstrong, David Airlie, Liviu Dudau,
dri-devel, Sandy Huang, Laurent Pinchart, Benjamin Gaignard,
linux-amlogic, linux-stm32, Yannick Fertre, Kevin Hilman,
Xinwei Kong, Xinliang Liu, linux-rockchip, Chen-Yu Tsai,
Noralf Trønnes, CK Hu, Philipp Zabel, Alexandre Torgue,
Chen Feng, Maarten Lankhorst, linux-renesas-soc, Maxime Ripard,
linux-mediatek, Matthias Brugger, Daniel Vetter, nd, Sean Paul,
linux-arm-kernel, Philippe Cornu, Vincent Abriou, Kieran Bingham,
Maxime Coquelin, Rongrong Zou, Brian Starkey
On Mon, Oct 21, 2019 at 04:45:48PM -0500, Rob Herring wrote:
> Add support in CMA helpers to handle callers specifying
> DRM_MODE_DUMB_KERNEL_MAP flag. Existing behavior is maintained with this
> change. drm_gem_cma_dumb_create() always creates a kernel mapping as
> before. drm_gem_cma_dumb_create_internal() lets the caller set the flags
> as desired. Therefore, update all the existing callers of
> drm_gem_cma_dumb_create_internal() to also set the
> DRM_MODE_DUMB_KERNEL_MAP flag.
>
> Cc: Maarten Lankhorst <maarten.lankhorst@linux.intel.com>
> Cc: Maxime Ripard <mripard@kernel.org>
> Cc: Sean Paul <sean@poorly.run>
> Cc: David Airlie <airlied@linux.ie>
> Cc: Daniel Vetter <daniel@ffwll.ch>
> Cc: "James (Qian) Wang" <james.qian.wang@arm.com>
> Cc: Liviu Dudau <liviu.dudau@arm.com>
> Cc: Brian Starkey <brian.starkey@arm.com>
> Cc: Neil Armstrong <narmstrong@baylibre.com>
> Cc: Kevin Hilman <khilman@baylibre.com>
> Cc: Laurent Pinchart <laurent.pinchart@ideasonboard.com>
> Cc: Kieran Bingham <kieran.bingham+renesas@ideasonboard.com>
> Cc: Sandy Huang <hjc@rock-chips.com>
> Cc: "Heiko Stübner" <heiko@sntech.de>
> Cc: Yannick Fertre <yannick.fertre@st.com>
> Cc: Philippe Cornu <philippe.cornu@st.com>
> Cc: Benjamin Gaignard <benjamin.gaignard@linaro.org>
> Cc: Vincent Abriou <vincent.abriou@st.com>
> Cc: Maxime Coquelin <mcoquelin.stm32@gmail.com>
> Cc: Alexandre Torgue <alexandre.torgue@st.com>
> Cc: Chen-Yu Tsai <wens@csie.org>
> Cc: linux-amlogic@lists.infradead.org
> Cc: linux-arm-kernel@lists.infradead.org
> Cc: linux-renesas-soc@vger.kernel.org
> Cc: linux-rockchip@lists.infradead.org
> Cc: linux-stm32@st-md-mailman.stormreply.com
> Signed-off-by: Rob Herring <robh@kernel.org>
> ---
> .../gpu/drm/arm/display/komeda/komeda_kms.c | 1 +
> drivers/gpu/drm/arm/malidp_drv.c | 1 +
> drivers/gpu/drm/drm_gem_cma_helper.c | 48 +++++++++++--------
> drivers/gpu/drm/meson/meson_drv.c | 1 +
> drivers/gpu/drm/rcar-du/rcar_du_kms.c | 1 +
> drivers/gpu/drm/rockchip/rockchip_drm_gem.c | 1 +
> drivers/gpu/drm/stm/drv.c | 1 +
> drivers/gpu/drm/sun4i/sun4i_drv.c | 1 +
> 8 files changed, 36 insertions(+), 19 deletions(-)
>
> diff --git a/drivers/gpu/drm/arm/display/komeda/komeda_kms.c b/drivers/gpu/drm/arm/display/komeda/komeda_kms.c
> index d49772de93e0..7cf0dc4cbfc1 100644
> --- a/drivers/gpu/drm/arm/display/komeda/komeda_kms.c
> +++ b/drivers/gpu/drm/arm/display/komeda/komeda_kms.c
> @@ -31,6 +31,7 @@ static int komeda_gem_cma_dumb_create(struct drm_file *file,
> u32 pitch = DIV_ROUND_UP(args->width * args->bpp, 8);
>
> args->pitch = ALIGN(pitch, mdev->chip.bus_width);
> + args->flags = DRM_MODE_DUMB_KERNEL_MAP;
>
Hi Rob:
komeda doesn't need the kernel map either, so you can del this line for komeda.
and with this.
Reviewed-by: James Qian Wang (Arm Technology China) <james.qian.wang@arm.com>
> return drm_gem_cma_dumb_create_internal(file, dev, args);
> }
> diff --git a/drivers/gpu/drm/arm/malidp_drv.c b/drivers/gpu/drm/arm/malidp_drv.c
> index 8a76315aaa0f..aeb1a779ecc1 100644
> --- a/drivers/gpu/drm/arm/malidp_drv.c
> +++ b/drivers/gpu/drm/arm/malidp_drv.c
> @@ -465,6 +465,7 @@ static int malidp_dumb_create(struct drm_file *file_priv,
> u8 alignment = malidp_hw_get_pitch_align(malidp->dev, 1);
>
> args->pitch = ALIGN(DIV_ROUND_UP(args->width * args->bpp, 8), alignment);
> + args->flags = DRM_MODE_DUMB_KERNEL_MAP;
>
> return drm_gem_cma_dumb_create_internal(file_priv, drm, args);
> }
> diff --git a/drivers/gpu/drm/drm_gem_cma_helper.c b/drivers/gpu/drm/drm_gem_cma_helper.c
> index 4cebfe01e6ea..f91e9e8adeaf 100644
> --- a/drivers/gpu/drm/drm_gem_cma_helper.c
> +++ b/drivers/gpu/drm/drm_gem_cma_helper.c
> @@ -78,21 +78,8 @@ __drm_gem_cma_create(struct drm_device *drm, size_t size)
> return ERR_PTR(ret);
> }
>
> -/**
> - * drm_gem_cma_create - allocate an object with the given size
> - * @drm: DRM device
> - * @size: size of the object to allocate
> - *
> - * This function creates a CMA GEM object and allocates a contiguous chunk of
> - * memory as backing store. The backing memory has the writecombine attribute
> - * set.
> - *
> - * Returns:
> - * A struct drm_gem_cma_object * on success or an ERR_PTR()-encoded negative
> - * error code on failure.
> - */
> -struct drm_gem_cma_object *drm_gem_cma_create(struct drm_device *drm,
> - size_t size)
> +static struct drm_gem_cma_object *
> +drm_gem_cma_create_flags(struct drm_device *drm, size_t size, u32 flags)
> {
> struct drm_gem_cma_object *cma_obj;
> int ret;
> @@ -103,6 +90,9 @@ struct drm_gem_cma_object *drm_gem_cma_create(struct drm_device *drm,
> if (IS_ERR(cma_obj))
> return cma_obj;
>
> + if (!(flags & DRM_MODE_DUMB_KERNEL_MAP))
> + cma_obj->dma_attrs |= DMA_ATTR_NO_KERNEL_MAPPING;
> +
> cma_obj->vaddr = dma_alloc_attrs(drm->dev, size, &cma_obj->paddr,
> GFP_KERNEL | __GFP_NOWARN,
> cma_obj->dma_attrs);
> @@ -119,6 +109,25 @@ struct drm_gem_cma_object *drm_gem_cma_create(struct drm_device *drm,
> drm_gem_object_put_unlocked(&cma_obj->base);
> return ERR_PTR(ret);
> }
> +
> +/**
> + * drm_gem_cma_create - allocate an object with the given size
> + * @drm: DRM device
> + * @size: size of the object to allocate
> + *
> + * This function creates a CMA GEM object and allocates a contiguous chunk of
> + * memory as backing store. The backing memory has the writecombine attribute
> + * set.
> + *
> + * Returns:
> + * A struct drm_gem_cma_object * on success or an ERR_PTR()-encoded negative
> + * error code on failure.
> + */
> +struct drm_gem_cma_object *drm_gem_cma_create(struct drm_device *drm,
> + size_t size)
> +{
> + return drm_gem_cma_create_flags(drm, size, DRM_MODE_DUMB_KERNEL_MAP);
> +}
> EXPORT_SYMBOL_GPL(drm_gem_cma_create);
>
> /**
> @@ -139,14 +148,14 @@ EXPORT_SYMBOL_GPL(drm_gem_cma_create);
> */
> static struct drm_gem_cma_object *
> drm_gem_cma_create_with_handle(struct drm_file *file_priv,
> - struct drm_device *drm, size_t size,
> + struct drm_device *drm, size_t size, u32 flags,
> uint32_t *handle)
> {
> struct drm_gem_cma_object *cma_obj;
> struct drm_gem_object *gem_obj;
> int ret;
>
> - cma_obj = drm_gem_cma_create(drm, size);
> + cma_obj = drm_gem_cma_create_flags(drm, size, flags);
> if (IS_ERR(cma_obj))
> return cma_obj;
>
> @@ -225,7 +234,7 @@ int drm_gem_cma_dumb_create_internal(struct drm_file *file_priv,
> args->size = args->pitch * args->height;
>
> cma_obj = drm_gem_cma_create_with_handle(file_priv, drm, args->size,
> - &args->handle);
> + args->flags, &args->handle);
> return PTR_ERR_OR_ZERO(cma_obj);
> }
> EXPORT_SYMBOL_GPL(drm_gem_cma_dumb_create_internal);
> @@ -256,9 +265,10 @@ int drm_gem_cma_dumb_create(struct drm_file *file_priv,
>
> args->pitch = DIV_ROUND_UP(args->width * args->bpp, 8);
> args->size = args->pitch * args->height;
> + args->flags = DRM_MODE_DUMB_KERNEL_MAP;
>
> cma_obj = drm_gem_cma_create_with_handle(file_priv, drm, args->size,
> - &args->handle);
> + args->flags, &args->handle);
> return PTR_ERR_OR_ZERO(cma_obj);
> }
> EXPORT_SYMBOL_GPL(drm_gem_cma_dumb_create);
> diff --git a/drivers/gpu/drm/meson/meson_drv.c b/drivers/gpu/drm/meson/meson_drv.c
> index 397c33182f4f..1593518dcbe4 100644
> --- a/drivers/gpu/drm/meson/meson_drv.c
> +++ b/drivers/gpu/drm/meson/meson_drv.c
> @@ -81,6 +81,7 @@ static int meson_dumb_create(struct drm_file *file, struct drm_device *dev,
> */
> args->pitch = ALIGN(DIV_ROUND_UP(args->width * args->bpp, 8), SZ_64);
> args->size = PAGE_ALIGN(args->pitch * args->height);
> + args->flags = DRM_MODE_DUMB_KERNEL_MAP;
>
> return drm_gem_cma_dumb_create_internal(file, dev, args);
> }
> diff --git a/drivers/gpu/drm/rcar-du/rcar_du_kms.c b/drivers/gpu/drm/rcar-du/rcar_du_kms.c
> index 2dc9caee8767..c9b1f298ce7e 100644
> --- a/drivers/gpu/drm/rcar-du/rcar_du_kms.c
> +++ b/drivers/gpu/drm/rcar-du/rcar_du_kms.c
> @@ -299,6 +299,7 @@ int rcar_du_dumb_create(struct drm_file *file, struct drm_device *dev,
> align = 16 * args->bpp / 8;
>
> args->pitch = roundup(min_pitch, align);
> + args->flags = DRM_MODE_DUMB_KERNEL_MAP;
>
> return drm_gem_cma_dumb_create_internal(file, dev, args);
> }
> diff --git a/drivers/gpu/drm/rockchip/rockchip_drm_gem.c b/drivers/gpu/drm/rockchip/rockchip_drm_gem.c
> index 7582d0e6a60a..f09b9a035376 100644
> --- a/drivers/gpu/drm/rockchip/rockchip_drm_gem.c
> +++ b/drivers/gpu/drm/rockchip/rockchip_drm_gem.c
> @@ -419,6 +419,7 @@ int rockchip_gem_dumb_create(struct drm_file *file_priv,
> * align to 64 bytes since Mali requires it.
> */
> args->pitch = ALIGN(min_pitch, 64);
> + args->flags = DRM_MODE_DUMB_KERNEL_MAP;
> args->size = args->pitch * args->height;
>
> rk_obj = rockchip_gem_create_with_handle(file_priv, dev, args->size,
> diff --git a/drivers/gpu/drm/stm/drv.c b/drivers/gpu/drm/stm/drv.c
> index 5a9f9aca8bc2..0f76a4ac95b3 100644
> --- a/drivers/gpu/drm/stm/drv.c
> +++ b/drivers/gpu/drm/stm/drv.c
> @@ -47,6 +47,7 @@ static int stm_gem_cma_dumb_create(struct drm_file *file,
> */
> args->pitch = roundup(min_pitch, 128);
> args->height = roundup(args->height, 4);
> + args->flags = DRM_MODE_DUMB_KERNEL_MAP;
>
> return drm_gem_cma_dumb_create_internal(file, dev, args);
> }
> diff --git a/drivers/gpu/drm/sun4i/sun4i_drv.c b/drivers/gpu/drm/sun4i/sun4i_drv.c
> index a5757b11b730..f653a5d1e2d6 100644
> --- a/drivers/gpu/drm/sun4i/sun4i_drv.c
> +++ b/drivers/gpu/drm/sun4i/sun4i_drv.c
> @@ -34,6 +34,7 @@ static int drm_sun4i_gem_dumb_create(struct drm_file *file_priv,
> {
> /* The hardware only allows even pitches for YUV buffers. */
> args->pitch = ALIGN(DIV_ROUND_UP(args->width * args->bpp, 8), 2);
> + args->flags = DRM_MODE_DUMB_KERNEL_MAP;
>
> return drm_gem_cma_dumb_create_internal(file_priv, drm, args);
> }
> --
> 2.20.1
_______________________________________________
linux-amlogic mailing list
linux-amlogic@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-amlogic
^ permalink raw reply [flat|nested] 8+ messages in thread* Re: [PATCH 4/6] drm/cma-helper: Support DRM_MODE_DUMB_KERNEL_MAP flag
2019-10-21 21:45 ` [PATCH 4/6] drm/cma-helper: Support DRM_MODE_DUMB_KERNEL_MAP flag Rob Herring
2019-10-22 6:13 ` james qian wang (Arm Technology China)
@ 2019-10-22 7:50 ` Neil Armstrong
2019-10-22 11:30 ` Laurent Pinchart
2 siblings, 0 replies; 8+ messages in thread
From: Neil Armstrong @ 2019-10-22 7:50 UTC (permalink / raw)
To: Rob Herring, dri-devel
Cc: Heiko Stübner, David Airlie, Liviu Dudau, Sandy Huang,
Laurent Pinchart, Benjamin Gaignard, linux-amlogic, linux-stm32,
Yannick Fertre, Kevin Hilman, Xinwei Kong, Xinliang Liu,
linux-rockchip, Chen-Yu Tsai, Noralf Trønnes,
James (Qian) Wang, CK Hu, Philipp Zabel, Alexandre Torgue,
Chen Feng, Maarten Lankhorst, linux-renesas-soc, Maxime Ripard,
linux-mediatek, Matthias Brugger, Daniel Vetter, Sean Paul,
linux-arm-kernel, Philippe Cornu, Vincent Abriou, Kieran Bingham,
Maxime Coquelin, Rongrong Zou, Brian Starkey
On 21/10/2019 23:45, Rob Herring wrote:
> Add support in CMA helpers to handle callers specifying
> DRM_MODE_DUMB_KERNEL_MAP flag. Existing behavior is maintained with this
> change. drm_gem_cma_dumb_create() always creates a kernel mapping as
> before. drm_gem_cma_dumb_create_internal() lets the caller set the flags
> as desired. Therefore, update all the existing callers of
> drm_gem_cma_dumb_create_internal() to also set the
> DRM_MODE_DUMB_KERNEL_MAP flag.
>
> Cc: Maarten Lankhorst <maarten.lankhorst@linux.intel.com>
> Cc: Maxime Ripard <mripard@kernel.org>
> Cc: Sean Paul <sean@poorly.run>
> Cc: David Airlie <airlied@linux.ie>
> Cc: Daniel Vetter <daniel@ffwll.ch>
> Cc: "James (Qian) Wang" <james.qian.wang@arm.com>
> Cc: Liviu Dudau <liviu.dudau@arm.com>
> Cc: Brian Starkey <brian.starkey@arm.com>
> Cc: Neil Armstrong <narmstrong@baylibre.com>
> Cc: Kevin Hilman <khilman@baylibre.com>
> Cc: Laurent Pinchart <laurent.pinchart@ideasonboard.com>
> Cc: Kieran Bingham <kieran.bingham+renesas@ideasonboard.com>
> Cc: Sandy Huang <hjc@rock-chips.com>
> Cc: "Heiko Stübner" <heiko@sntech.de>
> Cc: Yannick Fertre <yannick.fertre@st.com>
> Cc: Philippe Cornu <philippe.cornu@st.com>
> Cc: Benjamin Gaignard <benjamin.gaignard@linaro.org>
> Cc: Vincent Abriou <vincent.abriou@st.com>
> Cc: Maxime Coquelin <mcoquelin.stm32@gmail.com>
> Cc: Alexandre Torgue <alexandre.torgue@st.com>
> Cc: Chen-Yu Tsai <wens@csie.org>
> Cc: linux-amlogic@lists.infradead.org
> Cc: linux-arm-kernel@lists.infradead.org
> Cc: linux-renesas-soc@vger.kernel.org
> Cc: linux-rockchip@lists.infradead.org
> Cc: linux-stm32@st-md-mailman.stormreply.com
> Signed-off-by: Rob Herring <robh@kernel.org>
> ---
> .../gpu/drm/arm/display/komeda/komeda_kms.c | 1 +
> drivers/gpu/drm/arm/malidp_drv.c | 1 +
> drivers/gpu/drm/drm_gem_cma_helper.c | 48 +++++++++++--------
> drivers/gpu/drm/meson/meson_drv.c | 1 +
> drivers/gpu/drm/rcar-du/rcar_du_kms.c | 1 +
> drivers/gpu/drm/rockchip/rockchip_drm_gem.c | 1 +
> drivers/gpu/drm/stm/drv.c | 1 +
> drivers/gpu/drm/sun4i/sun4i_drv.c | 1 +
> 8 files changed, 36 insertions(+), 19 deletions(-)
>
> diff --git a/drivers/gpu/drm/arm/display/komeda/komeda_kms.c b/drivers/gpu/drm/arm/display/komeda/komeda_kms.c
> index d49772de93e0..7cf0dc4cbfc1 100644
> --- a/drivers/gpu/drm/arm/display/komeda/komeda_kms.c
> +++ b/drivers/gpu/drm/arm/display/komeda/komeda_kms.c
> @@ -31,6 +31,7 @@ static int komeda_gem_cma_dumb_create(struct drm_file *file,
> u32 pitch = DIV_ROUND_UP(args->width * args->bpp, 8);
>
> args->pitch = ALIGN(pitch, mdev->chip.bus_width);
> + args->flags = DRM_MODE_DUMB_KERNEL_MAP;
>
> return drm_gem_cma_dumb_create_internal(file, dev, args);
> }
> diff --git a/drivers/gpu/drm/arm/malidp_drv.c b/drivers/gpu/drm/arm/malidp_drv.c
> index 8a76315aaa0f..aeb1a779ecc1 100644
> --- a/drivers/gpu/drm/arm/malidp_drv.c
> +++ b/drivers/gpu/drm/arm/malidp_drv.c
> @@ -465,6 +465,7 @@ static int malidp_dumb_create(struct drm_file *file_priv,
> u8 alignment = malidp_hw_get_pitch_align(malidp->dev, 1);
>
> args->pitch = ALIGN(DIV_ROUND_UP(args->width * args->bpp, 8), alignment);
> + args->flags = DRM_MODE_DUMB_KERNEL_MAP;
>
> return drm_gem_cma_dumb_create_internal(file_priv, drm, args);
> }
> diff --git a/drivers/gpu/drm/drm_gem_cma_helper.c b/drivers/gpu/drm/drm_gem_cma_helper.c
> index 4cebfe01e6ea..f91e9e8adeaf 100644
> --- a/drivers/gpu/drm/drm_gem_cma_helper.c
> +++ b/drivers/gpu/drm/drm_gem_cma_helper.c
> @@ -78,21 +78,8 @@ __drm_gem_cma_create(struct drm_device *drm, size_t size)
> return ERR_PTR(ret);
> }
>
> -/**
> - * drm_gem_cma_create - allocate an object with the given size
> - * @drm: DRM device
> - * @size: size of the object to allocate
> - *
> - * This function creates a CMA GEM object and allocates a contiguous chunk of
> - * memory as backing store. The backing memory has the writecombine attribute
> - * set.
> - *
> - * Returns:
> - * A struct drm_gem_cma_object * on success or an ERR_PTR()-encoded negative
> - * error code on failure.
> - */
> -struct drm_gem_cma_object *drm_gem_cma_create(struct drm_device *drm,
> - size_t size)
> +static struct drm_gem_cma_object *
> +drm_gem_cma_create_flags(struct drm_device *drm, size_t size, u32 flags)
> {
> struct drm_gem_cma_object *cma_obj;
> int ret;
> @@ -103,6 +90,9 @@ struct drm_gem_cma_object *drm_gem_cma_create(struct drm_device *drm,
> if (IS_ERR(cma_obj))
> return cma_obj;
>
> + if (!(flags & DRM_MODE_DUMB_KERNEL_MAP))
> + cma_obj->dma_attrs |= DMA_ATTR_NO_KERNEL_MAPPING;
> +
> cma_obj->vaddr = dma_alloc_attrs(drm->dev, size, &cma_obj->paddr,
> GFP_KERNEL | __GFP_NOWARN,
> cma_obj->dma_attrs);
> @@ -119,6 +109,25 @@ struct drm_gem_cma_object *drm_gem_cma_create(struct drm_device *drm,
> drm_gem_object_put_unlocked(&cma_obj->base);
> return ERR_PTR(ret);
> }
> +
> +/**
> + * drm_gem_cma_create - allocate an object with the given size
> + * @drm: DRM device
> + * @size: size of the object to allocate
> + *
> + * This function creates a CMA GEM object and allocates a contiguous chunk of
> + * memory as backing store. The backing memory has the writecombine attribute
> + * set.
> + *
> + * Returns:
> + * A struct drm_gem_cma_object * on success or an ERR_PTR()-encoded negative
> + * error code on failure.
> + */
> +struct drm_gem_cma_object *drm_gem_cma_create(struct drm_device *drm,
> + size_t size)
> +{
> + return drm_gem_cma_create_flags(drm, size, DRM_MODE_DUMB_KERNEL_MAP);
> +}
> EXPORT_SYMBOL_GPL(drm_gem_cma_create);
>
> /**
> @@ -139,14 +148,14 @@ EXPORT_SYMBOL_GPL(drm_gem_cma_create);
> */
> static struct drm_gem_cma_object *
> drm_gem_cma_create_with_handle(struct drm_file *file_priv,
> - struct drm_device *drm, size_t size,
> + struct drm_device *drm, size_t size, u32 flags,
> uint32_t *handle)
> {
> struct drm_gem_cma_object *cma_obj;
> struct drm_gem_object *gem_obj;
> int ret;
>
> - cma_obj = drm_gem_cma_create(drm, size);
> + cma_obj = drm_gem_cma_create_flags(drm, size, flags);
> if (IS_ERR(cma_obj))
> return cma_obj;
>
> @@ -225,7 +234,7 @@ int drm_gem_cma_dumb_create_internal(struct drm_file *file_priv,
> args->size = args->pitch * args->height;
>
> cma_obj = drm_gem_cma_create_with_handle(file_priv, drm, args->size,
> - &args->handle);
> + args->flags, &args->handle);
> return PTR_ERR_OR_ZERO(cma_obj);
> }
> EXPORT_SYMBOL_GPL(drm_gem_cma_dumb_create_internal);
> @@ -256,9 +265,10 @@ int drm_gem_cma_dumb_create(struct drm_file *file_priv,
>
> args->pitch = DIV_ROUND_UP(args->width * args->bpp, 8);
> args->size = args->pitch * args->height;
> + args->flags = DRM_MODE_DUMB_KERNEL_MAP;
>
> cma_obj = drm_gem_cma_create_with_handle(file_priv, drm, args->size,
> - &args->handle);
> + args->flags, &args->handle);
> return PTR_ERR_OR_ZERO(cma_obj);
> }
> EXPORT_SYMBOL_GPL(drm_gem_cma_dumb_create);
> diff --git a/drivers/gpu/drm/meson/meson_drv.c b/drivers/gpu/drm/meson/meson_drv.c
> index 397c33182f4f..1593518dcbe4 100644
> --- a/drivers/gpu/drm/meson/meson_drv.c
> +++ b/drivers/gpu/drm/meson/meson_drv.c
> @@ -81,6 +81,7 @@ static int meson_dumb_create(struct drm_file *file, struct drm_device *dev,
> */
> args->pitch = ALIGN(DIV_ROUND_UP(args->width * args->bpp, 8), SZ_64);
> args->size = PAGE_ALIGN(args->pitch * args->height);
> + args->flags = DRM_MODE_DUMB_KERNEL_MAP;
>
> return drm_gem_cma_dumb_create_internal(file, dev, args);
> }
> diff --git a/drivers/gpu/drm/rcar-du/rcar_du_kms.c b/drivers/gpu/drm/rcar-du/rcar_du_kms.c
> index 2dc9caee8767..c9b1f298ce7e 100644
> --- a/drivers/gpu/drm/rcar-du/rcar_du_kms.c
> +++ b/drivers/gpu/drm/rcar-du/rcar_du_kms.c
> @@ -299,6 +299,7 @@ int rcar_du_dumb_create(struct drm_file *file, struct drm_device *dev,
> align = 16 * args->bpp / 8;
>
> args->pitch = roundup(min_pitch, align);
> + args->flags = DRM_MODE_DUMB_KERNEL_MAP;
>
> return drm_gem_cma_dumb_create_internal(file, dev, args);
> }
> diff --git a/drivers/gpu/drm/rockchip/rockchip_drm_gem.c b/drivers/gpu/drm/rockchip/rockchip_drm_gem.c
> index 7582d0e6a60a..f09b9a035376 100644
> --- a/drivers/gpu/drm/rockchip/rockchip_drm_gem.c
> +++ b/drivers/gpu/drm/rockchip/rockchip_drm_gem.c
> @@ -419,6 +419,7 @@ int rockchip_gem_dumb_create(struct drm_file *file_priv,
> * align to 64 bytes since Mali requires it.
> */
> args->pitch = ALIGN(min_pitch, 64);
> + args->flags = DRM_MODE_DUMB_KERNEL_MAP;
> args->size = args->pitch * args->height;
>
> rk_obj = rockchip_gem_create_with_handle(file_priv, dev, args->size,
> diff --git a/drivers/gpu/drm/stm/drv.c b/drivers/gpu/drm/stm/drv.c
> index 5a9f9aca8bc2..0f76a4ac95b3 100644
> --- a/drivers/gpu/drm/stm/drv.c
> +++ b/drivers/gpu/drm/stm/drv.c
> @@ -47,6 +47,7 @@ static int stm_gem_cma_dumb_create(struct drm_file *file,
> */
> args->pitch = roundup(min_pitch, 128);
> args->height = roundup(args->height, 4);
> + args->flags = DRM_MODE_DUMB_KERNEL_MAP;
>
> return drm_gem_cma_dumb_create_internal(file, dev, args);
> }
> diff --git a/drivers/gpu/drm/sun4i/sun4i_drv.c b/drivers/gpu/drm/sun4i/sun4i_drv.c
> index a5757b11b730..f653a5d1e2d6 100644
> --- a/drivers/gpu/drm/sun4i/sun4i_drv.c
> +++ b/drivers/gpu/drm/sun4i/sun4i_drv.c
> @@ -34,6 +34,7 @@ static int drm_sun4i_gem_dumb_create(struct drm_file *file_priv,
> {
> /* The hardware only allows even pitches for YUV buffers. */
> args->pitch = ALIGN(DIV_ROUND_UP(args->width * args->bpp, 8), 2);
> + args->flags = DRM_MODE_DUMB_KERNEL_MAP;
>
> return drm_gem_cma_dumb_create_internal(file_priv, drm, args);
> }
>
Reviewed-by: Neil Armstrong <narmstrong@baylibre.com> # for drm_gem_cma_helper and meson drm driver
_______________________________________________
linux-amlogic mailing list
linux-amlogic@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-amlogic
^ permalink raw reply [flat|nested] 8+ messages in thread* Re: [PATCH 4/6] drm/cma-helper: Support DRM_MODE_DUMB_KERNEL_MAP flag
2019-10-21 21:45 ` [PATCH 4/6] drm/cma-helper: Support DRM_MODE_DUMB_KERNEL_MAP flag Rob Herring
2019-10-22 6:13 ` james qian wang (Arm Technology China)
2019-10-22 7:50 ` Neil Armstrong
@ 2019-10-22 11:30 ` Laurent Pinchart
2019-10-22 11:40 ` Geert Uytterhoeven
2019-10-22 20:02 ` Rob Herring
2 siblings, 2 replies; 8+ messages in thread
From: Laurent Pinchart @ 2019-10-22 11:30 UTC (permalink / raw)
To: Rob Herring
Cc: Heiko Stübner, Neil Armstrong, David Airlie, Liviu Dudau,
dri-devel, Sandy Huang, Benjamin Gaignard, linux-amlogic,
linux-stm32, Yannick Fertre, Kevin Hilman, Xinwei Kong,
Xinliang Liu, linux-rockchip, Chen-Yu Tsai, Noralf Trønnes,
James (Qian) Wang, CK Hu, Philipp Zabel, Alexandre Torgue,
Chen Feng, Maarten Lankhorst, linux-renesas-soc, Maxime Ripard,
linux-mediatek, Matthias Brugger, Daniel Vetter, Sean Paul,
linux-arm-kernel, Philippe Cornu, Vincent Abriou, Kieran Bingham,
Maxime Coquelin, Rongrong Zou, Brian Starkey
Hi Rob,
Thank you for the patch.
On Mon, Oct 21, 2019 at 04:45:48PM -0500, Rob Herring wrote:
> Add support in CMA helpers to handle callers specifying
> DRM_MODE_DUMB_KERNEL_MAP flag. Existing behavior is maintained with this
> change. drm_gem_cma_dumb_create() always creates a kernel mapping as
> before. drm_gem_cma_dumb_create_internal() lets the caller set the flags
> as desired. Therefore, update all the existing callers of
> drm_gem_cma_dumb_create_internal() to also set the
> DRM_MODE_DUMB_KERNEL_MAP flag.
>
> Cc: Maarten Lankhorst <maarten.lankhorst@linux.intel.com>
> Cc: Maxime Ripard <mripard@kernel.org>
> Cc: Sean Paul <sean@poorly.run>
> Cc: David Airlie <airlied@linux.ie>
> Cc: Daniel Vetter <daniel@ffwll.ch>
> Cc: "James (Qian) Wang" <james.qian.wang@arm.com>
> Cc: Liviu Dudau <liviu.dudau@arm.com>
> Cc: Brian Starkey <brian.starkey@arm.com>
> Cc: Neil Armstrong <narmstrong@baylibre.com>
> Cc: Kevin Hilman <khilman@baylibre.com>
> Cc: Laurent Pinchart <laurent.pinchart@ideasonboard.com>
> Cc: Kieran Bingham <kieran.bingham+renesas@ideasonboard.com>
> Cc: Sandy Huang <hjc@rock-chips.com>
> Cc: "Heiko Stübner" <heiko@sntech.de>
> Cc: Yannick Fertre <yannick.fertre@st.com>
> Cc: Philippe Cornu <philippe.cornu@st.com>
> Cc: Benjamin Gaignard <benjamin.gaignard@linaro.org>
> Cc: Vincent Abriou <vincent.abriou@st.com>
> Cc: Maxime Coquelin <mcoquelin.stm32@gmail.com>
> Cc: Alexandre Torgue <alexandre.torgue@st.com>
> Cc: Chen-Yu Tsai <wens@csie.org>
> Cc: linux-amlogic@lists.infradead.org
> Cc: linux-arm-kernel@lists.infradead.org
> Cc: linux-renesas-soc@vger.kernel.org
> Cc: linux-rockchip@lists.infradead.org
> Cc: linux-stm32@st-md-mailman.stormreply.com
> Signed-off-by: Rob Herring <robh@kernel.org>
> ---
> .../gpu/drm/arm/display/komeda/komeda_kms.c | 1 +
> drivers/gpu/drm/arm/malidp_drv.c | 1 +
> drivers/gpu/drm/drm_gem_cma_helper.c | 48 +++++++++++--------
> drivers/gpu/drm/meson/meson_drv.c | 1 +
> drivers/gpu/drm/rcar-du/rcar_du_kms.c | 1 +
> drivers/gpu/drm/rockchip/rockchip_drm_gem.c | 1 +
> drivers/gpu/drm/stm/drv.c | 1 +
> drivers/gpu/drm/sun4i/sun4i_drv.c | 1 +
> 8 files changed, 36 insertions(+), 19 deletions(-)
>
> diff --git a/drivers/gpu/drm/arm/display/komeda/komeda_kms.c b/drivers/gpu/drm/arm/display/komeda/komeda_kms.c
> index d49772de93e0..7cf0dc4cbfc1 100644
> --- a/drivers/gpu/drm/arm/display/komeda/komeda_kms.c
> +++ b/drivers/gpu/drm/arm/display/komeda/komeda_kms.c
> @@ -31,6 +31,7 @@ static int komeda_gem_cma_dumb_create(struct drm_file *file,
> u32 pitch = DIV_ROUND_UP(args->width * args->bpp, 8);
>
> args->pitch = ALIGN(pitch, mdev->chip.bus_width);
> + args->flags = DRM_MODE_DUMB_KERNEL_MAP;
>
> return drm_gem_cma_dumb_create_internal(file, dev, args);
> }
> diff --git a/drivers/gpu/drm/arm/malidp_drv.c b/drivers/gpu/drm/arm/malidp_drv.c
> index 8a76315aaa0f..aeb1a779ecc1 100644
> --- a/drivers/gpu/drm/arm/malidp_drv.c
> +++ b/drivers/gpu/drm/arm/malidp_drv.c
> @@ -465,6 +465,7 @@ static int malidp_dumb_create(struct drm_file *file_priv,
> u8 alignment = malidp_hw_get_pitch_align(malidp->dev, 1);
>
> args->pitch = ALIGN(DIV_ROUND_UP(args->width * args->bpp, 8), alignment);
> + args->flags = DRM_MODE_DUMB_KERNEL_MAP;
>
> return drm_gem_cma_dumb_create_internal(file_priv, drm, args);
> }
> diff --git a/drivers/gpu/drm/drm_gem_cma_helper.c b/drivers/gpu/drm/drm_gem_cma_helper.c
> index 4cebfe01e6ea..f91e9e8adeaf 100644
> --- a/drivers/gpu/drm/drm_gem_cma_helper.c
> +++ b/drivers/gpu/drm/drm_gem_cma_helper.c
> @@ -78,21 +78,8 @@ __drm_gem_cma_create(struct drm_device *drm, size_t size)
> return ERR_PTR(ret);
> }
>
> -/**
> - * drm_gem_cma_create - allocate an object with the given size
> - * @drm: DRM device
> - * @size: size of the object to allocate
> - *
> - * This function creates a CMA GEM object and allocates a contiguous chunk of
> - * memory as backing store. The backing memory has the writecombine attribute
> - * set.
> - *
> - * Returns:
> - * A struct drm_gem_cma_object * on success or an ERR_PTR()-encoded negative
> - * error code on failure.
> - */
> -struct drm_gem_cma_object *drm_gem_cma_create(struct drm_device *drm,
> - size_t size)
> +static struct drm_gem_cma_object *
> +drm_gem_cma_create_flags(struct drm_device *drm, size_t size, u32 flags)
> {
> struct drm_gem_cma_object *cma_obj;
> int ret;
> @@ -103,6 +90,9 @@ struct drm_gem_cma_object *drm_gem_cma_create(struct drm_device *drm,
> if (IS_ERR(cma_obj))
> return cma_obj;
>
> + if (!(flags & DRM_MODE_DUMB_KERNEL_MAP))
> + cma_obj->dma_attrs |= DMA_ATTR_NO_KERNEL_MAPPING;
> +
> cma_obj->vaddr = dma_alloc_attrs(drm->dev, size, &cma_obj->paddr,
> GFP_KERNEL | __GFP_NOWARN,
> cma_obj->dma_attrs);
> @@ -119,6 +109,25 @@ struct drm_gem_cma_object *drm_gem_cma_create(struct drm_device *drm,
> drm_gem_object_put_unlocked(&cma_obj->base);
> return ERR_PTR(ret);
> }
> +
> +/**
> + * drm_gem_cma_create - allocate an object with the given size
> + * @drm: DRM device
> + * @size: size of the object to allocate
> + *
> + * This function creates a CMA GEM object and allocates a contiguous chunk of
> + * memory as backing store. The backing memory has the writecombine attribute
> + * set.
> + *
Shouldn't you mention here that the function always creates a kernel
mapping, and that callers that don't need the mapping should use
drm_gem_cma_dumb_create_internal() instead ?
drm_gem_cma_dumb_create_internal() operates at a different level though,
and drm_gem_cma_create() is only exported for a single driver. There's
no equivalent to drm_gem_cma_create() that can skip the kernel mapping.
Apart from this,
Reviewed-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>
> + * Returns:
> + * A struct drm_gem_cma_object * on success or an ERR_PTR()-encoded negative
> + * error code on failure.
> + */
> +struct drm_gem_cma_object *drm_gem_cma_create(struct drm_device *drm,
> + size_t size)
> +{
> + return drm_gem_cma_create_flags(drm, size, DRM_MODE_DUMB_KERNEL_MAP);
> +}
> EXPORT_SYMBOL_GPL(drm_gem_cma_create);
>
> /**
> @@ -139,14 +148,14 @@ EXPORT_SYMBOL_GPL(drm_gem_cma_create);
> */
> static struct drm_gem_cma_object *
> drm_gem_cma_create_with_handle(struct drm_file *file_priv,
> - struct drm_device *drm, size_t size,
> + struct drm_device *drm, size_t size, u32 flags,
> uint32_t *handle)
> {
> struct drm_gem_cma_object *cma_obj;
> struct drm_gem_object *gem_obj;
> int ret;
>
> - cma_obj = drm_gem_cma_create(drm, size);
> + cma_obj = drm_gem_cma_create_flags(drm, size, flags);
> if (IS_ERR(cma_obj))
> return cma_obj;
>
> @@ -225,7 +234,7 @@ int drm_gem_cma_dumb_create_internal(struct drm_file *file_priv,
> args->size = args->pitch * args->height;
>
> cma_obj = drm_gem_cma_create_with_handle(file_priv, drm, args->size,
> - &args->handle);
> + args->flags, &args->handle);
> return PTR_ERR_OR_ZERO(cma_obj);
> }
> EXPORT_SYMBOL_GPL(drm_gem_cma_dumb_create_internal);
> @@ -256,9 +265,10 @@ int drm_gem_cma_dumb_create(struct drm_file *file_priv,
>
> args->pitch = DIV_ROUND_UP(args->width * args->bpp, 8);
> args->size = args->pitch * args->height;
> + args->flags = DRM_MODE_DUMB_KERNEL_MAP;
>
> cma_obj = drm_gem_cma_create_with_handle(file_priv, drm, args->size,
> - &args->handle);
> + args->flags, &args->handle);
> return PTR_ERR_OR_ZERO(cma_obj);
> }
> EXPORT_SYMBOL_GPL(drm_gem_cma_dumb_create);
> diff --git a/drivers/gpu/drm/meson/meson_drv.c b/drivers/gpu/drm/meson/meson_drv.c
> index 397c33182f4f..1593518dcbe4 100644
> --- a/drivers/gpu/drm/meson/meson_drv.c
> +++ b/drivers/gpu/drm/meson/meson_drv.c
> @@ -81,6 +81,7 @@ static int meson_dumb_create(struct drm_file *file, struct drm_device *dev,
> */
> args->pitch = ALIGN(DIV_ROUND_UP(args->width * args->bpp, 8), SZ_64);
> args->size = PAGE_ALIGN(args->pitch * args->height);
> + args->flags = DRM_MODE_DUMB_KERNEL_MAP;
>
> return drm_gem_cma_dumb_create_internal(file, dev, args);
> }
> diff --git a/drivers/gpu/drm/rcar-du/rcar_du_kms.c b/drivers/gpu/drm/rcar-du/rcar_du_kms.c
> index 2dc9caee8767..c9b1f298ce7e 100644
> --- a/drivers/gpu/drm/rcar-du/rcar_du_kms.c
> +++ b/drivers/gpu/drm/rcar-du/rcar_du_kms.c
> @@ -299,6 +299,7 @@ int rcar_du_dumb_create(struct drm_file *file, struct drm_device *dev,
> align = 16 * args->bpp / 8;
>
> args->pitch = roundup(min_pitch, align);
> + args->flags = DRM_MODE_DUMB_KERNEL_MAP;
>
> return drm_gem_cma_dumb_create_internal(file, dev, args);
> }
> diff --git a/drivers/gpu/drm/rockchip/rockchip_drm_gem.c b/drivers/gpu/drm/rockchip/rockchip_drm_gem.c
> index 7582d0e6a60a..f09b9a035376 100644
> --- a/drivers/gpu/drm/rockchip/rockchip_drm_gem.c
> +++ b/drivers/gpu/drm/rockchip/rockchip_drm_gem.c
> @@ -419,6 +419,7 @@ int rockchip_gem_dumb_create(struct drm_file *file_priv,
> * align to 64 bytes since Mali requires it.
> */
> args->pitch = ALIGN(min_pitch, 64);
> + args->flags = DRM_MODE_DUMB_KERNEL_MAP;
> args->size = args->pitch * args->height;
My OCD gets triggered by flags appearing in the middle here while it is
at the end in other drivers :-)
>
> rk_obj = rockchip_gem_create_with_handle(file_priv, dev, args->size,
> diff --git a/drivers/gpu/drm/stm/drv.c b/drivers/gpu/drm/stm/drv.c
> index 5a9f9aca8bc2..0f76a4ac95b3 100644
> --- a/drivers/gpu/drm/stm/drv.c
> +++ b/drivers/gpu/drm/stm/drv.c
> @@ -47,6 +47,7 @@ static int stm_gem_cma_dumb_create(struct drm_file *file,
> */
> args->pitch = roundup(min_pitch, 128);
> args->height = roundup(args->height, 4);
> + args->flags = DRM_MODE_DUMB_KERNEL_MAP;
>
> return drm_gem_cma_dumb_create_internal(file, dev, args);
> }
> diff --git a/drivers/gpu/drm/sun4i/sun4i_drv.c b/drivers/gpu/drm/sun4i/sun4i_drv.c
> index a5757b11b730..f653a5d1e2d6 100644
> --- a/drivers/gpu/drm/sun4i/sun4i_drv.c
> +++ b/drivers/gpu/drm/sun4i/sun4i_drv.c
> @@ -34,6 +34,7 @@ static int drm_sun4i_gem_dumb_create(struct drm_file *file_priv,
> {
> /* The hardware only allows even pitches for YUV buffers. */
> args->pitch = ALIGN(DIV_ROUND_UP(args->width * args->bpp, 8), 2);
> + args->flags = DRM_MODE_DUMB_KERNEL_MAP;
>
> return drm_gem_cma_dumb_create_internal(file_priv, drm, args);
> }
--
Regards,
Laurent Pinchart
_______________________________________________
linux-amlogic mailing list
linux-amlogic@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-amlogic
^ permalink raw reply [flat|nested] 8+ messages in thread* Re: [PATCH 4/6] drm/cma-helper: Support DRM_MODE_DUMB_KERNEL_MAP flag
2019-10-22 11:30 ` Laurent Pinchart
@ 2019-10-22 11:40 ` Geert Uytterhoeven
2019-10-22 19:52 ` Rob Herring
2019-10-22 20:02 ` Rob Herring
1 sibling, 1 reply; 8+ messages in thread
From: Geert Uytterhoeven @ 2019-10-22 11:40 UTC (permalink / raw)
To: Laurent Pinchart
Cc: Daniel Vetter, Heiko Stübner, Neil Armstrong, David Airlie,
Liviu Dudau, DRI Development, Philippe Cornu, Benjamin Gaignard,
linux-stm32, Rob Herring, Kevin Hilman, Chen-Yu Tsai,
Xinliang Liu, open list:ARM/Rockchip SoC...,
Xinwei Kong, Kieran Bingham, James (Qian) Wang, CK Hu,
Alexandre Torgue, Chen Feng, Maarten Lankhorst, Linux-Renesas,
Maxime Ripard, linux-mediatek, Vincent Abriou, Matthias Brugger,
open list:ARM/Amlogic Meson...,
Sean Paul, Linux ARM, Maxime Coquelin, Sandy Huang,
Yannick Fertre, Noralf Trønnes, Philipp Zabel, Rongrong Zou,
Brian Starkey
Hi Laurent,
On Tue, Oct 22, 2019 at 1:30 PM Laurent Pinchart
<laurent.pinchart@ideasonboard.com> wrote:
> On Mon, Oct 21, 2019 at 04:45:48PM -0500, Rob Herring wrote:
> > --- a/drivers/gpu/drm/rockchip/rockchip_drm_gem.c
> > +++ b/drivers/gpu/drm/rockchip/rockchip_drm_gem.c
> > @@ -419,6 +419,7 @@ int rockchip_gem_dumb_create(struct drm_file *file_priv,
> > * align to 64 bytes since Mali requires it.
> > */
> > args->pitch = ALIGN(min_pitch, 64);
> > + args->flags = DRM_MODE_DUMB_KERNEL_MAP;
> > args->size = args->pitch * args->height;
>
> My OCD gets triggered by flags appearing in the middle here while it is
> at the end in other drivers :-)
... while "flags" appears before "pitch" and "size" in the actual struct
definition... Aaarghl ;-)
Gr{oetje,eeting}s,
Geert
--
Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- geert@linux-m68k.org
In personal conversations with technical people, I call myself a hacker. But
when I'm talking to journalists I just say "programmer" or something like that.
-- Linus Torvalds
_______________________________________________
linux-amlogic mailing list
linux-amlogic@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-amlogic
^ permalink raw reply [flat|nested] 8+ messages in thread* Re: [PATCH 4/6] drm/cma-helper: Support DRM_MODE_DUMB_KERNEL_MAP flag
2019-10-22 11:40 ` Geert Uytterhoeven
@ 2019-10-22 19:52 ` Rob Herring
0 siblings, 0 replies; 8+ messages in thread
From: Rob Herring @ 2019-10-22 19:52 UTC (permalink / raw)
To: Geert Uytterhoeven
Cc: Daniel Vetter, Heiko Stübner, Neil Armstrong, David Airlie,
Liviu Dudau, DRI Development, Philippe Cornu, Laurent Pinchart,
Benjamin Gaignard, linux-stm32, Linux-Renesas, Kevin Hilman,
Chen-Yu Tsai, Xinliang Liu, open list:ARM/Rockchip SoC...,
Xinwei Kong, Kieran Bingham, James (Qian) Wang, CK Hu,
Alexandre Torgue, Chen Feng, Maarten Lankhorst, Maxime Ripard,
moderated list:ARM/Mediatek SoC support, Vincent Abriou,
Matthias Brugger, open list:ARM/Amlogic Meson...,
Sean Paul, Linux ARM, Maxime Coquelin, Sandy Huang,
Yannick Fertre, Noralf Trønnes, Philipp Zabel, Rongrong Zou,
Brian Starkey
On Tue, Oct 22, 2019 at 6:40 AM Geert Uytterhoeven <geert@linux-m68k.org> wrote:
>
> Hi Laurent,
>
> On Tue, Oct 22, 2019 at 1:30 PM Laurent Pinchart
> <laurent.pinchart@ideasonboard.com> wrote:
> > On Mon, Oct 21, 2019 at 04:45:48PM -0500, Rob Herring wrote:
>
> > > --- a/drivers/gpu/drm/rockchip/rockchip_drm_gem.c
> > > +++ b/drivers/gpu/drm/rockchip/rockchip_drm_gem.c
> > > @@ -419,6 +419,7 @@ int rockchip_gem_dumb_create(struct drm_file *file_priv,
> > > * align to 64 bytes since Mali requires it.
> > > */
> > > args->pitch = ALIGN(min_pitch, 64);
> > > + args->flags = DRM_MODE_DUMB_KERNEL_MAP;
> > > args->size = args->pitch * args->height;
> >
> > My OCD gets triggered by flags appearing in the middle here while it is
> > at the end in other drivers :-)
>
> ... while "flags" appears before "pitch" and "size" in the actual struct
> definition... Aaarghl ;-)
There was some reasoning here as my CMA helper conversion patch is
going to insert code between flags and size.
But to keep everyone's OCD in check, I can set flags first.
Rob
_______________________________________________
linux-amlogic mailing list
linux-amlogic@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-amlogic
^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [PATCH 4/6] drm/cma-helper: Support DRM_MODE_DUMB_KERNEL_MAP flag
2019-10-22 11:30 ` Laurent Pinchart
2019-10-22 11:40 ` Geert Uytterhoeven
@ 2019-10-22 20:02 ` Rob Herring
2019-10-23 14:27 ` Laurent Pinchart
1 sibling, 1 reply; 8+ messages in thread
From: Rob Herring @ 2019-10-22 20:02 UTC (permalink / raw)
To: Laurent Pinchart
Cc: Heiko Stübner, Neil Armstrong, David Airlie, Liviu Dudau,
dri-devel, Sandy Huang, Benjamin Gaignard, linux-amlogic,
linux-stm32, Yannick Fertre, Kevin Hilman, Xinwei Kong,
Xinliang Liu, open list:ARM/Rockchip SoC...,
Chen-Yu Tsai, Noralf Trønnes, James (Qian) Wang, CK Hu,
Philipp Zabel, Alexandre Torgue, Chen Feng, Maarten Lankhorst,
open list:MEDIA DRIVERS FOR RENESAS - FCP, Maxime Ripard,
moderated list:ARM/Mediatek SoC support, Matthias Brugger,
Daniel Vetter, Sean Paul,
moderated list:ARM/FREESCALE IMX / MXC ARM ARCHITECTURE,
Philippe Cornu, Vincent Abriou, Kieran Bingham, Maxime Coquelin,
Rongrong Zou, Brian Starkey
On Tue, Oct 22, 2019 at 6:30 AM Laurent Pinchart
<laurent.pinchart@ideasonboard.com> wrote:
>
> Hi Rob,
>
> Thank you for the patch.
>
> On Mon, Oct 21, 2019 at 04:45:48PM -0500, Rob Herring wrote:
> > Add support in CMA helpers to handle callers specifying
> > DRM_MODE_DUMB_KERNEL_MAP flag. Existing behavior is maintained with this
> > change. drm_gem_cma_dumb_create() always creates a kernel mapping as
> > before. drm_gem_cma_dumb_create_internal() lets the caller set the flags
> > as desired. Therefore, update all the existing callers of
> > drm_gem_cma_dumb_create_internal() to also set the
> > DRM_MODE_DUMB_KERNEL_MAP flag.
> >
> > Cc: Maarten Lankhorst <maarten.lankhorst@linux.intel.com>
> > Cc: Maxime Ripard <mripard@kernel.org>
> > Cc: Sean Paul <sean@poorly.run>
> > Cc: David Airlie <airlied@linux.ie>
> > Cc: Daniel Vetter <daniel@ffwll.ch>
> > Cc: "James (Qian) Wang" <james.qian.wang@arm.com>
> > Cc: Liviu Dudau <liviu.dudau@arm.com>
> > Cc: Brian Starkey <brian.starkey@arm.com>
> > Cc: Neil Armstrong <narmstrong@baylibre.com>
> > Cc: Kevin Hilman <khilman@baylibre.com>
> > Cc: Laurent Pinchart <laurent.pinchart@ideasonboard.com>
> > Cc: Kieran Bingham <kieran.bingham+renesas@ideasonboard.com>
> > Cc: Sandy Huang <hjc@rock-chips.com>
> > Cc: "Heiko Stübner" <heiko@sntech.de>
> > Cc: Yannick Fertre <yannick.fertre@st.com>
> > Cc: Philippe Cornu <philippe.cornu@st.com>
> > Cc: Benjamin Gaignard <benjamin.gaignard@linaro.org>
> > Cc: Vincent Abriou <vincent.abriou@st.com>
> > Cc: Maxime Coquelin <mcoquelin.stm32@gmail.com>
> > Cc: Alexandre Torgue <alexandre.torgue@st.com>
> > Cc: Chen-Yu Tsai <wens@csie.org>
> > Cc: linux-amlogic@lists.infradead.org
> > Cc: linux-arm-kernel@lists.infradead.org
> > Cc: linux-renesas-soc@vger.kernel.org
> > Cc: linux-rockchip@lists.infradead.org
> > Cc: linux-stm32@st-md-mailman.stormreply.com
> > Signed-off-by: Rob Herring <robh@kernel.org>
> > ---
> > .../gpu/drm/arm/display/komeda/komeda_kms.c | 1 +
> > drivers/gpu/drm/arm/malidp_drv.c | 1 +
> > drivers/gpu/drm/drm_gem_cma_helper.c | 48 +++++++++++--------
> > drivers/gpu/drm/meson/meson_drv.c | 1 +
> > drivers/gpu/drm/rcar-du/rcar_du_kms.c | 1 +
> > drivers/gpu/drm/rockchip/rockchip_drm_gem.c | 1 +
> > drivers/gpu/drm/stm/drv.c | 1 +
> > drivers/gpu/drm/sun4i/sun4i_drv.c | 1 +
> > 8 files changed, 36 insertions(+), 19 deletions(-)
> >
> > diff --git a/drivers/gpu/drm/arm/display/komeda/komeda_kms.c b/drivers/gpu/drm/arm/display/komeda/komeda_kms.c
> > index d49772de93e0..7cf0dc4cbfc1 100644
> > --- a/drivers/gpu/drm/arm/display/komeda/komeda_kms.c
> > +++ b/drivers/gpu/drm/arm/display/komeda/komeda_kms.c
> > @@ -31,6 +31,7 @@ static int komeda_gem_cma_dumb_create(struct drm_file *file,
> > u32 pitch = DIV_ROUND_UP(args->width * args->bpp, 8);
> >
> > args->pitch = ALIGN(pitch, mdev->chip.bus_width);
> > + args->flags = DRM_MODE_DUMB_KERNEL_MAP;
> >
> > return drm_gem_cma_dumb_create_internal(file, dev, args);
> > }
> > diff --git a/drivers/gpu/drm/arm/malidp_drv.c b/drivers/gpu/drm/arm/malidp_drv.c
> > index 8a76315aaa0f..aeb1a779ecc1 100644
> > --- a/drivers/gpu/drm/arm/malidp_drv.c
> > +++ b/drivers/gpu/drm/arm/malidp_drv.c
> > @@ -465,6 +465,7 @@ static int malidp_dumb_create(struct drm_file *file_priv,
> > u8 alignment = malidp_hw_get_pitch_align(malidp->dev, 1);
> >
> > args->pitch = ALIGN(DIV_ROUND_UP(args->width * args->bpp, 8), alignment);
> > + args->flags = DRM_MODE_DUMB_KERNEL_MAP;
> >
> > return drm_gem_cma_dumb_create_internal(file_priv, drm, args);
> > }
> > diff --git a/drivers/gpu/drm/drm_gem_cma_helper.c b/drivers/gpu/drm/drm_gem_cma_helper.c
> > index 4cebfe01e6ea..f91e9e8adeaf 100644
> > --- a/drivers/gpu/drm/drm_gem_cma_helper.c
> > +++ b/drivers/gpu/drm/drm_gem_cma_helper.c
> > @@ -78,21 +78,8 @@ __drm_gem_cma_create(struct drm_device *drm, size_t size)
> > return ERR_PTR(ret);
> > }
> >
> > -/**
> > - * drm_gem_cma_create - allocate an object with the given size
> > - * @drm: DRM device
> > - * @size: size of the object to allocate
> > - *
> > - * This function creates a CMA GEM object and allocates a contiguous chunk of
> > - * memory as backing store. The backing memory has the writecombine attribute
> > - * set.
> > - *
> > - * Returns:
> > - * A struct drm_gem_cma_object * on success or an ERR_PTR()-encoded negative
> > - * error code on failure.
> > - */
> > -struct drm_gem_cma_object *drm_gem_cma_create(struct drm_device *drm,
> > - size_t size)
> > +static struct drm_gem_cma_object *
> > +drm_gem_cma_create_flags(struct drm_device *drm, size_t size, u32 flags)
> > {
> > struct drm_gem_cma_object *cma_obj;
> > int ret;
> > @@ -103,6 +90,9 @@ struct drm_gem_cma_object *drm_gem_cma_create(struct drm_device *drm,
> > if (IS_ERR(cma_obj))
> > return cma_obj;
> >
> > + if (!(flags & DRM_MODE_DUMB_KERNEL_MAP))
> > + cma_obj->dma_attrs |= DMA_ATTR_NO_KERNEL_MAPPING;
> > +
> > cma_obj->vaddr = dma_alloc_attrs(drm->dev, size, &cma_obj->paddr,
> > GFP_KERNEL | __GFP_NOWARN,
> > cma_obj->dma_attrs);
> > @@ -119,6 +109,25 @@ struct drm_gem_cma_object *drm_gem_cma_create(struct drm_device *drm,
> > drm_gem_object_put_unlocked(&cma_obj->base);
> > return ERR_PTR(ret);
> > }
> > +
> > +/**
> > + * drm_gem_cma_create - allocate an object with the given size
> > + * @drm: DRM device
> > + * @size: size of the object to allocate
> > + *
> > + * This function creates a CMA GEM object and allocates a contiguous chunk of
> > + * memory as backing store. The backing memory has the writecombine attribute
> > + * set.
> > + *
>
> Shouldn't you mention here that the function always creates a kernel
> mapping, and that callers that don't need the mapping should use
> drm_gem_cma_dumb_create_internal() instead ?
Are you confusing drm_gem_cma_create with drm_gem_cma_dumb_create?
drm_gem_cma_dumb_create() uses defaults and
drm_gem_cma_dumb_create_internal() allows the caller to tweak
parameters. Nothing new there other than an additional param to tweak.
> drm_gem_cma_dumb_create_internal() operates at a different level though,
> and drm_gem_cma_create() is only exported for a single driver. There's
> no equivalent to drm_gem_cma_create() that can skip the kernel mapping.
Because we don't yet need one. drm_gem_cma_create_flags() can be made
public when we do. I could do that now I guess and make
drm_gem_cma_create an inline wrapper.
Rob
_______________________________________________
linux-amlogic mailing list
linux-amlogic@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-amlogic
^ permalink raw reply [flat|nested] 8+ messages in thread* Re: [PATCH 4/6] drm/cma-helper: Support DRM_MODE_DUMB_KERNEL_MAP flag
2019-10-22 20:02 ` Rob Herring
@ 2019-10-23 14:27 ` Laurent Pinchart
0 siblings, 0 replies; 8+ messages in thread
From: Laurent Pinchart @ 2019-10-23 14:27 UTC (permalink / raw)
To: Rob Herring
Cc: Heiko Stübner, Neil Armstrong, David Airlie, Liviu Dudau,
dri-devel, Sandy Huang, Benjamin Gaignard, linux-amlogic,
linux-stm32, Yannick Fertre, Kevin Hilman, Xinwei Kong,
Xinliang Liu, open list:ARM/Rockchip SoC...,
Chen-Yu Tsai, Noralf Trønnes, James (Qian) Wang, CK Hu,
Philipp Zabel, Alexandre Torgue, Chen Feng, Maarten Lankhorst,
open list:MEDIA DRIVERS FOR RENESAS - FCP, Maxime Ripard,
moderated list:ARM/Mediatek SoC support, Matthias Brugger,
Daniel Vetter, Sean Paul,
moderated list:ARM/FREESCALE IMX / MXC ARM ARCHITECTURE,
Philippe Cornu, Vincent Abriou, Kieran Bingham, Maxime Coquelin,
Rongrong Zou, Brian Starkey
Hi Rob,
On Tue, Oct 22, 2019 at 03:02:06PM -0500, Rob Herring wrote:
> On Tue, Oct 22, 2019 at 6:30 AM Laurent Pinchart wrote:
> > On Mon, Oct 21, 2019 at 04:45:48PM -0500, Rob Herring wrote:
> >> Add support in CMA helpers to handle callers specifying
> >> DRM_MODE_DUMB_KERNEL_MAP flag. Existing behavior is maintained with this
> >> change. drm_gem_cma_dumb_create() always creates a kernel mapping as
> >> before. drm_gem_cma_dumb_create_internal() lets the caller set the flags
> >> as desired. Therefore, update all the existing callers of
> >> drm_gem_cma_dumb_create_internal() to also set the
> >> DRM_MODE_DUMB_KERNEL_MAP flag.
> >>
> >> Cc: Maarten Lankhorst <maarten.lankhorst@linux.intel.com>
> >> Cc: Maxime Ripard <mripard@kernel.org>
> >> Cc: Sean Paul <sean@poorly.run>
> >> Cc: David Airlie <airlied@linux.ie>
> >> Cc: Daniel Vetter <daniel@ffwll.ch>
> >> Cc: "James (Qian) Wang" <james.qian.wang@arm.com>
> >> Cc: Liviu Dudau <liviu.dudau@arm.com>
> >> Cc: Brian Starkey <brian.starkey@arm.com>
> >> Cc: Neil Armstrong <narmstrong@baylibre.com>
> >> Cc: Kevin Hilman <khilman@baylibre.com>
> >> Cc: Laurent Pinchart <laurent.pinchart@ideasonboard.com>
> >> Cc: Kieran Bingham <kieran.bingham+renesas@ideasonboard.com>
> >> Cc: Sandy Huang <hjc@rock-chips.com>
> >> Cc: "Heiko Stübner" <heiko@sntech.de>
> >> Cc: Yannick Fertre <yannick.fertre@st.com>
> >> Cc: Philippe Cornu <philippe.cornu@st.com>
> >> Cc: Benjamin Gaignard <benjamin.gaignard@linaro.org>
> >> Cc: Vincent Abriou <vincent.abriou@st.com>
> >> Cc: Maxime Coquelin <mcoquelin.stm32@gmail.com>
> >> Cc: Alexandre Torgue <alexandre.torgue@st.com>
> >> Cc: Chen-Yu Tsai <wens@csie.org>
> >> Cc: linux-amlogic@lists.infradead.org
> >> Cc: linux-arm-kernel@lists.infradead.org
> >> Cc: linux-renesas-soc@vger.kernel.org
> >> Cc: linux-rockchip@lists.infradead.org
> >> Cc: linux-stm32@st-md-mailman.stormreply.com
> >> Signed-off-by: Rob Herring <robh@kernel.org>
> >> ---
> >> .../gpu/drm/arm/display/komeda/komeda_kms.c | 1 +
> >> drivers/gpu/drm/arm/malidp_drv.c | 1 +
> >> drivers/gpu/drm/drm_gem_cma_helper.c | 48 +++++++++++--------
> >> drivers/gpu/drm/meson/meson_drv.c | 1 +
> >> drivers/gpu/drm/rcar-du/rcar_du_kms.c | 1 +
> >> drivers/gpu/drm/rockchip/rockchip_drm_gem.c | 1 +
> >> drivers/gpu/drm/stm/drv.c | 1 +
> >> drivers/gpu/drm/sun4i/sun4i_drv.c | 1 +
> >> 8 files changed, 36 insertions(+), 19 deletions(-)
> >>
> >> diff --git a/drivers/gpu/drm/arm/display/komeda/komeda_kms.c b/drivers/gpu/drm/arm/display/komeda/komeda_kms.c
> >> index d49772de93e0..7cf0dc4cbfc1 100644
> >> --- a/drivers/gpu/drm/arm/display/komeda/komeda_kms.c
> >> +++ b/drivers/gpu/drm/arm/display/komeda/komeda_kms.c
> >> @@ -31,6 +31,7 @@ static int komeda_gem_cma_dumb_create(struct drm_file *file,
> >> u32 pitch = DIV_ROUND_UP(args->width * args->bpp, 8);
> >>
> >> args->pitch = ALIGN(pitch, mdev->chip.bus_width);
> >> + args->flags = DRM_MODE_DUMB_KERNEL_MAP;
> >>
> >> return drm_gem_cma_dumb_create_internal(file, dev, args);
> >> }
> >> diff --git a/drivers/gpu/drm/arm/malidp_drv.c b/drivers/gpu/drm/arm/malidp_drv.c
> >> index 8a76315aaa0f..aeb1a779ecc1 100644
> >> --- a/drivers/gpu/drm/arm/malidp_drv.c
> >> +++ b/drivers/gpu/drm/arm/malidp_drv.c
> >> @@ -465,6 +465,7 @@ static int malidp_dumb_create(struct drm_file *file_priv,
> >> u8 alignment = malidp_hw_get_pitch_align(malidp->dev, 1);
> >>
> >> args->pitch = ALIGN(DIV_ROUND_UP(args->width * args->bpp, 8), alignment);
> >> + args->flags = DRM_MODE_DUMB_KERNEL_MAP;
> >>
> >> return drm_gem_cma_dumb_create_internal(file_priv, drm, args);
> >> }
> >> diff --git a/drivers/gpu/drm/drm_gem_cma_helper.c b/drivers/gpu/drm/drm_gem_cma_helper.c
> >> index 4cebfe01e6ea..f91e9e8adeaf 100644
> >> --- a/drivers/gpu/drm/drm_gem_cma_helper.c
> >> +++ b/drivers/gpu/drm/drm_gem_cma_helper.c
> >> @@ -78,21 +78,8 @@ __drm_gem_cma_create(struct drm_device *drm, size_t size)
> >> return ERR_PTR(ret);
> >> }
> >>
> >> -/**
> >> - * drm_gem_cma_create - allocate an object with the given size
> >> - * @drm: DRM device
> >> - * @size: size of the object to allocate
> >> - *
> >> - * This function creates a CMA GEM object and allocates a contiguous chunk of
> >> - * memory as backing store. The backing memory has the writecombine attribute
> >> - * set.
> >> - *
> >> - * Returns:
> >> - * A struct drm_gem_cma_object * on success or an ERR_PTR()-encoded negative
> >> - * error code on failure.
> >> - */
> >> -struct drm_gem_cma_object *drm_gem_cma_create(struct drm_device *drm,
> >> - size_t size)
> >> +static struct drm_gem_cma_object *
> >> +drm_gem_cma_create_flags(struct drm_device *drm, size_t size, u32 flags)
> >> {
> >> struct drm_gem_cma_object *cma_obj;
> >> int ret;
> >> @@ -103,6 +90,9 @@ struct drm_gem_cma_object *drm_gem_cma_create(struct drm_device *drm,
> >> if (IS_ERR(cma_obj))
> >> return cma_obj;
> >>
> >> + if (!(flags & DRM_MODE_DUMB_KERNEL_MAP))
> >> + cma_obj->dma_attrs |= DMA_ATTR_NO_KERNEL_MAPPING;
> >> +
> >> cma_obj->vaddr = dma_alloc_attrs(drm->dev, size, &cma_obj->paddr,
> >> GFP_KERNEL | __GFP_NOWARN,
> >> cma_obj->dma_attrs);
> >> @@ -119,6 +109,25 @@ struct drm_gem_cma_object *drm_gem_cma_create(struct drm_device *drm,
> >> drm_gem_object_put_unlocked(&cma_obj->base);
> >> return ERR_PTR(ret);
> >> }
> >> +
> >> +/**
> >> + * drm_gem_cma_create - allocate an object with the given size
> >> + * @drm: DRM device
> >> + * @size: size of the object to allocate
> >> + *
> >> + * This function creates a CMA GEM object and allocates a contiguous chunk of
> >> + * memory as backing store. The backing memory has the writecombine attribute
> >> + * set.
> >> + *
> >
> > Shouldn't you mention here that the function always creates a kernel
> > mapping, and that callers that don't need the mapping should use
> > drm_gem_cma_dumb_create_internal() instead ?
>
> Are you confusing drm_gem_cma_create with drm_gem_cma_dumb_create?
> drm_gem_cma_dumb_create() uses defaults and
> drm_gem_cma_dumb_create_internal() allows the caller to tweak
> parameters. Nothing new there other than an additional param to tweak.
>
> > drm_gem_cma_dumb_create_internal() operates at a different level though,
> > and drm_gem_cma_create() is only exported for a single driver. There's
> > no equivalent to drm_gem_cma_create() that can skip the kernel mapping.
>
> Because we don't yet need one. drm_gem_cma_create_flags() can be made
> public when we do. I could do that now I guess and make
> drm_gem_cma_create an inline wrapper.
I don't mind not having drm_gem_cma_create_flags() made public (but you
can do so already if you prefer) if there's no user. My point is that we
now have a mechanism to skip creation of kernel mappings, and that
drm_gem_cma_create() will always result in the creation of a kernel
mapping. I thought it was worth mentioning it, but if you think that's
not needed, feel free to ignore the comment.
--
Regards,
Laurent Pinchart
_______________________________________________
linux-amlogic mailing list
linux-amlogic@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-amlogic
^ permalink raw reply [flat|nested] 8+ messages in thread