[PATCH 4/6] drm/cma-helper: Support DRM_MODE_DUMB_KERNEL_MAP flag

Rob Herring robh at kernel.org
Tue Oct 22 20:02:06 UTC 2019


On Tue, Oct 22, 2019 at 6:30 AM Laurent Pinchart
<laurent.pinchart at 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 at linux.intel.com>
> > Cc: Maxime Ripard <mripard at kernel.org>
> > Cc: Sean Paul <sean at poorly.run>
> > Cc: David Airlie <airlied at linux.ie>
> > Cc: Daniel Vetter <daniel at ffwll.ch>
> > Cc: "James (Qian) Wang" <james.qian.wang at arm.com>
> > Cc: Liviu Dudau <liviu.dudau at arm.com>
> > Cc: Brian Starkey <brian.starkey at arm.com>
> > Cc: Neil Armstrong <narmstrong at baylibre.com>
> > Cc: Kevin Hilman <khilman at baylibre.com>
> > Cc: Laurent Pinchart <laurent.pinchart at ideasonboard.com>
> > Cc: Kieran Bingham <kieran.bingham+renesas at ideasonboard.com>
> > Cc: Sandy Huang <hjc at rock-chips.com>
> > Cc: "Heiko Stübner" <heiko at sntech.de>
> > Cc: Yannick Fertre <yannick.fertre at st.com>
> > Cc: Philippe Cornu <philippe.cornu at st.com>
> > Cc: Benjamin Gaignard <benjamin.gaignard at linaro.org>
> > Cc: Vincent Abriou <vincent.abriou at st.com>
> > Cc: Maxime Coquelin <mcoquelin.stm32 at gmail.com>
> > Cc: Alexandre Torgue <alexandre.torgue at st.com>
> > Cc: Chen-Yu Tsai <wens at csie.org>
> > Cc: linux-amlogic at lists.infradead.org
> > Cc: linux-arm-kernel at lists.infradead.org
> > Cc: linux-renesas-soc at vger.kernel.org
> > Cc: linux-rockchip at lists.infradead.org
> > Cc: linux-stm32 at st-md-mailman.stormreply.com
> > Signed-off-by: Rob Herring <robh at 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


More information about the dri-devel mailing list