Re: [PATCH RFC PKS/PMEM 09/58] drivers/gpu: Utilize new kmap_thread()

2020-10-10 Thread Ira Weiny
On Sat, Oct 10, 2020 at 12:03:49AM +0200, Daniel Vetter wrote:
> On Fri, Oct 09, 2020 at 12:49:44PM -0700, ira.we...@intel.com wrote:
> > From: Ira Weiny 
> > 
> > These kmap() calls in the gpu stack are localized to a single thread.
> > To avoid the over head of global PKRS updates use the new kmap_thread()
> > call.
> > 
> > Cc: David Airlie 
> > Cc: Daniel Vetter 
> > Cc: Patrik Jakobsson 
> > Signed-off-by: Ira Weiny 
> 
> I'm guessing the entire pile goes in through some other tree.
>

Apologies for not realizing there were multiple maintainers here.

But, I was thinking it would land together through the mm tree once the core
support lands.  I've tried to split these out in a way they can be easily
reviewed/acked by the correct developers.

> If so:
> 
> Acked-by: Daniel Vetter 
> 
> If you want this to land through maintainer trees, then we need a
> per-driver split (since aside from amdgpu and radeon they're all different
> subtrees).

It is just RFC for the moment.  I need to get the core support accepted first
then this can land.

> 
> btw the two kmap calls in drm you highlight in the cover letter should
> also be convertible to kmap_thread. We only hold vmalloc mappings for a
> longer time (or it'd be quite a driver bug). So if you want maybe throw
> those two as two additional patches on top, and we can do some careful
> review & testing for them.

Cool.  I'll add them in.

Ira

> -Daniel
> 
> > ---
> >  drivers/gpu/drm/amd/amdgpu/amdgpu_ttm.c  | 12 ++--
> >  drivers/gpu/drm/gma500/gma_display.c |  4 ++--
> >  drivers/gpu/drm/gma500/mmu.c | 10 +-
> >  drivers/gpu/drm/i915/gem/i915_gem_shmem.c|  4 ++--
> >  .../gpu/drm/i915/gem/selftests/i915_gem_context.c|  4 ++--
> >  drivers/gpu/drm/i915/gem/selftests/i915_gem_mman.c   |  8 
> >  drivers/gpu/drm/i915/gt/intel_ggtt_fencing.c |  4 ++--
> >  drivers/gpu/drm/i915/gt/intel_gtt.c  |  4 ++--
> >  drivers/gpu/drm/i915/gt/shmem_utils.c|  4 ++--
> >  drivers/gpu/drm/i915/i915_gem.c  |  8 
> >  drivers/gpu/drm/i915/i915_gpu_error.c|  4 ++--
> >  drivers/gpu/drm/i915/selftests/i915_perf.c   |  4 ++--
> >  drivers/gpu/drm/radeon/radeon_ttm.c  |  4 ++--
> >  13 files changed, 37 insertions(+), 37 deletions(-)
> > 
> > diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_ttm.c 
> > b/drivers/gpu/drm/amd/amdgpu/amdgpu_ttm.c
> > index 978bae731398..bd564bccb7a3 100644
> > --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_ttm.c
> > +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_ttm.c
> > @@ -2437,11 +2437,11 @@ static ssize_t amdgpu_ttm_gtt_read(struct file *f, 
> > char __user *buf,
> >  
> > page = adev->gart.pages[p];
> > if (page) {
> > -   ptr = kmap(page);
> > +   ptr = kmap_thread(page);
> > ptr += off;
> >  
> > r = copy_to_user(buf, ptr, cur_size);
> > -   kunmap(adev->gart.pages[p]);
> > +   kunmap_thread(adev->gart.pages[p]);
> > } else
> > r = clear_user(buf, cur_size);
> >  
> > @@ -2507,9 +2507,9 @@ static ssize_t amdgpu_iomem_read(struct file *f, char 
> > __user *buf,
> > if (p->mapping != adev->mman.bdev.dev_mapping)
> > return -EPERM;
> >  
> > -   ptr = kmap(p);
> > +   ptr = kmap_thread(p);
> > r = copy_to_user(buf, ptr + off, bytes);
> > -   kunmap(p);
> > +   kunmap_thread(p);
> > if (r)
> > return -EFAULT;
> >  
> > @@ -2558,9 +2558,9 @@ static ssize_t amdgpu_iomem_write(struct file *f, 
> > const char __user *buf,
> > if (p->mapping != adev->mman.bdev.dev_mapping)
> > return -EPERM;
> >  
> > -   ptr = kmap(p);
> > +   ptr = kmap_thread(p);
> > r = copy_from_user(ptr + off, buf, bytes);
> > -   kunmap(p);
> > +   kunmap_thread(p);
> > if (r)
> > return -EFAULT;
> >  
> > diff --git a/drivers/gpu/drm/gma500/gma_display.c 
> > b/drivers/gpu/drm/gma500/gma_display.c
> > index 3df6d6e850f5..35f4e55c941f 100644
> > --- a/drivers/gpu/drm/gma500/gma_display.c
> > +++ b/drivers/gpu/drm/gma500/gma_display.c
> > @@ -400,9 +400,9 @@ int gma_crtc_cursor_set(struct drm_crtc *crtc,
> > /* Copy the cursor to cursor mem */
> > tmp_dst = dev_priv->vram_addr + cursor_gt->offset;
> > for (i = 0; i < cursor_pages; i++) {
> > -   tmp_src = kmap(gt->pages[i]);
> > +   tmp_src = kmap_thread(gt->pages[i]);
> > memcpy(tmp_dst, tmp_src, PAGE_SIZE);
> > -   kunmap(gt->pages[i]);
> > +   kunmap_thread(gt->pages[i]);
> > tmp_dst += PAGE_SIZE;
> > }
> >  
> > diff --git 

Re: [PATCH v2 09/17] mm: Add unsafe_follow_pfn

2020-10-10 Thread Daniel Vetter
On Sat, Oct 10, 2020 at 11:36 PM Laurent Pinchart
 wrote:
>
> Hi Tomasz,
>
> On Sat, Oct 10, 2020 at 07:22:48PM +0200, Tomasz Figa wrote:
> > On Fri, Oct 9, 2020 at 7:52 PM Daniel Vetter  wrote:
> > > On Fri, Oct 9, 2020 at 2:48 PM Jason Gunthorpe  wrote:
> > > > On Fri, Oct 09, 2020 at 02:37:23PM +0200, Mauro Carvalho Chehab wrote:
> > > >
> > > > > I'm not a mm/ expert, but, from what I understood from Daniel's patch
> > > > > description is that this is unsafe *only if*  __GFP_MOVABLE is used.
> > > >
> > > > No, it is unconditionally unsafe. The CMA movable mappings are
> > > > specific VMAs that will have bad issues here, but there are other
> > > > types too.
> > > >
> > > > The only way to do something at a VMA level is to have a list of OK
> > > > VMAs, eg because they were creatd via a special mmap helper from the
> > > > media subsystem.
> > > >
> > > > > Well, no drivers inside the media subsystem uses such flag, although
> > > > > they may rely on some infrastructure that could be using it behind
> > > > > the bars.
> > > >
> > > > It doesn't matter, nothing prevents the user from calling media APIs
> > > > on mmaps it gets from other subsystems.
> > >
> > > I think a good first step would be to disable userptr of non struct
> > > page backed storage going forward for any new hw support. Even on
> > > existing drivers. dma-buf sharing has been around for long enough now
> > > that this shouldn't be a problem. Unfortunately right now this doesn't
> > > seem to exist, so the entire problem keeps getting perpetuated.
> > >
> > > > > If this is the case, the proper fix seems to have a GFP_NOT_MOVABLE
> > > > > flag that it would be denying the core mm code to set __GFP_MOVABLE.
> > > >
> > > > We can't tell from the VMA these kinds of details..
> > > >
> > > > It has to go the other direction, evey mmap that might be used as a
> > > > userptr here has to be found and the VMA specially created to allow
> > > > its use. At least that is a kernel only change, but will need people
> > > > with the HW to do this work.
> > >
> > > I think the only reasonable way to keep this working is:
> > > - add a struct dma_buf *vma_tryget_dma_buf(struct vm_area_struct *vma);
> > > - add dma-buf export support to fbdev and v4l
> >
> > I assume you mean V4L2 and not the obsolete V4L that is emulated in
> > the userspace by libv4l. If so, every video device that uses videobuf2
> > gets DMA-buf export for free and there is nothing needed to enable it.

Yeah. And I missed that v4l2 added dma-buf export too.

> > We probably still have a few legacy drivers using videobuf (non-2),
> > but IMHO those should be safe to put behind some disabled-by-default
> > Kconfig symbol or even completely drop, as the legacy framework has
> > been deprecated for many years already.
>
> There's 8 drivers left, and they support a very large number of devices.
> I expect unhappy users distros stop shipping them. On the other hand,
> videobuf has been deprecated for a lng time, so there has been
> plenty of time to convert the remaining drivers to videobuf2. If nobody
> can do it, then we'll have to drop support for these devices given the
> security issues.

Again, the issue here is _only_ with follow_pfn. For videobuf1 this
means videbuf-dma-contig.c userptr support is broken. Unlike videobuf2
it means it's broken for all usage (not just zero-copy userptr),
because videbuf-dma-contig.c lacks the pin_user_pages path. But that
would be easy to add if this poses a  problem I think - we just need
to carry over the pin_user_pages_fast logic from videbuf2, no driver
changes required. But of course I don't think we should do that before
someone reports the regression, since videobuf1 userptr is doubly
deprecated :-)

Everything else keeps working with videobuf1 with my patch series. So
depending upon which videobuf1 implementations these 8 drivers use,
you might not even have any real breakage there.

> We have moved media drivers to staging in the past when there wasn't
> enough maintenance effort, we could do so here too.

I'm not breaking the world with this, it's really very minimal
use-case. At least as far as I'm understanding the entire media
subsystem here.
-Daniel

> > > - roll this out everywhere we still need it.
> > >
> > > Realistically this just isn't going to happen. And anything else just
> > > reimplements half of dma-buf, which is kinda pointless (you need
> > > minimally refcounting and some way to get at a promise of a permanent
> > > sg list for dma. Plus probably the vmap for kernel cpu access.
> > >
> > > > > Please let address the issue on this way, instead of broken an
> > > > > userspace API that it is there since 1991.
> > > >
> > > > It has happened before :( It took 4 years for RDMA to undo the uAPI
> > > > breakage caused by a security fix for something that was a 15 years
> > > > old bug.
> > >
> > > Yeah we have a bunch of these on the drm side too. Some of them are
> > > really just "you have to upgrade 

Re: [PATCH v2 03/17] misc/habana: Stop using frame_vector helpers

2020-10-10 Thread Oded Gabbay
On Sun, Oct 11, 2020 at 12:41 AM Daniel Vetter  wrote:
>
> On Sat, Oct 10, 2020 at 11:32 PM Daniel Vetter  wrote:
> >
> > On Sat, Oct 10, 2020 at 10:27 PM Oded Gabbay  wrote:
> > >
> > > On Fri, Oct 9, 2020 at 10:59 AM Daniel Vetter  
> > > wrote:
> > > >
> > > > All we need are a pages array, pin_user_pages_fast can give us that
> > > > directly. Plus this avoids the entire raw pfn side of get_vaddr_frames.
> > > >
> > > Thanks for the patch Daniel.
> > >
> > > > Signed-off-by: Daniel Vetter 
> > > > Cc: Jason Gunthorpe 
> > > > Cc: Andrew Morton 
> > > > Cc: John Hubbard 
> > > > Cc: Jérôme Glisse 
> > > > Cc: Jan Kara 
> > > > Cc: Dan Williams 
> > > > Cc: linux...@kvack.org
> > > > Cc: linux-arm-ker...@lists.infradead.org
> > > > Cc: linux-samsung-...@vger.kernel.org
> > > > Cc: linux-me...@vger.kernel.org
> > > > Cc: Oded Gabbay 
> > > > Cc: Omer Shpigelman 
> > > > Cc: Ofir Bitton 
> > > > Cc: Tomer Tayar 
> > > > Cc: Moti Haimovski 
> > > > Cc: Daniel Vetter 
> > > > Cc: Greg Kroah-Hartman 
> > > > Cc: Pawel Piskorski 
> > > > --
> > > > v2: Use unpin_user_pages_dirty_lock (John)
> > > > ---
> > > >  drivers/misc/habanalabs/Kconfig |  1 -
> > > >  drivers/misc/habanalabs/common/habanalabs.h |  3 +-
> > > >  drivers/misc/habanalabs/common/memory.c | 49 -
> > > >  3 files changed, 20 insertions(+), 33 deletions(-)
> > > >
> > > > diff --git a/drivers/misc/habanalabs/Kconfig 
> > > > b/drivers/misc/habanalabs/Kconfig
> > > > index 8eb5d38c618e..2f04187f7167 100644
> > > > --- a/drivers/misc/habanalabs/Kconfig
> > > > +++ b/drivers/misc/habanalabs/Kconfig
> > > > @@ -6,7 +6,6 @@
> > > >  config HABANA_AI
> > > > tristate "HabanaAI accelerators (habanalabs)"
> > > > depends on PCI && HAS_IOMEM
> > > > -   select FRAME_VECTOR
> > > > select DMA_SHARED_BUFFER
> > > > select GENERIC_ALLOCATOR
> > > > select HWMON
> > > > diff --git a/drivers/misc/habanalabs/common/habanalabs.h 
> > > > b/drivers/misc/habanalabs/common/habanalabs.h
> > > > index edbd627b29d2..c1b3ad613b15 100644
> > > > --- a/drivers/misc/habanalabs/common/habanalabs.h
> > > > +++ b/drivers/misc/habanalabs/common/habanalabs.h
> > > > @@ -881,7 +881,8 @@ struct hl_ctx_mgr {
> > > >  struct hl_userptr {
> > > > enum vm_type_t  vm_type; /* must be first */
> > > > struct list_headjob_node;
> > > > -   struct frame_vector *vec;
> > > > +   struct page **pages;
> > > > +   unsigned intnpages;
> > > Can you please update the kerneldoc comment section of this structure
> > > according to your changes ?
> >
> > Apologies I missed the nice kerneldoc. I'll fix that in the next round.
> >
> >
> > > > struct sg_table *sgt;
> > > > enum dma_data_direction dir;
> > > > struct list_headdebugfs_list;
> > > > diff --git a/drivers/misc/habanalabs/common/memory.c 
> > > > b/drivers/misc/habanalabs/common/memory.c
> > > > index 5ff4688683fd..327b64479f97 100644
> > > > --- a/drivers/misc/habanalabs/common/memory.c
> > > > +++ b/drivers/misc/habanalabs/common/memory.c
> > > > @@ -1281,45 +1281,41 @@ static int get_user_memory(struct hl_device 
> > > > *hdev, u64 addr, u64 size,
> > > > return -EFAULT;
> > > > }
> > > >
> > > > -   userptr->vec = frame_vector_create(npages);
> > > > -   if (!userptr->vec) {
> > > > +   userptr->pages = kvmalloc_array(npages, sizeof(*userptr->pages),
> > > > +   GFP_KERNEL);
> > > > +   if (!userptr->pages) {
> > > > dev_err(hdev->dev, "Failed to create frame vector\n");
> > > > return -ENOMEM;
> > > > }
> > > >
> > > > -   rc = get_vaddr_frames(start, npages, FOLL_FORCE | FOLL_WRITE,
> > > > -   userptr->vec);
> > > > +   rc = pin_user_pages_fast(start, npages, FOLL_FORCE | FOLL_WRITE,
> > > > +userptr->pages);
> > > >
> > > > if (rc != npages) {
> > > > dev_err(hdev->dev,
> > > > "Failed to map host memory, user ptr probably 
> > > > wrong\n");
> > > > if (rc < 0)
> > > > -   goto destroy_framevec;
> > > > +   goto destroy_pages;
> > > > +   npages = rc;
> > > > rc = -EFAULT;
> > > > -   goto put_framevec;
> > > > -   }
> > > > -
> > > > -   if (frame_vector_to_pages(userptr->vec) < 0) {
> > > > -   dev_err(hdev->dev,
> > > > -   "Failed to translate frame vector to pages\n");
> > > > -   rc = -EFAULT;
> > > > -   goto put_framevec;
> > > > +   goto put_pages;
> > > > }
> > > > +   userptr->npages = npages;
> > > >
> > > > rc = sg_alloc_table_from_pages(userptr->sgt,
> > > > -   
> > > > 

Re: [PATCH v2 03/17] misc/habana: Stop using frame_vector helpers

2020-10-10 Thread Daniel Vetter
On Sat, Oct 10, 2020 at 11:32 PM Daniel Vetter  wrote:
>
> On Sat, Oct 10, 2020 at 10:27 PM Oded Gabbay  wrote:
> >
> > On Fri, Oct 9, 2020 at 10:59 AM Daniel Vetter  
> > wrote:
> > >
> > > All we need are a pages array, pin_user_pages_fast can give us that
> > > directly. Plus this avoids the entire raw pfn side of get_vaddr_frames.
> > >
> > Thanks for the patch Daniel.
> >
> > > Signed-off-by: Daniel Vetter 
> > > Cc: Jason Gunthorpe 
> > > Cc: Andrew Morton 
> > > Cc: John Hubbard 
> > > Cc: Jérôme Glisse 
> > > Cc: Jan Kara 
> > > Cc: Dan Williams 
> > > Cc: linux...@kvack.org
> > > Cc: linux-arm-ker...@lists.infradead.org
> > > Cc: linux-samsung-...@vger.kernel.org
> > > Cc: linux-me...@vger.kernel.org
> > > Cc: Oded Gabbay 
> > > Cc: Omer Shpigelman 
> > > Cc: Ofir Bitton 
> > > Cc: Tomer Tayar 
> > > Cc: Moti Haimovski 
> > > Cc: Daniel Vetter 
> > > Cc: Greg Kroah-Hartman 
> > > Cc: Pawel Piskorski 
> > > --
> > > v2: Use unpin_user_pages_dirty_lock (John)
> > > ---
> > >  drivers/misc/habanalabs/Kconfig |  1 -
> > >  drivers/misc/habanalabs/common/habanalabs.h |  3 +-
> > >  drivers/misc/habanalabs/common/memory.c | 49 -
> > >  3 files changed, 20 insertions(+), 33 deletions(-)
> > >
> > > diff --git a/drivers/misc/habanalabs/Kconfig 
> > > b/drivers/misc/habanalabs/Kconfig
> > > index 8eb5d38c618e..2f04187f7167 100644
> > > --- a/drivers/misc/habanalabs/Kconfig
> > > +++ b/drivers/misc/habanalabs/Kconfig
> > > @@ -6,7 +6,6 @@
> > >  config HABANA_AI
> > > tristate "HabanaAI accelerators (habanalabs)"
> > > depends on PCI && HAS_IOMEM
> > > -   select FRAME_VECTOR
> > > select DMA_SHARED_BUFFER
> > > select GENERIC_ALLOCATOR
> > > select HWMON
> > > diff --git a/drivers/misc/habanalabs/common/habanalabs.h 
> > > b/drivers/misc/habanalabs/common/habanalabs.h
> > > index edbd627b29d2..c1b3ad613b15 100644
> > > --- a/drivers/misc/habanalabs/common/habanalabs.h
> > > +++ b/drivers/misc/habanalabs/common/habanalabs.h
> > > @@ -881,7 +881,8 @@ struct hl_ctx_mgr {
> > >  struct hl_userptr {
> > > enum vm_type_t  vm_type; /* must be first */
> > > struct list_headjob_node;
> > > -   struct frame_vector *vec;
> > > +   struct page **pages;
> > > +   unsigned intnpages;
> > Can you please update the kerneldoc comment section of this structure
> > according to your changes ?
>
> Apologies I missed the nice kerneldoc. I'll fix that in the next round.
>
>
> > > struct sg_table *sgt;
> > > enum dma_data_direction dir;
> > > struct list_headdebugfs_list;
> > > diff --git a/drivers/misc/habanalabs/common/memory.c 
> > > b/drivers/misc/habanalabs/common/memory.c
> > > index 5ff4688683fd..327b64479f97 100644
> > > --- a/drivers/misc/habanalabs/common/memory.c
> > > +++ b/drivers/misc/habanalabs/common/memory.c
> > > @@ -1281,45 +1281,41 @@ static int get_user_memory(struct hl_device 
> > > *hdev, u64 addr, u64 size,
> > > return -EFAULT;
> > > }
> > >
> > > -   userptr->vec = frame_vector_create(npages);
> > > -   if (!userptr->vec) {
> > > +   userptr->pages = kvmalloc_array(npages, sizeof(*userptr->pages),
> > > +   GFP_KERNEL);
> > > +   if (!userptr->pages) {
> > > dev_err(hdev->dev, "Failed to create frame vector\n");
> > > return -ENOMEM;
> > > }
> > >
> > > -   rc = get_vaddr_frames(start, npages, FOLL_FORCE | FOLL_WRITE,
> > > -   userptr->vec);
> > > +   rc = pin_user_pages_fast(start, npages, FOLL_FORCE | FOLL_WRITE,
> > > +userptr->pages);
> > >
> > > if (rc != npages) {
> > > dev_err(hdev->dev,
> > > "Failed to map host memory, user ptr probably 
> > > wrong\n");
> > > if (rc < 0)
> > > -   goto destroy_framevec;
> > > +   goto destroy_pages;
> > > +   npages = rc;
> > > rc = -EFAULT;
> > > -   goto put_framevec;
> > > -   }
> > > -
> > > -   if (frame_vector_to_pages(userptr->vec) < 0) {
> > > -   dev_err(hdev->dev,
> > > -   "Failed to translate frame vector to pages\n");
> > > -   rc = -EFAULT;
> > > -   goto put_framevec;
> > > +   goto put_pages;
> > > }
> > > +   userptr->npages = npages;
> > >
> > > rc = sg_alloc_table_from_pages(userptr->sgt,
> > > -   frame_vector_pages(userptr->vec),
> > > -   npages, offset, size, GFP_ATOMIC);
> > > +  userptr->pages,
> > > +  npages, offset, size, GFP_ATOMIC);
> > I think that because the call to 

Re: [PATCH] ARM: davinci_all_defconfig: Add CONFIG_DRM_DISPLAY_CONNECTOR=m

2020-10-10 Thread Laurent Pinchart
Hi Jyri,

Thank you for the patch.

On Sat, Oct 10, 2020 at 07:08:50PM +0300, Jyri Sarha wrote:
> Current dumb-vga-dac driver requires CONFIG_DRM_DISPLAY_CONNECTOR
> for it to work.
> 
> Signed-off-by: Jyri Sarha 

Reviewed-by: Laurent Pinchart 

> ---
> An alternative would be selecting CONFIG_DRM_DISPLAY_CONNECTOR from all
> bridges requiring DRM connector.
> 
>  arch/arm/configs/davinci_all_defconfig | 1 +
>  1 file changed, 1 insertion(+)
> 
> diff --git a/arch/arm/configs/davinci_all_defconfig 
> b/arch/arm/configs/davinci_all_defconfig
> index e849367c0566..28471757c129 100644
> --- a/arch/arm/configs/davinci_all_defconfig
> +++ b/arch/arm/configs/davinci_all_defconfig
> @@ -160,6 +160,7 @@ CONFIG_DRM=m
>  CONFIG_DRM_TILCDC=m
>  CONFIG_DRM_SIMPLE_BRIDGE=m
>  CONFIG_DRM_TINYDRM=m
> +CONFIG_DRM_DISPLAY_CONNECTOR=m
>  CONFIG_TINYDRM_ST7586=m
>  CONFIG_FB=y
>  CONFIG_FIRMWARE_EDID=y

-- 
Regards,

Laurent Pinchart
___
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel


Re: [PATCH v2 09/17] mm: Add unsafe_follow_pfn

2020-10-10 Thread Laurent Pinchart
Hi Tomasz,

On Sat, Oct 10, 2020 at 07:22:48PM +0200, Tomasz Figa wrote:
> On Fri, Oct 9, 2020 at 7:52 PM Daniel Vetter  wrote:
> > On Fri, Oct 9, 2020 at 2:48 PM Jason Gunthorpe  wrote:
> > > On Fri, Oct 09, 2020 at 02:37:23PM +0200, Mauro Carvalho Chehab wrote:
> > >
> > > > I'm not a mm/ expert, but, from what I understood from Daniel's patch
> > > > description is that this is unsafe *only if*  __GFP_MOVABLE is used.
> > >
> > > No, it is unconditionally unsafe. The CMA movable mappings are
> > > specific VMAs that will have bad issues here, but there are other
> > > types too.
> > >
> > > The only way to do something at a VMA level is to have a list of OK
> > > VMAs, eg because they were creatd via a special mmap helper from the
> > > media subsystem.
> > >
> > > > Well, no drivers inside the media subsystem uses such flag, although
> > > > they may rely on some infrastructure that could be using it behind
> > > > the bars.
> > >
> > > It doesn't matter, nothing prevents the user from calling media APIs
> > > on mmaps it gets from other subsystems.
> >
> > I think a good first step would be to disable userptr of non struct
> > page backed storage going forward for any new hw support. Even on
> > existing drivers. dma-buf sharing has been around for long enough now
> > that this shouldn't be a problem. Unfortunately right now this doesn't
> > seem to exist, so the entire problem keeps getting perpetuated.
> >
> > > > If this is the case, the proper fix seems to have a GFP_NOT_MOVABLE
> > > > flag that it would be denying the core mm code to set __GFP_MOVABLE.
> > >
> > > We can't tell from the VMA these kinds of details..
> > >
> > > It has to go the other direction, evey mmap that might be used as a
> > > userptr here has to be found and the VMA specially created to allow
> > > its use. At least that is a kernel only change, but will need people
> > > with the HW to do this work.
> >
> > I think the only reasonable way to keep this working is:
> > - add a struct dma_buf *vma_tryget_dma_buf(struct vm_area_struct *vma);
> > - add dma-buf export support to fbdev and v4l
> 
> I assume you mean V4L2 and not the obsolete V4L that is emulated in
> the userspace by libv4l. If so, every video device that uses videobuf2
> gets DMA-buf export for free and there is nothing needed to enable it.
> 
> We probably still have a few legacy drivers using videobuf (non-2),
> but IMHO those should be safe to put behind some disabled-by-default
> Kconfig symbol or even completely drop, as the legacy framework has
> been deprecated for many years already.

There's 8 drivers left, and they support a very large number of devices.
I expect unhappy users distros stop shipping them. On the other hand,
videobuf has been deprecated for a lng time, so there has been
plenty of time to convert the remaining drivers to videobuf2. If nobody
can do it, then we'll have to drop support for these devices given the
security issues.

We have moved media drivers to staging in the past when there wasn't
enough maintenance effort, we could do so here too.

> > - roll this out everywhere we still need it.
> >
> > Realistically this just isn't going to happen. And anything else just
> > reimplements half of dma-buf, which is kinda pointless (you need
> > minimally refcounting and some way to get at a promise of a permanent
> > sg list for dma. Plus probably the vmap for kernel cpu access.
> >
> > > > Please let address the issue on this way, instead of broken an
> > > > userspace API that it is there since 1991.
> > >
> > > It has happened before :( It took 4 years for RDMA to undo the uAPI
> > > breakage caused by a security fix for something that was a 15 years
> > > old bug.
> >
> > Yeah we have a bunch of these on the drm side too. Some of them are
> > really just "you have to upgrade userspace", and there's no real fix
> > for the security nightmare without that.
> 
> I think we need to phase out such userspace indeed. The Kconfig symbol
> allows enabling the unsafe functionality for anyone who still needs
> it, so I think it's not entirely a breakage.

-- 
Regards,

Laurent Pinchart
___
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel


Re: [PATCH v2 03/17] misc/habana: Stop using frame_vector helpers

2020-10-10 Thread Daniel Vetter
On Sat, Oct 10, 2020 at 10:27 PM Oded Gabbay  wrote:
>
> On Fri, Oct 9, 2020 at 10:59 AM Daniel Vetter  wrote:
> >
> > All we need are a pages array, pin_user_pages_fast can give us that
> > directly. Plus this avoids the entire raw pfn side of get_vaddr_frames.
> >
> Thanks for the patch Daniel.
>
> > Signed-off-by: Daniel Vetter 
> > Cc: Jason Gunthorpe 
> > Cc: Andrew Morton 
> > Cc: John Hubbard 
> > Cc: Jérôme Glisse 
> > Cc: Jan Kara 
> > Cc: Dan Williams 
> > Cc: linux...@kvack.org
> > Cc: linux-arm-ker...@lists.infradead.org
> > Cc: linux-samsung-...@vger.kernel.org
> > Cc: linux-me...@vger.kernel.org
> > Cc: Oded Gabbay 
> > Cc: Omer Shpigelman 
> > Cc: Ofir Bitton 
> > Cc: Tomer Tayar 
> > Cc: Moti Haimovski 
> > Cc: Daniel Vetter 
> > Cc: Greg Kroah-Hartman 
> > Cc: Pawel Piskorski 
> > --
> > v2: Use unpin_user_pages_dirty_lock (John)
> > ---
> >  drivers/misc/habanalabs/Kconfig |  1 -
> >  drivers/misc/habanalabs/common/habanalabs.h |  3 +-
> >  drivers/misc/habanalabs/common/memory.c | 49 -
> >  3 files changed, 20 insertions(+), 33 deletions(-)
> >
> > diff --git a/drivers/misc/habanalabs/Kconfig 
> > b/drivers/misc/habanalabs/Kconfig
> > index 8eb5d38c618e..2f04187f7167 100644
> > --- a/drivers/misc/habanalabs/Kconfig
> > +++ b/drivers/misc/habanalabs/Kconfig
> > @@ -6,7 +6,6 @@
> >  config HABANA_AI
> > tristate "HabanaAI accelerators (habanalabs)"
> > depends on PCI && HAS_IOMEM
> > -   select FRAME_VECTOR
> > select DMA_SHARED_BUFFER
> > select GENERIC_ALLOCATOR
> > select HWMON
> > diff --git a/drivers/misc/habanalabs/common/habanalabs.h 
> > b/drivers/misc/habanalabs/common/habanalabs.h
> > index edbd627b29d2..c1b3ad613b15 100644
> > --- a/drivers/misc/habanalabs/common/habanalabs.h
> > +++ b/drivers/misc/habanalabs/common/habanalabs.h
> > @@ -881,7 +881,8 @@ struct hl_ctx_mgr {
> >  struct hl_userptr {
> > enum vm_type_t  vm_type; /* must be first */
> > struct list_headjob_node;
> > -   struct frame_vector *vec;
> > +   struct page **pages;
> > +   unsigned intnpages;
> Can you please update the kerneldoc comment section of this structure
> according to your changes ?

Apologies I missed the nice kerneldoc. I'll fix that in the next round.


> > struct sg_table *sgt;
> > enum dma_data_direction dir;
> > struct list_headdebugfs_list;
> > diff --git a/drivers/misc/habanalabs/common/memory.c 
> > b/drivers/misc/habanalabs/common/memory.c
> > index 5ff4688683fd..327b64479f97 100644
> > --- a/drivers/misc/habanalabs/common/memory.c
> > +++ b/drivers/misc/habanalabs/common/memory.c
> > @@ -1281,45 +1281,41 @@ static int get_user_memory(struct hl_device *hdev, 
> > u64 addr, u64 size,
> > return -EFAULT;
> > }
> >
> > -   userptr->vec = frame_vector_create(npages);
> > -   if (!userptr->vec) {
> > +   userptr->pages = kvmalloc_array(npages, sizeof(*userptr->pages),
> > +   GFP_KERNEL);
> > +   if (!userptr->pages) {
> > dev_err(hdev->dev, "Failed to create frame vector\n");
> > return -ENOMEM;
> > }
> >
> > -   rc = get_vaddr_frames(start, npages, FOLL_FORCE | FOLL_WRITE,
> > -   userptr->vec);
> > +   rc = pin_user_pages_fast(start, npages, FOLL_FORCE | FOLL_WRITE,
> > +userptr->pages);
> >
> > if (rc != npages) {
> > dev_err(hdev->dev,
> > "Failed to map host memory, user ptr probably 
> > wrong\n");
> > if (rc < 0)
> > -   goto destroy_framevec;
> > +   goto destroy_pages;
> > +   npages = rc;
> > rc = -EFAULT;
> > -   goto put_framevec;
> > -   }
> > -
> > -   if (frame_vector_to_pages(userptr->vec) < 0) {
> > -   dev_err(hdev->dev,
> > -   "Failed to translate frame vector to pages\n");
> > -   rc = -EFAULT;
> > -   goto put_framevec;
> > +   goto put_pages;
> > }
> > +   userptr->npages = npages;
> >
> > rc = sg_alloc_table_from_pages(userptr->sgt,
> > -   frame_vector_pages(userptr->vec),
> > -   npages, offset, size, GFP_ATOMIC);
> > +  userptr->pages,
> > +  npages, offset, size, GFP_ATOMIC);
> I think that because the call to kvmalloc_array() is done with
> GFP_KERNEL, there is no point in using GFP_ATOMIC here.
> And actually, this path only needs to avoid yielding when using a
> special debug mode.
> So I suggest putting here GFP_KERNEL.

Huh, I didn't even notice the GFP_ATOMIC here. This looks indeed
strange and GFP_KERNEL should 

Re: [PATCH v2 09/17] mm: Add unsafe_follow_pfn

2020-10-10 Thread Laurent Pinchart
Hi Daniel,

On Fri, Oct 09, 2020 at 07:52:05PM +0200, Daniel Vetter wrote:
> On Fri, Oct 9, 2020 at 2:48 PM Jason Gunthorpe  wrote:
> > On Fri, Oct 09, 2020 at 02:37:23PM +0200, Mauro Carvalho Chehab wrote:
> >
> > > I'm not a mm/ expert, but, from what I understood from Daniel's patch
> > > description is that this is unsafe *only if*  __GFP_MOVABLE is used.
> >
> > No, it is unconditionally unsafe. The CMA movable mappings are
> > specific VMAs that will have bad issues here, but there are other
> > types too.
> >
> > The only way to do something at a VMA level is to have a list of OK
> > VMAs, eg because they were creatd via a special mmap helper from the
> > media subsystem.
> >
> > > Well, no drivers inside the media subsystem uses such flag, although
> > > they may rely on some infrastructure that could be using it behind
> > > the bars.
> >
> > It doesn't matter, nothing prevents the user from calling media APIs
> > on mmaps it gets from other subsystems.
> 
> I think a good first step would be to disable userptr of non struct
> page backed storage going forward for any new hw support. Even on
> existing drivers. dma-buf sharing has been around for long enough now
> that this shouldn't be a problem. Unfortunately right now this doesn't
> seem to exist, so the entire problem keeps getting perpetuated.

On the V4L2 side, I think we should disable USERPTR for any new driver,
period. That's what I've been recommended when reviewing patches for
several years already. It's a deprecated API, it should be phased out,
which starts by not allowing any new use case.

> > > If this is the case, the proper fix seems to have a GFP_NOT_MOVABLE
> > > flag that it would be denying the core mm code to set __GFP_MOVABLE.
> >
> > We can't tell from the VMA these kinds of details..
> >
> > It has to go the other direction, evey mmap that might be used as a
> > userptr here has to be found and the VMA specially created to allow
> > its use. At least that is a kernel only change, but will need people
> > with the HW to do this work.
> 
> I think the only reasonable way to keep this working is:
> - add a struct dma_buf *vma_tryget_dma_buf(struct vm_area_struct *vma);
> - add dma-buf export support to fbdev and v4l
> - roll this out everywhere we still need it.
> 
> Realistically this just isn't going to happen. And anything else just
> reimplements half of dma-buf, which is kinda pointless (you need
> minimally refcounting and some way to get at a promise of a permanent
> sg list for dma. Plus probably the vmap for kernel cpu access.
> 
> > > Please let address the issue on this way, instead of broken an
> > > userspace API that it is there since 1991.
> >
> > It has happened before :( It took 4 years for RDMA to undo the uAPI
> > breakage caused by a security fix for something that was a 15 years
> > old bug.
> 
> Yeah we have a bunch of these on the drm side too. Some of them are
> really just "you have to upgrade userspace", and there's no real fix
> for the security nightmare without that.

-- 
Regards,

Laurent Pinchart
___
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel


[PATCH v3 0/2] drm/tilcdc: Couple of minor feature improvements

2020-10-10 Thread Jyri Sarha
Since v2:
- "drm/tilcdc: Remove tilcdc_crtc_max_width(), use private data"
 - Add reviewed-by tag
 - fix:
   -:106: CHECK:PARENTHESIS_ALIGNMENT: Alignment should match open parenthesis
   #106: FILE: drivers/gpu/drm/tilcdc/tilcdc_drv.c:289:
   +   if (of_property_read_u32(node, "max-pixelclock",
   +   >max_pixelclock))

Since first version. 
- "drm/tilcdc: Do not keep vblank interrupts enabled all the time"
  - Add reviewed-by tags
- "drm/tilcdc: Remove tilcdc_crtc_max_width(), use private data"
  - Put TILCDC_DEFAULT_MAX_WIDTH_V1 before *_V2 and


The vblank interrupts have been always on when the display is on for a
very long time, so I decided that it is about time to fix it. Then the
following patch is just a cleanup.

BR,
Jyri

Jyri Sarha (2):
  drm/tilcdc: Do not keep vblank interrupts enabled all the time
  drm/tilcdc: Remove tilcdc_crtc_max_width(), use private data

 drivers/gpu/drm/tilcdc/tilcdc_crtc.c | 52 ++--
 drivers/gpu/drm/tilcdc/tilcdc_drv.c  | 38 +++-
 drivers/gpu/drm/tilcdc/tilcdc_drv.h  |  7 ++--
 3 files changed, 59 insertions(+), 38 deletions(-)

-- 
Texas Instruments Finland Oy, Porkkalankatu 22, 00180 Helsinki. 
Y-tunnus/Business ID: 0615521-4. Kotipaikka/Domicile: Helsinki

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


[PATCH v3 2/2] drm/tilcdc: Remove tilcdc_crtc_max_width(), use private data

2020-10-10 Thread Jyri Sarha
We already have a private data member for maximum display width so
let's use it and get rid of the redundant tilcdc_crtc_max_width().

The LCDC version probing is moved to before reading the device tree
properties so that the version information is available when private
data maximum width is initialized, if "max-width" property is not
found.

Signed-off-by: Jyri Sarha 
Reviewed-by: Tomi Valkeinen 
---
 drivers/gpu/drm/tilcdc/tilcdc_crtc.c | 16 +---
 drivers/gpu/drm/tilcdc/tilcdc_drv.c  | 38 +++-
 drivers/gpu/drm/tilcdc/tilcdc_drv.h  |  7 ++---
 3 files changed, 26 insertions(+), 35 deletions(-)

diff --git a/drivers/gpu/drm/tilcdc/tilcdc_crtc.c 
b/drivers/gpu/drm/tilcdc/tilcdc_crtc.c
index 0fd3dafe6404..da2ab2aa3577 100644
--- a/drivers/gpu/drm/tilcdc/tilcdc_crtc.c
+++ b/drivers/gpu/drm/tilcdc/tilcdc_crtc.c
@@ -754,20 +754,6 @@ static const struct drm_crtc_funcs tilcdc_crtc_funcs = {
.disable_vblank = tilcdc_crtc_disable_vblank,
 };
 
-int tilcdc_crtc_max_width(struct drm_crtc *crtc)
-{
-   struct drm_device *dev = crtc->dev;
-   struct tilcdc_drm_private *priv = dev->dev_private;
-   int max_width = 0;
-
-   if (priv->rev == 1)
-   max_width = 1024;
-   else if (priv->rev == 2)
-   max_width = 2048;
-
-   return max_width;
-}
-
 static enum drm_mode_status
 tilcdc_crtc_mode_valid(struct drm_crtc *crtc,
   const struct drm_display_mode *mode)
@@ -780,7 +766,7 @@ tilcdc_crtc_mode_valid(struct drm_crtc *crtc,
 * check to see if the width is within the range that
 * the LCD Controller physically supports
 */
-   if (mode->hdisplay > tilcdc_crtc_max_width(crtc))
+   if (mode->hdisplay > priv->max_width)
return MODE_VIRTUAL_X;
 
/* width must be multiple of 16 */
diff --git a/drivers/gpu/drm/tilcdc/tilcdc_drv.c 
b/drivers/gpu/drm/tilcdc/tilcdc_drv.c
index 4f5fc3e87383..c5f82e693f1a 100644
--- a/drivers/gpu/drm/tilcdc/tilcdc_drv.c
+++ b/drivers/gpu/drm/tilcdc/tilcdc_drv.c
@@ -105,7 +105,7 @@ static void modeset_init(struct drm_device *dev)
 
dev->mode_config.min_width = 0;
dev->mode_config.min_height = 0;
-   dev->mode_config.max_width = tilcdc_crtc_max_width(priv->crtc);
+   dev->mode_config.max_width = priv->max_width;
dev->mode_config.max_height = 2048;
dev->mode_config.funcs = _config_funcs;
 }
@@ -218,22 +218,6 @@ static int tilcdc_init(struct drm_driver *ddrv, struct 
device *dev)
goto init_failed;
}
 
-   if (of_property_read_u32(node, "max-bandwidth", >max_bandwidth))
-   priv->max_bandwidth = TILCDC_DEFAULT_MAX_BANDWIDTH;
-
-   DBG("Maximum Bandwidth Value %d", priv->max_bandwidth);
-
-   if (of_property_read_u32(node, "max-width", >max_width))
-   priv->max_width = TILCDC_DEFAULT_MAX_WIDTH;
-
-   DBG("Maximum Horizontal Pixel Width Value %dpixels", priv->max_width);
-
-   if (of_property_read_u32(node, "max-pixelclock",
-   >max_pixelclock))
-   priv->max_pixelclock = TILCDC_DEFAULT_MAX_PIXELCLOCK;
-
-   DBG("Maximum Pixel Clock Value %dKHz", priv->max_pixelclock);
-
pm_runtime_enable(dev);
 
/* Determine LCD IP Version */
@@ -287,6 +271,26 @@ static int tilcdc_init(struct drm_driver *ddrv, struct 
device *dev)
}
}
 
+   if (of_property_read_u32(node, "max-bandwidth", >max_bandwidth))
+   priv->max_bandwidth = TILCDC_DEFAULT_MAX_BANDWIDTH;
+
+   DBG("Maximum Bandwidth Value %d", priv->max_bandwidth);
+
+   if (of_property_read_u32(node, "max-width", >max_width)) {
+   if (priv->rev == 1)
+   priv->max_width = TILCDC_DEFAULT_MAX_WIDTH_V1;
+   else
+   priv->max_width = TILCDC_DEFAULT_MAX_WIDTH_V2;
+   }
+
+   DBG("Maximum Horizontal Pixel Width Value %dpixels", priv->max_width);
+
+   if (of_property_read_u32(node, "max-pixelclock",
+>max_pixelclock))
+   priv->max_pixelclock = TILCDC_DEFAULT_MAX_PIXELCLOCK;
+
+   DBG("Maximum Pixel Clock Value %dKHz", priv->max_pixelclock);
+
ret = tilcdc_crtc_create(ddev);
if (ret < 0) {
dev_err(dev, "failed to create crtc\n");
diff --git a/drivers/gpu/drm/tilcdc/tilcdc_drv.h 
b/drivers/gpu/drm/tilcdc/tilcdc_drv.h
index 18815e75ca4f..d29806ca8817 100644
--- a/drivers/gpu/drm/tilcdc/tilcdc_drv.h
+++ b/drivers/gpu/drm/tilcdc/tilcdc_drv.h
@@ -28,8 +28,10 @@ struct drm_plane;
 
 /* Defaulting to pixel clock defined on AM335x */
 #define TILCDC_DEFAULT_MAX_PIXELCLOCK  126000
-/* Defaulting to max width as defined on AM335x */
-#define TILCDC_DEFAULT_MAX_WIDTH  2048
+/* Maximum display width for LCDC V1 */
+#define TILCDC_DEFAULT_MAX_WIDTH_V1  1024
+/* ... and for LCDC V2 found on AM335x: */
+#define TILCDC_DEFAULT_MAX_WIDTH_V2  2048
 /*
 

[PATCH v3 1/2] drm/tilcdc: Do not keep vblank interrupts enabled all the time

2020-10-10 Thread Jyri Sarha
END_OF_FRAME interrupts have been enabled all the time since the
beginning of this driver. It is about time to add this feature.

Signed-off-by: Jyri Sarha 
Reviewed-by: Tomi Valkeinen 
Reviewed-by: Laurent Pinchart 
---
 drivers/gpu/drm/tilcdc/tilcdc_crtc.c | 36 +---
 1 file changed, 33 insertions(+), 3 deletions(-)

diff --git a/drivers/gpu/drm/tilcdc/tilcdc_crtc.c 
b/drivers/gpu/drm/tilcdc/tilcdc_crtc.c
index cf4ead0dc2e3..0fd3dafe6404 100644
--- a/drivers/gpu/drm/tilcdc/tilcdc_crtc.c
+++ b/drivers/gpu/drm/tilcdc/tilcdc_crtc.c
@@ -147,12 +147,9 @@ static void tilcdc_crtc_enable_irqs(struct drm_device *dev)
tilcdc_set(dev, LCDC_RASTER_CTRL_REG,
LCDC_V1_SYNC_LOST_INT_ENA | LCDC_V1_FRAME_DONE_INT_ENA |
LCDC_V1_UNDERFLOW_INT_ENA);
-   tilcdc_set(dev, LCDC_DMA_CTRL_REG,
-   LCDC_V1_END_OF_FRAME_INT_ENA);
} else {
tilcdc_write(dev, LCDC_INT_ENABLE_SET_REG,
LCDC_V2_UNDERFLOW_INT_ENA |
-   LCDC_V2_END_OF_FRAME0_INT_ENA |
LCDC_FRAME_DONE | LCDC_SYNC_LOST);
}
 }
@@ -678,11 +675,44 @@ static int tilcdc_crtc_atomic_check(struct drm_crtc *crtc,
 
 static int tilcdc_crtc_enable_vblank(struct drm_crtc *crtc)
 {
+   struct tilcdc_crtc *tilcdc_crtc = to_tilcdc_crtc(crtc);
+   struct drm_device *dev = crtc->dev;
+   struct tilcdc_drm_private *priv = dev->dev_private;
+   unsigned long flags;
+
+   spin_lock_irqsave(_crtc->irq_lock, flags);
+
+   tilcdc_clear_irqstatus(dev, LCDC_END_OF_FRAME0);
+
+   if (priv->rev == 1)
+   tilcdc_set(dev, LCDC_DMA_CTRL_REG,
+  LCDC_V1_END_OF_FRAME_INT_ENA);
+   else
+   tilcdc_set(dev, LCDC_INT_ENABLE_SET_REG,
+  LCDC_V2_END_OF_FRAME0_INT_ENA);
+
+   spin_unlock_irqrestore(_crtc->irq_lock, flags);
+
return 0;
 }
 
 static void tilcdc_crtc_disable_vblank(struct drm_crtc *crtc)
 {
+   struct tilcdc_crtc *tilcdc_crtc = to_tilcdc_crtc(crtc);
+   struct drm_device *dev = crtc->dev;
+   struct tilcdc_drm_private *priv = dev->dev_private;
+   unsigned long flags;
+
+   spin_lock_irqsave(_crtc->irq_lock, flags);
+
+   if (priv->rev == 1)
+   tilcdc_clear(dev, LCDC_DMA_CTRL_REG,
+LCDC_V1_END_OF_FRAME_INT_ENA);
+   else
+   tilcdc_clear(dev, LCDC_INT_ENABLE_SET_REG,
+LCDC_V2_END_OF_FRAME0_INT_ENA);
+
+   spin_unlock_irqrestore(_crtc->irq_lock, flags);
 }
 
 static void tilcdc_crtc_reset(struct drm_crtc *crtc)
-- 
Texas Instruments Finland Oy, Porkkalankatu 22, 00180 Helsinki. 
Y-tunnus/Business ID: 0615521-4. Kotipaikka/Domicile: Helsinki

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


Re: [PATCH v2 03/17] misc/habana: Stop using frame_vector helpers

2020-10-10 Thread Oded Gabbay
On Fri, Oct 9, 2020 at 10:59 AM Daniel Vetter  wrote:
>
> All we need are a pages array, pin_user_pages_fast can give us that
> directly. Plus this avoids the entire raw pfn side of get_vaddr_frames.
>
Thanks for the patch Daniel.

> Signed-off-by: Daniel Vetter 
> Cc: Jason Gunthorpe 
> Cc: Andrew Morton 
> Cc: John Hubbard 
> Cc: Jérôme Glisse 
> Cc: Jan Kara 
> Cc: Dan Williams 
> Cc: linux...@kvack.org
> Cc: linux-arm-ker...@lists.infradead.org
> Cc: linux-samsung-...@vger.kernel.org
> Cc: linux-me...@vger.kernel.org
> Cc: Oded Gabbay 
> Cc: Omer Shpigelman 
> Cc: Ofir Bitton 
> Cc: Tomer Tayar 
> Cc: Moti Haimovski 
> Cc: Daniel Vetter 
> Cc: Greg Kroah-Hartman 
> Cc: Pawel Piskorski 
> --
> v2: Use unpin_user_pages_dirty_lock (John)
> ---
>  drivers/misc/habanalabs/Kconfig |  1 -
>  drivers/misc/habanalabs/common/habanalabs.h |  3 +-
>  drivers/misc/habanalabs/common/memory.c | 49 -
>  3 files changed, 20 insertions(+), 33 deletions(-)
>
> diff --git a/drivers/misc/habanalabs/Kconfig b/drivers/misc/habanalabs/Kconfig
> index 8eb5d38c618e..2f04187f7167 100644
> --- a/drivers/misc/habanalabs/Kconfig
> +++ b/drivers/misc/habanalabs/Kconfig
> @@ -6,7 +6,6 @@
>  config HABANA_AI
> tristate "HabanaAI accelerators (habanalabs)"
> depends on PCI && HAS_IOMEM
> -   select FRAME_VECTOR
> select DMA_SHARED_BUFFER
> select GENERIC_ALLOCATOR
> select HWMON
> diff --git a/drivers/misc/habanalabs/common/habanalabs.h 
> b/drivers/misc/habanalabs/common/habanalabs.h
> index edbd627b29d2..c1b3ad613b15 100644
> --- a/drivers/misc/habanalabs/common/habanalabs.h
> +++ b/drivers/misc/habanalabs/common/habanalabs.h
> @@ -881,7 +881,8 @@ struct hl_ctx_mgr {
>  struct hl_userptr {
> enum vm_type_t  vm_type; /* must be first */
> struct list_headjob_node;
> -   struct frame_vector *vec;
> +   struct page **pages;
> +   unsigned intnpages;
Can you please update the kerneldoc comment section of this structure
according to your changes ?

> struct sg_table *sgt;
> enum dma_data_direction dir;
> struct list_headdebugfs_list;
> diff --git a/drivers/misc/habanalabs/common/memory.c 
> b/drivers/misc/habanalabs/common/memory.c
> index 5ff4688683fd..327b64479f97 100644
> --- a/drivers/misc/habanalabs/common/memory.c
> +++ b/drivers/misc/habanalabs/common/memory.c
> @@ -1281,45 +1281,41 @@ static int get_user_memory(struct hl_device *hdev, 
> u64 addr, u64 size,
> return -EFAULT;
> }
>
> -   userptr->vec = frame_vector_create(npages);
> -   if (!userptr->vec) {
> +   userptr->pages = kvmalloc_array(npages, sizeof(*userptr->pages),
> +   GFP_KERNEL);
> +   if (!userptr->pages) {
> dev_err(hdev->dev, "Failed to create frame vector\n");
> return -ENOMEM;
> }
>
> -   rc = get_vaddr_frames(start, npages, FOLL_FORCE | FOLL_WRITE,
> -   userptr->vec);
> +   rc = pin_user_pages_fast(start, npages, FOLL_FORCE | FOLL_WRITE,
> +userptr->pages);
>
> if (rc != npages) {
> dev_err(hdev->dev,
> "Failed to map host memory, user ptr probably 
> wrong\n");
> if (rc < 0)
> -   goto destroy_framevec;
> +   goto destroy_pages;
> +   npages = rc;
> rc = -EFAULT;
> -   goto put_framevec;
> -   }
> -
> -   if (frame_vector_to_pages(userptr->vec) < 0) {
> -   dev_err(hdev->dev,
> -   "Failed to translate frame vector to pages\n");
> -   rc = -EFAULT;
> -   goto put_framevec;
> +   goto put_pages;
> }
> +   userptr->npages = npages;
>
> rc = sg_alloc_table_from_pages(userptr->sgt,
> -   frame_vector_pages(userptr->vec),
> -   npages, offset, size, GFP_ATOMIC);
> +  userptr->pages,
> +  npages, offset, size, GFP_ATOMIC);
I think that because the call to kvmalloc_array() is done with
GFP_KERNEL, there is no point in using GFP_ATOMIC here.
And actually, this path only needs to avoid yielding when using a
special debug mode.
So I suggest putting here GFP_KERNEL.

In the meanwhile, I'll run this patch (coupled with the next patch) in
our C/I to make sure there are no regressions.
Thanks,
Oded

> if (rc < 0) {
> dev_err(hdev->dev, "failed to create SG table from pages\n");
> -   goto put_framevec;
> +   goto put_pages;
> }
>
> return 0;
>
> -put_framevec:
> -   put_vaddr_frames(userptr->vec);
> -destroy_framevec:
> -   

[PATCH] ARM: davinci_all_defconfig: Add CONFIG_DRM_DISPLAY_CONNECTOR=m

2020-10-10 Thread Jyri Sarha
Current dumb-vga-dac driver requires CONFIG_DRM_DISPLAY_CONNECTOR
for it to work.

Signed-off-by: Jyri Sarha 
---
An alternative would be selecting CONFIG_DRM_DISPLAY_CONNECTOR from all
bridges requiring DRM connector.

 arch/arm/configs/davinci_all_defconfig | 1 +
 1 file changed, 1 insertion(+)

diff --git a/arch/arm/configs/davinci_all_defconfig 
b/arch/arm/configs/davinci_all_defconfig
index e849367c0566..28471757c129 100644
--- a/arch/arm/configs/davinci_all_defconfig
+++ b/arch/arm/configs/davinci_all_defconfig
@@ -160,6 +160,7 @@ CONFIG_DRM=m
 CONFIG_DRM_TILCDC=m
 CONFIG_DRM_SIMPLE_BRIDGE=m
 CONFIG_DRM_TINYDRM=m
+CONFIG_DRM_DISPLAY_CONNECTOR=m
 CONFIG_TINYDRM_ST7586=m
 CONFIG_FB=y
 CONFIG_FIRMWARE_EDID=y
-- 
Texas Instruments Finland Oy, Porkkalankatu 22, 00180 Helsinki. 
Y-tunnus/Business ID: 0615521-4. Kotipaikka/Domicile: Helsinki

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


Re: [PATCH v2 09/17] mm: Add unsafe_follow_pfn

2020-10-10 Thread Tomasz Figa
Hi Mauro,

On Fri, Oct 9, 2020 at 2:37 PM Mauro Carvalho Chehab
 wrote:
>
> Em Fri, 9 Oct 2020 09:21:11 -0300
> Jason Gunthorpe  escreveu:
>
> > On Fri, Oct 09, 2020 at 12:34:21PM +0200, Mauro Carvalho Chehab wrote:
> > > Hi,
> > >
> > > Em Fri,  9 Oct 2020 09:59:26 +0200
> > > Daniel Vetter  escreveu:
> > >
> > > > Way back it was a reasonable assumptions that iomem mappings never
> > > > change the pfn range they point at. But this has changed:
> > > >
> > > > - gpu drivers dynamically manage their memory nowadays, invalidating
> > > > ptes with unmap_mapping_range when buffers get moved
> > > >
> > > > - contiguous dma allocations have moved from dedicated carvetouts to
> > > > cma regions. This means if we miss the unmap the pfn might contain
> > > > pagecache or anon memory (well anything allocated with GFP_MOVEABLE)
> > > >
> > > > - even /dev/mem now invalidates mappings when the kernel requests that
> > > > iomem region when CONFIG_IO_STRICT_DEVMEM is set, see 3234ac664a87
> > > > ("/dev/mem: Revoke mappings when a driver claims the region")
> > > >
> > > > Accessing pfns obtained from ptes without holding all the locks is
> > > > therefore no longer a good idea.
> > > >
> > > > Unfortunately there's some users where this is not fixable (like v4l
> > > > userptr of iomem mappings) or involves a pile of work (vfio type1
> > > > iommu). For now annotate these as unsafe and splat appropriately.
> > > >
> > > > This patch adds an unsafe_follow_pfn, which later patches will then
> > > > roll out to all appropriate places.
> > >
> > > NACK, as this breaks an existing userspace API on media.
> >
> > It doesn't break it. You get a big warning the thing is broken and it
> > keeps working.
> >
> > We can't leave such a huge security hole open - it impacts other
> > subsystems, distros need to be able to run in a secure mode.
>
> Well, if distros disable it, then apps will break.
>

Do we have any information on userspace that actually needs this functionality?

Note that we're _not_ talking here about the complete USERPTR
functionality, but rather just the very corner case of carveout memory
not backed by struct pages.

Given that the current in-tree ways of reserving carveout memory, such
as shared-dma-pool, actually give memory backed by struct pages, do we
even have a source of such legacy memory in the kernel today?

I think that given that this is a very niche functionality, we could
have it disabled by default for security reasons and if someone
_really_ (i.e. there is no replacement) needs it, they probably need
to use a custom kernel build anyway for their exotic hardware setup
(with PFN-backed carveout memory), so they can enable it.

> > > While I agree that using the userptr on media is something that
> > > new drivers may not support, as DMABUF is a better way of
> > > handling it, changing this for existing ones is a big no,
> > > as it may break usersapace.
> >
> > media community needs to work to fix this, not pretend it is OK to
> > keep going as-is.
>
> > Dealing with security issues is the one case where an uABI break might
> > be acceptable.
> >
> > If you want to NAK it then you need to come up with the work to do
> > something here correctly that will support the old drivers without the
> > kernel taint.
> >
> > Unfortunately making things uncomfortable for the subsystem is the big
> > hammer the core kernel needs to use to actually get this security work
> > done by those responsible.
>
>
> I'm not pretending that this is ok. Just pointing that the approach
> taken is NOT OK.
>
> I'm not a mm/ expert, but, from what I understood from Daniel's patch
> description is that this is unsafe *only if*  __GFP_MOVABLE is used.
>
> Well, no drivers inside the media subsystem uses such flag, although
> they may rely on some infrastructure that could be using it behind
> the bars.
>
> If this is the case, the proper fix seems to have a GFP_NOT_MOVABLE
> flag that it would be denying the core mm code to set __GFP_MOVABLE.
>
> Please let address the issue on this way, instead of broken an
> userspace API that it is there since 1991.

Note that USERPTR as a whole generally has been considered deprecated
in V4L2 for many years and people have been actively discouraged to
use it. And, still, we're just talking here about the very rare corner
case, not the whole USERPTR API.

Best regards,
Tomasz
___
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel


Re: [PATCH v2 09/17] mm: Add unsafe_follow_pfn

2020-10-10 Thread Tomasz Figa
Hi Daniel,

On Fri, Oct 9, 2020 at 7:52 PM Daniel Vetter  wrote:
>
> On Fri, Oct 9, 2020 at 2:48 PM Jason Gunthorpe  wrote:
> >
> > On Fri, Oct 09, 2020 at 02:37:23PM +0200, Mauro Carvalho Chehab wrote:
> >
> > > I'm not a mm/ expert, but, from what I understood from Daniel's patch
> > > description is that this is unsafe *only if*  __GFP_MOVABLE is used.
> >
> > No, it is unconditionally unsafe. The CMA movable mappings are
> > specific VMAs that will have bad issues here, but there are other
> > types too.
> >
> > The only way to do something at a VMA level is to have a list of OK
> > VMAs, eg because they were creatd via a special mmap helper from the
> > media subsystem.
> >
> > > Well, no drivers inside the media subsystem uses such flag, although
> > > they may rely on some infrastructure that could be using it behind
> > > the bars.
> >
> > It doesn't matter, nothing prevents the user from calling media APIs
> > on mmaps it gets from other subsystems.
>
> I think a good first step would be to disable userptr of non struct
> page backed storage going forward for any new hw support. Even on
> existing drivers. dma-buf sharing has been around for long enough now
> that this shouldn't be a problem. Unfortunately right now this doesn't
> seem to exist, so the entire problem keeps getting perpetuated.
>
> > > If this is the case, the proper fix seems to have a GFP_NOT_MOVABLE
> > > flag that it would be denying the core mm code to set __GFP_MOVABLE.
> >
> > We can't tell from the VMA these kinds of details..
> >
> > It has to go the other direction, evey mmap that might be used as a
> > userptr here has to be found and the VMA specially created to allow
> > its use. At least that is a kernel only change, but will need people
> > with the HW to do this work.
>
> I think the only reasonable way to keep this working is:
> - add a struct dma_buf *vma_tryget_dma_buf(struct vm_area_struct *vma);
> - add dma-buf export support to fbdev and v4l

I assume you mean V4L2 and not the obsolete V4L that is emulated in
the userspace by libv4l. If so, every video device that uses videobuf2
gets DMA-buf export for free and there is nothing needed to enable it.

We probably still have a few legacy drivers using videobuf (non-2),
but IMHO those should be safe to put behind some disabled-by-default
Kconfig symbol or even completely drop, as the legacy framework has
been deprecated for many years already.

> - roll this out everywhere we still need it.
>
> Realistically this just isn't going to happen. And anything else just
> reimplements half of dma-buf, which is kinda pointless (you need
> minimally refcounting and some way to get at a promise of a permanent
> sg list for dma. Plus probably the vmap for kernel cpu access.
>
> > > Please let address the issue on this way, instead of broken an
> > > userspace API that it is there since 1991.
> >
> > It has happened before :( It took 4 years for RDMA to undo the uAPI
> > breakage caused by a security fix for something that was a 15 years
> > old bug.
>
> Yeah we have a bunch of these on the drm side too. Some of them are
> really just "you have to upgrade userspace", and there's no real fix
> for the security nightmare without that.

I think we need to phase out such userspace indeed. The Kconfig symbol
allows enabling the unsafe functionality for anyone who still needs
it, so I think it's not entirely a breakage.

Best regards,
Tomasz
___
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel


[Bug 205649] Daisy Chain (MST) Session Crash after Screen Lock Resume

2020-10-10 Thread bugzilla-daemon
https://bugzilla.kernel.org/show_bug.cgi?id=205649

Michael Rauch (mich...@rauch.be) changed:

   What|Removed |Added

 Kernel Version|5.3, 5.4, 5.5-rc3   |5.3, 5.4, 5.5-rc3, 5.9-rc7

--- Comment #4 from Michael Rauch (mich...@rauch.be) ---
Error message:

[drm] DM_MST: stopping TM on aconnector: 9b31f0b7 [id: 74]

-- 
You are receiving this mail because:
You are watching the assignee of the bug.
___
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel


[Bug 205649] Daisy Chain (MST) Session Crash after Screen Lock Resume

2020-10-10 Thread bugzilla-daemon
https://bugzilla.kernel.org/show_bug.cgi?id=205649

--- Comment #3 from Michael Rauch (mich...@rauch.be) ---
Same behavior with Kernel 5.9.0-050900rc7-generic and Mesa 20.0.8.

-- 
You are receiving this mail because:
You are watching the assignee of the bug.
___
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel


Re: [PATCH v2 09/17] mm: Add unsafe_follow_pfn

2020-10-10 Thread Daniel Vetter
On Sat, Oct 10, 2020 at 1:39 PM Mauro Carvalho Chehab
 wrote:
>
> Em Sat, 10 Oct 2020 12:53:49 +0200
> Daniel Vetter  escreveu:
>
> > Hi Mauro,
> >
> > You might want to read the patches more carefully, because what you're
> > demanding is what my patches do. Short summary:
> >
> > - if STRICT_FOLLOW_PFN is set:
> > a) normal memory is handled as-is (i.e. your example works) through
> > the addition of FOLL_LONGTERM. This is the "pin the pages correctly"
> > approach you're demanding
> > b) for non-page memory (zerocopy sharing before dma-buf was upstreamed
> > is the only use-case for this) it is correctly rejected with -EINVAL
> >
> > - if you do have blobby userspace which requires the zero-copy using
> > userptr to work, and doesn't have any of the fallbacks implemented
> > that you describe, this would be a regression. That's why
> > STRICT_FOLLOW_PFN can be unset. And yes there's a real security issue
> > in this usage, Marek also confirmed that the removal of the vma copy
> > code a few years ago essentially broke even the weak assumptions that
> > made the code work 10+ years ago when it was merged.
> >
> > so tdlr; Everything you described will keep working even with the new
> > flag set, and everything you demand must be implemented _is_
> > implemented in this patch series.
> >
> > Also please keep in mind that we are _not_ talking about the general
> > userptr support that was merge ~20 years ago. This patch series here
> > is _only_ about the zerocpy userptr support merged with 50ac952d2263
> > ("[media] videobuf2-dma-sg: Support io userptr operations on io
> > memory") in 2013.
>
> Ok, now it is making more sense. Please update the comments for
> patch 10/17 to describe the above.

Will do.

> We need some time to test this though, in order to check if no
> regressions were added (except the ones due to changeset 50ac952d2263).

Yeah testing of the previous patches to switch to FOLL_LONGTERM would
be really good. I also need that for habanalabs and ideally exynos
too. All the userptr for normal memory should keep working, and with
FOLL_LONGTERM it should actually work better, since with that it
should now correctly interact with pagecache and fs code, not just
with anon memory from malloc.

Thanks, Daniel

> > Why this hack was merged in 2013 when we merged dma-buf almost 2 years
> > before that I have no idea about. Imo that patch simply should never
> > have landed, and instead dma-buf support prioritized.
>
> If I recall correctly, we didn't have any DMABUF support
> at the media subsystem, back on 2013.
>
> It took some time for the DMA-BUF to arrive at media, as this
> was not a top priority. Also, there aren't many developers that
> understand the memory model well enough to implement DMA-BUF support
> and touch the VB2 code, which is quite complex, as it supports
> lots of different ways for I/O, plus works with vmalloc, DMA
> contig and DMA scatter/gather.
>
> Changes there should carefully be tested against different
> drivers, in order to avoid regressions on it.
>
> > Cheers, Daniel
>
> Thanks,
> Mauro



-- 
Daniel Vetter
Software Engineer, Intel Corporation
http://blog.ffwll.ch
___
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel


Re: [PATCH v2 09/17] mm: Add unsafe_follow_pfn

2020-10-10 Thread Mauro Carvalho Chehab
Em Sat, 10 Oct 2020 12:53:49 +0200
Daniel Vetter  escreveu:

> Hi Mauro,
> 
> You might want to read the patches more carefully, because what you're
> demanding is what my patches do. Short summary:
> 
> - if STRICT_FOLLOW_PFN is set:
> a) normal memory is handled as-is (i.e. your example works) through
> the addition of FOLL_LONGTERM. This is the "pin the pages correctly"
> approach you're demanding
> b) for non-page memory (zerocopy sharing before dma-buf was upstreamed
> is the only use-case for this) it is correctly rejected with -EINVAL
> 
> - if you do have blobby userspace which requires the zero-copy using
> userptr to work, and doesn't have any of the fallbacks implemented
> that you describe, this would be a regression. That's why
> STRICT_FOLLOW_PFN can be unset. And yes there's a real security issue
> in this usage, Marek also confirmed that the removal of the vma copy
> code a few years ago essentially broke even the weak assumptions that
> made the code work 10+ years ago when it was merged.
> 
> so tdlr; Everything you described will keep working even with the new
> flag set, and everything you demand must be implemented _is_
> implemented in this patch series.
> 
> Also please keep in mind that we are _not_ talking about the general
> userptr support that was merge ~20 years ago. This patch series here
> is _only_ about the zerocpy userptr support merged with 50ac952d2263
> ("[media] videobuf2-dma-sg: Support io userptr operations on io
> memory") in 2013.

Ok, now it is making more sense. Please update the comments for
patch 10/17 to describe the above.

We need some time to test this though, in order to check if no
regressions were added (except the ones due to changeset 50ac952d2263).

> 
> Why this hack was merged in 2013 when we merged dma-buf almost 2 years
> before that I have no idea about. Imo that patch simply should never
> have landed, and instead dma-buf support prioritized.

If I recall correctly, we didn't have any DMABUF support
at the media subsystem, back on 2013.

It took some time for the DMA-BUF to arrive at media, as this
was not a top priority. Also, there aren't many developers that
understand the memory model well enough to implement DMA-BUF support
and touch the VB2 code, which is quite complex, as it supports
lots of different ways for I/O, plus works with vmalloc, DMA
contig and DMA scatter/gather. 

Changes there should carefully be tested against different
drivers, in order to avoid regressions on it.

> Cheers, Daniel

Thanks,
Mauro
___
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel


Re: [PATCH v2 09/17] mm: Add unsafe_follow_pfn

2020-10-10 Thread Daniel Vetter
Hi Mauro,

You might want to read the patches more carefully, because what you're
demanding is what my patches do. Short summary:

- if STRICT_FOLLOW_PFN is set:
a) normal memory is handled as-is (i.e. your example works) through
the addition of FOLL_LONGTERM. This is the "pin the pages correctly"
approach you're demanding
b) for non-page memory (zerocopy sharing before dma-buf was upstreamed
is the only use-case for this) it is correctly rejected with -EINVAL

- if you do have blobby userspace which requires the zero-copy using
userptr to work, and doesn't have any of the fallbacks implemented
that you describe, this would be a regression. That's why
STRICT_FOLLOW_PFN can be unset. And yes there's a real security issue
in this usage, Marek also confirmed that the removal of the vma copy
code a few years ago essentially broke even the weak assumptions that
made the code work 10+ years ago when it was merged.

so tdlr; Everything you described will keep working even with the new
flag set, and everything you demand must be implemented _is_
implemented in this patch series.

Also please keep in mind that we are _not_ talking about the general
userptr support that was merge ~20 years ago. This patch series here
is _only_ about the zerocpy userptr support merged with 50ac952d2263
("[media] videobuf2-dma-sg: Support io userptr operations on io
memory") in 2013.

Why this hack was merged in 2013 when we merged dma-buf almost 2 years
before that I have no idea about. Imo that patch simply should never
have landed, and instead dma-buf support prioritized.

Cheers, Daniel


On Sat, Oct 10, 2020 at 11:21 AM Mauro Carvalho Chehab
 wrote:
>
> Em Fri, 9 Oct 2020 19:52:05 +0200
> Daniel Vetter  escreveu:
>
> > On Fri, Oct 9, 2020 at 2:48 PM Jason Gunthorpe  wrote:
> > >
> > > On Fri, Oct 09, 2020 at 02:37:23PM +0200, Mauro Carvalho Chehab wrote:
> > >
> > > > I'm not a mm/ expert, but, from what I understood from Daniel's patch
> > > > description is that this is unsafe *only if*  __GFP_MOVABLE is used.
> > >
> > > No, it is unconditionally unsafe. The CMA movable mappings are
> > > specific VMAs that will have bad issues here, but there are other
> > > types too.
>
> I didn't check the mm dirty details, but I strongly suspect that the mm
> code has a way to prevent moving a mmapped page while it is still in usage.
>
> If not, then the entire movable pages concept sounds broken to me, and
> has to be fixed at mm subsystem.
>
> > >
> > > The only way to do something at a VMA level is to have a list of OK
> > > VMAs, eg because they were creatd via a special mmap helper from the
> > > media subsystem.
>
> I'm not sure if I'm following you on that. The media API can work with
> different ways of sending buffers to userspace:
>
> - read();
>
> - using the overlay mode. This interface is deprecated in
>   practice, being replaced by DMABUF. Only a few older hardware
>   supports it, and it depends on an special X11 helper driver
>   for it to work.
>
> - via DMABUF:
> 
> https://linuxtv.org/downloads/v4l-dvb-apis-new/userspace-api/v4l/dmabuf.html
>
> - via mmap, using a mmap helper:
> 
> https://linuxtv.org/downloads/v4l-dvb-apis-new/userspace-api/v4l/mmap.html
>
> - via mmap, using userspace-provided pointers:
> 
> https://linuxtv.org/downloads/v4l-dvb-apis-new/userspace-api/v4l/userp.html
>
> The existing open-source programs usually chose one or more of the above
> modes. if the Kernel driver returns -EINVAL when an mmapped streaming I/O
> mode is not supported, userspace has to select a different method.
>
> Most userspace open source programs have fallback support: if one
> mmap I/O method fails, it selects another one, although this is not
> a mandatory requirement. I found (and fixed) a few ones that were
> relying exclusively on userptr support, but I didn't make a
> comprehensive check.
>
> Also there are a number of relevant closed-source apps that we have no
> idea about what methods they use, like Skype, and other similar
> videoconferencing programs. Breaking support for those, specially at
> a time where people are relying on it in order to participate on
> conferences and doing internal meetings is a **very bad** idea.
>
> So, whatever solution is taken, it should not be dumping warning
> messages at the system and tainting the Kernel, but, instead, checking
> if the userspace request is valid or not. If it is invalid, return the
> proper error code via the right V4L2 ioctl, in order to let userspace
> choose a different method. I the request is valid, refcount the pages
> for them to not be moved while they're still in usage.
>
> -
>
> Let me provide some background about how things work at the media
> subsytem. If you want to know more, the userspace-provided memory
> mapped pointers work is described here:
>
> 
> 

[PATCH] drm/amd/display: remove no need return value

2020-10-10 Thread Bernard Zhao
Functions (disable_all_writeback_pipes_for_stream &
dc_enable_stereo & dc_post_update_surfaces_to_stream)
always return true, there is no need to keep the return value.
This change is to make the code a bit more readable.

Signed-off-by: Bernard Zhao 
---
 drivers/gpu/drm/amd/display/dc/core/dc.c   | 17 +
 drivers/gpu/drm/amd/display/dc/dc.h|  2 +-
 drivers/gpu/drm/amd/display/dc/dc_stream.h |  2 +-
 3 files changed, 7 insertions(+), 14 deletions(-)

diff --git a/drivers/gpu/drm/amd/display/dc/core/dc.c 
b/drivers/gpu/drm/amd/display/dc/core/dc.c
index 92eb1ca1634f..8dc598a632b5 100644
--- a/drivers/gpu/drm/amd/display/dc/core/dc.c
+++ b/drivers/gpu/drm/amd/display/dc/core/dc.c
@@ -761,7 +761,7 @@ static bool dc_construct(struct dc *dc,
return false;
 }
 
-static bool disable_all_writeback_pipes_for_stream(
+static void disable_all_writeback_pipes_for_stream(
const struct dc *dc,
struct dc_stream_state *stream,
struct dc_state *context)
@@ -770,8 +770,6 @@ static bool disable_all_writeback_pipes_for_stream(
 
for (i = 0; i < stream->num_wb_info; i++)
stream->writeback_info[i].wb_enabled = false;
-
-   return true;
 }
 
 void apply_ctx_interdependent_lock(struct dc *dc, struct dc_state *context, 
struct dc_stream_state *stream, bool lock)
@@ -1213,13 +1211,12 @@ bool dc_validate_seamless_boot_timing(const struct dc 
*dc,
return true;
 }
 
-bool dc_enable_stereo(
+void dc_enable_stereo(
struct dc *dc,
struct dc_state *context,
struct dc_stream_state *streams[],
uint8_t stream_count)
 {
-   bool ret = true;
int i, j;
struct pipe_ctx *pipe;
 
@@ -1234,8 +1231,6 @@ bool dc_enable_stereo(
dc->hwss.setup_stereo(pipe, dc);
}
}
-
-   return ret;
 }
 
 /*
@@ -1448,18 +1443,18 @@ static bool is_flip_pending_in_pipes(struct dc *dc, 
struct dc_state *context)
return false;
 }
 
-bool dc_post_update_surfaces_to_stream(struct dc *dc)
+void dc_post_update_surfaces_to_stream(struct dc *dc)
 {
int i;
struct dc_state *context = dc->current_state;
 
if ((!dc->optimized_required) || dc->optimize_seamless_boot_streams > 0)
-   return true;
+   return;
 
post_surface_trace(dc);
 
if (is_flip_pending_in_pipes(dc, context))
-   return true;
+   return;
 
for (i = 0; i < dc->res_pool->pipe_count; i++)
if (context->res_ctx.pipe_ctx[i].stream == NULL ||
@@ -1472,8 +1467,6 @@ bool dc_post_update_surfaces_to_stream(struct dc *dc)
 
dc->optimized_required = false;
dc->wm_optimized_required = false;
-
-   return true;
 }
 
 struct dc_state *dc_create_state(struct dc *dc)
diff --git a/drivers/gpu/drm/amd/display/dc/dc.h 
b/drivers/gpu/drm/amd/display/dc/dc.h
index f50ef4255020..f79a3c318757 100644
--- a/drivers/gpu/drm/amd/display/dc/dc.h
+++ b/drivers/gpu/drm/amd/display/dc/dc.h
@@ -962,7 +962,7 @@ struct dc_flip_addrs {
bool triplebuffer_flips;
 };
 
-bool dc_post_update_surfaces_to_stream(
+void dc_post_update_surfaces_to_stream(
struct dc *dc);
 
 #include "dc_stream.h"
diff --git a/drivers/gpu/drm/amd/display/dc/dc_stream.h 
b/drivers/gpu/drm/amd/display/dc/dc_stream.h
index d9888f316da6..0047ab33f88e 100644
--- a/drivers/gpu/drm/amd/display/dc/dc_stream.h
+++ b/drivers/gpu/drm/amd/display/dc/dc_stream.h
@@ -391,7 +391,7 @@ enum dc_status dc_validate_stream(struct dc *dc, struct 
dc_stream_state *stream)
  * Enable stereo when commit_streams is not required,
  * for example, frame alternate.
  */
-bool dc_enable_stereo(
+void dc_enable_stereo(
struct dc *dc,
struct dc_state *context,
struct dc_stream_state *streams[],
-- 
2.28.0

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


[PULL] drm-misc-next-fixes

2020-10-10 Thread Maxime Ripard
Hi Dave, Daniel,

Here's this week PR for drm-misc-next-fixes

Maxime

drm-misc-next-fixes-2020-10-09:
One MAINTAINERS change and a revert for a compilation breakage in next for
ingenic
The following changes since commit 089d83418914abd4d908db117d9a3eca7f51a68c:

  drm/vc4: hvs: Pull the state of all the CRTCs prior to PV muxing (2020-09-21 
16:43:11 +0200)

are available in the Git repository at:

  git://anongit.freedesktop.org/drm/drm-misc tags/drm-misc-next-fixes-2020-10-09

for you to fetch changes up to 6561e0aa4627da90f59076fec5e3a1b72a8aa63f:

  MAINTAINERS: Update entry for st7703 driver after the rename (2020-10-09 
08:55:00 +0200)


One MAINTAINERS change and a revert for a compilation breakage in next for
ingenic


Maxime Ripard (3):
  drm/vc4: kms: Assign a FIFO to enabled CRTCs instead of active
  drm/vc4: crtc: Rework a bit the CRTC state code
  drm/vc4: crtc: Keep the previously assigned HVS FIFO

Ondrej Jirman (1):
  MAINTAINERS: Update entry for st7703 driver after the rename

Paul Cercueil (1):
  Revert "gpu/drm: ingenic: Add option to mmap GEM buffers cached"

 MAINTAINERS   |   7 +-
 drivers/gpu/drm/ingenic/ingenic-drm-drv.c | 116 +++---
 drivers/gpu/drm/ingenic/ingenic-drm.h |   4 --
 drivers/gpu/drm/ingenic/ingenic-ipu.c |  12 +---
 drivers/gpu/drm/vc4/vc4_crtc.c|  14 +++-
 drivers/gpu/drm/vc4/vc4_drv.h |   2 +
 drivers/gpu/drm/vc4/vc4_kms.c |  22 --
 7 files changed, 46 insertions(+), 131 deletions(-)


signature.asc
Description: PGP signature
___
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel


Re: [PATCH 1/6] mm: mmap: fix fput in error path

2020-10-10 Thread Jason Gunthorpe
On Fri, Oct 09, 2020 at 05:03:37PM +0200, Christian König wrote:
> Patch "495c10cc1c0c CHROMIUM: dma-buf: restore args..."
> adds a workaround for a bug in mmap_region.
> 
> As the comment states ->mmap() callback can change
> vma->vm_file and so we might call fput() on the wrong file.
> 
> Revert the workaround and proper fix this in mmap_region.
> 
> Signed-off-by: Christian König 
>  drivers/dma-buf/dma-buf.c | 22 +-
>  mm/mmap.c |  2 +-
>  2 files changed, 6 insertions(+), 18 deletions(-)
> 
> diff --git a/drivers/dma-buf/dma-buf.c b/drivers/dma-buf/dma-buf.c
> index a6ba4d598f0e..edd57402a48a 100644
> +++ b/drivers/dma-buf/dma-buf.c
> @@ -1143,9 +1143,6 @@ EXPORT_SYMBOL_GPL(dma_buf_end_cpu_access);
>  int dma_buf_mmap(struct dma_buf *dmabuf, struct vm_area_struct *vma,
>unsigned long pgoff)
>  {
> - struct file *oldfile;
> - int ret;
> -
>   if (WARN_ON(!dmabuf || !vma))
>   return -EINVAL;
>  
> @@ -1163,22 +1160,13 @@ int dma_buf_mmap(struct dma_buf *dmabuf, struct 
> vm_area_struct *vma,
>   return -EINVAL;
>  
>   /* readjust the vma */
> - get_file(dmabuf->file);
> - oldfile = vma->vm_file;
> - vma->vm_file = dmabuf->file;
> - vma->vm_pgoff = pgoff;
> + if (vma->vm_file)
> + fput(vma->vm_file);

This if is redundant too

But otherwise

Reviewed-by: Jason Gunthorpe 

Thanks,
Jason
___
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel


Re: [PATCH 1/4] mm: introduce vma_set_file function v2

2020-10-10 Thread Jason Gunthorpe
On Fri, Oct 09, 2020 at 09:39:00AM +0200, Daniel Vetter wrote:
> I just noticed this here in the patch because everyone else does not do
> this. But looking at the mmap_region() code in mmap.c we seem to indeed
> have this problem for the error path:
> 
> unmap_and_free_vma:
>   vma->vm_file = NULL;
>   fput(file);
> 
> Note that the success path does things correctly (a bit above):
> 
>   file = vma->vm_file;
> out:
> 
> So it indeed looks like dma-buf is the only one that does this fully
> correctly. So maybe we should do a follow-up patch to change the
> mmap_region exit code to pick up whatever vma->vm_file was set instead,
> and fput that?

Given that this new vma_set_file() should be the only way to
manipulate vm_file from the mmap op, I think this reflects a bug in
mm/mmap.c.. Should be:

unmap_and_free_vma:
fput(vma->vm_file);
vma->vm_file = NULL;

Then everything works the way you'd expect without tricky error
handling

Jason
___
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel


[PATCH 2/2] drm/i915: Force DPCD backlight mode for BOE 2270 panel

2020-10-10 Thread Aaron Ma
BOE 2270 panel failed to control backlight brightness.
Add it in edid quirks to force using DPCD backlight control.
Then the brightness can be controlled.

Signed-off-by: Aaron Ma 
---
 drivers/gpu/drm/drm_dp_helper.c | 1 +
 1 file changed, 1 insertion(+)

diff --git a/drivers/gpu/drm/drm_dp_helper.c b/drivers/gpu/drm/drm_dp_helper.c
index 092c8c985911..417ed10bbf83 100644
--- a/drivers/gpu/drm/drm_dp_helper.c
+++ b/drivers/gpu/drm/drm_dp_helper.c
@@ -1324,6 +1324,7 @@ static const struct edid_quirk edid_quirk_list[] = {
{ MFG(0x4d, 0x10), PROD_ID(0xc7, 0x14), 
BIT(DP_QUIRK_FORCE_DPCD_BACKLIGHT) },
{ MFG(0x4d, 0x10), PROD_ID(0xe6, 0x14), 
BIT(DP_QUIRK_FORCE_DPCD_BACKLIGHT) },
{ MFG(0x4c, 0x83), PROD_ID(0x47, 0x41), 
BIT(DP_QUIRK_FORCE_DPCD_BACKLIGHT) },
+   { MFG(0x09, 0xe5), PROD_ID(0xde, 0x08), 
BIT(DP_QUIRK_FORCE_DPCD_BACKLIGHT) },
 };
 
 #undef MFG
-- 
2.25.1

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


Re: [PATCH v2 3/7] dt-bindings: display: mxsfb: Add a bus-width endpoint property

2020-10-10 Thread Marek Vasut
On 10/10/20 1:58 AM, Laurent Pinchart wrote:
> Hi Marek,

Hi,

> On Wed, Oct 07, 2020 at 10:40:26AM +0200, Marek Vasut wrote:
>> On 10/7/20 3:24 AM, Laurent Pinchart wrote:
>>
>> [...]
>>
>>> +  bus-width:
>>> +enum: [16, 18, 24]
>>> +description: |
>>> +  The output bus width. This value overrides the configuration
>>> +  derived from the connected device (encoder or panel). It 
>>> should
>>> +  only be specified when PCB routing of the data signals 
>>> require a
>>> +  different bus width on the LCDIF and the connected device. 
>>> For
>>> +  instance, when a 18-bit RGB panel has its R[5:0], G[5:0] and
>>> +  B[5:0] signals connected to LCD_DATA[7:2], LCD_DATA[15:10] 
>>> and
>>> +  LCD_DATA[23:18] instead of LCD_DATA[5:0], LCD_DATA[11:6] and
>>> +  LCD_DATA[17:12], bus-width should be set to 24.
>>
>> The iMX6 IPUv3 uses interface-pix-fmt which is a bit more flexible, but
>> I'm not sure whether it's the right way to go about this, see:
>> Documentation/devicetree/bindings/display/imx/fsl-imx-drm.txt
> 
> I think specifying the bus with is better. It's a standard property, but
> more than that, a given bus width can carry different formats. For
> instance, a 24-bus could carry RGB666 data (with dithering for the
> LSBs).

I think that's exactly what the interface-pix-fmt was trying to solve
for the IPUv3, there you could have e.g. both RGB666 and LVDS666 , which
were different.
___
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel


Re: [PATCH RFC PKS/PMEM 51/58] kernel: Utilize new kmap_thread()

2020-10-10 Thread Eric W. Biederman
ira.we...@intel.com writes:

> From: Ira Weiny 
>
> This kmap() call is localized to a single thread.  To avoid the over
> head of global PKRS updates use the new kmap_thread() call.

Acked-by: "Eric W. Biederman" 

>
> Cc: Eric Biederman 
> Signed-off-by: Ira Weiny 
> ---
>  kernel/kexec_core.c | 8 
>  1 file changed, 4 insertions(+), 4 deletions(-)
>
> diff --git a/kernel/kexec_core.c b/kernel/kexec_core.c
> index c19c0dad1ebe..272a9920c0d6 100644
> --- a/kernel/kexec_core.c
> +++ b/kernel/kexec_core.c
> @@ -815,7 +815,7 @@ static int kimage_load_normal_segment(struct kimage 
> *image,
>   if (result < 0)
>   goto out;
>  
> - ptr = kmap(page);
> + ptr = kmap_thread(page);
>   /* Start with a clear page */
>   clear_page(ptr);
>   ptr += maddr & ~PAGE_MASK;
> @@ -828,7 +828,7 @@ static int kimage_load_normal_segment(struct kimage 
> *image,
>   memcpy(ptr, kbuf, uchunk);
>   else
>   result = copy_from_user(ptr, buf, uchunk);
> - kunmap(page);
> + kunmap_thread(page);
>   if (result) {
>   result = -EFAULT;
>   goto out;
> @@ -879,7 +879,7 @@ static int kimage_load_crash_segment(struct kimage *image,
>   goto out;
>   }
>   arch_kexec_post_alloc_pages(page_address(page), 1, 0);
> - ptr = kmap(page);
> + ptr = kmap_thread(page);
>   ptr += maddr & ~PAGE_MASK;
>   mchunk = min_t(size_t, mbytes,
>   PAGE_SIZE - (maddr & ~PAGE_MASK));
> @@ -895,7 +895,7 @@ static int kimage_load_crash_segment(struct kimage *image,
>   else
>   result = copy_from_user(ptr, buf, uchunk);
>   kexec_flush_icache_page(page);
> - kunmap(page);
> + kunmap_thread(page);
>   arch_kexec_pre_free_pages(page_address(page), 1);
>   if (result) {
>   result = -EFAULT;
___
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel


Re: [Nouveau] nouveau broken on Riva TNT2 in 5.9.0-rc8: GPU not supported on big-endian

2020-10-10 Thread Ondrej Zary
On Saturday 10 October 2020 00:23:38 Ilia Mirkin wrote:
> On Fri, Oct 9, 2020 at 5:54 PM Karol Herbst  wrote:
> >
> > On Fri, Oct 9, 2020 at 11:35 PM Ondrej Zary  wrote:
> > >
> > > Hello,
> > > I'm testing 5.9.0-rc8 and found that Riva TNT2 stopped working:
> > > [0.00] Linux version 5.9.0-rc8+ (zary@gsql) (gcc (Debian 8.3.0-6) 
> > > 8.3.0, GNU ld (GNU Binutils for Debian) 2.31.1) #326 SMP Fri Oct 9 
> > > 22:31:40 CEST 2020
> > > ...
> > > [   14.771464] nouveau :01:00.0: GPU not supported on big-endian
> > > [   14.771782] nouveau: probe of :01:00.0 failed with error -38
> > >
> > > big-endian? WTF? The machine is x86.
> > >
> >
> > mhh, we reworked the endianess checks a bit and apparently that broke
> > something... I will give it some thoughts, but could you be so kind
> > and create an mmiotrace under 5.9 with nouveau? You won't need to
> > start X or anything while doing it. Just enable the trace and modprobe
> > nouveau and collect the trace.
> 
> Looks like nvkm_device_endianness unconditionally reads out 0x4. I
> don't think that reg is there pre-NV11. At least NV4, NV5, NV10 and
> maybe NV15 (which is logically pre-NV11) don't support big-endian
> mode. Not sure about NV1A, which was the IGP of the series and IIRC
> logically pre-NV11 as well (but clearly could only be used with x86
> chips, since it was part of the motherboard).

Yes, you're right. Forcing nvkm_device_endianness to return true allows
5.9.0-rc8 to work:
[0.00] Linux version 5.9.0-rc8+ (zary@gsql) (gcc (Debian 8.3.0-6) 
8.3.0, GNU ld (GNU Binutils for Debian) 2.31.1) #326 SMP Fri Oct 9 22:31:40 
CEST 2020
...
[   12.311258] nouveau :01:00.0: bios: DCB table not found
[   12.311583] nouveau :01:00.0: bios: DCB table not found
[   12.311834] nouveau :01:00.0: bios: DCB table not found
[   12.311847] nouveau :01:00.0: bios: DCB table not found
[   12.311989] agpgart-intel :00:00.0: AGP 3.0 bridge
[   12.312017] agpgart-intel :00:00.0: bridge is in legacy mode, falling 
back to 2.x
[   12.312031] agpgart-intel :00:00.0: putting AGP V2 device into 4x mode
[   12.312066] nouveau :01:00.0: putting AGP V2 device into 4x mode
[   12.312162] agpgart-intel :00:00.0: AGP 3.0 bridge
[   12.312182] agpgart-intel :00:00.0: bridge is in legacy mode, falling 
back to 2.x
[   12.312195] agpgart-intel :00:00.0: putting AGP V2 device into 4x mode
[   12.312230] nouveau :01:00.0: putting AGP V2 device into 4x mode
[   12.312247] nouveau :01:00.0: tmr: unknown input clock freq
[   12.318341] nouveau :01:00.0: fb: 32 MiB SDRAM
[   12.76] [TTM] Zone  kernel: Available graphics memory: 385048 KiB
[   12.92] [TTM] Initializing pool allocator
[   12.333434] nouveau :01:00.0: DRM: VRAM: 31 MiB
[   12.333443] nouveau :01:00.0: DRM: GART: 128 MiB
[   12.333453] nouveau :01:00.0: DRM: BMP version 5.6
[   12.333460] nouveau :01:00.0: DRM: No DCB data found in VBIOS
[   12.335355] nouveau :01:00.0: DRM: MM: using M2MF for buffer copies
[   12.335443] nouveau :01:00.0: bios: DCB table not found
[   12.336033] nouveau :01:00.0: DRM: Saving VGA fonts
[   12.376420] nouveau :01:00.0: DRM: No DCB data found in VBIOS
[   12.410397] nouveau :01:00.0: DRM: allocated 1280x1024 fb: 0x4000, bo 
b68d2ac4
[   12.441217] fbcon: nouveaudrmfb (fb0) is primary device
[   12.591964] Console: switching to colour frame buffer device 160x64
[   12.593876] nouveau :01:00.0: [drm] fb0: nouveaudrmfb frame buffer device
[   12.594944] [drm] Initialized nouveau 1.3.1 20120801 for :01:00.0 on 
minor 0

BTW. 5.8 kernel (that appeared today in Debian packports) is broken the same 
way.

> Aha, it's documented in rnndb:
> 
> https://github.com/envytools/envytools/blob/master/rnndb/bus/pmc.xml
> 
> 
>   -ilia
> 


-- 
Ondrej Zary
___
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel


Re: [PATCH v2 14/17] resource: Move devmem revoke code to resource framework

2020-10-10 Thread Jason Gunthorpe
On Fri, Oct 09, 2020 at 04:24:45PM +0200, Daniel Vetter wrote:
> On Fri, Oct 9, 2020 at 2:31 PM Jason Gunthorpe  wrote:
> >
> > On Fri, Oct 09, 2020 at 09:59:31AM +0200, Daniel Vetter wrote:
> >
> > > +struct address_space *iomem_get_mapping(void)
> > > +{
> > > + return iomem_inode->i_mapping;
> >
> > This should pair an acquire with the release below
> >
> > > + /*
> > > +  * Publish /dev/mem initialized.
> > > +  * Pairs with smp_load_acquire() in revoke_iomem().
> > > +  */
> > > + smp_store_release(_inode, inode);
> >
> > However, this seems abnormal, initcalls rarely do this kind of stuff
> > with global data..
> >
> > The kernel crashes if this fs_initcall is raced with
> > iomem_get_mapping() due to the unconditional dereference, so I think
> > it can be safely switched to a simple assignment.
> 
> Ah yes I checked this all, but forgot to correctly annotate the
> iomem_get_mapping access. For reference, see b34e7e298d7a ("/dev/mem:
> Add missing memory barriers for devmem_inode").

Oh yikes, so revoke_iomem can run concurrently during early boot,
tricky.

> The reasons for the annotations is that iomem requests can happen
> fairly early, way before fs_initcalls happen. That means revoke_iomem
> needs to check for that and bail out if we race - nothing bad can
> happen since userspace isn't running at this point anyway. And
> apparently it needs to be a full acquire fence since we don't just
> write a value, but need a barrier for the struct stuff.

Yes, if that is what is happening it release/acquire is needed.

Jason
___
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel


[PATCH] drm/nouveau/kms: Fix NULL pointer dereference in nouveau_connector_detect_depth

2020-10-10 Thread Alexander Kapshuk
This oops manifests itself on the following hardware:
01:00.0 VGA compatible controller: NVIDIA Corporation G98M [GeForce G 103M] 
(rev a1)

Oct 09 14:17:46 lp-sasha kernel: BUG: kernel NULL pointer dereference, address: 

Oct 09 14:17:46 lp-sasha kernel: #PF: supervisor read access in kernel mode
Oct 09 14:17:46 lp-sasha kernel: #PF: error_code(0x) - not-present page
Oct 09 14:17:46 lp-sasha kernel: PGD 0 P4D 0
Oct 09 14:17:46 lp-sasha kernel: Oops:  [#1] SMP PTI
Oct 09 14:17:46 lp-sasha kernel: CPU: 1 PID: 191 Comm: systemd-udevd Not 
tainted 5.9.0-rc8-next-20201009 #38
Oct 09 14:17:46 lp-sasha kernel: Hardware name: Hewlett-Packard Compaq Presario 
CQ61 Notebook PC/306A, BIOS F.03 03/23/2009
Oct 09 14:17:46 lp-sasha kernel: RIP: 
0010:nouveau_connector_detect_depth+0x71/0xc0 [nouveau]
Oct 09 14:17:46 lp-sasha kernel: Code: 0a 00 00 48 8b 49 48 c7 87 b8 00 00 00 
06 00 00 00 80 b9 4d 0a 00 00 00 75 1e 83 fa 41 75 05 48 85 c0 75 29 8b 81 10 
0d 00 00 <39> 06 7c 25 f6 81 14 0d 00 00 02 75 b7 c3 80 b9 0c 0d 00 00 00 75
Oct 09 14:17:46 lp-sasha kernel: RSP: 0018:c928f8c0 EFLAGS: 00010297
Oct 09 14:17:46 lp-sasha kernel: RAX: 00014c08 RBX: 8880369d4000 
RCX: 8880369d3000
Oct 09 14:17:46 lp-sasha kernel: RDX: 0040 RSI:  
RDI: 8880369d4000
Oct 09 14:17:46 lp-sasha kernel: RBP: 88800601cc00 R08: 8880051da298 
R09: 8226201a
Oct 09 14:17:46 lp-sasha kernel: R10: 88800469aa80 R11: 888004c84ff8 
R12: 
Oct 09 14:17:46 lp-sasha kernel: R13: 8880051da000 R14: 2000 
R15: 0003
Oct 09 14:17:46 lp-sasha kernel: FS:  7fd0192b3440() 
GS:8880bc90() knlGS:
Oct 09 14:17:46 lp-sasha kernel: CS:  0010 DS:  ES:  CR0: 
80050033
Oct 09 14:17:46 lp-sasha kernel: CR2:  CR3: 04976000 
CR4: 06e0
Oct 09 14:17:46 lp-sasha kernel: Call Trace:
Oct 09 14:17:46 lp-sasha kernel:  nouveau_connector_get_modes+0x1e6/0x240 
[nouveau]
Oct 09 14:17:46 lp-sasha kernel:  ? kfree+0xb9/0x240
Oct 09 14:17:46 lp-sasha kernel:  ? drm_connector_list_iter_next+0x7c/0xa0
Oct 09 14:17:46 lp-sasha kernel:  
drm_helper_probe_single_connector_modes+0x1ba/0x7c0
Oct 09 14:17:46 lp-sasha kernel:  drm_client_modeset_probe+0x27e/0x1360
Oct 09 14:17:46 lp-sasha kernel:  ? nvif_object_sclass_put+0xc/0x20 [nouveau]
Oct 09 14:17:46 lp-sasha kernel:  ? nouveau_cli_init+0x3cc/0x440 [nouveau]
Oct 09 14:17:46 lp-sasha kernel:  ? ktime_get_mono_fast_ns+0x49/0xa0
Oct 09 14:17:46 lp-sasha kernel:  ? nouveau_drm_open+0x4e/0x180 [nouveau]
Oct 09 14:17:46 lp-sasha kernel:  
__drm_fb_helper_initial_config_and_unlock+0x3f/0x4a0
Oct 09 14:17:46 lp-sasha kernel:  ? drm_file_alloc+0x18f/0x260
Oct 09 14:17:46 lp-sasha kernel:  ? mutex_lock+0x9/0x40
Oct 09 14:17:46 lp-sasha kernel:  ? drm_client_init+0x110/0x160
Oct 09 14:17:46 lp-sasha kernel:  nouveau_fbcon_init+0x14d/0x1c0 [nouveau]
Oct 09 14:17:46 lp-sasha kernel:  nouveau_drm_device_init+0x1c0/0x880 [nouveau]
Oct 09 14:17:46 lp-sasha kernel:  nouveau_drm_probe+0x11a/0x1e0 [nouveau]
Oct 09 14:17:46 lp-sasha kernel:  pci_device_probe+0xcd/0x140
Oct 09 14:17:46 lp-sasha kernel:  really_probe+0xd8/0x400
Oct 09 14:17:46 lp-sasha kernel:  driver_probe_device+0x4a/0xa0
Oct 09 14:17:46 lp-sasha kernel:  device_driver_attach+0x9c/0xc0
Oct 09 14:17:46 lp-sasha kernel:  __driver_attach+0x6f/0x100
Oct 09 14:17:46 lp-sasha kernel:  ? device_driver_attach+0xc0/0xc0
Oct 09 14:17:46 lp-sasha kernel:  bus_for_each_dev+0x75/0xc0
Oct 09 14:17:46 lp-sasha kernel:  bus_add_driver+0x106/0x1c0
Oct 09 14:17:46 lp-sasha kernel:  driver_register+0x86/0xe0
Oct 09 14:17:46 lp-sasha kernel:  ? 0xa044e000
Oct 09 14:17:46 lp-sasha kernel:  do_one_initcall+0x48/0x1e0
Oct 09 14:17:46 lp-sasha kernel:  ? _cond_resched+0x11/0x60
Oct 09 14:17:46 lp-sasha kernel:  ? kmem_cache_alloc_trace+0x19c/0x1e0
Oct 09 14:17:46 lp-sasha kernel:  do_init_module+0x57/0x220
Oct 09 14:17:46 lp-sasha kernel:  __do_sys_finit_module+0xa0/0xe0
Oct 09 14:17:46 lp-sasha kernel:  do_syscall_64+0x33/0x40
Oct 09 14:17:46 lp-sasha kernel:  entry_SYSCALL_64_after_hwframe+0x44/0xa9
Oct 09 14:17:46 lp-sasha kernel: RIP: 0033:0x7fd01a060d5d
Oct 09 14:17:46 lp-sasha kernel: Code: 00 c3 66 2e 0f 1f 84 00 00 00 00 00 90 
f3 0f 1e fa 48 89 f8 48 89 f7 48 89 d6 48 89 ca 4d 89 c2 4d 89 c8 4c 8b 4c 24 
08 0f 05 <48> 3d 01 f0 ff ff 73 01 c3 48 8b 0d e3 70 0c 00 f7 d8 64 89 01 48
Oct 09 14:17:46 lp-sasha kernel: RSP: 002b:7ffc8ad38a98 EFLAGS: 0246 
ORIG_RAX: 0139
Oct 09 14:17:46 lp-sasha kernel: RAX: ffda RBX: 563f6e7fd530 
RCX: 7fd01a060d5d
Oct 09 14:17:46 lp-sasha kernel: RDX:  RSI: 7fd01a19f95d 
RDI: 000f
Oct 09 14:17:46 lp-sasha kernel: RBP: 0002 R08:  
R09: 0007
Oct 09 14:17:46 lp-sasha kernel: R10: 000f R11: 0246 
R12: 

Re: [PATCH v2 09/17] mm: Add unsafe_follow_pfn

2020-10-10 Thread Jason Gunthorpe
On Fri, Oct 09, 2020 at 02:37:23PM +0200, Mauro Carvalho Chehab wrote:

> I'm not a mm/ expert, but, from what I understood from Daniel's patch
> description is that this is unsafe *only if*  __GFP_MOVABLE is used.

No, it is unconditionally unsafe. The CMA movable mappings are
specific VMAs that will have bad issues here, but there are other
types too.

The only way to do something at a VMA level is to have a list of OK
VMAs, eg because they were creatd via a special mmap helper from the
media subsystem.

> Well, no drivers inside the media subsystem uses such flag, although
> they may rely on some infrastructure that could be using it behind
> the bars.

It doesn't matter, nothing prevents the user from calling media APIs
on mmaps it gets from other subsystems.

> If this is the case, the proper fix seems to have a GFP_NOT_MOVABLE
> flag that it would be denying the core mm code to set __GFP_MOVABLE.

We can't tell from the VMA these kinds of details..

It has to go the other direction, evey mmap that might be used as a
userptr here has to be found and the VMA specially created to allow
its use. At least that is a kernel only change, but will need people
with the HW to do this work.

> Please let address the issue on this way, instead of broken an
> userspace API that it is there since 1991.

It has happened before :( It took 4 years for RDMA to undo the uAPI
breakage caused by a security fix for something that was a 15 years
old bug. 

Jason
___
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel


Re: [PATCH 2/6] mm: introduce vma_set_file function v3

2020-10-10 Thread Jason Gunthorpe
On Fri, Oct 09, 2020 at 05:03:38PM +0200, Christian König wrote:
> +/*
> + * Change backing file, only valid to use during initial VMA setup.
> + */
> +void vma_set_file(struct vm_area_struct *vma, struct file *file)
> +{
> + if (file)
> + get_file(file);
> +
> + swap(vma->vm_file, file);
> +
> + if (file)
> + fput(file);
> +}

fput crashes when file is NULL so the error handling after
unmap_and_free_vma: can't handle this case, similarly vm_file can't be
NULL either.

So just simply:

 swap(vma->vm_file, file);
 get_file(vma->vm_file);
 fput(file);
 
Will do?

Just let it crash if any of them are wrongly NULL.

Jason
___
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel


[PATCH] MAINTAINERS: Add myself as a maintainer for vc4

2020-10-10 Thread Maxime Ripard
Eric isn't working on vc4 anymore and I've been working on it, as well as
merging patches for it, recently so let's make it official so I don't miss
patches.

Signed-off-by: Maxime Ripard 
---
 MAINTAINERS | 1 +
 1 file changed, 1 insertion(+)

diff --git a/MAINTAINERS b/MAINTAINERS
index 91d46806e511..a248e3a2b537 100644
--- a/MAINTAINERS
+++ b/MAINTAINERS
@@ -5957,6 +5957,7 @@ F:include/uapi/drm/v3d_drm.h
 
 DRM DRIVERS FOR VC4
 M: Eric Anholt 
+M: Maxime Ripard 
 S: Supported
 T: git git://github.com/anholt/linux
 T: git git://anongit.freedesktop.org/drm/drm-misc
-- 
2.26.2

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


Re: [PATCH 1/6] mm: mmap: fix fput in error path

2020-10-10 Thread linmiaohe
Jason Gunthorpe  wrote:
> On Fri, Oct 09, 2020 at 03:04:20PM -0700, Andrew Morton wrote:
>> On Fri,  9 Oct 2020 17:03:37 +0200 "Christian König" 
>>  wrote:
>> 
>> > Patch "495c10cc1c0c CHROMIUM: dma-buf: restore args..."
>> > adds a workaround for a bug in mmap_region.
>> > 
>> > As the comment states ->mmap() callback can change
>> > vma->vm_file and so we might call fput() on the wrong file.
>> > 
>> > Revert the workaround and proper fix this in mmap_region.
>> > 
>> 
>> Doesn't this patch series address the same thing as 
>> https://lkml.kernel.org/r/20200916090733.31427-1-linmia...@huawei.com?
>
>Same basic issue, looks like both of these patches should be combined to plug 
>it fully.
>
>Jason 

I think so too. Both of these patches fix the fput at possible wrong @file due 
to ->mmap() callback can change vma->vm_file.

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


nouveau broken on Riva TNT2 in 5.9.0-rc8: GPU not supported on big-endian

2020-10-10 Thread Ondrej Zary
Hello,
I'm testing 5.9.0-rc8 and found that Riva TNT2 stopped working:
[0.00] Linux version 5.9.0-rc8+ (zary@gsql) (gcc (Debian 8.3.0-6) 
8.3.0, GNU ld (GNU Binutils for Debian) 2.31.1) #326 SMP Fri Oct 9 22:31:40 
CEST 2020
...
[   14.771464] nouveau :01:00.0: GPU not supported on big-endian
[   14.771782] nouveau: probe of :01:00.0 failed with error -38

big-endian? WTF? The machine is x86.

It works fine with Debian 5.7 kernel (5.7.10-1~bpo10+1):
[0.00] Linux version 5.7.0-0.bpo.2-686 (debian-ker...@lists.debian.org) 
(gcc version 8.3.0 (Debian 8.3.0-6), GNU ld (GNU Binutils for Debian) 2.31.1) 
#1 SMP Debian 5.7.10-1~bpo10+1 (2020-07-30)
...
[   23.266196] nouveau :01:00.0: NVIDIA NV05 (20154000)
[   23.288582] nouveau :01:00.0: bios: version 02.05.20.02.00
[   23.288869] nouveau :01:00.0: bios: DCB table not found
[   23.289595] nouveau :01:00.0: bios: DCB table not found
[   23.289956] nouveau :01:00.0: bios: DCB table not found
[   23.290015] nouveau :01:00.0: bios: DCB table not found
[   23.290215] agpgart-intel :00:00.0: AGP 3.0 bridge
[   23.290287] agpgart-intel :00:00.0: bridge is in legacy mode, falling 
back to 2.x
[   23.290351] agpgart-intel :00:00.0: putting AGP V2 device into 4x mode
[   23.290430] nouveau :01:00.0: putting AGP V2 device into 4x mode
[   23.290565] agpgart-intel :00:00.0: AGP 3.0 bridge
[   23.290627] agpgart-intel :00:00.0: bridge is in legacy mode, falling 
back to 2.x
[   23.290690] agpgart-intel :00:00.0: putting AGP V2 device into 4x mode
[   23.290768] nouveau :01:00.0: putting AGP V2 device into 4x mode
[   23.290830] nouveau :01:00.0: tmr: unknown input clock freq
[   23.293026] nouveau :01:00.0: fb: 32 MiB SDRAM
[   23.301269] [TTM] Zone  kernel: Available graphics memory: 382728 KiB
[   23.301327] [TTM] Initializing pool allocator
[   23.301414] nouveau :01:00.0: DRM: VRAM: 31 MiB
[   23.301465] nouveau :01:00.0: DRM: GART: 128 MiB
[   23.301518] nouveau :01:00.0: DRM: BMP version 5.6
[   23.301570] nouveau :01:00.0: DRM: No DCB data found in VBIOS
[   23.303594] nouveau :01:00.0: DRM: MM: using M2MF for buffer copies
[   23.303719] nouveau :01:00.0: bios: DCB table not found
[   23.304904] nouveau :01:00.0: DRM: Saving VGA fonts
[   23.349089] nouveau :01:00.0: DRM: No DCB data found in VBIOS
[   23.349681] [drm] Supports vblank timestamp caching Rev 2 (21.10.2013).
[   23.383066] nouveau :01:00.0: DRM: allocated 1280x1024 fb: 0x4000, bo 
b10d2f17
[   23.413903] fbcon: nouveaudrmfb (fb0) is primary device
[   23.569851] Console: switching to colour frame buffer device 160x64
[   23.571050] nouveau :01:00.0: fb0: nouveaudrmfb frame buffer device


-- 
Ondrej Zary
___
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel


[PATCH] drm/msm/disp: add error value record in for circle`s error index

2020-10-10 Thread Bernard Zhao
In function dpu_core_irq_enable & dpu_core_irq_disable, when
some index enable or disable failed, return value will be
covered by next success index. Upper call function will not
catch this error, this maybe does not meet the expectation.
This change is to make the code a bit more readable.

Signed-off-by: Bernard Zhao 
---
 drivers/gpu/drm/msm/disp/dpu1/dpu_core_irq.c | 8 
 1 file changed, 4 insertions(+), 4 deletions(-)

diff --git a/drivers/gpu/drm/msm/disp/dpu1/dpu_core_irq.c 
b/drivers/gpu/drm/msm/disp/dpu1/dpu_core_irq.c
index f1bc6a1af7a7..e6da0469b743 100644
--- a/drivers/gpu/drm/msm/disp/dpu1/dpu_core_irq.c
+++ b/drivers/gpu/drm/msm/disp/dpu1/dpu_core_irq.c
@@ -123,8 +123,8 @@ int dpu_core_irq_enable(struct dpu_kms *dpu_kms, int 
*irq_idxs, u32 irq_count)
DRM_ERROR("irq_idx=%d enable_count=%d\n", irq_idxs[0], counts);
 
for (i = 0; (i < irq_count) && !ret; i++)
-   ret = _dpu_core_irq_enable(dpu_kms, irq_idxs[i]);
-
+   if (_dpu_core_irq_enable(dpu_kms, irq_idxs[i]) != 0)
+   ret = -EINVAL;
return ret;
 }
 
@@ -178,8 +178,8 @@ int dpu_core_irq_disable(struct dpu_kms *dpu_kms, int 
*irq_idxs, u32 irq_count)
DRM_ERROR("irq_idx=%d enable_count=%d\n", irq_idxs[0], counts);
 
for (i = 0; (i < irq_count) && !ret; i++)
-   ret = _dpu_core_irq_disable(dpu_kms, irq_idxs[i]);
-
+   if (_dpu_core_irq_disable(dpu_kms, irq_idxs[i]) != 0)
+   ret = -EINVAL;
return ret;
 }
 
-- 
2.28.0

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


[tip: core/rcu] drm/i915: Cleanup PREEMPT_COUNT leftovers

2020-10-10 Thread tip-bot2 for Thomas Gleixner
The following commit has been merged into the core/rcu branch of tip:

Commit-ID: 5d35c1c982ffaaccd1ec974e96e7a5244bbadaa1
Gitweb:
https://git.kernel.org/tip/5d35c1c982ffaaccd1ec974e96e7a5244bbadaa1
Author:Thomas Gleixner 
AuthorDate:Mon, 14 Sep 2020 19:35:03 +02:00
Committer: Paul E. McKenney 
CommitterDate: Thu, 01 Oct 2020 09:02:13 -07:00

drm/i915: Cleanup PREEMPT_COUNT leftovers

CONFIG_PREEMPT_COUNT is now unconditionally enabled and will be
removed. Cleanup the leftovers before doing so.

Signed-off-by: Thomas Gleixner 
Reviewed-by: Joonas Lahtinen 
Cc: Jani Nikula 
Cc: Rodrigo Vivi 
Cc: David Airlie 
Cc: Daniel Vetter 
Cc: intel-...@lists.freedesktop.org
Cc: dri-devel@lists.freedesktop.org
Signed-off-by: Paul E. McKenney 
---
 drivers/gpu/drm/i915/Kconfig.debug | 1 -
 drivers/gpu/drm/i915/i915_utils.h  | 3 +--
 2 files changed, 1 insertion(+), 3 deletions(-)

diff --git a/drivers/gpu/drm/i915/Kconfig.debug 
b/drivers/gpu/drm/i915/Kconfig.debug
index 1cb28c2..17d9b00 100644
--- a/drivers/gpu/drm/i915/Kconfig.debug
+++ b/drivers/gpu/drm/i915/Kconfig.debug
@@ -20,7 +20,6 @@ config DRM_I915_DEBUG
bool "Enable additional driver debugging"
depends on DRM_I915
select DEBUG_FS
-   select PREEMPT_COUNT
select I2C_CHARDEV
select STACKDEPOT
select DRM_DP_AUX_CHARDEV
diff --git a/drivers/gpu/drm/i915/i915_utils.h 
b/drivers/gpu/drm/i915/i915_utils.h
index 5477337..ecfed86 100644
--- a/drivers/gpu/drm/i915/i915_utils.h
+++ b/drivers/gpu/drm/i915/i915_utils.h
@@ -337,8 +337,7 @@ wait_remaining_ms_from_jiffies(unsigned long 
timestamp_jiffies, int to_wait_ms)
   (Wmax))
 #define wait_for(COND, MS) _wait_for((COND), (MS) * 1000, 10, 1000)
 
-/* If CONFIG_PREEMPT_COUNT is disabled, in_atomic() always reports false. */
-#if defined(CONFIG_DRM_I915_DEBUG) && defined(CONFIG_PREEMPT_COUNT)
+#ifdef CONFIG_DRM_I915_DEBUG
 # define _WAIT_FOR_ATOMIC_CHECK(ATOMIC) WARN_ON_ONCE((ATOMIC) && !in_atomic())
 #else
 # define _WAIT_FOR_ATOMIC_CHECK(ATOMIC) do { } while (0)
___
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel


Re: [PATCH v2 09/17] mm: Add unsafe_follow_pfn

2020-10-10 Thread Jason Gunthorpe
On Fri, Oct 09, 2020 at 12:34:21PM +0200, Mauro Carvalho Chehab wrote:
> Hi,
> 
> Em Fri,  9 Oct 2020 09:59:26 +0200
> Daniel Vetter  escreveu:
> 
> > Way back it was a reasonable assumptions that iomem mappings never
> > change the pfn range they point at. But this has changed:
> > 
> > - gpu drivers dynamically manage their memory nowadays, invalidating
> > ptes with unmap_mapping_range when buffers get moved
> > 
> > - contiguous dma allocations have moved from dedicated carvetouts to
> > cma regions. This means if we miss the unmap the pfn might contain
> > pagecache or anon memory (well anything allocated with GFP_MOVEABLE)
> > 
> > - even /dev/mem now invalidates mappings when the kernel requests that
> > iomem region when CONFIG_IO_STRICT_DEVMEM is set, see 3234ac664a87
> > ("/dev/mem: Revoke mappings when a driver claims the region")
> > 
> > Accessing pfns obtained from ptes without holding all the locks is
> > therefore no longer a good idea.
> > 
> > Unfortunately there's some users where this is not fixable (like v4l
> > userptr of iomem mappings) or involves a pile of work (vfio type1
> > iommu). For now annotate these as unsafe and splat appropriately.
> > 
> > This patch adds an unsafe_follow_pfn, which later patches will then
> > roll out to all appropriate places.
> 
> NACK, as this breaks an existing userspace API on media.

It doesn't break it. You get a big warning the thing is broken and it
keeps working.

We can't leave such a huge security hole open - it impacts other
subsystems, distros need to be able to run in a secure mode.

> While I agree that using the userptr on media is something that
> new drivers may not support, as DMABUF is a better way of
> handling it, changing this for existing ones is a big no, 
> as it may break usersapace.

media community needs to work to fix this, not pretend it is OK to
keep going as-is.

Dealing with security issues is the one case where an uABI break might
be acceptable.

If you want to NAK it then you need to come up with the work to do
something here correctly that will support the old drivers without the
kernel taint.

Unfortunately making things uncomfortable for the subsystem is the big
hammer the core kernel needs to use to actually get this security work
done by those responsible.

Jason
___
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel


Re: [PATCH RFC PKS/PMEM 48/58] drivers/md: Utilize new kmap_thread()

2020-10-10 Thread Coly Li
On 2020/10/10 03:50, ira.we...@intel.com wrote:
> From: Ira Weiny 
> 
> These kmap() calls are localized to a single thread.  To avoid the over
> head of global PKRS updates use the new kmap_thread() call.
> 

Hi Ira,

There were a number of options considered.

1) Attempt to change all the thread local kmap() calls to kmap_atomic()
2) Introduce a flags parameter to kmap() to indicate if the mapping
should be global or not
3) Change ~20-30 call sites to 'kmap_global()' to indicate that they
require a global mapping of the pages
4) Change ~209 call sites to 'kmap_thread()' to indicate that the
mapping is to be used within that thread of execution only


I copied the above information from patch 00/58 to this message. The
idea behind kmap_thread() is fine to me, but as you said the new api is
very easy to be missed in new code (even for me). I would like to be
supportive to option 2) introduce a flag to kmap(), then we won't forget
the new thread-localized kmap method, and people won't ask why a
_thread() function is called but no kthread created.

Thanks.


Coly Li



> Cc: Coly Li  (maintainer:BCACHE (BLOCK LAYER CACHE))
> Cc: Kent Overstreet  (maintainer:BCACHE (BLOCK 
> LAYER CACHE))
> Signed-off-by: Ira Weiny 
> ---
>  drivers/md/bcache/request.c | 4 ++--
>  1 file changed, 2 insertions(+), 2 deletions(-)
> 
> diff --git a/drivers/md/bcache/request.c b/drivers/md/bcache/request.c
> index c7cadaafa947..a4571f6d09dd 100644
> --- a/drivers/md/bcache/request.c
> +++ b/drivers/md/bcache/request.c
> @@ -44,10 +44,10 @@ static void bio_csum(struct bio *bio, struct bkey *k)
>   uint64_t csum = 0;
>  
>   bio_for_each_segment(bv, bio, iter) {
> - void *d = kmap(bv.bv_page) + bv.bv_offset;
> + void *d = kmap_thread(bv.bv_page) + bv.bv_offset;
>  
>   csum = bch_crc64_update(csum, d, bv.bv_len);
> - kunmap(bv.bv_page);
> + kunmap_thread(bv.bv_page);
>   }
>  
>   k->ptr[KEY_PTRS(k)] = csum & (~0ULL >> 1);
> 

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


Re: [PATCH v2 14/17] resource: Move devmem revoke code to resource framework

2020-10-10 Thread Jason Gunthorpe
On Fri, Oct 09, 2020 at 09:59:31AM +0200, Daniel Vetter wrote:

> +struct address_space *iomem_get_mapping(void)
> +{
> + return iomem_inode->i_mapping;

This should pair an acquire with the release below

> + /*
> +  * Publish /dev/mem initialized.
> +  * Pairs with smp_load_acquire() in revoke_iomem().
> +  */
> + smp_store_release(_inode, inode);

However, this seems abnormal, initcalls rarely do this kind of stuff
with global data..

The kernel crashes if this fs_initcall is raced with
iomem_get_mapping() due to the unconditional dereference, so I think
it can be safely switched to a simple assignment.

Jason
___
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel


Re: [PATCH v2 09/17] mm: Add unsafe_follow_pfn

2020-10-10 Thread Jason Gunthorpe
On Fri, Oct 09, 2020 at 07:52:05PM +0200, Daniel Vetter wrote:

> > > If this is the case, the proper fix seems to have a GFP_NOT_MOVABLE
> > > flag that it would be denying the core mm code to set __GFP_MOVABLE.
> >
> > We can't tell from the VMA these kinds of details..
> >
> > It has to go the other direction, evey mmap that might be used as a
> > userptr here has to be found and the VMA specially created to allow
> > its use. At least that is a kernel only change, but will need people
> > with the HW to do this work.
> 
> I think the only reasonable way to keep this working is:
> - add a struct dma_buf *vma_tryget_dma_buf(struct vm_area_struct *vma);
> - add dma-buf export support to fbdev and v4l
> - roll this out everywhere we still need it.

It seems to me there is a technical way forward to restore user
compat, so it is really no different than RDMA/DRM pain we both
suffered before.

Thus no justification to NAK it. If media wants things to keep working
they have to do the technical path like you outline above.

> Realistically this just isn't going to happen. 

If your series goes ahead it will get solved. Someone will take on the
huge project to either add DMA buf to the drivers people still care
about, or do the work above to transparently handle in kernel.

If we allow things to keep working without consequence then nobody
will do it.

The only reason we did the 4 years of work in RDMA was because Linus
went in and broke the uABI for a security fix. It was hundreds of
patches to fix it, so I don't have much sympathy for "it is too hard"
here.

Jason
___
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel


Re: [PATCH RFC PKS/PMEM 22/58] fs/f2fs: Utilize new kmap_thread()

2020-10-10 Thread Matthew Wilcox
On Fri, Oct 09, 2020 at 02:34:34PM -0700, Eric Biggers wrote:
> On Fri, Oct 09, 2020 at 12:49:57PM -0700, ira.we...@intel.com wrote:
> > The kmap() calls in this FS are localized to a single thread.  To avoid
> > the over head of global PKRS updates use the new kmap_thread() call.
> >
> > @@ -2410,12 +2410,12 @@ static inline struct page *f2fs_pagecache_get_page(
> >  
> >  static inline void f2fs_copy_page(struct page *src, struct page *dst)
> >  {
> > -   char *src_kaddr = kmap(src);
> > -   char *dst_kaddr = kmap(dst);
> > +   char *src_kaddr = kmap_thread(src);
> > +   char *dst_kaddr = kmap_thread(dst);
> >  
> > memcpy(dst_kaddr, src_kaddr, PAGE_SIZE);
> > -   kunmap(dst);
> > -   kunmap(src);
> > +   kunmap_thread(dst);
> > +   kunmap_thread(src);
> >  }
> 
> Wouldn't it make more sense to switch cases like this to kmap_atomic()?
> The pages are only mapped to do a memcpy(), then they're immediately unmapped.

Maybe you missed the earlier thread from Thomas trying to do something
similar for rather different reasons ...

https://lore.kernel.org/lkml/20200919091751.06...@linutronix.de/
___
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel


Re: [PATCH 1/6] mm: mmap: fix fput in error path

2020-10-10 Thread Jason Gunthorpe
On Fri, Oct 09, 2020 at 03:04:20PM -0700, Andrew Morton wrote:
> On Fri,  9 Oct 2020 17:03:37 +0200 "Christian König" 
>  wrote:
> 
> > Patch "495c10cc1c0c CHROMIUM: dma-buf: restore args..."
> > adds a workaround for a bug in mmap_region.
> > 
> > As the comment states ->mmap() callback can change
> > vma->vm_file and so we might call fput() on the wrong file.
> > 
> > Revert the workaround and proper fix this in mmap_region.
> > 
> 
> Doesn't this patch series address the same thing as
> https://lkml.kernel.org/r/20200916090733.31427-1-linmia...@huawei.com?

Same basic issue, looks like both of these patches should be combined
to plug it fully.

Jason 
___
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel


[PATCH 1/2] drm/i915/dpcd_bl: uncheck PWM_PIN_CAP when detect eDP backlight capabilities

2020-10-10 Thread Aaron Ma
BOE panel with ID 2270 claims both PWM_PIN_CAP and AUX_SET_CAP backlight
control bits, but default chip backlight failed to control brightness.

Check AUX_SET_CAP and proceed to check quirks or VBT backlight type.
DPCD can control the brightness of this pannel.

Signed-off-by: Aaron Ma 
---
 drivers/gpu/drm/i915/display/intel_dp_aux_backlight.c | 3 +--
 1 file changed, 1 insertion(+), 2 deletions(-)

diff --git a/drivers/gpu/drm/i915/display/intel_dp_aux_backlight.c 
b/drivers/gpu/drm/i915/display/intel_dp_aux_backlight.c
index acbd7eb66cbe..308b14159b7c 100644
--- a/drivers/gpu/drm/i915/display/intel_dp_aux_backlight.c
+++ b/drivers/gpu/drm/i915/display/intel_dp_aux_backlight.c
@@ -334,8 +334,7 @@ intel_dp_aux_display_control_capable(struct intel_connector 
*connector)
 * the panel can support backlight control over the aux channel
 */
if (intel_dp->edp_dpcd[1] & DP_EDP_TCON_BACKLIGHT_ADJUSTMENT_CAP &&
-   (intel_dp->edp_dpcd[2] & DP_EDP_BACKLIGHT_BRIGHTNESS_AUX_SET_CAP) &&
-   !(intel_dp->edp_dpcd[2] & DP_EDP_BACKLIGHT_BRIGHTNESS_PWM_PIN_CAP)) 
{
+   (intel_dp->edp_dpcd[2] & DP_EDP_BACKLIGHT_BRIGHTNESS_AUX_SET_CAP)) {
drm_dbg_kms(>drm, "AUX Backlight Control Supported!\n");
return true;
}
-- 
2.25.1

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


Re: [PATCH v2 10/17] media/videbuf1|2: Mark follow_pfn usage as unsafe

2020-10-10 Thread Mauro Carvalho Chehab
Em Fri,  9 Oct 2020 09:59:27 +0200
Daniel Vetter  escreveu:

> The media model assumes that buffers are all preallocated, so that
> when a media pipeline is running we never miss a deadline because the
> buffers aren't allocated or available.
> 
> This means we cannot fix the v4l follow_pfn usage through
> mmu_notifier, without breaking how this all works. The only real fix
> is to deprecate userptr support for VM_IO | VM_PFNMAP mappings and
> tell everyone to cut over to dma-buf memory sharing for zerocopy.
> 
> userptr for normal memory will keep working as-is.

I won't repeat here the discussions for patch 09/17, but
just to be clear about this one:

NACK.

We need a better alternative to avoid breaking existing
media applications.

> 
> Signed-off-by: Daniel Vetter 
> Cc: Jason Gunthorpe 
> Cc: Kees Cook 
> Cc: Dan Williams 
> Cc: Andrew Morton 
> Cc: John Hubbard 
> Cc: Jérôme Glisse 
> Cc: Jan Kara 
> Cc: Dan Williams 
> Cc: linux...@kvack.org
> Cc: linux-arm-ker...@lists.infradead.org
> Cc: linux-samsung-...@vger.kernel.org
> Cc: linux-me...@vger.kernel.org
> Cc: Pawel Osciak 
> Cc: Marek Szyprowski 
> Cc: Kyungmin Park 
> Cc: Tomasz Figa 
> Cc: Laurent Dufour 
> Cc: Vlastimil Babka 
> Cc: Daniel Jordan 
> Cc: Michel Lespinasse 
> ---
>  drivers/media/common/videobuf2/frame_vector.c | 2 +-
>  drivers/media/v4l2-core/videobuf-dma-contig.c | 2 +-
>  2 files changed, 2 insertions(+), 2 deletions(-)
> 
> diff --git a/drivers/media/common/videobuf2/frame_vector.c 
> b/drivers/media/common/videobuf2/frame_vector.c
> index 2b0b97761d15..a1b85fe9e7c1 100644
> --- a/drivers/media/common/videobuf2/frame_vector.c
> +++ b/drivers/media/common/videobuf2/frame_vector.c
> @@ -69,7 +69,7 @@ int get_vaddr_frames(unsigned long start, unsigned int 
> nr_frames,
>   break;
>  
>   while (ret < nr_frames && start + PAGE_SIZE <= vma->vm_end) {
> - err = follow_pfn(vma, start, [ret]);
> + err = unsafe_follow_pfn(vma, start, [ret]);
>   if (err) {
>   if (ret == 0)
>   ret = err;
> diff --git a/drivers/media/v4l2-core/videobuf-dma-contig.c 
> b/drivers/media/v4l2-core/videobuf-dma-contig.c
> index 52312ce2ba05..821c4a76ab96 100644
> --- a/drivers/media/v4l2-core/videobuf-dma-contig.c
> +++ b/drivers/media/v4l2-core/videobuf-dma-contig.c
> @@ -183,7 +183,7 @@ static int videobuf_dma_contig_user_get(struct 
> videobuf_dma_contig_memory *mem,
>   user_address = untagged_baddr;
>  
>   while (pages_done < (mem->size >> PAGE_SHIFT)) {
> - ret = follow_pfn(vma, user_address, _pfn);
> + ret = unsafe_follow_pfn(vma, user_address, _pfn);
>   if (ret)
>   break;
>  



Thanks,
Mauro
___
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel


Re: [PATCH v2 09/17] mm: Add unsafe_follow_pfn

2020-10-10 Thread Mauro Carvalho Chehab
Em Fri, 9 Oct 2020 19:52:05 +0200
Daniel Vetter  escreveu:

> On Fri, Oct 9, 2020 at 2:48 PM Jason Gunthorpe  wrote:
> >
> > On Fri, Oct 09, 2020 at 02:37:23PM +0200, Mauro Carvalho Chehab wrote:
> >  
> > > I'm not a mm/ expert, but, from what I understood from Daniel's patch
> > > description is that this is unsafe *only if*  __GFP_MOVABLE is used.  
> >
> > No, it is unconditionally unsafe. The CMA movable mappings are
> > specific VMAs that will have bad issues here, but there are other
> > types too.

I didn't check the mm dirty details, but I strongly suspect that the mm
code has a way to prevent moving a mmapped page while it is still in usage.

If not, then the entire movable pages concept sounds broken to me, and
has to be fixed at mm subsystem.

> >
> > The only way to do something at a VMA level is to have a list of OK
> > VMAs, eg because they were creatd via a special mmap helper from the
> > media subsystem.

I'm not sure if I'm following you on that. The media API can work with
different ways of sending buffers to userspace:

- read();

- using the overlay mode. This interface is deprecated in
  practice, being replaced by DMABUF. Only a few older hardware
  supports it, and it depends on an special X11 helper driver
  for it to work.

- via DMABUF:

https://linuxtv.org/downloads/v4l-dvb-apis-new/userspace-api/v4l/dmabuf.html

- via mmap, using a mmap helper:

https://linuxtv.org/downloads/v4l-dvb-apis-new/userspace-api/v4l/mmap.html

- via mmap, using userspace-provided pointers:

https://linuxtv.org/downloads/v4l-dvb-apis-new/userspace-api/v4l/userp.html

The existing open-source programs usually chose one or more of the above
modes. if the Kernel driver returns -EINVAL when an mmapped streaming I/O
mode is not supported, userspace has to select a different method.

Most userspace open source programs have fallback support: if one
mmap I/O method fails, it selects another one, although this is not
a mandatory requirement. I found (and fixed) a few ones that were
relying exclusively on userptr support, but I didn't make a 
comprehensive check.

Also there are a number of relevant closed-source apps that we have no 
idea about what methods they use, like Skype, and other similar
videoconferencing programs. Breaking support for those, specially at
a time where people are relying on it in order to participate on
conferences and doing internal meetings is a **very bad** idea.

So, whatever solution is taken, it should not be dumping warning
messages at the system and tainting the Kernel, but, instead, checking
if the userspace request is valid or not. If it is invalid, return the
proper error code via the right V4L2 ioctl, in order to let userspace
choose a different method. I the request is valid, refcount the pages 
for them to not be moved while they're still in usage.

-

Let me provide some background about how things work at the media
subsytem. If you want to know more, the userspace-provided memory 
mapped pointers work is described here:


https://linuxtv.org/downloads/v4l-dvb-apis-new/userspace-api/v4l/userp.html#userp

Basically, userspace calls either one of those ioctls:

VIDIOC_CREATE_BUFS:

https://linuxtv.org/downloads/v4l-dvb-apis-new/userspace-api/v4l/vidioc-create-bufs.html

Which is translated into a videobuf2 call to: vb2_ioctl_create_bufs()

VIDIOC_REQBUFS

https://linuxtv.org/downloads/v4l-dvb-apis-new/userspace-api/v4l/vidioc-reqbufs.html#vidioc-reqbufs

Which is translated into a videobuf2 call to: vb2_ioctl_reqbufs()

Both internally calls vb2_verify_memory_type(), which is responsible
for checking if the provided pointers are OK for the usage and/or do
all necessary page allocations, and taking care of any special 
requirements. This could easily have some additional checks to
verify if the requested VMA address has pages that are movable or
not, ensuring that ensure that the VMA is OK, and locking them in
order to prevent the mm code to move such pages while they are in
usage by the media subsystem.

Now, as I said before, I don't know the dirty details about how
to lock those pages at the mm subsystem in order to avoid it
to move the used pages. Yet, when vb2_create_framevec()
is called, the check if VMA is OK should already be happened
at vb2_verify_memory_type().

-

It should be noticed that the dirty hack added by patch 09/17
and 10/17 affects *all* types of memory allocations at V4L2, 
as this kAPI is called by the 3 different memory models
supported at the media subsystem:

drivers/media/common/videobuf2/videobuf2-vmalloc.c
drivers/media/common/videobuf2/videobuf2-dma-contig.c
drivers/media/common/videobuf2/videobuf2-dma-sg.c

In other words, with this code:

int unsafe_follow_pfn(struct vm_area_struct *vma, unsigned long address,
unsigned long 

Re: [PATCH v8 3/4] drm/kmb: Mipi DSI part of the display driver

2020-10-10 Thread Daniel Vetter
On Fri, Oct 02, 2020 at 07:17:01PM -0700, Anitha Chrisanthus wrote:
> Initializes Mipi DSI and sets up connects to ADV bridge
> 
> v2: removed license text
> upclassed dev_private, removed HAVE_IRQ. (Sam)
> 
> v3: Squashed all 59 commits to one
> 
> v4: review changes from Sam Ravnborg
>   renamed dev_p to kmb
> 
> v5: corrected spellings
> v6: corrected checkpatch warnings
> v7: review changes Sam Ravnborg and Thomas Zimmerman
>   removed unnecessary logs and defines and ifdef codes (Sam)
>   split dphy_init_sequence smaller (Sam)
>   removed redundant checks in kmb_dsi (Sam)
>   changed kmb_dsi_init to drm_bridge_connector_init and
>   drm_connector_attach_encoder to bridge's connector (Sam)
> v8: call drm_bridge_attach with DRM_BRIDGE_ATTACH_NO_CONNECTOR
> 
> Cc: Sam Ravnborg 
> Signed-off-by: Anitha Chrisanthus 
> Reviewed-by: Bob Paauwe 
> ---
>  drivers/gpu/drm/kmb/kmb_dsi.c | 1524 
> +
>  drivers/gpu/drm/kmb/kmb_dsi.h |  350 ++
>  2 files changed, 1874 insertions(+)
>  create mode 100644 drivers/gpu/drm/kmb/kmb_dsi.c
>  create mode 100644 drivers/gpu/drm/kmb/kmb_dsi.h
> 
> diff --git a/drivers/gpu/drm/kmb/kmb_dsi.c b/drivers/gpu/drm/kmb/kmb_dsi.c
> new file mode 100644
> index 000..849f1cc
> --- /dev/null
> +++ b/drivers/gpu/drm/kmb/kmb_dsi.c
> @@ -0,0 +1,1524 @@
> +// SPDX-License-Identifier: GPL-2.0-only
> +/*
> + * Copyright © 2019-2020 Intel Corporation
> + */
> +
> +#include 
> +#include 
> +#include 
> +#include 
> +
> +#include 
> +#include 
> +#include 
> +#include 
> +#include 
> +#include 
> +#include 
> +#include 
> +#include 
> +#include 
> +
> +#include "kmb_drv.h"
> +#include "kmb_dsi.h"
> +#include "kmb_regs.h"
> +
> +static struct mipi_dsi_host *dsi_host;
> +static struct mipi_dsi_device *dsi_device;
> +
> +/* Default setting is 1080p, 4 lanes */
> +#define IMG_HEIGHT_LINES  1080
> +#define IMG_WIDTH_PX  1920
> +#define MIPI_TX_ACTIVE_LANES 4
> +
> +struct mipi_tx_frame_section_cfg mipi_tx_frame0_sect_cfg = {
> + .width_pixels = IMG_WIDTH_PX,
> + .height_lines = IMG_HEIGHT_LINES,
> + .data_type = DSI_LP_DT_PPS_RGB888_24B,
> + .data_mode = MIPI_DATA_MODE1,
> + .dma_packed = 0
> +};
> +
> +struct mipi_tx_frame_cfg mipitx_frame0_cfg = {
> + .sections[0] = _tx_frame0_sect_cfg,
> + .sections[1] = NULL,
> + .sections[2] = NULL,
> + .sections[3] = NULL,
> + .vsync_width = 5,
> + .v_backporch = 36,
> + .v_frontporch = 4,
> + .hsync_width = 44,
> + .h_backporch = 148,
> + .h_frontporch = 88
> +};
> +
> +struct mipi_tx_dsi_cfg mipitx_dsi_cfg = {
> + .hfp_blank_en = 0,
> + .eotp_en = 0,
> + .lpm_last_vfp_line = 0,
> + .lpm_first_vsa_line = 0,
> + .sync_pulse_eventn = DSI_VIDEO_MODE_NO_BURST_EVENT,
> + .hfp_blanking = SEND_BLANK_PACKET,
> + .hbp_blanking = SEND_BLANK_PACKET,
> + .hsa_blanking = SEND_BLANK_PACKET,
> + .v_blanking = SEND_BLANK_PACKET,
> +};
> +
> +struct mipi_ctrl_cfg mipi_tx_init_cfg = {
> + .active_lanes = MIPI_TX_ACTIVE_LANES,
> + .lane_rate_mbps = MIPI_TX_LANE_DATA_RATE_MBPS,
> + .ref_clk_khz = MIPI_TX_REF_CLK_KHZ,
> + .cfg_clk_khz = MIPI_TX_CFG_CLK_KHZ,
> + .tx_ctrl_cfg = {
> + .frames[0] = _frame0_cfg,
> + .frames[1] = NULL,
> + .frames[2] = NULL,
> + .frames[3] = NULL,
> + .tx_dsi_cfg = _dsi_cfg,
> + .line_sync_pkt_en = 0,
> + .line_counter_active = 0,
> + .frame_counter_active = 0,
> + .tx_always_use_hact = 1,
> + .tx_hact_wait_stop = 1,
> + }
> +};
> +
> +struct  mipi_hs_freq_range_cfg {
> + u16 default_bit_rate_mbps;
> + u8 hsfreqrange_code;
> +};
> +
> +struct vco_params {
> + u32 freq;
> + u32 range;
> + u32 divider;
> +};
> +
> +static struct vco_params vco_table[] = {
> + {52, 0x3f, 8},
> + {80, 0x39, 8},
> + {105, 0x2f, 4},
> + {160, 0x29, 4},
> + {210, 0x1f, 2},
> + {320, 0x19, 2},
> + {420, 0x0f, 1},
> + {630, 0x09, 1},
> + {1100, 0x03, 1},
> + {0x, 0x01, 1},
> +};
> +
> +static struct mipi_hs_freq_range_cfg
> +mipi_hs_freq_range[MIPI_DPHY_DEFAULT_BIT_RATES] = {
> + {.default_bit_rate_mbps = 80, .hsfreqrange_code = 0x00},
> + {.default_bit_rate_mbps = 90, .hsfreqrange_code = 0x10},
> + {.default_bit_rate_mbps = 100, .hsfreqrange_code = 0x20},
> + {.default_bit_rate_mbps = 110, .hsfreqrange_code = 0x30},
> + {.default_bit_rate_mbps = 120, .hsfreqrange_code = 0x01},
> + {.default_bit_rate_mbps = 130, .hsfreqrange_code = 0x11},
> + {.default_bit_rate_mbps = 140, .hsfreqrange_code = 0x21},
> + {.default_bit_rate_mbps = 150, .hsfreqrange_code = 0x31},
> + {.default_bit_rate_mbps = 160, .hsfreqrange_code = 0x02},
> + {.default_bit_rate_mbps = 170, 

Re: [PATCH v1 1/4] drm: Add Keem Bay VPU codec DRM

2020-10-10 Thread Ezequiel Garcia
Hello everyone,

(Adding some Hantro developers)

On Fri, 9 Oct 2020 at 19:15, Daniel Vetter  wrote:
>
> On Fri, Oct 09, 2020 at 07:57:52PM +0800, kuhanh.murugasen.krish...@intel.com 
> wrote:
> > From: "Murugasen Krishnan, Kuhanh" 
> >
> > This is a new DRM media codec driver for Intel's Keem Bay SOC which
> > integrates the Verisilicon's Hantro Video Processor Unit (VPU) IP.
> > The SoC couples an ARM Cortex A53 CPU with an Intel Movidius VPU.
> >
> > Hantro VPU IP is a series of video decoder and encoder semiconductor IP 
> > cores,
> > which can be flexibly configured for video surveillance, multimedia consumer
> > products, Internet of Things, cloud service products, data centers, aerial
> > photography and recorders, thereby providing video transcoding and 
> > multi-channel
> > HD video encoding and decoding.
> >
> > Hantro VPU IP consists of Hantro VC8000D for decoder and Hantro VC8000E for 
> > encoder.
> >

Before you guys even start reviewing or discussing this: good news everyone!
Verisilicon Hantro VPU support is in mainline since a few releases now.

How about you run a quick "git grep hantro -- drivers/" and see for
yourself :-) ?

Spoiler alert: we currently support G1 core, supporting MPEG-2, H.264, VP8
and some post-processor features.

We are working on G2 for HEVC and VP9, and we have patches ready
for VC8000D for H264.

Given the VPU is stateless, it requires quite a bit of work on the
application side.
There are implementations in GStreamer (see v4l2codecs plugin), Chromium,
and Ffmpeg.

Given all the stateless codec drivers depend on the stateless controls APIs,
and given this API is still marked as experimental/unstable, the drivers
are in staging. Other than this, these drivers are just as good as any,
and have been shipped for quite some time now.

I expect to move them out of staging soon, just as soon as we clean and
stabilize this control API.

I will be happy to review patches adding Keem Bay support,
to be honest, unsure what that implies, but we'll see.

Thanks,
Ezequiel
___
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel


[PATCH v4 2/2] drm/mediatek: dsi: fix scrolling of panel with small hfp or hbp

2020-10-10 Thread Jitao Shi
Replace horizontal_backporch_byte with vm->hback_porch * bpp to aovid
flowing judgement negative number.

if ((vm->hfront_porch * dsi_tmp_buf_bpp + horizontal_backporch_byte) >
data_phy_cycles * dsi->lanes + delta)

Signed-off-by: Jitao Shi 
---
 drivers/gpu/drm/mediatek/mtk_dsi.c | 54 ++
 1 file changed, 19 insertions(+), 35 deletions(-)

diff --git a/drivers/gpu/drm/mediatek/mtk_dsi.c 
b/drivers/gpu/drm/mediatek/mtk_dsi.c
index 16fd99dcdacf..f69ebeaf 100644
--- a/drivers/gpu/drm/mediatek/mtk_dsi.c
+++ b/drivers/gpu/drm/mediatek/mtk_dsi.c
@@ -445,6 +445,7 @@ static void mtk_dsi_config_vdo_timing(struct mtk_dsi *dsi)
u32 horizontal_backporch_byte;
u32 horizontal_frontporch_byte;
u32 dsi_tmp_buf_bpp, data_phy_cycles;
+   u32 delta;
struct mtk_phy_timing *timing = >phy_timing;
 
struct videomode *vm = >vm;
@@ -475,42 +476,25 @@ static void mtk_dsi_config_vdo_timing(struct mtk_dsi *dsi)
data_phy_cycles = timing->lpx + timing->da_hs_prepare +
  timing->da_hs_zero + timing->da_hs_exit + 3;
 
-   if (dsi->mode_flags & MIPI_DSI_MODE_VIDEO_BURST) {
-   if ((vm->hfront_porch + vm->hback_porch) * dsi_tmp_buf_bpp >
-   data_phy_cycles * dsi->lanes + 18) {
-   horizontal_frontporch_byte =
-   vm->hfront_porch * dsi_tmp_buf_bpp -
-   (data_phy_cycles * dsi->lanes + 18) *
-   vm->hfront_porch /
-   (vm->hfront_porch + vm->hback_porch);
-
-   horizontal_backporch_byte =
-   horizontal_backporch_byte -
-   (data_phy_cycles * dsi->lanes + 18) *
-   vm->hback_porch /
-   (vm->hfront_porch + vm->hback_porch);
-   } else {
-   DRM_WARN("HFP less than d-phy, FPS will under 60Hz\n");
-   horizontal_frontporch_byte = vm->hfront_porch *
-dsi_tmp_buf_bpp;
-   }
+   delta = (dsi->mode_flags & MIPI_DSI_MODE_VIDEO_BURST) ? 18 : 12;
+
+   if ((vm->hfront_porch * dsi_tmp_buf_bpp + horizontal_backporch_byte) >
+   data_phy_cycles * dsi->lanes + delta) {
+   horizontal_frontporch_byte =
+   vm->hfront_porch * dsi_tmp_buf_bpp -
+   (data_phy_cycles * dsi->lanes + delta) *
+   vm->hfront_porch /
+   (vm->hfront_porch + vm->hback_porch);
+
+   horizontal_backporch_byte =
+   horizontal_backporch_byte -
+   (data_phy_cycles * dsi->lanes + delta) *
+   vm->hback_porch /
+   (vm->hfront_porch + vm->hback_porch);
} else {
-   if ((vm->hfront_porch + vm->hback_porch) * dsi_tmp_buf_bpp >
-   data_phy_cycles * dsi->lanes + 12) {
-   horizontal_frontporch_byte =
-   vm->hfront_porch * dsi_tmp_buf_bpp -
-   (data_phy_cycles * dsi->lanes + 12) *
-   vm->hfront_porch /
-   (vm->hfront_porch + vm->hback_porch);
-   horizontal_backporch_byte = horizontal_backporch_byte -
-   (data_phy_cycles * dsi->lanes + 12) *
-   vm->hback_porch /
-   (vm->hfront_porch + vm->hback_porch);
-   } else {
-   DRM_WARN("HFP less than d-phy, FPS will under 60Hz\n");
-   horizontal_frontporch_byte = vm->hfront_porch *
-dsi_tmp_buf_bpp;
-   }
+   DRM_WARN("HFP + HBP less than d-phy, FPS will under 60Hz\n");
+   horizontal_frontporch_byte = vm->hfront_porch *
+dsi_tmp_buf_bpp;
}
 
writel(horizontal_sync_active_byte, dsi->regs + DSI_HSA_WC);
-- 
2.12.5
___
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel


[PATCH v4 1/2] Revert "drm/mediatek: dsi: Fix scrolling of panel with small hfp or hbp"

2020-10-10 Thread Jitao Shi
This reverts commit 35bf948f1edbf507f6e57e0879fa6ea36d2d2930.

Signed-off-by: Jitao Shi 
---
 drivers/gpu/drm/mediatek/mtk_dsi.c | 9 +
 1 file changed, 5 insertions(+), 4 deletions(-)

diff --git a/drivers/gpu/drm/mediatek/mtk_dsi.c 
b/drivers/gpu/drm/mediatek/mtk_dsi.c
index 80b7a082e874..16fd99dcdacf 100644
--- a/drivers/gpu/drm/mediatek/mtk_dsi.c
+++ b/drivers/gpu/drm/mediatek/mtk_dsi.c
@@ -466,13 +466,14 @@ static void mtk_dsi_config_vdo_timing(struct mtk_dsi *dsi)
horizontal_sync_active_byte = (vm->hsync_len * dsi_tmp_buf_bpp - 10);
 
if (dsi->mode_flags & MIPI_DSI_MODE_VIDEO_SYNC_PULSE)
-   horizontal_backporch_byte = vm->hback_porch * dsi_tmp_buf_bpp;
+   horizontal_backporch_byte =
+   (vm->hback_porch * dsi_tmp_buf_bpp - 10);
else
-   horizontal_backporch_byte = (vm->hback_porch + vm->hsync_len) *
-   dsi_tmp_buf_bpp;
+   horizontal_backporch_byte = ((vm->hback_porch + vm->hsync_len) *
+   dsi_tmp_buf_bpp - 10);
 
data_phy_cycles = timing->lpx + timing->da_hs_prepare +
- timing->da_hs_zero + timing->da_hs_exit;
+ timing->da_hs_zero + timing->da_hs_exit + 3;
 
if (dsi->mode_flags & MIPI_DSI_MODE_VIDEO_BURST) {
if ((vm->hfront_porch + vm->hback_porch) * dsi_tmp_buf_bpp >
-- 
2.12.5
___
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel


[v4 PATCH 0/2] fix scrolling of panel with small hfp or hbp

2020-10-10 Thread Jitao Shi
Changes since v3:
 - Revert v2, for v2 will cause some bridge ic no output. the cause
   the video linetime doesn't match display mode from get mode.
 - Make sure the horizontal_frontporch_byte and horizontal_backporch_byte
   are > 0.

Jitao Shi (2):
  Revert "drm/mediatek: dsi: Fix scrolling of panel with small hfp or
hbp"
  drm/mediatek: dsi: fix scrolling of panel with small hfp or hbp

 drivers/gpu/drm/mediatek/mtk_dsi.c | 65 +++---
 1 file changed, 25 insertions(+), 40 deletions(-)

-- 
2.12.5
___
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel