On Tue, Oct 22, 2019 at 6:30 AM Laurent Pinchart
<laurent.pinch...@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.lankho...@linux.intel.com>
> > Cc: Maxime Ripard <mrip...@kernel.org>
> > Cc: Sean Paul <s...@poorly.run>
> > Cc: David Airlie <airl...@linux.ie>
> > Cc: Daniel Vetter <dan...@ffwll.ch>
> > Cc: "James (Qian) Wang" <james.qian.w...@arm.com>
> > Cc: Liviu Dudau <liviu.du...@arm.com>
> > Cc: Brian Starkey <brian.star...@arm.com>
> > Cc: Neil Armstrong <narmstr...@baylibre.com>
> > Cc: Kevin Hilman <khil...@baylibre.com>
> > Cc: Laurent Pinchart <laurent.pinch...@ideasonboard.com>
> > Cc: Kieran Bingham <kieran.bingham+rene...@ideasonboard.com>
> > Cc: Sandy Huang <h...@rock-chips.com>
> > Cc: "Heiko Stübner" <he...@sntech.de>
> > Cc: Yannick Fertre <yannick.fer...@st.com>
> > Cc: Philippe Cornu <philippe.co...@st.com>
> > Cc: Benjamin Gaignard <benjamin.gaign...@linaro.org>
> > Cc: Vincent Abriou <vincent.abr...@st.com>
> > Cc: Maxime Coquelin <mcoquelin.st...@gmail.com>
> > Cc: Alexandre Torgue <alexandre.tor...@st.com>
> > Cc: Chen-Yu Tsai <w...@csie.org>
> > Cc: linux-amlo...@lists.infradead.org
> > Cc: linux-arm-ker...@lists.infradead.org
> > Cc: linux-renesas-...@vger.kernel.org
> > Cc: linux-rockc...@lists.infradead.org
> > Cc: linux-st...@st-md-mailman.stormreply.com
> > Signed-off-by: Rob Herring <r...@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
