from:"Sumit Semwal"

Re: [Intel-gfx] [PATCH v5 00/21] Move all drivers to a common dma-buf locking convention

2022-09-20 Thread Sumit Semwal

Hi Dmitry,


On Wed, 14 Sept 2022 at 00:58, Dmitry Osipenko
 wrote:
>
> Hello,
>
> This series moves all drivers to a dynamic dma-buf locking specification.
> From now on all dma-buf importers are made responsible for holding
> dma-buf's reservation lock around all operations performed over dma-bufs
> in accordance to the locking specification. This allows us to utilize
> reservation lock more broadly around kernel without fearing of a potential
> deadlocks.
Thank you for the excellent work on this series - apart from a minor
nit in patch 15, please feel free to add my:
Acked-by: Sumit Semwal 
for the relevant dma-buf patches (1, 2, 15-19, 21).

Best regards,
Sumit.

>
> This patchset passes all i915 selftests. It was also tested using VirtIO,
> Panfrost, Lima, Tegra, udmabuf, AMDGPU and Nouveau drivers. I tested cases
> of display+GPU, display+V4L and GPU+V4L dma-buf sharing (where appropriate),
> which covers majority of kernel drivers since rest of the drivers share
> same or similar code paths.
>
> Changelog:
>
> v5: - Added acks and r-bs that were given to v4.
>
> - Changed i915 preparation patch like was suggested by Michael Ruhl.
>   The scope of reservation locking is smaller now.
>
> v4: - Added dma_buf_mmap() to the "locking convention" documentation,
>   which was missed by accident in v3.
>
> - Added acks from Christian König, Tomasz Figa and Hans Verkuil that
>   they gave to couple v3 patches.
>
> - Dropped the "_unlocked" postfix from function names that don't have
>   the locked variant, as was requested by Christian König.
>
> - Factored out the per-driver preparations into separate patches
>   to ease reviewing of the changes, which is now doable without the
>   global dma-buf functions renaming.
>
> - Factored out the dynamic locking convention enforcements into separate
>   patches which add the final dma_resv_assert_held(dmabuf->resv) to the
>   dma-buf API functions.
>
> v3: - Factored out dma_buf_mmap_unlocked() and attachment functions
>   into aseparate patches, like was suggested by Christian König.
>
> - Corrected and factored out dma-buf locking documentation into
>   a separate patch, like was suggested by Christian König.
>
> - Intel driver dropped the reservation locking fews days ago from
>   its BO-release code path, but we need that locking for the imported
>   GEMs because in the end that code path unmaps the imported GEM.
>   So I added back the locking needed by the imported GEMs, updating
>   the "dma-buf attachment locking specification" patch appropriately.
>
> - Tested Nouveau+Intel dma-buf import/export combo.
>
> - Tested udmabuf import to i915/Nouveau/AMDGPU.
>
> - Fixed few places in Etnaviv, Panfrost and Lima drivers that I missed
>   to switch to locked dma-buf vmapping in the drm/gem: Take reservation
>   lock for vmap/vunmap operations" patch. In a result invalidated the
>   Christian's r-b that he gave to v2.
>
> - Added locked dma-buf vmap/vunmap functions that are needed for fixing
>   vmappping of Etnaviv, Panfrost and Lima drivers mentioned above.
>   I actually had this change stashed for the drm-shmem shrinker patchset,
>   but then realized that it's already needed by the dma-buf patches.
>   Also improved my tests to better cover these code paths.
>
> v2: - Changed locking specification to avoid problems with a cross-driver
>   ww locking, like was suggested by Christian König. Now the attach/detach
>   callbacks are invoked without the held lock and exporter should take the
>   lock.
>
> - Added "locking convention" documentation that explains which dma-buf
>   functions and callbacks are locked/unlocked for importers and exporters,
>   which was requested by Christian König.
>
> - Added ack from Tomasz Figa to the V4L patches that he gave to v1.
>
> Dmitry Osipenko (21):
>   dma-buf: Add unlocked variant of vmapping functions
>   dma-buf: Add unlocked variant of attachment-mapping functions
>   drm/gem: Take reservation lock for vmap/vunmap operations
>   drm/prime: Prepare to dynamic dma-buf locking specification
>   drm/armada: Prepare to dynamic dma-buf locking specification
>   drm/i915: Prepare to dynamic dma-buf locking specification
>   drm/omapdrm: Prepare to dynamic dma-buf locking specification
>   drm/tegra: Prepare to dynamic dma-buf locking specification
>   drm/etnaviv: Prepare to dynamic dma-buf locking specification
>   RDMA/umem: Prepare to dynamic dma-buf locking specification
>   misc: fastrpc: Prepare to dynamic dma-buf locking specification
>   x

Re: [Intel-gfx] [PATCH v5 15/21] dma-buf: Move dma_buf_vmap() to dynamic locking specification

2022-09-20 Thread Sumit Semwal

Hi Dmitry,

Thanks very much for the series.

On Wed, 14 Sept 2022 at 00:59, Dmitry Osipenko
 wrote:
>
> Move dma_buf_vmap/vunmap_unlocked() functions to the dynamic locking
> specification by asserting that the reservation lock is held.
Thanks for the patch; just a minor nit - I think you mean dma_buf_vmap
/ vunmap() here, and not _unlocked?

Best,
Sumit.


>
> Acked-by: Christian König 
> Signed-off-by: Dmitry Osipenko 
> ---
>  drivers/dma-buf/dma-buf.c | 4 
>  1 file changed, 4 insertions(+)
>
> diff --git a/drivers/dma-buf/dma-buf.c b/drivers/dma-buf/dma-buf.c
> index 50db7571dc4b..80fd6ccc88c6 100644
> --- a/drivers/dma-buf/dma-buf.c
> +++ b/drivers/dma-buf/dma-buf.c
> @@ -1450,6 +1450,8 @@ int dma_buf_vmap(struct dma_buf *dmabuf, struct 
> iosys_map *map)
> if (WARN_ON(!dmabuf))
> return -EINVAL;
>
> +   dma_resv_assert_held(dmabuf->resv);
> +
> if (!dmabuf->ops->vmap)
> return -EINVAL;
>
> @@ -1510,6 +1512,8 @@ void dma_buf_vunmap(struct dma_buf *dmabuf, struct 
> iosys_map *map)
> if (WARN_ON(!dmabuf))
> return;
>
> +   dma_resv_assert_held(dmabuf->resv);
> +
> BUG_ON(iosys_map_is_null(>vmap_ptr));
> BUG_ON(dmabuf->vmapping_counter == 0);
> BUG_ON(!iosys_map_is_equal(>vmap_ptr, map));
> --
> 2.37.3
>


--
Thanks and regards,

Sumit Semwal (he / him)
Tech Lead - LCG, Vertical Technologies
Linaro.org │ Open source software for ARM SoCs

Re: [Intel-gfx] [PATCH v2] dma-buf-map: Rename to iosys-map

2022-02-02 Thread Sumit Semwal

Hello Lucas,

On Wed, 2 Feb 2022 at 15:08, Lucas De Marchi  wrote:
>
> On Wed, Feb 02, 2022 at 10:25:28AM +0100, Christian König wrote:
> >Am 02.02.22 um 10:11 schrieb Lucas De Marchi:
> >>[SNIP]
> >>diff --git a/MAINTAINERS b/MAINTAINERS
> >>index d03ad8da1f36..112676f11792 100644
> >>--- a/MAINTAINERS
> >>+++ b/MAINTAINERS
> >>@@ -5675,7 +5675,6 @@ T:  git git://anongit.freedesktop.org/drm/drm-misc
> >>  F:  Documentation/driver-api/dma-buf.rst
> >>  F:  drivers/dma-buf/
> >>  F:  include/linux/*fence.h
> >
> >Oh, that is not correct to begin with.
>
> right, include/linux/dma-fence*
>
> >
> >>-F:   include/linux/dma-buf*
> >
> >That here should probably be changed to point directly to
> >include/linux/dma-buf.h, but that can come later on.
>
> thanks for catching that. I will change it on next version and add your
> (and any additional) ack.
>
> Lucas De Marchi
>
> >
> >Feel free to add an Acked-by: Christian König
> > to the patch.
> >
> >If nobody objects I'm going to push it drm-misc-next and provide a
> >follow up to cleanup the MAINTAINERS file a bit more.
Thank you for the patch; apologies for not being able to respond
earlier (was out due to covid, just getting back slowly).

With Christian's suggestions, looks good to me as well - feel free to add an
Acked-by: Sumit Semwal  to the same.

> >
> >Regards,
> >Christian.
Best,
Sumit.

> >
> >>  F:  include/linux/dma-resv.h
> >>  K:  \bdma_(?:buf|fence|resv)\b
> >>@@ -9976,6 +9975,13 @@ F: include/linux/iova.h
> >>  F:  include/linux/of_iommu.h
> >>  F:  include/uapi/linux/iommu.h
> >>+IOSYS-MAP HELPERS
> >>+M:   Thomas Zimmermann 
> >>+L:   dri-de...@lists.freedesktop.org
> >>+S:   Maintained
> >>+T:   git git://anongit.freedesktop.org/drm/drm-misc
> >>+F:   include/linux/iosys-map.h
> >>+
> >

Re: [Intel-gfx] [PATCH 4/7] dma-buf: Document DMA_BUF_IOCTL_SYNC

2021-05-27 Thread Sumit Semwal

Hi Daniel,

On Thu, 27 May 2021 at 16:08, Daniel Vetter  wrote:

> On Tue, May 25, 2021 at 04:17:50PM -0500, Jason Ekstrand wrote:
> > This adds a new "DMA Buffer ioctls" section to the dma-buf docs and adds
> > documentation for DMA_BUF_IOCTL_SYNC.
> >
> > Signed-off-by: Jason Ekstrand 
> > Cc: Daniel Vetter 
> > Cc: Christian König 
> > Cc: Sumit Semwal 
>
> We're still missing the doc for the SET_NAME ioctl, but maybe Sumit can be
> motivated to fix that?
>

Yes, certainly, I'll cook up a patch and send it soon.

>
> > ---
> >  Documentation/driver-api/dma-buf.rst |  8 +++
> >  include/uapi/linux/dma-buf.h | 32 +++-
> >  2 files changed, 39 insertions(+), 1 deletion(-)
> >
> > diff --git a/Documentation/driver-api/dma-buf.rst
> b/Documentation/driver-api/dma-buf.rst
> > index 7f37ec30d9fd7..784f84fe50a5e 100644
> > --- a/Documentation/driver-api/dma-buf.rst
> > +++ b/Documentation/driver-api/dma-buf.rst
> > @@ -88,6 +88,9 @@ consider though:
> >  - The DMA buffer FD is also pollable, see `Implicit Fence Poll
> Support`_ below for
> >details.
> >
> > +- The DMA buffer FD also supports a few dma-buf-specific ioctls, see
> > +  `DMA Buffer ioctls`_ below for details.
> > +
> >  Basic Operation and Device DMA Access
> >  ~
> >
> > @@ -106,6 +109,11 @@ Implicit Fence Poll Support
> >  .. kernel-doc:: drivers/dma-buf/dma-buf.c
> > :doc: implicit fence polling
> >
> > +DMA Buffer ioctls
> > +~
> > +
> > +.. kernel-doc:: include/uapi/linux/dma-buf.h
> > +
> >  Kernel Functions and Structures Reference
> >  ~
> >
> > diff --git a/include/uapi/linux/dma-buf.h b/include/uapi/linux/dma-buf.h
> > index 7f30393b92c3b..1f67ced853b14 100644
> > --- a/include/uapi/linux/dma-buf.h
> > +++ b/include/uapi/linux/dma-buf.h
> > @@ -22,8 +22,38 @@
> >
> >  #include 
> >
> > -/* begin/end dma-buf functions used for userspace mmap. */
> > +/**
> > + * struct dma_buf_sync - Synchronize with CPU access.
> > + *
> > + * When a DMA buffer is accessed from the CPU via mmap, it is not always
> > + * possible to guarantee coherency between the CPU-visible map and
> underlying
> > + * memory.  To manage coherency, DMA_BUF_IOCTL_SYNC must be used to
> bracket
> > + * any CPU access to give the kernel the chance to shuffle memory
> around if
> > + * needed.
> > + *
> > + * Prior to accessing the map, the client should call DMA_BUF_IOCTL_SYNC
>
> s/should/must/
>
> > + * with DMA_BUF_SYNC_START and the appropriate read/write flags.  Once
> the
> > + * access is complete, the client should call DMA_BUF_IOCTL_SYNC with
> > + * DMA_BUF_SYNC_END and the same read/write flags.
>
> I think we should make it really clear here that this is _only_ for cache
> coherency, and that furthermore if you want coherency with gpu access you
> either need to use poll() for implicit sync (link to the relevant section)
> or handle explicit sync with sync_file (again link would be awesome).
>
> > + */
> >  struct dma_buf_sync {
> > + /**
> > +  * @flags: Set of access flags
> > +  *
> > +  * - DMA_BUF_SYNC_START: Indicates the start of a map access
>
> Bikeshed, but I think the item list format instead of bullet point list
> looks neater, e.g.  DOC: standard plane properties in drm_plane.c.
>
>
> > +  *   session.
> > +  *
> > +  * - DMA_BUF_SYNC_END: Indicates the end of a map access session.
> > +  *
> > +  * - DMA_BUF_SYNC_READ: Indicates that the mapped DMA buffer will
> > +  *   be read by the client via the CPU map.
> > +  *
> > +  * - DMA_BUF_SYNC_READ: Indicates that the mapped DMA buffer will
>
> s/READ/WRITE/
>
> > +  *   be written by the client via the CPU map.
> > +  *
> > +  * - DMA_BUF_SYNC_RW: An alias for DMA_BUF_SYNC_READ |
> > +  *   DMA_BUF_SYNC_WRITE.
> > +  */
>
> With the nits addressed: Reviewed-by: Daniel Vetter <
> daniel.vet...@ffwll.ch>
>
> >   __u64 flags;
> >  };
> >
> > --
> > 2.31.1
> >
>
> --
> Daniel Vetter
> Software Engineer, Intel Corporation
> http://blog.ffwll.ch
>

Best,
Sumit.

-- 
Thanks and regards,

Sumit Semwal
Tech Lead - Android, Vertical Technologies
Linaro.org │ Open source software for ARM SoCs
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

Re: [Intel-gfx] [PATCH 0/3] dma-buf: Flag vmap'ed memory as system or I/O memory

2020-09-18 Thread Sumit Semwal

Hi Thomas,

On Fri, 18 Sep 2020 at 11:36, Sumit Semwal  wrote:
>
> Hello Thomas,
>
> On Mon, 14 Sep 2020 at 16:55, Thomas Zimmermann  wrote:
> >
> > Dma-buf provides vmap() and vunmap() for retrieving and releasing mappings
> > of dma-buf memory in kernel address space. The functions operate with plain
> > addresses and the assumption is that the memory can be accessed with load
> > and store operations. This is not the case on some architectures (e.g.,
> > sparc64) where I/O memory can only be accessed with dedicated instructions.
> >
> > This patchset introduces struct dma_buf_map, which contains the address of
> > a buffer and a flag that tells whether system- or I/O-memory instructions
> > are required.
>
> Thank you for the patchset - it is a really nice, clean bit to add!
> >
> > Some background: updating the DRM framebuffer console on sparc64 makes the
> > kernel panic. This is because the framebuffer memory cannot be accessed with
> > system-memory instructions. We currently employ a workaround in DRM to
> > address this specific problem. [1]
> >
> > To resolve the problem, we'd like to address it at the most common point,
> > which is the dma-buf framework. The dma-buf mapping ideally knows if I/O
> > instructions are required and exports this information to it's users. The
> > new structure struct dma_buf_map stores the buffer address and a flag that
> > signals I/O memory. Affected users of the buffer (e.g., drivers, frameworks)
> > can then access the memory accordingly.
> >
> > This patchset only introduces struct dma_buf_map, and updates struct dma_buf
> > and it's interfaces. Further patches can update dma-buf users. For example,
> > there's a prototype patchset for DRM that fixes the framebuffer problem. [2]
> >
> > Further work: TTM, one of DRM's memory managers, already exports an
> > is_iomem flag of its own. It could later be switched over to exporting 
> > struct
> > dma_buf_map, thus simplifying some code. Several DRM drivers expect their
> > fbdev console to operate on I/O memory. These could possibly be switched 
> > over
> > to the generic fbdev emulation, as soon as the generic code uses struct
> > dma_buf_map.
> >
> > [1] https://lore.kernel.org/dri-devel/20200725191012.ga434...@ravnborg.org/
> > [2] 
> > https://lore.kernel.org/dri-devel/20200806085239.4606-1-tzimmerm...@suse.de/
> >
> > Thomas Zimmermann (3):
> >   dma-buf: Add struct dma-buf-map for storing struct dma_buf.vaddr_ptr
> >   dma-buf: Use struct dma_buf_map in dma_buf_vmap() interfaces
> >   dma-buf: Use struct dma_buf_map in dma_buf_vunmap() interfaces
>
> FWIW, for the series, please feel free to add my
> Acked-by: Sumit Semwal 
Of course, once the errors found by kernel test robot are fixed :).
>
> >
> >  Documentation/driver-api/dma-buf.rst  |   3 +
> >  drivers/dma-buf/dma-buf.c |  40 +++---
> >  drivers/gpu/drm/drm_gem_cma_helper.c  |  16 ++-
> >  drivers/gpu/drm/drm_gem_shmem_helper.c|  17 ++-
> >  drivers/gpu/drm/drm_prime.c   |  14 +-
> >  drivers/gpu/drm/etnaviv/etnaviv_gem_prime.c   |  13 +-
> >  drivers/gpu/drm/i915/gem/i915_gem_dmabuf.c|  13 +-
> >  .../drm/i915/gem/selftests/i915_gem_dmabuf.c  |  18 ++-
> >  drivers/gpu/drm/tegra/gem.c   |  23 ++--
> >  .../common/videobuf2/videobuf2-dma-contig.c   |  17 ++-
> >  .../media/common/videobuf2/videobuf2-dma-sg.c |  19 ++-
> >  .../common/videobuf2/videobuf2-vmalloc.c  |  21 ++-
> >  include/drm/drm_prime.h   |   5 +-
> >  include/linux/dma-buf-map.h   | 126 ++
> >  include/linux/dma-buf.h   |  11 +-
> >  15 files changed, 274 insertions(+), 82 deletions(-)
> >  create mode 100644 include/linux/dma-buf-map.h
> >
> > --
> > 2.28.0
> >
>
> Best,
> Sumit.

Best,
Sumit.
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

Re: [Intel-gfx] [PATCH 0/3] dma-buf: Flag vmap'ed memory as system or I/O memory

2020-09-18 Thread Sumit Semwal

Hello Thomas,

On Mon, 14 Sep 2020 at 16:55, Thomas Zimmermann  wrote:
>
> Dma-buf provides vmap() and vunmap() for retrieving and releasing mappings
> of dma-buf memory in kernel address space. The functions operate with plain
> addresses and the assumption is that the memory can be accessed with load
> and store operations. This is not the case on some architectures (e.g.,
> sparc64) where I/O memory can only be accessed with dedicated instructions.
>
> This patchset introduces struct dma_buf_map, which contains the address of
> a buffer and a flag that tells whether system- or I/O-memory instructions
> are required.

Thank you for the patchset - it is a really nice, clean bit to add!
>
> Some background: updating the DRM framebuffer console on sparc64 makes the
> kernel panic. This is because the framebuffer memory cannot be accessed with
> system-memory instructions. We currently employ a workaround in DRM to
> address this specific problem. [1]
>
> To resolve the problem, we'd like to address it at the most common point,
> which is the dma-buf framework. The dma-buf mapping ideally knows if I/O
> instructions are required and exports this information to it's users. The
> new structure struct dma_buf_map stores the buffer address and a flag that
> signals I/O memory. Affected users of the buffer (e.g., drivers, frameworks)
> can then access the memory accordingly.
>
> This patchset only introduces struct dma_buf_map, and updates struct dma_buf
> and it's interfaces. Further patches can update dma-buf users. For example,
> there's a prototype patchset for DRM that fixes the framebuffer problem. [2]
>
> Further work: TTM, one of DRM's memory managers, already exports an
> is_iomem flag of its own. It could later be switched over to exporting struct
> dma_buf_map, thus simplifying some code. Several DRM drivers expect their
> fbdev console to operate on I/O memory. These could possibly be switched over
> to the generic fbdev emulation, as soon as the generic code uses struct
> dma_buf_map.
>
> [1] https://lore.kernel.org/dri-devel/20200725191012.ga434...@ravnborg.org/
> [2] 
> https://lore.kernel.org/dri-devel/20200806085239.4606-1-tzimmerm...@suse.de/
>
> Thomas Zimmermann (3):
>   dma-buf: Add struct dma-buf-map for storing struct dma_buf.vaddr_ptr
>   dma-buf: Use struct dma_buf_map in dma_buf_vmap() interfaces
>   dma-buf: Use struct dma_buf_map in dma_buf_vunmap() interfaces

FWIW, for the series, please feel free to add my
Acked-by: Sumit Semwal 

>
>  Documentation/driver-api/dma-buf.rst  |   3 +
>  drivers/dma-buf/dma-buf.c |  40 +++---
>  drivers/gpu/drm/drm_gem_cma_helper.c  |  16 ++-
>  drivers/gpu/drm/drm_gem_shmem_helper.c|  17 ++-
>  drivers/gpu/drm/drm_prime.c   |  14 +-
>  drivers/gpu/drm/etnaviv/etnaviv_gem_prime.c   |  13 +-
>  drivers/gpu/drm/i915/gem/i915_gem_dmabuf.c|  13 +-
>  .../drm/i915/gem/selftests/i915_gem_dmabuf.c  |  18 ++-
>  drivers/gpu/drm/tegra/gem.c   |  23 ++--
>  .../common/videobuf2/videobuf2-dma-contig.c   |  17 ++-
>  .../media/common/videobuf2/videobuf2-dma-sg.c |  19 ++-
>  .../common/videobuf2/videobuf2-vmalloc.c  |  21 ++-
>  include/drm/drm_prime.h   |   5 +-
>  include/linux/dma-buf-map.h   | 126 ++
>  include/linux/dma-buf.h   |  11 +-
>  15 files changed, 274 insertions(+), 82 deletions(-)
>  create mode 100644 include/linux/dma-buf-map.h
>
> --
> 2.28.0
>

Best,
Sumit.
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

Re: [Intel-gfx] [PATCH 1/2] Revert "dma-buf: Report signaled links inside dma-fence-chain"

2020-06-26 Thread Sumit Semwal

Hi Christian,

On Fri, 26 Jun 2020, 18:10 Daniel Vetter,  wrote:

> On Fri, Jun 26, 2020 at 9:03 AM Christian König
>  wrote:
> >
> > Am 26.06.20 um 06:43 schrieb Sumit Semwal:
> > > On Fri, 26 Jun 2020 at 01:24, Daniel Vetter  wrote:
> > >> Ignoring everything else ...
> > >>
> > >> On Thu, Jun 25, 2020 at 9:28 PM Jani Nikula <
> jani.nik...@linux.intel.com> wrote:
> > >>> As a side note, there seem to be extra checks in place for acks when
> > >>> applying non-i915 patches to drm-intel; there are no such checks for
> > >>> drm-misc.
> > >> One option to generalize that that I pondered is to consult
> > >> get_maintainers.pl asking for git repo link, and if that returns
> > >> something else, then insist that there's an ack from a relevant
> > >> maintainer. It's a bit of typing, but I think the bigger problem is
> > >> that there's a ton of false positives.
> > > Right; for the particular patch, I wasn't even in the to: or cc: field
> > > and that made it slip from my radar. I would definitely ask any one
> > > sending patches for dma-buf directory to follow the get_maintainers.pl
> > > religiously.
> > >> But maybe that's a good thing, would give some motivation to keep
> > >> MAINTAINERS updated.
> >
> > Should I maybe add myself as maintainer as well? I've written enough
> > stuff in there to know the code quite a bit.
>
> I think that makes lots of sense, since defacto you already are :-)
>
> If you feel like bikeshed, get_maintainers.pl also supports R: for
> reviewer, but given that you also push patches to drm-misc M: for
> maintainer feels more accurate.
>

I think given you've been reviewing and changing most of the code around
dma-fences, it should be ok to add you as the maintainer for those bits?

-Daniel
>

Best,
Sumit.
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

Re: [Intel-gfx] [PATCH 1/2] Revert "dma-buf: Report signaled links inside dma-fence-chain"

2020-06-25 Thread Sumit Semwal

On Fri, 26 Jun 2020 at 01:24, Daniel Vetter  wrote:
>
> Ignoring everything else ...
>
> On Thu, Jun 25, 2020 at 9:28 PM Jani Nikula  
> wrote:
> > As a side note, there seem to be extra checks in place for acks when
> > applying non-i915 patches to drm-intel; there are no such checks for
> > drm-misc.
>
> One option to generalize that that I pondered is to consult
> get_maintainers.pl asking for git repo link, and if that returns
> something else, then insist that there's an ack from a relevant
> maintainer. It's a bit of typing, but I think the bigger problem is
> that there's a ton of false positives.

Right; for the particular patch, I wasn't even in the to: or cc: field
and that made it slip from my radar. I would definitely ask any one
sending patches for dma-buf directory to follow the get_maintainers.pl
religiously.
>
> But maybe that's a good thing, would give some motivation to keep
> MAINTAINERS updated.
>
> The other issue is though that drm-misc is plenty used to merge
> patches even when the respective maintainers are absent for weeks, or
> unresponsive. If we just blindly implement that rule, then the only
> possible Ack for these would be Dave as subsystem maintainers, and
> I don't want to be in the business of stamping approvals for all this
> stuff. Much better if people just collaborate.
>
> So I think an ack check would be nice, but probably not practical.
>
> Plus in this situation here drm-misc.git actually is the main repo,
> and we wont ever be able to teach a script to make a judgement call of
> whether that patch has the right amount of review on it.
> -Daniel

Best,
Sumit.
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

Re: [Intel-gfx] [PATCH 00/15] Retire dma_buf_k(un)map

2019-11-18 Thread Sumit Semwal

Hello Daniel,

On Mon, 18 Nov 2019 at 16:05, Daniel Vetter  wrote:
>
> Hi all,
>
> Way back when we created the dma-buf spec it made sense to have kmap/unmap
> interfaces, since 32bit kernels with limited vmalloc space were still
> rather ubiquitous. But that idea (like many others) never caught on, was
> quickly replaced by vmaps covering the entire buffer for all real uses,
> and nowadays 64bit kernels rule the world. Currently merged upstream
> drivers (and we have a pile now) don't even bother to kmap for their
> private buffers, much less for anything shared.
>
> So since it was never used, and this idea's time is clearly over, let's
> remove it all.
As always, thanks for helping keep this code sane :)

Fwiw, for the series, please feel free to add my
Acked-by: Sumit Semwal 

>
> Only real change I had to do (aside from deleting lots of dead code) was
> in the tegra driver. But even there I suspect the dma-buf kmap path has
> never been run in anger anywhere, it just doesn't make sense to put relocs
> into a dma-buf (as opposed to using a dma-buf for the target address of a
> reloc).
>
> Comments, reviews and testing very much appreciated.
>
> Cheers, Daniel
>
> Daniel Vetter (15):
>   drm/tegra: Map cmdbuf once for reloc processing
>   drm/tegra: Delete host1x_bo_ops->k(un)map
>   drm/i915: Remove dma_buf_kmap selftest
>   staging/android/ion: delete dma_buf->kmap/unmap implemenation
>   drm/armada: Delete dma_buf->k(un)map implemenation
>   drm/i915: Drop dma_buf->k(un)map
>   drm/omapdrm: Drop dma_buf->k(un)map
>   drm/tegra: Remove dma_buf->k(un)map
>   dma-buf: Drop dma_buf_k(un)map
>   drm/vmwgfx: Delete mmaping functions
>   media/videobuf2: Drop dma_buf->k(un)map support
>   drm/tee_shm: Drop dma_buf_k(unmap) support
>   xen/gntdev-dmabuf: Ditch dummy map functions
>   sample/vfio-mdev/mbocs: Remove dma_buf_k(un)map support
>   dma-buf: Remove kernel map/unmap hooks
>
>  drivers/dma-buf/dma-buf.c |  63 +--
>  drivers/gpu/drm/armada/armada_gem.c   |  12 ---
>  drivers/gpu/drm/i915/gem/i915_gem_dmabuf.c|  36 ---
>  .../drm/i915/gem/selftests/i915_gem_dmabuf.c  | 101 --
>  .../gpu/drm/i915/gem/selftests/mock_dmabuf.c  |  16 ---
>  drivers/gpu/drm/omapdrm/omap_gem_dmabuf.c |  21 
>  drivers/gpu/drm/tegra/gem.c   |  40 ---
>  drivers/gpu/drm/vmwgfx/vmwgfx_prime.c |  33 --
>  drivers/gpu/host1x/job.c  |  21 ++--
>  .../common/videobuf2/videobuf2-dma-contig.c   |   8 --
>  .../media/common/videobuf2/videobuf2-dma-sg.c |   8 --
>  .../common/videobuf2/videobuf2-vmalloc.c  |   8 --
>  drivers/misc/fastrpc.c|   8 --
>  drivers/staging/android/ion/ion.c |  14 ---
>  drivers/tee/tee_shm.c |   6 --
>  drivers/xen/gntdev-dmabuf.c   |  23 
>  include/linux/dma-buf.h   |  27 -
>  include/linux/host1x.h|  13 ---
>  samples/vfio-mdev/mbochs.c|  16 ---
>  19 files changed, 10 insertions(+), 464 deletions(-)
>
> --
> 2.24.0
>
> ___
> dri-devel mailing list
> dri-de...@lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/dri-devel

Best,
Sumit.
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

Re: [Intel-gfx] [PATCH] dma-buf: rename reservation_object to dma_resv

2019-08-14 Thread Sumit Semwal

Hello Christian,

On Sun, 11 Aug 2019 at 14:48, Chris Wilson  wrote:

> Quoting Christian König (2019-08-11 09:56:01)
> > Be more consistent with the naming of the other DMA-buf objects.
>
> From the tip of my fingers, \o/
>
> > Signed-off-by: Christian König 
>
Thanks for doing this!
Acked-by: Sumit Semwal 


>
> Letting the compiler do the real work (for the bits I spot checked it
> was the expected mechanical translation), and overwhelmingly agreeing with
> the motivation,
> Reviewed-by: Chris Wilson 
> -Chris
>

Best,
Sumit.
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

Re: [Intel-gfx] [PATCHv2] drm/prime: Forward declare struct device

2017-05-10 Thread Sumit Semwal

Hi Laura,

On 10 May 2017 at 22:35, Laura Abbott <labb...@redhat.com> wrote:
>
> We need a declaration of struct device to avoid warnings:
>
> In file included from include/drm/drm_file.h:38:0,
>  from drivers/gpu/drm/drm_file.c:38:
> include/drm/drm_prime.h:71:14: warning: 'struct device' declared inside
>   parameter list will not be visible outside of this definition or
>   declaration
>   struct device *attach_dev);
>  ^~
>
> Forward declare it.
>
> Signed-off-by: Laura Abbott <labb...@redhat.com>
Thanks for the patch; feel free to add my
Reviewed-by: Sumit Semwal <sumit.sem...@linaro.org>

> ---
> v2: Switch to foward declaration instead of including a header.
> ---
>  include/drm/drm_prime.h | 2 ++
>  1 file changed, 2 insertions(+)
>
> diff --git a/include/drm/drm_prime.h b/include/drm/drm_prime.h
> index 46fd1fb..59ccab4 100644
> --- a/include/drm/drm_prime.h
> +++ b/include/drm/drm_prime.h
> @@ -50,6 +50,8 @@ struct drm_prime_file_private {
> struct rb_root handles;
>  };
>
> +struct device;
> +
>  struct dma_buf_export_info;
>  struct dma_buf;
>
> --
> 2.7.4
>

Best,
Sumit.
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

Re: [Intel-gfx] [PULL] drm-misc-next-fixes

2017-04-21 Thread Sumit Semwal

On 21 April 2017 at 03:20, Daniel Vetter  wrote:

>> - Rename dma_buf_ops->kmap_* to avoid naming collision (Logan)
>
> This one touches v4l and ion and is acked by the corresponding
> maintainers (but Sumit forgot to record that when applying the patch,
> and Sean didn't highlight it in the summary).
>
Hmm - I assumed that the mbox I downloaded (after all the acks got
registered in patchworks) would've had the Acks? Didn't cross-check,
my bad.
Will of course take care from next time on.

> Sean, should we augment the template that cross-subsystem stuff
> (outside of what's on-topic for drm-misc) needs to be highlighted
> specifically in the pull summary?
>
> Off for vacation for real now.
>
> Cheers, Daniel
>
Best,
Sumit.
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

Re: [Intel-gfx] [PATCH v2] dma-buf: Rename dma-ops to prevent conflict with kunmap_atomic macro

2017-04-20 Thread Sumit Semwal

= vmw_prime_dmabuf_kmap_atomic,
>>> +   .unmap = vmw_prime_dmabuf_kunmap,
>>> +   .unmap_atomic = vmw_prime_dmabuf_kunmap_atomic,
>>> .mmap = vmw_prime_dmabuf_mmap,
>>> .vmap = vmw_prime_dmabuf_vmap,
>>> .vunmap = vmw_prime_dmabuf_vunmap,
>>> diff --git a/drivers/media/v4l2-core/videobuf2-dma-contig.c
>>> b/drivers/media/v4l2-core/videobuf2-dma-contig.c
>>> index fb6a177..2db0413 100644
>>> --- a/drivers/media/v4l2-core/videobuf2-dma-contig.c
>>> +++ b/drivers/media/v4l2-core/videobuf2-dma-contig.c
>>> @@ -356,8 +356,8 @@ static struct dma_buf_ops vb2_dc_dmabuf_ops = {
>>> .detach = vb2_dc_dmabuf_ops_detach,
>>> .map_dma_buf = vb2_dc_dmabuf_ops_map,
>>> .unmap_dma_buf = vb2_dc_dmabuf_ops_unmap,
>>> -   .kmap = vb2_dc_dmabuf_ops_kmap,
>>> -   .kmap_atomic = vb2_dc_dmabuf_ops_kmap,
>>> +   .map = vb2_dc_dmabuf_ops_kmap,
>>> +   .map_atomic = vb2_dc_dmabuf_ops_kmap,
>>> .vmap = vb2_dc_dmabuf_ops_vmap,
>>> .mmap = vb2_dc_dmabuf_ops_mmap,
>>> .release = vb2_dc_dmabuf_ops_release,
>>> diff --git a/drivers/media/v4l2-core/videobuf2-dma-sg.c
>>> b/drivers/media/v4l2-core/videobuf2-dma-sg.c
>>> index ecff8f4..6fd1343 100644
>>> --- a/drivers/media/v4l2-core/videobuf2-dma-sg.c
>>> +++ b/drivers/media/v4l2-core/videobuf2-dma-sg.c
>>> @@ -504,8 +504,8 @@ static struct dma_buf_ops vb2_dma_sg_dmabuf_ops = {
>>> .detach = vb2_dma_sg_dmabuf_ops_detach,
>>> .map_dma_buf = vb2_dma_sg_dmabuf_ops_map,
>>> .unmap_dma_buf = vb2_dma_sg_dmabuf_ops_unmap,
>>> -   .kmap = vb2_dma_sg_dmabuf_ops_kmap,
>>> -   .kmap_atomic = vb2_dma_sg_dmabuf_ops_kmap,
>>> +   .map = vb2_dma_sg_dmabuf_ops_kmap,
>>> +   .map_atomic = vb2_dma_sg_dmabuf_ops_kmap,
>>> .vmap = vb2_dma_sg_dmabuf_ops_vmap,
>>> .mmap = vb2_dma_sg_dmabuf_ops_mmap,
>>> .release = vb2_dma_sg_dmabuf_ops_release,
>>> diff --git a/drivers/media/v4l2-core/videobuf2-vmalloc.c
>>> b/drivers/media/v4l2-core/videobuf2-vmalloc.c
>>> index 3f77814..27d1db3 100644
>>> --- a/drivers/media/v4l2-core/videobuf2-vmalloc.c
>>> +++ b/drivers/media/v4l2-core/videobuf2-vmalloc.c
>>> @@ -342,8 +342,8 @@ static struct dma_buf_ops vb2_vmalloc_dmabuf_ops = {
>>> .detach = vb2_vmalloc_dmabuf_ops_detach,
>>> .map_dma_buf = vb2_vmalloc_dmabuf_ops_map,
>>> .unmap_dma_buf = vb2_vmalloc_dmabuf_ops_unmap,
>>> -   .kmap = vb2_vmalloc_dmabuf_ops_kmap,
>>> -   .kmap_atomic = vb2_vmalloc_dmabuf_ops_kmap,
>>> +   .map = vb2_vmalloc_dmabuf_ops_kmap,
>>> +   .map_atomic = vb2_vmalloc_dmabuf_ops_kmap,
>>> .vmap = vb2_vmalloc_dmabuf_ops_vmap,
>>> .mmap = vb2_vmalloc_dmabuf_ops_mmap,
>>> .release = vb2_vmalloc_dmabuf_ops_release,
>>> diff --git a/drivers/staging/android/ion/ion.c
>>> b/drivers/staging/android/ion/ion.c
>>> index f45115f..95a7f16 100644
>>> --- a/drivers/staging/android/ion/ion.c
>>> +++ b/drivers/staging/android/ion/ion.c
>>> @@ -1020,10 +1020,10 @@ static const struct dma_buf_ops dma_buf_ops = {
>>> .release = ion_dma_buf_release,
>>> .begin_cpu_access = ion_dma_buf_begin_cpu_access,
>>> .end_cpu_access = ion_dma_buf_end_cpu_access,
>>> -   .kmap_atomic = ion_dma_buf_kmap,
>>> -   .kunmap_atomic = ion_dma_buf_kunmap,
>>> -   .kmap = ion_dma_buf_kmap,
>>> -   .kunmap = ion_dma_buf_kunmap,
>>> +   .map_atomic = ion_dma_buf_kmap,
>>> +   .unmap_atomic = ion_dma_buf_kunmap,
>>> +   .map = ion_dma_buf_kmap,
>>> +   .unmap = ion_dma_buf_kunmap,
>>>   };
>>>
>>>   struct dma_buf *ion_share_dma_buf(struct ion_client *client,
>>> diff --git a/include/linux/dma-buf.h b/include/linux/dma-buf.h
>>> index bfb3704..79f27d6 100644
>>> --- a/include/linux/dma-buf.h
>>> +++ b/include/linux/dma-buf.h
>>> @@ -39,13 +39,13 @@ struct dma_buf_attachment;
>>>
>>>   /**
>>>* struct dma_buf_ops - operations possible on struct dma_buf
>>> - * @kmap_atomic: maps a page from the buffer into kernel address
>>> - *  space, users may not block until the subsequent unmap
>>> call.
>>> - *  This callback must not sleep.
>>> - * @kunmap_atomic: [optional] unmaps a atomically mapped page from the
>>> buffer.
>>> - *This Callback must not sleep.
>>> - * @kmap: maps a page from the buffer into kernel address space.
>>> - * @kunmap: [optional] unmaps a page from the buffer.
>>> + * @map_atomic: maps a page from the buffer into kernel address
>>> + * space, users may not block until the subsequent unmap
>>> call.
>>> + * This callback must not sleep.
>>> + * @unmap_atomic: [optional] unmaps a atomically mapped page from the
>>> buffer.
>>> + *   This Callback must not sleep.
>>> + * @map: maps a page from the buffer into kernel address space.
>>> + * @unmap: [optional] unmaps a page from the buffer.
>>>* @vmap: [optional] creates a virtual mapping for the buffer into
>>> kernel
>>>*  address space. Same restrictions as for vmap and friends apply.
>>>* @vunmap: [optional] unmaps a vmap from the buffer
>>> @@ -206,10 +206,10 @@ struct dma_buf_ops {
>>>  * to be restarted.
>>>  */
>>> int (*end_cpu_access)(struct dma_buf *, enum dma_data_direction);
>>> -   void *(*kmap_atomic)(struct dma_buf *, unsigned long);
>>> -   void (*kunmap_atomic)(struct dma_buf *, unsigned long, void *);
>>> -   void *(*kmap)(struct dma_buf *, unsigned long);
>>> -   void (*kunmap)(struct dma_buf *, unsigned long, void *);
>>> +   void *(*map_atomic)(struct dma_buf *, unsigned long);
>>> +   void (*unmap_atomic)(struct dma_buf *, unsigned long, void *);
>>> +   void *(*map)(struct dma_buf *, unsigned long);
>>> +   void (*unmap)(struct dma_buf *, unsigned long, void *);
>>>
>>> /**
>>>  * @mmap:
>
>
> Best regards
> --
> Marek Szyprowski, PhD
> Samsung R Institute Poland
>



-- 
Thanks and regards,

Sumit Semwal
Linaro Mobile Group - Kernel Team Lead
Linaro.org │ Open source software for ARM SoCs
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

Re: [Intel-gfx] [PATCH v2] dma-buf: Rename dma-ops to prevent conflict with kunmap_atomic macro

2017-04-20 Thread Sumit Semwal

Hi Logan,

Thanks for the patch.

On 20 April 2017 at 13:21, Daniel Vetter <dan...@ffwll.ch> wrote:
> On Wed, Apr 19, 2017 at 01:36:10PM -0600, Logan Gunthorpe wrote:
>> Seeing the kunmap_atomic dma_buf_ops share the same name with a macro
>> in highmem.h, the former can be aliased if any dma-buf user includes
>> that header.
>>
>> I'm personally trying to include highmem.h inside scatterlist.h and this
>> breaks the dma-buf code proper.
>>
>> Christoph Hellwig suggested [1] renaming it and pushing this patch ASAP.
>>
>> To maintain consistency I've renamed all four of kmap* and kunmap* to be
>> map* and unmap*. (Even though only kmap_atomic presently conflicts.)
>>
>> [1] https://www.spinics.net/lists/target-devel/msg15070.html
>>
>> Signed-off-by: Logan Gunthorpe <log...@deltatee.com>
>> Reviewed-by: Sinclair Yeh <s...@vmware.com>
>
> Acked-by: Daniel Vetter <daniel.vet...@ffwll.ch>
Acked-by: Sumit Semwal <sumit.sem...@linaro.org>
>
> Probably simplest if we pull this in through the drm-misc tree for 4.12.
> Can we have an ack for the v4l side for that pls?
>
> Thanks, Daniel
>
>> ---
>>
>> Changes since v1:
>>
>> - Added the missing tegra driver (noticed by kbuild robot)
>> - Rebased off of drm-intel-next to get the i915 selftest that is new
>> - Fixed nits Sinclair pointed out.
>>
>>  drivers/dma-buf/dma-buf.c  | 16 
>>  drivers/gpu/drm/armada/armada_gem.c|  8 
>>  drivers/gpu/drm/drm_prime.c|  8 
>>  drivers/gpu/drm/i915/i915_gem_dmabuf.c |  8 
>>  drivers/gpu/drm/i915/selftests/mock_dmabuf.c   |  8 
>>  drivers/gpu/drm/omapdrm/omap_gem_dmabuf.c  |  8 
>>  drivers/gpu/drm/tegra/gem.c|  8 
>>  drivers/gpu/drm/udl/udl_dmabuf.c   |  8 
>>  drivers/gpu/drm/vmwgfx/vmwgfx_prime.c  |  8 
>>  drivers/media/v4l2-core/videobuf2-dma-contig.c |  4 ++--
>>  drivers/media/v4l2-core/videobuf2-dma-sg.c |  4 ++--
>>  drivers/media/v4l2-core/videobuf2-vmalloc.c|  4 ++--
>>  drivers/staging/android/ion/ion.c  |  8 
>>  include/linux/dma-buf.h| 22 +++---
>>  14 files changed, 61 insertions(+), 61 deletions(-)
>>
>> diff --git a/drivers/dma-buf/dma-buf.c b/drivers/dma-buf/dma-buf.c
>> index f72aaac..512bdbc 100644
>> --- a/drivers/dma-buf/dma-buf.c
>> +++ b/drivers/dma-buf/dma-buf.c
>> @@ -405,8 +405,8 @@ struct dma_buf *dma_buf_export(const struct 
>> dma_buf_export_info *exp_info)
>> || !exp_info->ops->map_dma_buf
>> || !exp_info->ops->unmap_dma_buf
>> || !exp_info->ops->release
>> -   || !exp_info->ops->kmap_atomic
>> -   || !exp_info->ops->kmap
>> +   || !exp_info->ops->map_atomic
>> +   || !exp_info->ops->map
>> || !exp_info->ops->mmap)) {
>>   return ERR_PTR(-EINVAL);
>>   }
>> @@ -872,7 +872,7 @@ void *dma_buf_kmap_atomic(struct dma_buf *dmabuf, 
>> unsigned long page_num)
>>  {
>>   WARN_ON(!dmabuf);
>>
>> - return dmabuf->ops->kmap_atomic(dmabuf, page_num);
>> + return dmabuf->ops->map_atomic(dmabuf, page_num);
>>  }
>>  EXPORT_SYMBOL_GPL(dma_buf_kmap_atomic);
>>
>> @@ -889,8 +889,8 @@ void dma_buf_kunmap_atomic(struct dma_buf *dmabuf, 
>> unsigned long page_num,
>>  {
>>   WARN_ON(!dmabuf);
>>
>> - if (dmabuf->ops->kunmap_atomic)
>> - dmabuf->ops->kunmap_atomic(dmabuf, page_num, vaddr);
>> + if (dmabuf->ops->unmap_atomic)
>> + dmabuf->ops->unmap_atomic(dmabuf, page_num, vaddr);
>>  }
>>  EXPORT_SYMBOL_GPL(dma_buf_kunmap_atomic);
>>
>> @@ -907,7 +907,7 @@ void *dma_buf_kmap(struct dma_buf *dmabuf, unsigned long 
>> page_num)
>>  {
>>   WARN_ON(!dmabuf);
>>
>> - return dmabuf->ops->kmap(dmabuf, page_num);
>> + return dmabuf->ops->map(dmabuf, page_num);
>>  }
>>  EXPORT_SYMBOL_GPL(dma_buf_kmap);
>>
>> @@ -924,8 +924,8 @@ void dma_buf_kunmap(struct dma_buf *dmabuf, unsigned 
>> long page_num,
>>  {
>>   WARN_ON(!dmabuf);
>>
>> - if (dmabuf->ops->kunmap)
>> - dmabuf->ops

Re: [Intel-gfx] [PATCH] drm: Document code of conduct

2017-04-12 Thread Sumit Semwal

Hi Daniel,

On 11 April 2017 at 13:03, Sumit Semwal <sumit.sem...@linaro.org> wrote:
> On 11 April 2017 at 12:38, Daniel Stone <dan...@fooishbar.org> wrote:
>> Hi,
>>
>> On 11 April 2017 at 07:48, Daniel Vetter <daniel.vet...@ffwll.ch> wrote:
>>> Note: As Daniel Stone mentioned in the announcement fd.o admins
>>> started chatting with the communities their hosting, which includs the
>>> X.org foundation board, to figure out how to fan out enforcement and
>>> allow projects to run things on their own (with fd.o still as the
>>> fallback).  So the details of enforcement (and appealing decisions)
>>> might still change, but since this involves the board and lots more
>>> people it'll take a while to get there. For now this is good enough I
>>> think.
>>
>> All true.
>>
>> Reviewed-by: Daniel Stone <dani...@collabora.com>
>>
And of course,
Acked-by: Sumit Semwal <sumit.sem...@linaro.org>
:)
> Thanks for this, Daniel!
>
> Reviewed-by: Sumit Semwal <sumit.sem...@linaro.org>
>
> Best,
> Sumit.
>
>> Cheers,
>> Daniel



-- 
Thanks and regards,

Sumit Semwal
Linaro Mobile Group - Kernel Team Lead
Linaro.org │ Open source software for ARM SoCs
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

Re: [Intel-gfx] [PATCH] drm: Document code of conduct

2017-04-11 Thread Sumit Semwal

On 11 April 2017 at 12:38, Daniel Stone <dan...@fooishbar.org> wrote:
> Hi,
>
> On 11 April 2017 at 07:48, Daniel Vetter <daniel.vet...@ffwll.ch> wrote:
>> Note: As Daniel Stone mentioned in the announcement fd.o admins
>> started chatting with the communities their hosting, which includs the
>> X.org foundation board, to figure out how to fan out enforcement and
>> allow projects to run things on their own (with fd.o still as the
>> fallback).  So the details of enforcement (and appealing decisions)
>> might still change, but since this involves the board and lots more
>> people it'll take a while to get there. For now this is good enough I
>> think.
>
> All true.
>
> Reviewed-by: Daniel Stone <dani...@collabora.com>
>
Thanks for this, Daniel!

Reviewed-by: Sumit Semwal <sumit.sem...@linaro.org>

Best,
Sumit.

> Cheers,
> Daniel
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

Re: [Intel-gfx] [PATCH] dma/fence: Export enable-signaling tracepoint for emission by drivers

2017-01-27 Thread Sumit Semwal

fwiw,


On 27 January 2017 at 20:25, Gustavo Padovan <gust...@padovan.org> wrote:
> Hi Chris,
>
> 2017-01-24 Chris Wilson <ch...@chris-wilson.co.uk>:
>
>> Currently this tracepoint is solely used by dma_fence_enable_sw_signaling,
>> however I have a need to manually perform the hw enabling of the
>> signaling and would like to emit this tracepoint for completeness.
>>
>> Signed-off-by: Chris Wilson <ch...@chris-wilson.co.uk>
>> Cc: Sumit Semwal <sumit.sem...@linaro.org>
>> Cc: Daniel Vetter <daniel.vet...@ffwll.ch>
>> ---
>>  drivers/dma-buf/dma-fence.c | 1 +
>>  1 file changed, 1 insertion(+)
>
> Reviewed-by: Gustavo Padovan <gustavo.pado...@collabora.com>
>
Feel free to add my a-b :)
> Gustavo
>
> ___
> dri-devel mailing list
> dri-de...@lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/dri-devel



-- 
Thanks and regards,

Sumit Semwal
Linaro Mobile Group - Kernel Team Lead
Linaro.org │ Open source software for ARM SoCs
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

Re: [Intel-gfx] [PATCH 3/3] dma-fence: Introduce drm_fence_set_error() helper

2017-01-09 Thread Sumit Semwal

Hi Chris,

On 4 January 2017 at 19:42, Chris Wilson  wrote:
> The dma_fence.error field (formerly known as dma_fence.status) is an
> optional field that may be set by drivers before calling
> dma_fence_signal(). The field can be used to indicate that the fence was
> completed in err rather than with success, and is visible to other
> consumers of the fence and to userspace via sync_file.
>
> This patch renames the field from status to error so that its meaning is
> hopefully more clear (and distinct from dma_fence_get_status() which is
> a composite between the error state and signal state) and adds a helper
> that validates the preconditions of when it is suitable to adjust the
> error field.
>
> Signed-off-by: Chris Wilson 
> ---
>  drivers/dma-buf/dma-fence.c |  2 +-
>  include/linux/dma-fence.h   | 30 +-
>  2 files changed, 26 insertions(+), 6 deletions(-)
>
> diff --git a/drivers/dma-buf/dma-fence.c b/drivers/dma-buf/dma-fence.c
> index 7d936d19e659..ee82f36cb25e 100644
> --- a/drivers/dma-buf/dma-fence.c
> +++ b/drivers/dma-buf/dma-fence.c
> @@ -559,7 +559,7 @@ dma_fence_init(struct dma_fence *fence, const struct 
> dma_fence_ops *ops,
> fence->context = context;
> fence->seqno = seqno;
> fence->flags = 0UL;
> -   fence->status = 0;
> +   fence->error = 0;
>
> trace_dma_fence_init(fence);
>  }
> diff --git a/include/linux/dma-fence.h b/include/linux/dma-fence.h
> index 19f7af905182..6048fa404e57 100644
> --- a/include/linux/dma-fence.h
> +++ b/include/linux/dma-fence.h
> @@ -47,7 +47,7 @@ struct dma_fence_cb;
>   * can be compared to decide which fence would be signaled later.
>   * @flags: A mask of DMA_FENCE_FLAG_* defined below
>   * @timestamp: Timestamp when the fence was signaled.
> - * @status: Optional, only valid if < 0, must be set before calling
> + * @error: Optional, only valid if < 0, must be set before calling
>   * dma_fence_signal, indicates that the fence has completed with an error.
>   *
>   * the flags member must be manipulated and read using the appropriate
> @@ -79,7 +79,7 @@ struct dma_fence {
> unsigned seqno;
> unsigned long flags;
> ktime_t timestamp;
> -   int status;
> +   int error;
>  };
>
>  enum dma_fence_flag_bits {
> @@ -133,7 +133,7 @@ struct dma_fence_cb {
>   * or some failure occurred that made it impossible to enable
>   * signaling. True indicates successful enabling.
>   *
> - * fence->status may be set in enable_signaling, but only when false is
> + * fence->error may be set in enable_signaling, but only when false is
>   * returned.
>   *
>   * Calling dma_fence_signal before enable_signaling is called allows
> @@ -145,7 +145,7 @@ struct dma_fence_cb {
>   * the second time will be a noop since it was already signaled.
>   *
>   * Notes on signaled:
> - * May set fence->status if returning true.
> + * May set fence->error if returning true.
>   *
>   * Notes on wait:
>   * Must not be NULL, set to dma_fence_default_wait for default 
> implementation.
> @@ -395,13 +395,33 @@ static inline struct dma_fence *dma_fence_later(struct 
> dma_fence *f1,
>  static inline int dma_fence_get_status_locked(struct dma_fence *fence)
>  {
> if (dma_fence_is_signaled_locked(fence))
> -   return fence->status < 0 ? fence->status : 1;
> +   return fence->error ?: 1;
> else
> return 0;
>  }
>
>  int dma_fence_get_status(struct dma_fence *fence);
>
> +/**
> + * dma_fence_set_error - flag an error condition on the fence
> + * @fence: [in]the dma_fence
> + * @error: [in]the error to store
> + *
> + * Drivers can supply an optional error status condition before they signal
> + * the fence, to indicate that the fence was completed due to an error
> + * rather than success. This must be set before signaling (so that the value
> + * is visible before any waiters on the signal callback are woken). This
> + * helper exists to help catching erroneous setting of #dma_fence.error.
> + */
> +static inline void dma_fence_set_error(struct dma_fence *fence,
> +  int error)
> +{
> +   BUG_ON(test_bit(DMA_FENCE_FLAG_SIGNALED_BIT, >flags));
> +   BUG_ON(error >= 0 || error < -MAX_ERRNO);
> +
> +   fence->error = error;
> +}
Sorry I missed this earlier, but are you sure you want to use a BUG_ON
here, and not a WARN_ON?
> +
>  signed long dma_fence_wait_timeout(struct dma_fence *,
>bool intr, signed long timeout);
>  signed long dma_fence_wait_any_timeout(struct dma_fence **fences,
> --
> 2.11.0
>
> ___
> dri-devel mailing list
> dri-de...@lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/dri-devel

Best,
Sumit.
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org

Re: [Intel-gfx] [PATCH 1/3] dma-fence: Clear fence->status during dma_fence_init()

2017-01-04 Thread Sumit Semwal

Hi Chris,

Thanks for your patches!

On 4 January 2017 at 20:40, Daniel Vetter <dan...@ffwll.ch> wrote:
> On Wed, Jan 04, 2017 at 02:12:20PM +, Chris Wilson wrote:
>> As the fence->status is an optional field that may be set before
>> dma_fence_signal() is called to convey that the fence completed with an
>> error, we have to ensure that it is always set to zero on initialisation
>> so that the typical use (i.e. unset) always flags a successful completion.
>>
>> Signed-off-by: Chris Wilson <ch...@chris-wilson.co.uk>
>> Reviewed-by: Tvrtko Ursulin <tvrtko.ursu...@intel.com>
>
> Yeah, this looks all pretty. On the series:
>
> Reviewed-by: Daniel Vetter <daniel.vet...@ffwll.ch>
>
FWIW, please feel free, for this series, to apply my

Reviewed-by: Sumit Semwal <sumit.sem...@linaro.org>

> I'll defer to Gustavo for another pass over it and merging it to drm-misc.
> -Daniel
>

Best,
Sumit.
>> ---
>>  drivers/dma-buf/dma-fence.c | 1 +
>>  1 file changed, 1 insertion(+)
>>
>> diff --git a/drivers/dma-buf/dma-fence.c b/drivers/dma-buf/dma-fence.c
>> index 3444f293ad4a..9130f790ebf3 100644
>> --- a/drivers/dma-buf/dma-fence.c
>> +++ b/drivers/dma-buf/dma-fence.c
>> @@ -534,6 +534,7 @@ dma_fence_init(struct dma_fence *fence, const struct 
>> dma_fence_ops *ops,
>>   fence->context = context;
>>   fence->seqno = seqno;
>>   fence->flags = 0UL;
>> + fence->status = 0;
>>
>>   trace_dma_fence_init(fence);
>>  }
>> --
>> 2.11.0
>>
>> ___
>> dri-devel mailing list
>> dri-de...@lists.freedesktop.org
>> https://lists.freedesktop.org/mailman/listinfo/dri-devel
>
> --
> Daniel Vetter
> Software Engineer, Intel Corporation
> http://blog.ffwll.ch
> ___
> dri-devel mailing list
> dri-de...@lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/dri-devel



-- 
Thanks and regards,

Sumit Semwal
Linaro Mobile Group - Kernel Team Lead
Linaro.org │ Open source software for ARM SoCs
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

Re: [Intel-gfx] [PATCH 0/5] sphinxification for dma-buf docs

2016-12-13 Thread Sumit Semwal

Thanks Jonathan!

On 12 December 2016 at 01:14, Jonathan Corbet  wrote:
> On Sun, 11 Dec 2016 18:35:42 +0100
> Daniel Vetter  wrote:
>
>> > Here's a thought, though: how about if we slip in a little version of
>> > dma-buf.rst now with a "coming soon, don't miss it!!" message?  Then the
>> > rest of the set could go through your tree without touching
>> > driver-api/index.rst and, thus, avoiding the otherwise inevitable
>> > conflicts?
>>
>> So just patch 1 still for 4.10 through your tree? Sounds good to me.
>
> Yeah, that will work just fine, I'll slip it in.
Pushed the rest of the patches in drm-misc-next, queued for 4.11.
>
> Thanks,
>
> jon
Best,
Sumit.
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

Re: [Intel-gfx] [PATCH 0/5] sphinxification for dma-buf docs

2016-12-11 Thread Sumit Semwal

Hi Daniel,

On 10 December 2016 at 02:45, Jonathan Corbet <cor...@lwn.net> wrote:
> On Fri,  9 Dec 2016 19:53:04 +0100
> Daniel Vetter <daniel.vet...@ffwll.ch> wrote:
>
>> Not yet everything in this area, I still want to sprinkle nice docs around 
>> all
>> the fence code. Especially some text to explain implicit vs. explicit fencing
>> and how it's all supposed to work.
>>
Thanks for the patch series; I had something in the works too, but you
beat me to it! :)

Looks good to me, so please feel free to add my
Acked-by: Sumit Semwal <sumit.sem...@linaro.org>

to the series.

>> But just cleanup in the dma-buf part was quite a bit of work, and I'd like to
>> get feedback on that before moving on.
>
> No complaints here - except that I had to go looking around to find this
> 0/5 posting explaining what the overall goal was...:)
>
> It seems like just the sort of thing we want to be doing to pull the docs
> together in a more rational way.
>
> jon

Best regards,
Sumit.
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

Re: [Intel-gfx] [PATCH 02/11] drm/etnaviv: Remove manual call to reservation_object_test_signaled_rcu before wait

2016-10-05 Thread Sumit Semwal

Hi Lucas,

On 23 September 2016 at 18:25, Daniel Vetter  wrote:
> On Mon, Aug 29, 2016 at 08:08:25AM +0100, Chris Wilson wrote:
>> Since fence_wait_timeout_reservation_object_wait_timeout_rcu() with a
>> timeout of 0 becomes reservation_object_test_signaled_rcu(), we do not
>> need to handle such conversion in the caller. The only challenge are
>> those callers that wish to differentiate the error code between the
>> nonblocking busy check and potentially blocking wait.
>>
>> Signed-off-by: Chris Wilson 
>> Cc: Lucas Stach 
>> Cc: Russell King 
>> Cc: Christian Gmeiner 
>
> Reviewed-by: Daniel Vetter 
>
Could you please let me know if this is in your tree already, or would
you like me to take it via drm-misc (in which case, an Acked-by would
be fabulous!)

Thanks and best,
Sumit.
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

Re: [Intel-gfx] [PATCH 05/11] drm/vmwgfx: Remove call to reservation_object_test_signaled_rcu before wait

2016-10-05 Thread Sumit Semwal

Hi Thomas, Sinclair,

On 23 September 2016 at 18:26, Daniel Vetter  wrote:
> On Mon, Aug 29, 2016 at 08:08:28AM +0100, Chris Wilson wrote:
>> Since fence_wait_timeout_reservation_object_wait_timeout_rcu() with a
>> timeout of 0 becomes reservation_object_test_signaled_rcu(), we do not
>> need to handle such conversion in the caller. The only challenge are
>> those callers that wish to differentiate the error code between the
>> nonblocking busy check and potentially blocking wait.
>>
>> Signed-off-by: Chris Wilson 
>> Cc: Sinclair Yeh 
>> Cc: Thomas Hellstrom 
>> Reviewed-by: Sinclair Yeh 
>
> Reviewed-by: Daniel Vetter 
>
Could you please let me know if this patch is already queued up at
your end, or should I just take it via drm-misc with Sinclair's r-b?

Thanks and Best,
Sumit.
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

Re: [Intel-gfx] [PATCH 04/11] drm/nouveau: Remove call to reservation_object_test_signaled_rcu before wait

2016-10-05 Thread Sumit Semwal

Hi Ben,

On 23 September 2016 at 18:25, Daniel Vetter  wrote:
> On Mon, Aug 29, 2016 at 08:08:27AM +0100, Chris Wilson wrote:
>> Since fence_wait_timeout_reservation_object_wait_timeout_rcu() with a
>> timeout of 0 becomes reservation_object_test_signaled_rcu(), we do not
>> need to handle such conversion in the caller. The only challenge are
>> those callers that wish to differentiate the error code between the
>> nonblocking busy check and potentially blocking wait.
>>
>> Signed-off-by: Chris Wilson 
>> Cc: Ben Skeggs 
>
> Reviewed-by: Daniel Vetter 
>
May I please know if this patch is already in your queue, or should I
take it through drm-misc (in which case an Acked-by would be highly
appreciated :) )


Best,
Sumit.
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

Re: [Intel-gfx] [PATCH 01/11] drm/amdgpu: Remove call to reservation_object_test_signaled_rcu before wait

2016-10-05 Thread Sumit Semwal

Hi Alex,

On 23 September 2016 at 18:24, Daniel Vetter  wrote:
> On Mon, Aug 29, 2016 at 08:08:24AM +0100, Chris Wilson wrote:
>> Since fence_wait_timeout_reservation_object_wait_timeout_rcu() with a
>> timeout of 0 becomes reservation_object_test_signaled_rcu(), we do not
>> need to handle such conversion in the caller. The only challenge are
>> those callers that wish to differentiate the error code between the
>> nonblocking busy check and potentially blocking wait.
>>
>> Signed-off-by: Chris Wilson 
>> Cc: Alex Deucher 
>> Cc: Christian König 
>
> Reviewed-by: Daniel Vetter 
>> ---
I couldn't find if its already applied to your tree, or your acked-by;
could you please let me know if it's there, or if you'd like me to
pick it up via drm-misc (and an Acked-by would be appreciated in the
latter case :) )

Best,
Sumit.
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

Re: [Intel-gfx] [PATCH] dma-buf: Release module reference on creation failure

2016-07-18 Thread Sumit Semwal

Hi Chris,

On 18 July 2016 at 17:40, Daniel Vetter <dan...@ffwll.ch> wrote:
> On Mon, Jul 18, 2016 at 12:16:22PM +0100, Chris Wilson wrote:
>> If we fail to create the anon file, we need to remember to release the
>> module reference on the owner.
>>
Thanks for the patch.

>> Signed-off-by: Chris Wilson <ch...@chris-wilson.co.uk>
>> Reviewed-by: Joonas Lahtinen <joonas.lahti...@linux.intel.com>
>> Cc: Joonas Lahtinen <joonas.lahti...@linux.intel.com>
>> Cc: Sumit Semwal <sumit.sem...@linaro.org>
>> Cc: Daniel Vetter <daniel.vet...@ffwll.ch>
>> Cc: linux-me...@vger.kernel.org
>> Cc: dri-de...@lists.freedesktop.org
>> Cc: linaro-mm-...@lists.linaro.org
>> ---
>>  drivers/dma-buf/dma-buf.c | 15 +++
>>  1 file changed, 11 insertions(+), 4 deletions(-)
>>
>> diff --git a/drivers/dma-buf/dma-buf.c b/drivers/dma-buf/dma-buf.c
>> index 20ce0687b111..ddaee60ae52a 100644
>> --- a/drivers/dma-buf/dma-buf.c
>> +++ b/drivers/dma-buf/dma-buf.c
>> @@ -334,6 +334,7 @@ struct dma_buf *dma_buf_export(const struct 
>> dma_buf_export_info *exp_info)
>>   struct reservation_object *resv = exp_info->resv;
>>   struct file *file;
>>   size_t alloc_size = sizeof(struct dma_buf);
>> + int ret;
>
> Not sure this really helps readability, but meh. Will apply to drm-misc,
> with a cc: stable.
Daniel, fwiw, please feel free to add
Acked-by: Sumit Semwal <sumit.sem...@linaro.org>

> -Daniel

BR,
~Sumit.
>
>>
>>   if (!exp_info->resv)
>>   alloc_size += sizeof(struct reservation_object);
>> @@ -357,8 +358,8 @@ struct dma_buf *dma_buf_export(const struct 
>> dma_buf_export_info *exp_info)
>>
>>   dmabuf = kzalloc(alloc_size, GFP_KERNEL);
>>   if (!dmabuf) {
>> - module_put(exp_info->owner);
>> - return ERR_PTR(-ENOMEM);
>> + ret = -ENOMEM;
>> + goto err_module;
>>   }
>>
>>   dmabuf->priv = exp_info->priv;
>> @@ -379,8 +380,8 @@ struct dma_buf *dma_buf_export(const struct 
>> dma_buf_export_info *exp_info)
>>   file = anon_inode_getfile("dmabuf", _buf_fops, dmabuf,
>>   exp_info->flags);
>>   if (IS_ERR(file)) {
>> - kfree(dmabuf);
>> - return ERR_CAST(file);
>> + ret = PTR_ERR(file);
>> + goto err_dmabuf;
>>   }
>>
>>   file->f_mode |= FMODE_LSEEK;
>> @@ -394,6 +395,12 @@ struct dma_buf *dma_buf_export(const struct 
>> dma_buf_export_info *exp_info)
>>   mutex_unlock(_list.lock);
>>
>>   return dmabuf;
>> +
>> +err_dmabuf:
>> + kfree(dmabuf);
>> +err_module:
>> + module_put(exp_info->owner);
>> + return ERR_PTR(ret);
>>  }
>>  EXPORT_SYMBOL_GPL(dma_buf_export);
>>
>> --
>> 2.8.1
>>
>
> --
> Daniel Vetter
> Software Engineer, Intel Corporation
> http://blog.ffwll.ch



-- 
Thanks and regards,

Sumit Semwal
Linaro Mobile Group - Kernel Team Lead
Linaro.org │ Open source software for ARM SoCs
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

Re: [Intel-gfx] [PATCH] dma-buf: Update docs for SYNC ioctl

2016-03-21 Thread Sumit Semwal

On 21 March 2016 at 13:00, Daniel Vetter <daniel.vet...@ffwll.ch> wrote:
> Just a bit of wording polish plus mentioning that it can fail and must
> be restarted.
>
> Requested by Sumit.
>
> Cc: Chris Wilson <ch...@chris-wilson.co.uk>
> Cc: Tiago Vignatti <tiago.vigna...@intel.com>
> Cc: Stéphane Marchesin <marc...@chromium.org>
> Cc: David Herrmann <dh.herrm...@gmail.com>
> Cc: Sumit Semwal <sumit.sem...@linaro.org>
> Cc: Daniel Vetter <daniel.vet...@intel.com>
> CC: linux-me...@vger.kernel.org
> Cc: dri-de...@lists.freedesktop.org
> Cc: linaro-mm-...@lists.linaro.org
> Cc: intel-gfx@lists.freedesktop.org
> Cc: de...@driverdev.osuosl.org
> Signed-off-by: Daniel Vetter <daniel.vet...@intel.com>
Acked-by: Sumit Semwal <sumit.sem...@linaro.org>

> ---
>  Documentation/dma-buf-sharing.txt | 11 ++-
>  drivers/dma-buf/dma-buf.c |  2 +-
>  2 files changed, 7 insertions(+), 6 deletions(-)
>
> diff --git a/Documentation/dma-buf-sharing.txt 
> b/Documentation/dma-buf-sharing.txt
> index 32ac32e773e1..5c4e3e586ec8 100644
> --- a/Documentation/dma-buf-sharing.txt
> +++ b/Documentation/dma-buf-sharing.txt
> @@ -352,7 +352,8 @@ Being able to mmap an export dma-buf buffer object has 2 
> main use-cases:
>
> No special interfaces, userspace simply calls mmap on the dma-buf fd, 
> making
> sure that the cache synchronization ioctl (DMA_BUF_IOCTL_SYNC) is *always*
> -   used when the access happens. This is discussed next paragraphs.
> +   used when the access happens. Note that DMA_BUF_IOCTL_SYNC can fail with
> +   -EGAIN or -EINTR, in which case it must be restarted.
>
> Some systems might need some sort of cache coherency management e.g. when
> CPU and GPU domains are being accessed through dma-buf at the same time. 
> To
> @@ -366,10 +367,10 @@ Being able to mmap an export dma-buf buffer object has 
> 2 main use-cases:
> want (with the new data being consumed by the GPU or say scanout 
> device)
>   - munmap once you don't need the buffer any more
>
> -Therefore, for correctness and optimal performance, systems with the 
> memory
> -cache shared by the GPU and CPU i.e. the "coherent" and also the
> -"incoherent" are always required to use SYNC_START and SYNC_END before 
> and
> -after, respectively, when accessing the mapped address.
> +For correctness and optimal performance, it is always required to use
> +SYNC_START and SYNC_END before and after, respectively, when accessing 
> the
> +mapped address. Userspace cannot on coherent access, even when there are
> +systems where it just works without calling these ioctls.
>
>  2. Supporting existing mmap interfaces in importers
>
> diff --git a/drivers/dma-buf/dma-buf.c b/drivers/dma-buf/dma-buf.c
> index 774a60f4309a..4a2c07ee6677 100644
> --- a/drivers/dma-buf/dma-buf.c
> +++ b/drivers/dma-buf/dma-buf.c
> @@ -612,7 +612,7 @@ EXPORT_SYMBOL_GPL(dma_buf_begin_cpu_access);
>   * @dmabuf:[in]buffer to complete cpu access for.
>   * @direction: [in]length of range for cpu access.
>   *
> - * This call must always succeed.
> + * Can return negative error values, returns 0 on success.
>   */
>  int dma_buf_end_cpu_access(struct dma_buf *dmabuf,
>enum dma_data_direction direction)
> --
> 2.8.0.rc3
>

Best regards,
Sumit.
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

Re: [Intel-gfx] [PATCH] dma-buf, drm, ion: Propagate error code from dma_buf_start_cpu_access()

2016-03-21 Thread Sumit Semwal

On 19 March 2016 at 15:39, Daniel Vetter <dan...@ffwll.ch> wrote:
> On Fri, Mar 18, 2016 at 08:02:39PM +, Chris Wilson wrote:
>> Drivers, especially i915.ko, can fail during the initial migration of a
>> dma-buf for CPU access. However, the error code from the driver was not
>> being propagated back to ioctl and so userspace was blissfully ignorant
>> of the failure. Rendering corruption ensues.
>>
>> Whilst fixing the ioctl to return the error code from
>> dma_buf_start_cpu_access(), also do the same for
>> dma_buf_end_cpu_access().  For most drivers, dma_buf_end_cpu_access()
>> cannot fail. i915.ko however, as most drivers would, wants to avoid being
>> uninterruptible (as would be required to guarrantee no failure when
>> flushing the buffer to the device). As userspace already has to handle
>> errors from the SYNC_IOCTL, take advantage of this to be able to restart
>> the syscall across signals.
>>
>> This fixes a coherency issue for i915.ko as well as reducing the
>> uninterruptible hold upon its BKL, the struct_mutex.
>>
>> Fixes commit c11e391da2a8fe973c3c2398452000bed505851e
>> Author: Daniel Vetter <daniel.vet...@ffwll.ch>
>> Date:   Thu Feb 11 20:04:51 2016 -0200
>>
>> dma-buf: Add ioctls to allow userspace to flush
>>
>> Testcase: igt/gem_concurrent_blit/*dmabuf*interruptible
>> Testcase: igt/prime_mmap_coherency/ioctl-errors
>> Signed-off-by: Chris Wilson <ch...@chris-wilson.co.uk>
>> Cc: Tiago Vignatti <tiago.vigna...@intel.com>
>> Cc: Stéphane Marchesin <marc...@chromium.org>
>> Cc: David Herrmann <dh.herrm...@gmail.com>
>> Cc: Sumit Semwal <sumit.sem...@linaro.org>
>> Cc: Daniel Vetter <daniel.vet...@intel.com>
>> CC: linux-me...@vger.kernel.org
>> Cc: dri-de...@lists.freedesktop.org
>> Cc: linaro-mm-...@lists.linaro.org
>> Cc: intel-gfx@lists.freedesktop.org
>> Cc: de...@driverdev.osuosl.org
>
> Applied to drm-misc, I'll send a pull to Dave the next few days if no one
> screams.
> -Daniel
Thanks for pulling it via drm-misc, Daniel.
Chris, I feel since this is an API change, it also needs an update to
the Documentation file.
With that, and a minor nit below, please feel free to add
Acked-by: Sumit Semwal <sumit.sem...@linaro.org>

>
>> ---
>>  drivers/dma-buf/dma-buf.c | 17 +++--
>>  drivers/gpu/drm/i915/i915_gem_dmabuf.c| 15 +--
>>  drivers/gpu/drm/omapdrm/omap_gem_dmabuf.c |  5 +++--
>>  drivers/gpu/drm/udl/udl_fb.c  |  4 ++--
>>  drivers/staging/android/ion/ion.c |  6 --
>>  include/linux/dma-buf.h   |  6 +++---
>>  6 files changed, 28 insertions(+), 25 deletions(-)
>>
>> diff --git a/drivers/dma-buf/dma-buf.c b/drivers/dma-buf/dma-buf.c
>> index 9810d1df0691..774a60f4309a 100644
>> --- a/drivers/dma-buf/dma-buf.c
>> +++ b/drivers/dma-buf/dma-buf.c
>> @@ -259,6 +259,7 @@ static long dma_buf_ioctl(struct file *file,
>>   struct dma_buf *dmabuf;
>>   struct dma_buf_sync sync;
>>   enum dma_data_direction direction;
>> + int ret;
>>
>>   dmabuf = file->private_data;
>>
>> @@ -285,11 +286,11 @@ static long dma_buf_ioctl(struct file *file,
>>   }
>>
>>   if (sync.flags & DMA_BUF_SYNC_END)
>> - dma_buf_end_cpu_access(dmabuf, direction);
>> + ret = dma_buf_end_cpu_access(dmabuf, direction);
>>   else
>> - dma_buf_begin_cpu_access(dmabuf, direction);
>> + ret = dma_buf_begin_cpu_access(dmabuf, direction);
>>
>> - return 0;
>> + return ret;
>>   default:
>>   return -ENOTTY;
>>   }
>> @@ -613,13 +614,17 @@ EXPORT_SYMBOL_GPL(dma_buf_begin_cpu_access);
>>   *
>>   * This call must always succeed.
>>   */
Perhaps update the above comment to reflect the change as well?

>> -void dma_buf_end_cpu_access(struct dma_buf *dmabuf,
>> - enum dma_data_direction direction)
>> +int dma_buf_end_cpu_access(struct dma_buf *dmabuf,
>> +enum dma_data_direction direction)
>>  {
>> + int ret = 0;
>> +
>>   WARN_ON(!dmabuf);
>>
>>   if (dmabuf->ops->end_cpu_access)
>> - dmabuf->ops->end_cpu_access(dmabuf, direction);
>> + ret = dmabuf->ops->end_cpu_access(dmabuf, direction);
>> +
>> + return ret;
>>  }
<< snip>>

Best regards,
Sumit.
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

Re: [Intel-gfx] [PATCH 2/4] dma-buf: Add ioctls to allow userspace to flush

2015-08-12 Thread Sumit Semwal


 ___
 dri-devel mailing list
 dri-de...@lists.freedesktop.org
 http://lists.freedesktop.org/mailman/listinfo/dri-devel



-- 
Thanks and regards,
Sumit Semwal.
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx

[Intel-gfx] [PATCH] dma-buf: add ref counting for module as exporter

2015-05-07 Thread Sumit Semwal

Add reference counting on a kernel module that exports dma-buf and
implements its operations. This prevents the module from being unloaded
while DMABUF file is in use.

The original patch [1] was submitted by Tomasz, but he's since shifted
jobs and a ping didn't elicit any response.

  [tomasz: Original author]
Signed-off-by: Tomasz Stanislawski t.stanisl...@samsung.com
Acked-by: Daniel Vetter dan...@ffwll.ch
  [sumits: updated  rebased as per review comments]
Signed-off-by: Sumit Semwal sumit.sem...@linaro.org

[1]: https://lkml.org/lkml/2012/8/8/163
---
 drivers/dma-buf/dma-buf.c  | 9 -
 drivers/gpu/drm/armada/armada_gem.c| 1 +
 drivers/gpu/drm/drm_prime.c| 1 +
 drivers/gpu/drm/exynos/exynos_drm_dmabuf.c | 1 +
 drivers/gpu/drm/i915/i915_gem_dmabuf.c | 1 +
 drivers/gpu/drm/omapdrm/omap_gem_dmabuf.c  | 1 +
 drivers/gpu/drm/tegra/gem.c| 1 +
 drivers/gpu/drm/udl/udl_dmabuf.c   | 1 +
 drivers/gpu/drm/vmwgfx/vmwgfx_prime.c  | 1 +
 drivers/media/v4l2-core/videobuf2-dma-contig.c | 1 +
 drivers/media/v4l2-core/videobuf2-dma-sg.c | 1 +
 drivers/media/v4l2-core/videobuf2-vmalloc.c| 1 +
 drivers/staging/android/ion/ion.c  | 1 +
 include/linux/dma-buf.h| 2 ++
 14 files changed, 22 insertions(+), 1 deletion(-)

diff --git a/drivers/dma-buf/dma-buf.c b/drivers/dma-buf/dma-buf.c
index c5a9138a6a8d..9583eac0238f 100644
--- a/drivers/dma-buf/dma-buf.c
+++ b/drivers/dma-buf/dma-buf.c
@@ -29,6 +29,7 @@
 #include linux/anon_inodes.h
 #include linux/export.h
 #include linux/debugfs.h
+#include linux/module.h
 #include linux/seq_file.h
 #include linux/poll.h
 #include linux/reservation.h
@@ -64,6 +65,7 @@ static int dma_buf_release(struct inode *inode, struct file 
*file)
BUG_ON(dmabuf-cb_shared.active || dmabuf-cb_excl.active);
 
dmabuf-ops-release(dmabuf);
+   module_put(dmabuf-ops-owner);
 
mutex_lock(db_list.lock);
list_del(dmabuf-list_node);
@@ -302,9 +304,14 @@ struct dma_buf *dma_buf_export(const struct 
dma_buf_export_info *exp_info)
return ERR_PTR(-EINVAL);
}
 
+   if (!try_module_get(exp_info-ops-owner))
+   return ERR_PTR(-ENOENT);
+
dmabuf = kzalloc(alloc_size, GFP_KERNEL);
-   if (dmabuf == NULL)
+   if (!dmabuf) {
+   module_put(exp_info-ops-owner);
return ERR_PTR(-ENOMEM);
+   }
 
dmabuf-priv = exp_info-priv;
dmabuf-ops = exp_info-ops;
diff --git a/drivers/gpu/drm/armada/armada_gem.c 
b/drivers/gpu/drm/armada/armada_gem.c
index 580e10acaa3a..d2c5fc1269b7 100644
--- a/drivers/gpu/drm/armada/armada_gem.c
+++ b/drivers/gpu/drm/armada/armada_gem.c
@@ -524,6 +524,7 @@ armada_gem_dmabuf_mmap(struct dma_buf *buf, struct 
vm_area_struct *vma)
 }
 
 static const struct dma_buf_ops armada_gem_prime_dmabuf_ops = {
+   .owner = THIS_MODULE,
.map_dma_buf= armada_gem_prime_map_dma_buf,
.unmap_dma_buf  = armada_gem_prime_unmap_dma_buf,
.release= drm_gem_dmabuf_release,
diff --git a/drivers/gpu/drm/drm_prime.c b/drivers/gpu/drm/drm_prime.c
index 7fec191b45f7..ed4a6e35dd91 100644
--- a/drivers/gpu/drm/drm_prime.c
+++ b/drivers/gpu/drm/drm_prime.c
@@ -289,6 +289,7 @@ static int drm_gem_dmabuf_mmap(struct dma_buf *dma_buf,
 }
 
 static const struct dma_buf_ops drm_gem_prime_dmabuf_ops =  {
+   .owner = THIS_MODULE,
.attach = drm_gem_map_attach,
.detach = drm_gem_map_detach,
.map_dma_buf = drm_gem_map_dma_buf,
diff --git a/drivers/gpu/drm/exynos/exynos_drm_dmabuf.c 
b/drivers/gpu/drm/exynos/exynos_drm_dmabuf.c
index cd485c091b30..35fa029353e9 100644
--- a/drivers/gpu/drm/exynos/exynos_drm_dmabuf.c
+++ b/drivers/gpu/drm/exynos/exynos_drm_dmabuf.c
@@ -169,6 +169,7 @@ static int exynos_gem_dmabuf_mmap(struct dma_buf *dma_buf,
 }
 
 static struct dma_buf_ops exynos_dmabuf_ops = {
+   .owner  = THIS_MODULE,
.attach = exynos_gem_attach_dma_buf,
.detach = exynos_gem_detach_dma_buf,
.map_dma_buf= exynos_gem_map_dma_buf,
diff --git a/drivers/gpu/drm/i915/i915_gem_dmabuf.c 
b/drivers/gpu/drm/i915/i915_gem_dmabuf.c
index 7998da27c500..b9355d8d4862 100644
--- a/drivers/gpu/drm/i915/i915_gem_dmabuf.c
+++ b/drivers/gpu/drm/i915/i915_gem_dmabuf.c
@@ -213,6 +213,7 @@ static int i915_gem_begin_cpu_access(struct dma_buf 
*dma_buf, size_t start, size
 }
 
 static const struct dma_buf_ops i915_dmabuf_ops =  {
+   .owner = THIS_MODULE,
.map_dma_buf = i915_gem_map_dma_buf,
.unmap_dma_buf = i915_gem_unmap_dma_buf,
.release = drm_gem_dmabuf_release,
diff --git a/drivers/gpu/drm/omapdrm/omap_gem_dmabuf.c 
b/drivers/gpu/drm/omapdrm/omap_gem_dmabuf.c
index 344fd789170d..977ece9be62c 100644
--- a/drivers/gpu/drm/omapdrm/omap_gem_dmabuf.c
+++ b/drivers/gpu/drm/omapdrm

Re: [Intel-gfx] [PATCH] dma-buf: add ref counting for module as exporter

2015-05-07 Thread Sumit Semwal

On 7 May 2015 at 13:40, Greg KH gre...@linuxfoundation.org wrote:
 On Thu, May 07, 2015 at 01:00:52PM +0530, Sumit Semwal wrote:
 Add reference counting on a kernel module that exports dma-buf and
 implements its operations. This prevents the module from being unloaded
 while DMABUF file is in use.

 The original patch [1] was submitted by Tomasz, but he's since shifted
 jobs and a ping didn't elicit any response.

   [tomasz: Original author]
 Signed-off-by: Tomasz Stanislawski t.stanisl...@samsung.com
 Acked-by: Daniel Vetter dan...@ffwll.ch
   [sumits: updated  rebased as per review comments]
 Signed-off-by: Sumit Semwal sumit.sem...@linaro.org

 [1]: https://lkml.org/lkml/2012/8/8/163
 ---
  drivers/dma-buf/dma-buf.c  | 9 -
  drivers/gpu/drm/armada/armada_gem.c| 1 +
  drivers/gpu/drm/drm_prime.c| 1 +
  drivers/gpu/drm/exynos/exynos_drm_dmabuf.c | 1 +
  drivers/gpu/drm/i915/i915_gem_dmabuf.c | 1 +
  drivers/gpu/drm/omapdrm/omap_gem_dmabuf.c  | 1 +
  drivers/gpu/drm/tegra/gem.c| 1 +
  drivers/gpu/drm/udl/udl_dmabuf.c   | 1 +
  drivers/gpu/drm/vmwgfx/vmwgfx_prime.c  | 1 +
  drivers/media/v4l2-core/videobuf2-dma-contig.c | 1 +
  drivers/media/v4l2-core/videobuf2-dma-sg.c | 1 +
  drivers/media/v4l2-core/videobuf2-vmalloc.c| 1 +
  drivers/staging/android/ion/ion.c  | 1 +
  include/linux/dma-buf.h| 2 ++
  14 files changed, 22 insertions(+), 1 deletion(-)

 diff --git a/drivers/dma-buf/dma-buf.c b/drivers/dma-buf/dma-buf.c
 index c5a9138a6a8d..9583eac0238f 100644
 --- a/drivers/dma-buf/dma-buf.c
 +++ b/drivers/dma-buf/dma-buf.c
 @@ -29,6 +29,7 @@
  #include linux/anon_inodes.h
  #include linux/export.h
  #include linux/debugfs.h
 +#include linux/module.h
  #include linux/seq_file.h
  #include linux/poll.h
  #include linux/reservation.h
 @@ -64,6 +65,7 @@ static int dma_buf_release(struct inode *inode, struct 
 file *file)
   BUG_ON(dmabuf-cb_shared.active || dmabuf-cb_excl.active);

   dmabuf-ops-release(dmabuf);
 + module_put(dmabuf-ops-owner);

   mutex_lock(db_list.lock);
   list_del(dmabuf-list_node);
 @@ -302,9 +304,14 @@ struct dma_buf *dma_buf_export(const struct 
 dma_buf_export_info *exp_info)
   return ERR_PTR(-EINVAL);
   }

 + if (!try_module_get(exp_info-ops-owner))
 + return ERR_PTR(-ENOENT);
 +
   dmabuf = kzalloc(alloc_size, GFP_KERNEL);
 - if (dmabuf == NULL)
 + if (!dmabuf) {
 + module_put(exp_info-ops-owner);
   return ERR_PTR(-ENOMEM);
 + }

   dmabuf-priv = exp_info-priv;
   dmabuf-ops = exp_info-ops;
 diff --git a/drivers/gpu/drm/armada/armada_gem.c 
 b/drivers/gpu/drm/armada/armada_gem.c
 index 580e10acaa3a..d2c5fc1269b7 100644
 --- a/drivers/gpu/drm/armada/armada_gem.c
 +++ b/drivers/gpu/drm/armada/armada_gem.c
 @@ -524,6 +524,7 @@ armada_gem_dmabuf_mmap(struct dma_buf *buf, struct 
 vm_area_struct *vma)
  }

  static const struct dma_buf_ops armada_gem_prime_dmabuf_ops = {
 + .owner = THIS_MODULE,
   .map_dma_buf= armada_gem_prime_map_dma_buf,
   .unmap_dma_buf  = armada_gem_prime_unmap_dma_buf,
   .release= drm_gem_dmabuf_release,

 The easier way to do this is change the registration function to add
 the module owner automatically, which keeps you from having to modify
 all of the individual drivers.  Look at how pci and usb do this for
 their driver registration functions.  That should result in a much
 smaller patch, that always works properly for everyone (there's no way
 for driver to get it wrong.)

Thanks Greg; but of course, you're right! We already have a
DEFINE_DMA_BUF_EXPORT_INFO macro, so this is far easier incorporated
into that.

I will spin up the (much simpler) patch and repost!

 thanks,

 greg k-h


Best regards,
Sumit.
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx

Re: [Intel-gfx] [PATCH v3] dma-buf: cleanup dma_buf_export() to make it easily extensible

2015-01-28 Thread Sumit Semwal

Hi Mauro,

On 28 January 2015 at 18:53, Mauro Carvalho Chehab
mche...@osg.samsung.com wrote:
 Em Wed, 28 Jan 2015 18:24:03 +0530
 Sumit Semwal sumit.sem...@linaro.org escreveu:

 +/**
 + * helper macro for exporters; zeros and fills in most common values
 + */
 +#define DEFINE_DMA_BUF_EXPORT_INFO(a)\
 + struct dma_buf_export_info a = { .exp_name = KBUILD_MODNAME }
 +

 I suspect that this will let the other fields not initialized.

 You likely need to do:

 #define DEFINE_DMA_BUF_EXPORT_INFO(a)   \
 struct dma_buf_export_info a = {\
 .exp_name = KBUILD_MODNAME; \
 .fields = 0;\
 ...
 }
I suspected the same, but Daniel kindly referred to the C99 standard,
which states:
 If there are fewer initializers in a brace-enclosed list than there
are elements or members
of an aggregate, or fewer characters in a string literal used to
initialize an array of known
size than there are elements in the array, the remainder of the
aggregate shall be
initialized implicitly the same as objects that have static storage duration.

So I think we're well covered there?

 Regards,
 Mauro



-- 
Thanks and regards,

Sumit Semwal
Kernel Team Lead - Linaro Mobile Group
Linaro.org │ Open source software for ARM SoCs
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx

[Intel-gfx] [PATCH v3] dma-buf: cleanup dma_buf_export() to make it easily extensible

2015-01-28 Thread Sumit Semwal

At present, dma_buf_export() takes a series of parameters, which
makes it difficult to add any new parameters for exporters, if required.

Make it simpler by moving all these parameters into a struct, and pass
the struct * as parameter to dma_buf_export().

While at it, unite dma_buf_export_named() with dma_buf_export(), and
change all callers accordingly.

Signed-off-by: Sumit Semwal sumit.sem...@linaro.org
---
v3: Daniel Thompson caught the C99 warning issue w/ using {0}; using
{.exp_name = xxx} instead.

v2: add macro to zero out local struct, and fill KBUILD_MODNAME by default

 drivers/dma-buf/dma-buf.c  | 47 +-
 drivers/gpu/drm/armada/armada_gem.c| 10 --
 drivers/gpu/drm/drm_prime.c| 12 ---
 drivers/gpu/drm/exynos/exynos_drm_dmabuf.c |  9 +++--
 drivers/gpu/drm/i915/i915_gem_dmabuf.c | 10 --
 drivers/gpu/drm/omapdrm/omap_gem_dmabuf.c  |  9 -
 drivers/gpu/drm/tegra/gem.c| 10 --
 drivers/gpu/drm/ttm/ttm_object.c   |  9 +++--
 drivers/gpu/drm/udl/udl_dmabuf.c   |  9 -
 drivers/media/v4l2-core/videobuf2-dma-contig.c |  8 -
 drivers/media/v4l2-core/videobuf2-dma-sg.c |  8 -
 drivers/media/v4l2-core/videobuf2-vmalloc.c|  8 -
 drivers/staging/android/ion/ion.c  |  9 +++--
 include/linux/dma-buf.h| 34 +++
 14 files changed, 142 insertions(+), 50 deletions(-)

diff --git a/drivers/dma-buf/dma-buf.c b/drivers/dma-buf/dma-buf.c
index 5be225c2ba98..6d3df3dd9310 100644
--- a/drivers/dma-buf/dma-buf.c
+++ b/drivers/dma-buf/dma-buf.c
@@ -265,7 +265,7 @@ static inline int is_dma_buf_file(struct file *file)
 }
 
 /**
- * dma_buf_export_named - Creates a new dma_buf, and associates an anon file
+ * dma_buf_export - Creates a new dma_buf, and associates an anon file
  * with this buffer, so it can be exported.
  * Also connect the allocator specific data and ops to the buffer.
  * Additionally, provide a name string for exporter; useful in debugging.
@@ -277,31 +277,32 @@ static inline int is_dma_buf_file(struct file *file)
  * @exp_name:  [in]name of the exporting module - useful for debugging.
  * @resv:  [in]reservation-object, NULL to allocate default one.
  *
+ * All the above info comes from struct dma_buf_export_info.
+ *
  * Returns, on success, a newly created dma_buf object, which wraps the
  * supplied private data and operations for dma_buf_ops. On either missing
  * ops, or error in allocating struct dma_buf, will return negative error.
  *
  */
-struct dma_buf *dma_buf_export_named(void *priv, const struct dma_buf_ops *ops,
-   size_t size, int flags, const char *exp_name,
-   struct reservation_object *resv)
+struct dma_buf *dma_buf_export(struct dma_buf_export_info *exp_info)
 {
struct dma_buf *dmabuf;
struct file *file;
size_t alloc_size = sizeof(struct dma_buf);
-   if (!resv)
+   if (!exp_info-resv)
alloc_size += sizeof(struct reservation_object);
else
/* prevent dma_buf[1] == dma_buf-resv */
alloc_size += 1;
 
-   if (WARN_ON(!priv || !ops
- || !ops-map_dma_buf
- || !ops-unmap_dma_buf
- || !ops-release
- || !ops-kmap_atomic
- || !ops-kmap
- || !ops-mmap)) {
+   if (WARN_ON(!exp_info-priv
+ || !exp_info-ops
+ || !exp_info-ops-map_dma_buf
+ || !exp_info-ops-unmap_dma_buf
+ || !exp_info-ops-release
+ || !exp_info-ops-kmap_atomic
+ || !exp_info-ops-kmap
+ || !exp_info-ops-mmap)) {
return ERR_PTR(-EINVAL);
}
 
@@ -309,21 +310,22 @@ struct dma_buf *dma_buf_export_named(void *priv, const 
struct dma_buf_ops *ops,
if (dmabuf == NULL)
return ERR_PTR(-ENOMEM);
 
-   dmabuf-priv = priv;
-   dmabuf-ops = ops;
-   dmabuf-size = size;
-   dmabuf-exp_name = exp_name;
+   dmabuf-priv = exp_info-priv;
+   dmabuf-ops = exp_info-ops;
+   dmabuf-size = exp_info-size;
+   dmabuf-exp_name = exp_info-exp_name;
init_waitqueue_head(dmabuf-poll);
dmabuf-cb_excl.poll = dmabuf-cb_shared.poll = dmabuf-poll;
dmabuf-cb_excl.active = dmabuf-cb_shared.active = 0;
 
-   if (!resv) {
-   resv = (struct reservation_object *)dmabuf[1];
-   reservation_object_init(resv);
+   if (!exp_info-resv) {
+   exp_info-resv = (struct reservation_object *)dmabuf[1];
+   reservation_object_init(exp_info-resv);
}
-   dmabuf-resv = resv;
+   dmabuf-resv = exp_info-resv

Re: [Intel-gfx] [PATCH v2] dma-buf: cleanup dma_buf_export() to make it easily extensible

2015-01-28 Thread Sumit Semwal

On 28 January 2015 at 16:50, Daniel Thompson daniel.thomp...@linaro.org wrote:
 On 28/01/15 06:00, Sumit Semwal wrote:
snip
 +/**
 + * helper macro for exporters; zeros and fills in most common values
 + */
 +#define DEFINE_DMA_BUF_EXPORT_INFO(a)\
 + struct dma_buf_export_info a = {0}; \
 + exp_info.exp_name = KBUILD_MODNAME
 +

 This risks generating C99 warnings unless used with care (and only once
 per function). Shouldn't this be more like:

 #define DEFINE_DMA_BUF_EXPORT_INFO(a) \
 struct dma_buf_export_info a = { .exp_name = KBUILD_MODNAME }


Ah! My bad; thanks for catching this, Daniel; I'll send out the
updated patch in a minute!
 Daniel.




-- 
Thanks and regards,

Sumit Semwal
Kernel Team Lead - Linaro Mobile Group
Linaro.org │ Open source software for ARM SoCs
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx

[Intel-gfx] [PATCH v2] dma-buf: cleanup dma_buf_export() to make it easily extensible

2015-01-27 Thread Sumit Semwal

At present, dma_buf_export() takes a series of parameters, which
makes it difficult to add any new parameters for exporters, if required.

Make it simpler by moving all these parameters into a struct, and pass
the struct * as parameter to dma_buf_export().

While at it, unite dma_buf_export_named() with dma_buf_export(), and
change all callers accordingly.

Signed-off-by: Sumit Semwal sumit.sem...@linaro.org
---
v2: add macro to zero out local struct, and fill KBUILD_MODNAME by default

 drivers/dma-buf/dma-buf.c  | 47 +-
 drivers/gpu/drm/armada/armada_gem.c| 10 --
 drivers/gpu/drm/drm_prime.c| 12 ---
 drivers/gpu/drm/exynos/exynos_drm_dmabuf.c |  9 +++--
 drivers/gpu/drm/i915/i915_gem_dmabuf.c | 10 --
 drivers/gpu/drm/omapdrm/omap_gem_dmabuf.c  |  9 -
 drivers/gpu/drm/tegra/gem.c| 10 --
 drivers/gpu/drm/ttm/ttm_object.c   |  9 +++--
 drivers/gpu/drm/udl/udl_dmabuf.c   |  9 -
 drivers/media/v4l2-core/videobuf2-dma-contig.c |  8 -
 drivers/media/v4l2-core/videobuf2-dma-sg.c |  8 -
 drivers/media/v4l2-core/videobuf2-vmalloc.c|  8 -
 drivers/staging/android/ion/ion.c  |  9 +++--
 include/linux/dma-buf.h| 35 +++
 14 files changed, 143 insertions(+), 50 deletions(-)

diff --git a/drivers/dma-buf/dma-buf.c b/drivers/dma-buf/dma-buf.c
index 5be225c2ba98..6d3df3dd9310 100644
--- a/drivers/dma-buf/dma-buf.c
+++ b/drivers/dma-buf/dma-buf.c
@@ -265,7 +265,7 @@ static inline int is_dma_buf_file(struct file *file)
 }
 
 /**
- * dma_buf_export_named - Creates a new dma_buf, and associates an anon file
+ * dma_buf_export - Creates a new dma_buf, and associates an anon file
  * with this buffer, so it can be exported.
  * Also connect the allocator specific data and ops to the buffer.
  * Additionally, provide a name string for exporter; useful in debugging.
@@ -277,31 +277,32 @@ static inline int is_dma_buf_file(struct file *file)
  * @exp_name:  [in]name of the exporting module - useful for debugging.
  * @resv:  [in]reservation-object, NULL to allocate default one.
  *
+ * All the above info comes from struct dma_buf_export_info.
+ *
  * Returns, on success, a newly created dma_buf object, which wraps the
  * supplied private data and operations for dma_buf_ops. On either missing
  * ops, or error in allocating struct dma_buf, will return negative error.
  *
  */
-struct dma_buf *dma_buf_export_named(void *priv, const struct dma_buf_ops *ops,
-   size_t size, int flags, const char *exp_name,
-   struct reservation_object *resv)
+struct dma_buf *dma_buf_export(struct dma_buf_export_info *exp_info)
 {
struct dma_buf *dmabuf;
struct file *file;
size_t alloc_size = sizeof(struct dma_buf);
-   if (!resv)
+   if (!exp_info-resv)
alloc_size += sizeof(struct reservation_object);
else
/* prevent dma_buf[1] == dma_buf-resv */
alloc_size += 1;
 
-   if (WARN_ON(!priv || !ops
- || !ops-map_dma_buf
- || !ops-unmap_dma_buf
- || !ops-release
- || !ops-kmap_atomic
- || !ops-kmap
- || !ops-mmap)) {
+   if (WARN_ON(!exp_info-priv
+ || !exp_info-ops
+ || !exp_info-ops-map_dma_buf
+ || !exp_info-ops-unmap_dma_buf
+ || !exp_info-ops-release
+ || !exp_info-ops-kmap_atomic
+ || !exp_info-ops-kmap
+ || !exp_info-ops-mmap)) {
return ERR_PTR(-EINVAL);
}
 
@@ -309,21 +310,22 @@ struct dma_buf *dma_buf_export_named(void *priv, const 
struct dma_buf_ops *ops,
if (dmabuf == NULL)
return ERR_PTR(-ENOMEM);
 
-   dmabuf-priv = priv;
-   dmabuf-ops = ops;
-   dmabuf-size = size;
-   dmabuf-exp_name = exp_name;
+   dmabuf-priv = exp_info-priv;
+   dmabuf-ops = exp_info-ops;
+   dmabuf-size = exp_info-size;
+   dmabuf-exp_name = exp_info-exp_name;
init_waitqueue_head(dmabuf-poll);
dmabuf-cb_excl.poll = dmabuf-cb_shared.poll = dmabuf-poll;
dmabuf-cb_excl.active = dmabuf-cb_shared.active = 0;
 
-   if (!resv) {
-   resv = (struct reservation_object *)dmabuf[1];
-   reservation_object_init(resv);
+   if (!exp_info-resv) {
+   exp_info-resv = (struct reservation_object *)dmabuf[1];
+   reservation_object_init(exp_info-resv);
}
-   dmabuf-resv = resv;
+   dmabuf-resv = exp_info-resv;
 
-   file = anon_inode_getfile(dmabuf, dma_buf_fops, dmabuf, flags);
+   file = anon_inode_getfile

[Intel-gfx] [RFC] dma-buf: cleanup dma_buf_export() to make it easily extensible

2015-01-22 Thread Sumit Semwal

At present, dma_buf_export() takes a series of parameters, which
makes it difficult to add any new parameters for exporters, if required.

Make it simpler by moving all these parameters into a struct, and pass
the struct * as parameter to dma_buf_export().

While at it, unite dma_buf_export_named() with dma_buf_export(), and
change all callers accordingly.

Signed-off-by: Sumit Semwal sumit.sem...@linaro.org
---
 drivers/dma-buf/dma-buf.c  | 47 +-
 drivers/gpu/drm/armada/armada_gem.c| 12 +--
 drivers/gpu/drm/drm_prime.c| 14 +---
 drivers/gpu/drm/exynos/exynos_drm_dmabuf.c | 13 +--
 drivers/gpu/drm/i915/i915_gem_dmabuf.c | 12 +--
 drivers/gpu/drm/omapdrm/omap_gem_dmabuf.c  | 11 +-
 drivers/gpu/drm/tegra/gem.c| 12 +--
 drivers/gpu/drm/ttm/ttm_object.c   | 11 --
 drivers/gpu/drm/udl/udl_dmabuf.c   | 10 +-
 drivers/media/v4l2-core/videobuf2-dma-contig.c | 10 +-
 drivers/media/v4l2-core/videobuf2-dma-sg.c | 10 +-
 drivers/media/v4l2-core/videobuf2-vmalloc.c| 10 +-
 drivers/staging/android/ion/ion.c  | 11 --
 include/linux/dma-buf.h| 28 +++
 14 files changed, 160 insertions(+), 51 deletions(-)

diff --git a/drivers/dma-buf/dma-buf.c b/drivers/dma-buf/dma-buf.c
index 5be225c2ba98..6d3df3dd9310 100644
--- a/drivers/dma-buf/dma-buf.c
+++ b/drivers/dma-buf/dma-buf.c
@@ -265,7 +265,7 @@ static inline int is_dma_buf_file(struct file *file)
 }
 
 /**
- * dma_buf_export_named - Creates a new dma_buf, and associates an anon file
+ * dma_buf_export - Creates a new dma_buf, and associates an anon file
  * with this buffer, so it can be exported.
  * Also connect the allocator specific data and ops to the buffer.
  * Additionally, provide a name string for exporter; useful in debugging.
@@ -277,31 +277,32 @@ static inline int is_dma_buf_file(struct file *file)
  * @exp_name:  [in]name of the exporting module - useful for debugging.
  * @resv:  [in]reservation-object, NULL to allocate default one.
  *
+ * All the above info comes from struct dma_buf_export_info.
+ *
  * Returns, on success, a newly created dma_buf object, which wraps the
  * supplied private data and operations for dma_buf_ops. On either missing
  * ops, or error in allocating struct dma_buf, will return negative error.
  *
  */
-struct dma_buf *dma_buf_export_named(void *priv, const struct dma_buf_ops *ops,
-   size_t size, int flags, const char *exp_name,
-   struct reservation_object *resv)
+struct dma_buf *dma_buf_export(struct dma_buf_export_info *exp_info)
 {
struct dma_buf *dmabuf;
struct file *file;
size_t alloc_size = sizeof(struct dma_buf);
-   if (!resv)
+   if (!exp_info-resv)
alloc_size += sizeof(struct reservation_object);
else
/* prevent dma_buf[1] == dma_buf-resv */
alloc_size += 1;
 
-   if (WARN_ON(!priv || !ops
- || !ops-map_dma_buf
- || !ops-unmap_dma_buf
- || !ops-release
- || !ops-kmap_atomic
- || !ops-kmap
- || !ops-mmap)) {
+   if (WARN_ON(!exp_info-priv
+ || !exp_info-ops
+ || !exp_info-ops-map_dma_buf
+ || !exp_info-ops-unmap_dma_buf
+ || !exp_info-ops-release
+ || !exp_info-ops-kmap_atomic
+ || !exp_info-ops-kmap
+ || !exp_info-ops-mmap)) {
return ERR_PTR(-EINVAL);
}
 
@@ -309,21 +310,22 @@ struct dma_buf *dma_buf_export_named(void *priv, const 
struct dma_buf_ops *ops,
if (dmabuf == NULL)
return ERR_PTR(-ENOMEM);
 
-   dmabuf-priv = priv;
-   dmabuf-ops = ops;
-   dmabuf-size = size;
-   dmabuf-exp_name = exp_name;
+   dmabuf-priv = exp_info-priv;
+   dmabuf-ops = exp_info-ops;
+   dmabuf-size = exp_info-size;
+   dmabuf-exp_name = exp_info-exp_name;
init_waitqueue_head(dmabuf-poll);
dmabuf-cb_excl.poll = dmabuf-cb_shared.poll = dmabuf-poll;
dmabuf-cb_excl.active = dmabuf-cb_shared.active = 0;
 
-   if (!resv) {
-   resv = (struct reservation_object *)dmabuf[1];
-   reservation_object_init(resv);
+   if (!exp_info-resv) {
+   exp_info-resv = (struct reservation_object *)dmabuf[1];
+   reservation_object_init(exp_info-resv);
}
-   dmabuf-resv = resv;
+   dmabuf-resv = exp_info-resv;
 
-   file = anon_inode_getfile(dmabuf, dma_buf_fops, dmabuf, flags);
+   file = anon_inode_getfile(dmabuf, dma_buf_fops, dmabuf

Re: [Intel-gfx] linux-next: manual merge of the dma-buf tree with the drm-intel tree

2014-05-22 Thread Sumit Semwal

On 20 May 2014 12:25, Stephen Rothwell s...@canb.auug.org.au wrote:
 Hi Sumit,

 Today's linux-next merge of the dma-buf tree got a conflict in
 drivers/gpu/drm/i915/i915_gem_dmabuf.c between commit 5cc9ed4b9a7a
 (drm/i915: Introduce mapping of user pages into video memory (userptr)
 ioctl) from the drm-intel tree and commit 8dfb1f0f8103 (dma-buf: use
 reservation objects) from the dma-buf tree.

 I fixed it up (see below) and can carry the fix as necessary (no action
 is required).

Hi Stephen!

This seems good, thanks!

Best regards,
~Sumit.
 --
 Cheers,
 Stephen Rothwells...@canb.auug.org.au

 diff --cc drivers/gpu/drm/i915/i915_gem_dmabuf.c
 index 580aa42443ed,817ec444c976..
 --- a/drivers/gpu/drm/i915/i915_gem_dmabuf.c
 +++ b/drivers/gpu/drm/i915/i915_gem_dmabuf.c
 @@@ -229,15 -233,8 +229,16 @@@ static const struct dma_buf_ops i915_dm
   struct dma_buf *i915_gem_prime_export(struct drm_device *dev,
   struct drm_gem_object *gem_obj, int 
 flags)
   {
  +  struct drm_i915_gem_object *obj = to_intel_bo(gem_obj);
  +
  +  if (obj-ops-dmabuf_export) {
  +  int ret = obj-ops-dmabuf_export(obj);
  +  if (ret)
  +  return ERR_PTR(ret);
  +  }
  +
 -   return dma_buf_export(gem_obj, i915_dmabuf_ops, gem_obj-size, 
 flags);
 +   return dma_buf_export(gem_obj, i915_dmabuf_ops, gem_obj-size, flags,
 +   NULL);
   }

   static int i915_gem_object_get_pages_dmabuf(struct drm_i915_gem_object *obj)
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx

37 matches

Mail list logo