Re: [Intel-gfx] [PATCH v2 02/21] drm/armada: Introduce GEM object functions
On Tue, Sep 15, 2020 at 04:59:39PM +0200, Thomas Zimmermann wrote:
> GEM object functions deprecate several similar callback interfaces in
> struct drm_driver. This patch replaces the per-driver callbacks with
> per-instance callbacks in armada.
>
> Signed-off-by: Thomas Zimmermann
Acked-by: Russell King
Thanks.
> ---
> drivers/gpu/drm/armada/armada_drv.c | 3 ---
> drivers/gpu/drm/armada/armada_gem.c | 12 +++-
> drivers/gpu/drm/armada/armada_gem.h | 2 --
> 3 files changed, 11 insertions(+), 6 deletions(-)
>
> diff --git a/drivers/gpu/drm/armada/armada_drv.c
> b/drivers/gpu/drm/armada/armada_drv.c
> index 980d3f1f8f16..22247cfce80b 100644
> --- a/drivers/gpu/drm/armada/armada_drv.c
> +++ b/drivers/gpu/drm/armada/armada_drv.c
> @@ -37,13 +37,10 @@ DEFINE_DRM_GEM_FOPS(armada_drm_fops);
>
> static struct drm_driver armada_drm_driver = {
> .lastclose = drm_fb_helper_lastclose,
> - .gem_free_object_unlocked = armada_gem_free_object,
> .prime_handle_to_fd = drm_gem_prime_handle_to_fd,
> .prime_fd_to_handle = drm_gem_prime_fd_to_handle,
> - .gem_prime_export = armada_gem_prime_export,
> .gem_prime_import = armada_gem_prime_import,
> .dumb_create= armada_gem_dumb_create,
> - .gem_vm_ops = _gem_vm_ops,
> .major = 1,
> .minor = 0,
> .name = "armada-drm",
> diff --git a/drivers/gpu/drm/armada/armada_gem.c
> b/drivers/gpu/drm/armada/armada_gem.c
> index ecf8a55e93d9..c343fbefe47c 100644
> --- a/drivers/gpu/drm/armada/armada_gem.c
> +++ b/drivers/gpu/drm/armada/armada_gem.c
> @@ -25,7 +25,7 @@ static vm_fault_t armada_gem_vm_fault(struct vm_fault *vmf)
> return vmf_insert_pfn(vmf->vma, vmf->address, pfn);
> }
>
> -const struct vm_operations_struct armada_gem_vm_ops = {
> +static const struct vm_operations_struct armada_gem_vm_ops = {
> .fault = armada_gem_vm_fault,
> .open = drm_gem_vm_open,
> .close = drm_gem_vm_close,
> @@ -184,6 +184,12 @@ armada_gem_map_object(struct drm_device *dev, struct
> armada_gem_object *dobj)
> return dobj->addr;
> }
>
> +static const struct drm_gem_object_funcs armada_gem_object_funcs = {
> + .free = armada_gem_free_object,
> + .export = armada_gem_prime_export,
> + .vm_ops = _gem_vm_ops,
> +};
> +
> struct armada_gem_object *
> armada_gem_alloc_private_object(struct drm_device *dev, size_t size)
> {
> @@ -195,6 +201,8 @@ armada_gem_alloc_private_object(struct drm_device *dev,
> size_t size)
> if (!obj)
> return NULL;
>
> + obj->obj.funcs = _gem_object_funcs;
> +
> drm_gem_private_object_init(dev, >obj, size);
>
> DRM_DEBUG_DRIVER("alloc private obj %p size %zu\n", obj, size);
> @@ -214,6 +222,8 @@ static struct armada_gem_object
> *armada_gem_alloc_object(struct drm_device *dev,
> if (!obj)
> return NULL;
>
> + obj->obj.funcs = _gem_object_funcs;
> +
> if (drm_gem_object_init(dev, >obj, size)) {
> kfree(obj);
> return NULL;
> diff --git a/drivers/gpu/drm/armada/armada_gem.h
> b/drivers/gpu/drm/armada/armada_gem.h
> index de04cc2c8f0e..ffcc7e8dd351 100644
> --- a/drivers/gpu/drm/armada/armada_gem.h
> +++ b/drivers/gpu/drm/armada/armada_gem.h
> @@ -21,8 +21,6 @@ struct armada_gem_object {
> void*update_data;
> };
>
> -extern const struct vm_operations_struct armada_gem_vm_ops;
> -
> #define drm_to_armada_gem(o) container_of(o, struct armada_gem_object, obj)
>
> void armada_gem_free_object(struct drm_gem_object *);
> --
> 2.28.0
>
>
--
RMK's Patch system: https://www.armlinux.org.uk/developer/patches/
FTTP is here! 40Mbps down 10Mbps up. Decent connectivity at last!
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [PATCH v2 5/5] uaccess: Rename user_access_begin/end() to user_full_access_begin/end()
On Tue, Apr 21, 2020 at 03:49:19AM +0100, Al Viro wrote: > The only source I'd been able to find speeks of >= 60 cycles > (and possibly much more) for non-pipelined coprocessor instructions; > the list of such does contain loads and stores to a bunch of registers. > However, the register in question (p15/c3) has only store mentioned there, > so loads might be cheap; no obvious reasons for those to be slow. > That's a question to arm folks, I'm afraid... rmk? I have no information on that; instruction timings are not defined at architecture level (architecture reference manual), nor do I find information in the CPU technical reference manual (which would be specific to the CPU). Instruction timings tend to be implementation dependent. I've always consulted Will Deacon when I've needed to know whether an instruction is expensive or not. -- RMK's Patch system: https://www.armlinux.org.uk/developer/patches/ FTTC broadband for 0.8mile line in suburbia: sync at 10.2Mbps down 587kbps up ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [PATCH 18/51] drm/: Use drmm_add_final_kfree
On Fri, Feb 21, 2020 at 10:02:46PM +0100, Daniel Vetter wrote: > These are the leftover drivers that didn't have a ->release hook that > needed to be updated. > > Signed-off-by: Daniel Vetter > Cc: "James (Qian) Wang" > Cc: Liviu Dudau > Cc: Mihail Atanassov > Cc: Russell King > Cc: Hans de Goede > --- > drivers/gpu/drm/arm/display/komeda/komeda_kms.c | 2 ++ > drivers/gpu/drm/armada/armada_drv.c | 2 ++ > drivers/gpu/drm/vboxvideo/vbox_drv.c| 2 ++ > 3 files changed, 6 insertions(+) > > diff --git a/drivers/gpu/drm/armada/armada_drv.c > b/drivers/gpu/drm/armada/armada_drv.c > index 197dca3fc84c..dd9ed71ed942 100644 > --- a/drivers/gpu/drm/armada/armada_drv.c > +++ b/drivers/gpu/drm/armada/armada_drv.c > @@ -12,6 +12,7 @@ > #include > #include > #include > +#include > #include > #include > #include > @@ -103,6 +104,7 @@ static int armada_drm_bind(struct device *dev) > kfree(priv); > return ret; > } > + drmm_add_final_kfree(>drm, priv); > > /* Remove early framebuffers */ > ret = drm_fb_helper_remove_conflicting_framebuffers(NULL, I have no visibility of what the changes behind this are, so I can't ack this change. -- RMK's Patch system: https://www.armlinux.org.uk/developer/patches/ FTTC broadband for 0.8mile line in suburbia: sync at 12.1Mbps down 622kbps up According to speedtest.net: 11.9Mbps down 500kbps up ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [PATCH 05/15] drm/armada: Delete dma_buf->k(un)map implemenation
On Mon, Nov 25, 2019 at 10:44:43PM +0100, Daniel Vetter wrote:
> On Mon, Nov 18, 2019 at 11:35:26AM +0100, Daniel Vetter wrote:
> > It's a dummy anyway.
> >
> > Signed-off-by: Daniel Vetter
> > Cc: Russell King
>
> I merged the entire series except this one and the final patch, sill
> waiting a bit more for an ack on this perhaps.
Acked-by: Russell King
I thought drm trees closed around -rc6?
> -Daniel
>
> > ---
> > drivers/gpu/drm/armada/armada_gem.c | 12
> > 1 file changed, 12 deletions(-)
> >
> > diff --git a/drivers/gpu/drm/armada/armada_gem.c
> > b/drivers/gpu/drm/armada/armada_gem.c
> > index 93cf8b8bfcff..976685f2939e 100644
> > --- a/drivers/gpu/drm/armada/armada_gem.c
> > +++ b/drivers/gpu/drm/armada/armada_gem.c
> > @@ -461,16 +461,6 @@ static void armada_gem_prime_unmap_dma_buf(struct
> > dma_buf_attachment *attach,
> > kfree(sgt);
> > }
> >
> > -static void *armada_gem_dmabuf_no_kmap(struct dma_buf *buf, unsigned long
> > n)
> > -{
> > - return NULL;
> > -}
> > -
> > -static void
> > -armada_gem_dmabuf_no_kunmap(struct dma_buf *buf, unsigned long n, void
> > *addr)
> > -{
> > -}
> > -
> > static int
> > armada_gem_dmabuf_mmap(struct dma_buf *buf, struct vm_area_struct *vma)
> > {
> > @@ -481,8 +471,6 @@ static const struct dma_buf_ops
> > armada_gem_prime_dmabuf_ops = {
> > .map_dma_buf= armada_gem_prime_map_dma_buf,
> > .unmap_dma_buf = armada_gem_prime_unmap_dma_buf,
> > .release= drm_gem_dmabuf_release,
> > - .map= armada_gem_dmabuf_no_kmap,
> > - .unmap = armada_gem_dmabuf_no_kunmap,
> > .mmap = armada_gem_dmabuf_mmap,
> > };
> >
> > --
> > 2.24.0
> >
>
> --
> Daniel Vetter
> Software Engineer, Intel Corporation
> http://blog.ffwll.ch
>
--
RMK's Patch system: https://www.armlinux.org.uk/developer/patches/
FTTC broadband for 0.8mile line in suburbia: sync at 12.1Mbps down 622kbps up
According to speedtest.net: 11.9Mbps down 500kbps up
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [PATCH] dma-buf: add struct dma_buf_attach_info v2
On Tue, Apr 30, 2019 at 01:10:02PM +0200, Christian König wrote:
> Add a structure for the parameters of dma_buf_attach, this makes it much
> easier
> to add new parameters later on.
I don't understand this reasoning. What are the "new parameters" that
are being proposed, and why do we need to put them into memory to pass
them across this interface?
If the intention is to make it easier to change the interface, passing
parameters in this manner mean that it's easy for the interface to
change and drivers not to notice the changes, since the compiler will
not warn (unless some member of the structure that the driver is using
gets removed, in which case it will error.)
Additions to the structure will go unnoticed by drivers - what if the
caller is expecting some different kind of behaviour, and the driver
ignores that new addition?
This doesn't seem to me like a good idea.
>
> v2: rebase cleanup and fix all new implementations as well
>
> Signed-off-by: Christian König
> ---
> drivers/dma-buf/dma-buf.c | 13 +++--
> drivers/gpu/drm/armada/armada_gem.c | 6 +-
> drivers/gpu/drm/drm_prime.c | 6 +-
> drivers/gpu/drm/i915/i915_gem_dmabuf.c | 6 +-
> drivers/gpu/drm/omapdrm/omap_gem_dmabuf.c | 6 +-
> drivers/gpu/drm/tegra/gem.c | 6 +-
> drivers/gpu/drm/udl/udl_dmabuf.c| 6 +-
> .../common/videobuf2/videobuf2-dma-contig.c | 6 +-
> .../media/common/videobuf2/videobuf2-dma-sg.c | 6 +-
> drivers/misc/fastrpc.c | 6 +-
> drivers/staging/media/tegra-vde/tegra-vde.c | 6 +-
> drivers/xen/gntdev-dmabuf.c | 4
> include/linux/dma-buf.h | 17 +++--
> 13 files changed, 76 insertions(+), 18 deletions(-)
>
> diff --git a/drivers/dma-buf/dma-buf.c b/drivers/dma-buf/dma-buf.c
> index 3ae6c0c2cc02..e295e76a8c57 100644
> --- a/drivers/dma-buf/dma-buf.c
> +++ b/drivers/dma-buf/dma-buf.c
> @@ -535,8 +535,9 @@ EXPORT_SYMBOL_GPL(dma_buf_put);
> /**
> * dma_buf_attach - Add the device to dma_buf's attachments list; optionally,
> * calls attach() of dma_buf_ops to allow device-specific attach
> functionality
> - * @dmabuf: [in]buffer to attach device to.
> - * @dev: [in]device to be attached.
> + * @info:[in]holds all the attach related information provided
> + * by the importer. see dma_buf_attach_info
> + * for further details.
> *
> * Returns struct dma_buf_attachment pointer for this attachment. Attachments
> * must be cleaned up by calling dma_buf_detach().
> @@ -550,20 +551,20 @@ EXPORT_SYMBOL_GPL(dma_buf_put);
> * accessible to @dev, and cannot be moved to a more suitable place. This is
> * indicated with the error code -EBUSY.
> */
> -struct dma_buf_attachment *dma_buf_attach(struct dma_buf *dmabuf,
> - struct device *dev)
> +struct dma_buf_attachment *dma_buf_attach(const struct dma_buf_attach_info
> *info)
> {
> + struct dma_buf *dmabuf = info->dmabuf;
> struct dma_buf_attachment *attach;
> int ret;
>
> - if (WARN_ON(!dmabuf || !dev))
> + if (WARN_ON(!dmabuf || !info->dev))
> return ERR_PTR(-EINVAL);
>
> attach = kzalloc(sizeof(*attach), GFP_KERNEL);
> if (!attach)
> return ERR_PTR(-ENOMEM);
>
> - attach->dev = dev;
> + attach->dev = info->dev;
> attach->dmabuf = dmabuf;
>
> mutex_lock(>lock);
> diff --git a/drivers/gpu/drm/armada/armada_gem.c
> b/drivers/gpu/drm/armada/armada_gem.c
> index 642d0e70d0f8..19c47821032f 100644
> --- a/drivers/gpu/drm/armada/armada_gem.c
> +++ b/drivers/gpu/drm/armada/armada_gem.c
> @@ -501,6 +501,10 @@ armada_gem_prime_export(struct drm_device *dev, struct
> drm_gem_object *obj,
> struct drm_gem_object *
> armada_gem_prime_import(struct drm_device *dev, struct dma_buf *buf)
> {
> + struct dma_buf_attach_info attach_info = {
> + .dev = dev->dev,
> + .dmabuf = buf
> + };
> struct dma_buf_attachment *attach;
> struct armada_gem_object *dobj;
>
> @@ -516,7 +520,7 @@ armada_gem_prime_import(struct drm_device *dev, struct
> dma_buf *buf)
> }
> }
>
> - attach = dma_buf_attach(buf, dev->dev);
> + attach = dma_buf_attach(_info);
> if (IS_ERR(attach))
> return ERR_CAST(attach);
>
> diff --git a/drivers/gpu/drm/drm_prime.c b/drivers/gpu/drm/drm_prime.c
> index dc079efb3b0f..1dd70fc095ee 100644
> --- a/drivers/gpu/drm/drm_prime.c
> +++ b/drivers/gpu/drm/drm_prime.c
> @@ -710,6 +710,10 @@ struct drm_gem_object *drm_gem_prime_import_dev(struct
> drm_device *dev,
> struct dma_buf *dma_buf,
> struct device *attach_dev)
> {
> + struct
Re: [Intel-gfx] [PATCH 2/4] components: multiple components for a device
On Fri, Feb 08, 2019 at 12:27:57AM +0100, Daniel Vetter wrote:
> Component framework is extended to support multiple components for
> a struct device. These will be matched with different masters based on
> its sub component value.
>
> We are introducing this, as I915 needs two different components
> with different subcomponent value, which will be matched to two
> different component masters(Audio and HDCP) based on the subcomponent
> values.
>
> v2: Add documenation.
>
> v3: Rebase on top of updated documenation.
>
> v4: Review from Rafael:
> - Remove redundant "This" from kerneldoc (also in the previous patch)
> - Streamline the logic in find_component() a bit.
>
> Signed-off-by: Daniel Vetter (v1 code)
> Signed-off-by: Ramalingam C (v1 commit message)
> Cc: Ramalingam C
> Cc: Greg Kroah-Hartman
> Cc: Russell King
> Cc: Rafael J. Wysocki
> Cc: Jaroslav Kysela
> Cc: Takashi Iwai
> Cc: Rodrigo Vivi
> Cc: Jani Nikula
> Signed-off-by: Daniel Vetter
> ---
> drivers/base/component.c | 158 +-
> include/linux/component.h | 10 ++-
> 2 files changed, 129 insertions(+), 39 deletions(-)
>
> diff --git a/drivers/base/component.c b/drivers/base/component.c
> index 1624c2a892a5..7dbc41cccd58 100644
> --- a/drivers/base/component.c
> +++ b/drivers/base/component.c
> @@ -47,6 +47,7 @@ struct component;
> struct component_match_array {
> void *data;
> int (*compare)(struct device *, void *);
> + int (*compare_typed)(struct device *, int, void *);
> void (*release)(struct device *, void *);
> struct component *component;
> bool duplicate;
> @@ -74,6 +75,7 @@ struct component {
> bool bound;
>
> const struct component_ops *ops;
> + int subcomponent;
> struct device *dev;
> };
>
> @@ -158,7 +160,7 @@ static struct master *__master_find(struct device *dev,
> }
>
> static struct component *find_component(struct master *master,
> - int (*compare)(struct device *, void *), void *compare_data)
> + struct component_match_array *mc)
> {
> struct component *c;
>
> @@ -166,7 +168,11 @@ static struct component *find_component(struct master
> *master,
> if (c->master && c->master != master)
> continue;
>
> - if (compare(c->dev, compare_data))
> + if (mc->compare && mc->compare(c->dev, mc->data))
> + return c;
> +
> + if (mc->compare_typed &&
> + mc->compare_typed(c->dev, c->subcomponent, mc->data))
> return c;
> }
>
> @@ -192,7 +198,7 @@ static int find_components(struct master *master)
> if (match->compare[i].component)
> continue;
>
> - c = find_component(master, mc->compare, mc->data);
> + c = find_component(master, mc);
> if (!c) {
> ret = -ENXIO;
> break;
> @@ -327,29 +333,12 @@ static int component_match_realloc(struct device *dev,
> return 0;
> }
>
> -/**
> - * component_match_add_release - add a component match with release callback
> - * @master: device with the aggregate driver
> - * @matchptr: pointer to the list of component matches
> - * @release: release function for @compare_data
> - * @compare: compare function to match against all components
> - * @compare_data: opaque pointer passed to the @compare function
> - *
> - * Adds a new component match to the list stored in @matchptr, which the
> @master
> - * aggregate driver needs to function. The list of component matches pointed
> to
> - * by @matchptr must be initialized to NULL before adding the first match.
> - *
> - * The allocated match list in @matchptr is automatically released using devm
> - * actions, where upon @release will be called to free any references held by
> - * @compare_data, e.g. when @compare_data is a _node that must be
> - * released with of_node_put().
> - *
> - * See also component_match_add().
> - */
> -void component_match_add_release(struct device *master,
> +static void __component_match_add(struct device *master,
> struct component_match **matchptr,
> void (*release)(struct device *, void *),
> - int (*compare)(struct device *, void *), void *compare_data)
> + int (*compare)(struct device *, void *),
> + int (*compare_typed)(struct device *, int, void *),
> + void *compare_data)
> {
> struct component_match *match = *matchptr;
>
> @@ -381,13 +370,69 @@ void component_match_add_release(struct device *master,
> }
>
> match->compare[match->num].compare = compare;
> + match->compare[match->num].compare_typed = compare_typed;
> match->compare[match->num].release = release;
> match->compare[match->num].data = compare_data;
> match->compare[match->num].component = NULL;
> match->num++;
> }
> +
> +/**
> + * component_match_add_release - add a component match with
