date:20210209


Hi

Am 09.02.21 um 11:54 schrieb Daniel Vetter:

*: vmwgfx is the only non-gem driver, but there's plans to move at least
vmwgfx internals (maybe not the uapi, we'll see) over to gem. Once that's
done it's truly all gpu memory.


Do you have a URL to the discussion?

While I recent worked on GEM, I thought that vmwgfx could easily switch 
to the GEM internals without adopting the interface.


Personally, I think we should consider renaming struct drm_gem_object et 
al. It's not strictly GEM UAPI, but nowadays anything memory-related. 
Maybe drm_mem_object would fit.


Best regards
Thomas


---
  drivers/gpu/drm/drm_gem.c | 89 +++
  include/drm/drm_gem.h | 17 
  2 files changed, 106 insertions(+)

diff --git a/drivers/gpu/drm/drm_gem.c b/drivers/gpu/drm/drm_gem.c
index c2ce78c4edc3..a12da41eaafe 100644
--- a/drivers/gpu/drm/drm_gem.c
+++ b/drivers/gpu/drm/drm_gem.c
@@ -29,6 +29,7 @@
  #include 
  #include 
  #include 
+#include 
  #include 
  #include 
  #include 
@@ -112,6 +113,89 @@ drm_gem_init(struct drm_device *dev)
return drmm_add_action(dev, drm_gem_init_release, NULL);
  }
  
+/**

+ * drm_gem_object_set_cgroup - associate GEM object with a cgroup
+ * @obj: GEM object which is being associated with a cgroup
+ * @task: task associated with process control group to use
+ *
+ * This will acquire a reference on cgroup and use for charging GEM
+ * memory allocations.
+ * This helper could be extended in future to migrate charges to another
+ * cgroup, print warning if this usage occurs.
+ */
+void drm_gem_object_set_cgroup(struct drm_gem_object *obj,
+  struct task_struct *task)
+{
+   /* if object has existing cgroup, we migrate the charge... */
+   if (obj->drmcg) {
+   pr_warn("DRM: need to migrate cgroup charge of %lld\n",
+   atomic64_read(&obj->drmcg_bytes_charged));
+   }
+   obj->drmcg = drmcg_get(task);
+}
+EXPORT_SYMBOL(drm_gem_object_set_cgroup);
+
+/**
+ * drm_gem_object_unset_cgroup - clear GEM object's associated cgroup
+ * @obj: GEM object
+ *
+ * This will release a reference on cgroup.
+ */
+void drm_gem_object_unset_cgroup(struct drm_gem_object *obj)
+{
+   WARN_ON(atomic64_read(&obj->drmcg_bytes_charged));
+   drmcg_put(obj->drmcg);
+}
+EXPORT_SYMBOL(drm_gem_object_unset_cgroup);
+
+/**
+ * drm_gem_object_charge_mem - try charging size bytes to DRM cgroup
+ * @obj: GEM object which is being charged
+ * @size: number of bytes to charge
+ *
+ * Try to charge @size bytes to GEM object's associated DRM cgroup.  This
+ * will fail if a successful charge would cause the current device memory
+ * usage to go above the cgroup's GPU memory maximum limit.
+ *
+ * Returns 0 on success.  Otherwise, an error code is returned.
+ */
+int drm_gem_object_charge_mem(struct drm_gem_object *obj, u64 size)
+{
+   int ret;
+
+   ret = drm_cgroup_try_charge(obj->drmcg, obj->dev,
+   DRMCG_TYPE_MEM_CURRENT, size);
+   if (!ret)
+   atomic64_add(size, &obj->drmcg_bytes_charged);
+   return ret;
+}
+EXPORT_SYMBOL(drm_gem_object_charge_mem);
+
+/**
+ * drm_gem_object_uncharge_mem - uncharge size bytes from DRM cgroup
+ * @obj: GEM object which is being uncharged
+ * @size: number of bytes to uncharge
+ *
+ * Uncharge @size bytes from the DRM cgroup associated with specified
+ * GEM object.
+ *
+ * Returns 0 on success.  Otherwise, an error code is returned.
+ */
+void drm_gem_object_uncharge_mem(struct drm_gem_object *obj, u64 size)
+{
+   u64 charged = atomic64_read(&obj->drmcg_bytes_charged);
+
+   if (WARN_ON(!charged))
+   return;
+   if (WARN_ON(size > charged))
+   size = charged;
+
+   atomic64_sub(size, &obj->drmcg_bytes_charged);
+   drm_cgroup_uncharge(obj->drmcg, obj->dev, DRMCG_TYPE_MEM_CURRENT,
+   size);
+}
+EXPORT_SYMBOL(drm_gem_object_uncharge_mem);
+
  /**
   * drm_gem_object_init - initialize an allocated shmem-backed GEM object
   * @dev: drm_device the object should be initialized for
@@ -156,6 +240,8 @@ void drm_gem_private_object_init(struct drm_device *dev,
obj->dev = dev;
obj->filp = NULL;
  
+	drm_gem_object_set_cgroup(obj, current);

+
kref_init(&obj->refcount);
obj->handle_count = 0;
obj->size = size;
@@ -950,6 +1036,9 @@ drm_gem_object_release(struct drm_gem_object *obj)
  
  	dma_resv_fini(&obj->_resv);

drm_gem_free_mmap_offset(obj);
+
+   /* Release reference on cgroup used with GEM object charging */
+   drm_gem_object_unset_cgroup(obj);
  }
  EXPORT_SYMBOL(drm_gem_object_release);
  
diff --git a/include/drm/drm_gem.h b/include/drm/drm_gem.h

index 240049566592..06ea10fc17bc 100644
--- a/include/drm/drm_gem.h
+++ b/include/drm/drm_gem.h
@@ -37,6 +37,7 @@
  #include 
  #include 
  
+#include 

  #include 
  
  struct dma_buf_map;

@@ -222,6 +223,17 @@ struct drm

[PATCH -next] drm/rockchip: cdn-dp: Mark cdn_dp_resume as __maybe_unused

2021-02-09 Thread Wei Yongjun

The function cdn_dp_resume() may have no callers depending
on configuration, so it must be marked __maybe_unused to
avoid harmless warning:

drivers/gpu/drm/rockchip/cdn-dp-core.c:1124:12: warning:
 'cdn_dp_resume' defined but not used [-Wunused-function]
 1124 | static int cdn_dp_resume(struct device *dev)
  |^

Reported-by: Hulk Robot 
Signed-off-by: Wei Yongjun 
---
 drivers/gpu/drm/rockchip/cdn-dp-core.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/gpu/drm/rockchip/cdn-dp-core.c 
b/drivers/gpu/drm/rockchip/cdn-dp-core.c
index a4a45daf93f2..1162e321aaed 100644
--- a/drivers/gpu/drm/rockchip/cdn-dp-core.c
+++ b/drivers/gpu/drm/rockchip/cdn-dp-core.c
@@ -1121,7 +1121,7 @@ static int cdn_dp_suspend(struct device *dev)
return ret;
 }
 
-static int cdn_dp_resume(struct device *dev)
+static int __maybe_unused cdn_dp_resume(struct device *dev)
 {
struct cdn_dp_device *dp = dev_get_drvdata(dev);
 

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

[nouveau] WARNING: possible circular locking dependency detected in linux-next

2021-02-09 Thread Alexander Kapshuk

I've been seeing these warnings for a couple of weeks now. Any
pointers on how to address this would be much appreciated.

[   57.207457] ==
[   57.207470] WARNING: possible circular locking dependency detected
[   57.207483] 5.11.0-rc7-next-20210209 #142 Tainted: GW
[   57.207497] --
[   57.207508] Xorg/459 is trying to acquire lock:
[   57.207521] 888016edc518 (&cli->mutex){+.+.}-{3:3}, at:
nouveau_bo_move+0x4bf/0x2ec0 [nouveau]

[faddr2line]
nouveau_bo_move+0x4bf/0x2ec0:
nouveau_bo_move_m2mf at
/home/sasha/linux-next/drivers/gpu/drm/nouveau/nouveau_bo.c:804
(inlined by) nouveau_bo_move at
/home/sasha/linux-next/drivers/gpu/drm/nouveau/nouveau_bo.c:1024

/home/sasha/linux-next/drivers/gpu/drm/nouveau/nouveau_bo.c:800,804
if (drm_drv_uses_atomic_modeset(drm->dev))
mutex_lock(&cli->mutex);
else
mutex_lock_nested(&cli->mutex, SINGLE_DEPTH_NESTING);
ret = nouveau_fence_sync(nouveau_bo(bo), chan, true, ctx->interruptible);

[   57.207923]
   but task is already holding lock:
[   57.207934] 88801f49e9a0
(reservation_ww_class_mutex){+.+.}-{3:3}, at:
nouveau_bo_pin+0xc1/0xb60 [nouveau]

[faddr2line]
nouveau_bo_pin+0xc1/0xb60:
ttm_bo_reserve at /home/sasha/linux-next/./include/drm/ttm/ttm_bo_driver.h:152
(inlined by) nouveau_bo_pin at
/home/sasha/linux-next/drivers/gpu/drm/nouveau/nouveau_bo.c:424

/home/sasha/linux-next/include/drm/ttm/ttm_bo_driver.h:148,154
if (interruptible)
ret = dma_resv_lock_interruptible(bo->base.resv, ticket);
else
ret = dma_resv_lock(bo->base.resv, ticket);
if (ret == -EINTR)
return -ERESTARTSYS;
return ret;

[   57.208317]
   which lock already depends on the new lock.

[   57.208329]
   the existing dependency chain (in reverse order) is:
[   57.208340]
   -> #1 (reservation_ww_class_mutex){+.+.}-{3:3}:
[   57.208373]__ww_mutex_lock.constprop.0+0x18a/0x2d40
[   57.208395]nouveau_bo_pin+0xc1/0xb60 [nouveau]
[   57.208753]nouveau_channel_prep+0x2c6/0xba0 [nouveau]
[   57.209105]nouveau_channel_new+0x127/0x2020 [nouveau]
[   57.209457]nouveau_abi16_ioctl_channel_alloc+0x33b/0xdf0 [nouveau]
[   57.209809]drm_ioctl_kernel+0x1cb/0x260
[   57.209826]drm_ioctl+0x420/0x850
[   57.209841]nouveau_drm_ioctl+0xdf/0x210 [nouveau]
[   57.210198]__x64_sys_ioctl+0x122/0x190
[   57.210214]do_syscall_64+0x33/0x40
[   57.210230]entry_SYSCALL_64_after_hwframe+0x44/0xa9
[   57.210247]
   -> #0 (&cli->mutex){+.+.}-{3:3}:
[   57.210280]__lock_acquire+0x2a01/0x5ab0
[   57.210298]lock_acquire+0x1a9/0x690
[   57.210314]__mutex_lock+0x125/0x1140
[   57.210329]nouveau_bo_move+0x4bf/0x2ec0 [nouveau]
[   57.210686]ttm_bo_handle_move_mem+0x1b6/0x570 [ttm]
[   57.210719]ttm_bo_validate+0x316/0x420 [ttm]
[   57.210750]nouveau_bo_pin+0x3c4/0xb60 [nouveau]
[   57.211107]nv50_wndw_prepare_fb+0x117/0xcb0 [nouveau]
[   57.211460]drm_atomic_helper_prepare_planes+0x1ec/0x600
[   57.211477]nv50_disp_atomic_commit+0x189/0x530 [nouveau]
[   57.211833]drm_atomic_helper_update_plane+0x2ac/0x380
[   57.211849]drm_mode_cursor_universal+0x3f3/0xb40
[   57.211865]drm_mode_cursor_common+0x27b/0x930
[   57.211880]drm_mode_cursor_ioctl+0x95/0xd0
[   57.211895]drm_ioctl_kernel+0x1cb/0x260
[   57.211910]drm_ioctl+0x420/0x850
[   57.211925]nouveau_drm_ioctl+0xdf/0x210 [nouveau]
[   57.212281]__x64_sys_ioctl+0x122/0x190
[   57.212297]do_syscall_64+0x33/0x40
[   57.212312]entry_SYSCALL_64_after_hwframe+0x44/0xa9
[   57.212328]
   other info that might help us debug this:

[   57.212339]  Possible unsafe locking scenario:

[   57.212350]CPU0CPU1
[   57.212360]
[   57.212370]   lock(reservation_ww_class_mutex);
[   57.212390]lock(&cli->mutex);
[   57.212410]lock(reservation_ww_class_mutex);
[   57.212430]   lock(&cli->mutex);
[   57.212449]
*** DEADLOCK ***

[   57.212460] 3 locks held by Xorg/459:
[   57.212473]  #0: c944fb38
(crtc_ww_class_acquire){+.+.}-{0:0}, at:
drm_mode_cursor_common+0x1fd/0x930
[   57.212520]  #1: 88800d9f2098
(crtc_ww_class_mutex){+.+.}-{3:3}, at: modeset_lock+0xdb/0x4c0
[   57.212564]  #2: 88801f49e9a0
(reservation_ww_class_mutex){+.+.}-{3:3}, at:
nouveau_bo_pin+0xc1/0xb60 [nouveau]
[   57.212949]

Re: [PATCH 0/3] drm/ttm: constify static vm_operations_structs


Reviewed-by: Christian König  for the series.

Am 10.02.21 um 00:48 schrieb Rikard Falkeborn:

Constify a few static vm_operations_struct that are never modified. Their
only usage is to assign their address to the vm_ops field in the
vm_area_struct, which is a pointer to const vm_operations_struct. Make them
const to allow the compiler to put them in read-only memory.

With this series applied, all static struct vm_operations_struct in the
kernel tree are const.

Rikard Falkeborn (3):
   drm/amdgpu/ttm: constify static vm_operations_struct
   drm/radeon/ttm: constify static vm_operations_struct
   drm/nouveau/ttm: constify static vm_operations_struct

  drivers/gpu/drm/amd/amdgpu/amdgpu_ttm.c | 2 +-
  drivers/gpu/drm/nouveau/nouveau_ttm.c   | 2 +-
  drivers/gpu/drm/radeon/radeon_ttm.c | 2 +-
  3 files changed, 3 insertions(+), 3 deletions(-)



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

RE: [RFC v3 2/3] virtio: Introduce Vdmabuf driver

2021-02-09 Thread Kasireddy, Vivek

Hi Gerd,

> -Original Message-
> From: Gerd Hoffmann 
> Sent: Tuesday, February 09, 2021 12:45 AM
> To: Kasireddy, Vivek 
> Cc: Daniel Vetter ; 
> virtualizat...@lists.linux-foundation.org; dri-
> de...@lists.freedesktop.org; Vetter, Daniel ;
> daniel.vet...@ffwll.ch; Kim, Dongwon ;
> sumit.sem...@linaro.org; christian.koe...@amd.com; linux-me...@vger.kernel.org
> Subject: Re: [RFC v3 2/3] virtio: Introduce Vdmabuf driver
> 
>   Hi,
> 
> > > > > Nack, this doesn't work on dma-buf. And it'll blow up at runtime
> > > > > when you enable the very recently merged CONFIG_DMABUF_DEBUG (would
> > > > > be good to test with that, just to make sure).
> > [Kasireddy, Vivek] Although, I have not tested it yet but it looks like 
> > this will
> > throw a wrench in our solution as we use sg_next to iterate over all the 
> > struct page *
> > and get their PFNs. I wonder if there is any other clean way to get the 
> > PFNs of all
> > the pages associated with a dmabuf.
> 
> Well, there is no guarantee that dma-buf backing storage actually has
> struct page ...
[Kasireddy, Vivek] What if I do mmap() on the fd followed by mlock() or mmap()
followed by get_user_pages()? If it still fails, would ioremapping the device 
memory
and poking at the backing storage be an option? Or, if I bind the passthrough'd 
GPU device
to vfio-pci and tap into the memory region associated with the device memory, 
can it be
made to work? 

And, I noticed that for PFNs that do not have valid struct page associated with 
it, KVM
does a memremap() to access/map them. Is this an option?

> 
> > [Kasireddy, Vivek] To exclude such cases, would it not be OK to limit the 
> > scope
> > of this solution (Vdmabuf) to make it clear that the dma-buf has to live in 
> > Guest RAM?
> > Or, are there any ways to pin the dma-buf pages in Guest RAM to make this
> > solution work?
> 
> At that point it becomes (i915) driver-specific.  If you go that route
> it doesn't look that useful to use dma-bufs in the first place ...
[Kasireddy, Vivek] I prefer not to make this driver specific if possible.

> 
> > IIUC, Virtio GPU is used to present a virtual GPU to the Guest and all the 
> > rendering
> > commands are captured and forwarded to the Host GPU via Virtio.
> 
> You don't have to use the rendering pipeline.  You can let the i915 gpu
> render into a dma-buf shared with virtio-gpu, then use virtio-gpu only for
> buffer sharing with the host.
[Kasireddy, Vivek] Is this the most viable path forward? I am not sure how 
complex or 
feasible it would be but I'll look into it.
Also, not using the rendering capabilities of virtio-gpu and turning it into a 
sharing only
device means there would be a giant mode switch with a lot of if() conditions 
sprinkled
across. Are you OK with that?

Thanks,
Vivek
> 
> take care,
>   Gerd

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

Re: [PATCH 1/9] mm/migrate.c: Always allow device private pages to migrate

2021-02-09 Thread Alistair Popple

On Wednesday, 10 February 2021 12:39:32 AM AEDT Jason Gunthorpe wrote:
> On Tue, Feb 09, 2021 at 12:07:14PM +1100, Alistair Popple wrote:
> > Device private pages are used to represent device memory that is not
> > directly accessible from the CPU. Extra references to a device private
> > page are only used to ensure the struct page itself remains valid whilst
> > waiting for migration entries. Therefore extra references should not
> > prevent device private page migration as this can lead to failures to
> > migrate pages back to the CPU which are fatal to the user process.
> 
> This should identify the extra references in expected_count, just
> disabling this protection seems unsafe, ZONE_DEVICE is not so special
> that the refcount means nothing

This is similar to what migarte_vma_check_page() does now. The issue is that a 
migration wait takes a reference on the device private page so you can end up 
with one thread stuck waiting for migration whilst the other can't migrate due 
to the extra refcount.

Given device private pages can't undergo GUP and that it's not possible to 
differentiate the migration wait refcount from any other refcount we assume 
any possible extra reference must be from migration wait.

> Is this a side effect of the extra refcounts that Ralph was trying to
> get rid of? I'd rather see that work finished :)

I'd like to see that finished too but I don't think it would help here as this 
is not a side effect of that.

 - Alistair

> Jason

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

linux-next: build failure after merge of the drm-misc tree

2021-02-09 Thread Stephen Rothwell

Hi all,

After merging the drm-misc tree, today's linux-next build (x86_64
allmodconfig) failed like this:

drivers/gpu/drm/v3d/v3d_sched.c:263:1: error: return type is an incomplete type
  263 | v3d_gpu_reset_for_timeout(struct v3d_dev *v3d, struct drm_sched_job 
*sched_job)
  | ^
drivers/gpu/drm/v3d/v3d_sched.c: In function 'v3d_gpu_reset_for_timeout':
drivers/gpu/drm/v3d/v3d_sched.c:289:9: error: 'return' with a value, in 
function returning void [-Werror=return-type]
  289 |  return DRM_GPU_SCHED_STAT_NOMINAL;
  | ^~
drivers/gpu/drm/v3d/v3d_sched.c:263:1: note: declared here
  263 | v3d_gpu_reset_for_timeout(struct v3d_dev *v3d, struct drm_sched_job 
*sched_job)
  | ^
drivers/gpu/drm/v3d/v3d_sched.c: At top level:
drivers/gpu/drm/v3d/v3d_sched.c:298:1: error: return type is an incomplete type
  298 | v3d_cl_job_timedout(struct drm_sched_job *sched_job, enum v3d_queue q,
  | ^~~
drivers/gpu/drm/v3d/v3d_sched.c: In function 'v3d_cl_job_timedout':
drivers/gpu/drm/v3d/v3d_sched.c:309:10: error: 'return' with a value, in 
function returning void [-Werror=return-type]
  309 |   return DRM_GPU_SCHED_STAT_NOMINAL;
  |  ^~
drivers/gpu/drm/v3d/v3d_sched.c:298:1: note: declared here
  298 | v3d_cl_job_timedout(struct drm_sched_job *sched_job, enum v3d_queue q,
  | ^~~
drivers/gpu/drm/v3d/v3d_sched.c: At top level:
drivers/gpu/drm/v3d/v3d_sched.c:316:1: error: return type is an incomplete type
  316 | v3d_bin_job_timedout(struct drm_sched_job *sched_job)
  | ^~~~
drivers/gpu/drm/v3d/v3d_sched.c:325:1: error: return type is an incomplete type
  325 | v3d_render_job_timedout(struct drm_sched_job *sched_job)
  | ^~~
drivers/gpu/drm/v3d/v3d_sched.c:334:1: error: return type is an incomplete type
  334 | v3d_generic_job_timedout(struct drm_sched_job *sched_job)
  | ^~~~
drivers/gpu/drm/v3d/v3d_sched.c:342:1: error: return type is an incomplete type
  342 | v3d_csd_job_timedout(struct drm_sched_job *sched_job)
  | ^~~~
drivers/gpu/drm/v3d/v3d_sched.c: In function 'v3d_csd_job_timedout':
drivers/gpu/drm/v3d/v3d_sched.c:353:10: error: 'return' with a value, in 
function returning void [-Werror=return-type]
  353 |   return DRM_GPU_SCHED_STAT_NOMINAL;
  |  ^~
drivers/gpu/drm/v3d/v3d_sched.c:342:1: note: declared here
  342 | v3d_csd_job_timedout(struct drm_sched_job *sched_job)
  | ^~~~
drivers/gpu/drm/v3d/v3d_sched.c: At top level:
drivers/gpu/drm/v3d/v3d_sched.c:362:18: error: initialization of 'enum 
drm_gpu_sched_stat (*)(struct drm_sched_job *)' from incompatible pointer type 
'void (*)(struct drm_sched_job *)' [-Werror=incompatible-pointer-types]
  362 |  .timedout_job = v3d_bin_job_timedout,
  |  ^~~~
drivers/gpu/drm/v3d/v3d_sched.c:362:18: note: (near initialization for 
'v3d_bin_sched_ops.timedout_job')
drivers/gpu/drm/v3d/v3d_sched.c:369:18: error: initialization of 'enum 
drm_gpu_sched_stat (*)(struct drm_sched_job *)' from incompatible pointer type 
'void (*)(struct drm_sched_job *)' [-Werror=incompatible-pointer-types]
  369 |  .timedout_job = v3d_render_job_timedout,
  |  ^~~
drivers/gpu/drm/v3d/v3d_sched.c:369:18: note: (near initialization for 
'v3d_render_sched_ops.timedout_job')
drivers/gpu/drm/v3d/v3d_sched.c:376:18: error: initialization of 'enum 
drm_gpu_sched_stat (*)(struct drm_sched_job *)' from incompatible pointer type 
'void (*)(struct drm_sched_job *)' [-Werror=incompatible-pointer-types]
  376 |  .timedout_job = v3d_generic_job_timedout,
  |  ^~~~
drivers/gpu/drm/v3d/v3d_sched.c:376:18: note: (near initialization for 
'v3d_tfu_sched_ops.timedout_job')
drivers/gpu/drm/v3d/v3d_sched.c:383:18: error: initialization of 'enum 
drm_gpu_sched_stat (*)(struct drm_sched_job *)' from incompatible pointer type 
'void (*)(struct drm_sched_job *)' [-Werror=incompatible-pointer-types]
  383 |  .timedout_job = v3d_csd_job_timedout,
  |  ^~~~
drivers/gpu/drm/v3d/v3d_sched.c:383:18: note: (near initialization for 
'v3d_csd_sched_ops.timedout_job')
drivers/gpu/drm/v3d/v3d_sched.c:390:18: error: initialization of 'enum 
drm_gpu_sched_stat (*)(struct drm_sched_job *)' from incompatible pointer type 
'void (*)(struct drm_sched_job *)' [-Werror=incompatible-pointer-types]
  390 |  .timedout_job = v3d_generic_job_timedout,
  |  ^~~~
drivers/gpu/drm/v3d/v3d_sched.c:390:18: note: (near initialization for 
'v3d_c

Re: [PATCH] drm/msm: fix a6xx_gmu_clear_oob

2021-02-09 Thread Jordan Crouse

On Mon, Feb 08, 2021 at 01:55:54PM -0500, Jonathan Marek wrote:
> The cleanup patch broke a6xx_gmu_clear_oob, fix it by adding the missing
> bitshift operation.
> 
> Fixes: 555c50a4a19b ("drm/msm: Clean up GMU OOB set/clear handling")
> Signed-off-by: Jonathan Marek 

Thanks.  I feel silly that I missed that.

Reviewed-by: Jordan Crouse 

> ---
>  drivers/gpu/drm/msm/adreno/a6xx_gmu.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/drivers/gpu/drm/msm/adreno/a6xx_gmu.c 
> b/drivers/gpu/drm/msm/adreno/a6xx_gmu.c
> index 71c917f909af..91cf46f84025 100644
> --- a/drivers/gpu/drm/msm/adreno/a6xx_gmu.c
> +++ b/drivers/gpu/drm/msm/adreno/a6xx_gmu.c
> @@ -339,7 +339,7 @@ void a6xx_gmu_clear_oob(struct a6xx_gmu *gmu, enum 
> a6xx_gmu_oob_state state)
>   else
>   bit = a6xx_gmu_oob_bits[state].ack_new;
>  
> - gmu_write(gmu, REG_A6XX_GMU_HOST2GMU_INTR_SET, bit);
> + gmu_write(gmu, REG_A6XX_GMU_HOST2GMU_INTR_SET, 1 << bit);
>  }
>  
>  /* Enable CPU control of SPTP power power collapse */
> -- 
> 2.26.1
> 

-- 
The Qualcomm Innovation Center, Inc. is a member of Code Aurora Forum,
a Linux Foundation Collaborative Project
___
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

[PATCH v2] drm/msm: a6xx: Make sure the SQE microcode is safe

2021-02-09 Thread Jordan Crouse

Most a6xx targets have security issues that were fixed with new versions
of the microcode(s). Make sure that we are booting with a safe version of
the microcode for the target and print a message and error if not.

v2: Add more informative error messages and fix typos

Signed-off-by: Jordan Crouse 
---

 drivers/gpu/drm/msm/adreno/a6xx_gpu.c | 77 ++-
 1 file changed, 64 insertions(+), 13 deletions(-)

diff --git a/drivers/gpu/drm/msm/adreno/a6xx_gpu.c 
b/drivers/gpu/drm/msm/adreno/a6xx_gpu.c
index ba8e9d3cf0fe..064b7face504 100644
--- a/drivers/gpu/drm/msm/adreno/a6xx_gpu.c
+++ b/drivers/gpu/drm/msm/adreno/a6xx_gpu.c
@@ -522,28 +522,73 @@ static int a6xx_cp_init(struct msm_gpu *gpu)
return a6xx_idle(gpu, ring) ? 0 : -EINVAL;
 }
 
-static void a6xx_ucode_check_version(struct a6xx_gpu *a6xx_gpu,
+/*
+ * Check that the microcode version is new enough to include several key
+ * security fixes. Return true if the ucode is safe.
+ */
+static bool a6xx_ucode_check_version(struct a6xx_gpu *a6xx_gpu,
struct drm_gem_object *obj)
 {
+   struct adreno_gpu *adreno_gpu = &a6xx_gpu->base;
+   struct msm_gpu *gpu = &adreno_gpu->base;
u32 *buf = msm_gem_get_vaddr(obj);
+   bool ret = false;
 
if (IS_ERR(buf))
-   return;
+   return false;
 
/*
-* If the lowest nibble is 0xa that is an indication that this microcode
-* has been patched. The actual version is in dword [3] but we only care
-* about the patchlevel which is the lowest nibble of dword [3]
-*
-* Otherwise check that the firmware is greater than or equal to 1.90
-* which was the first version that had this fix built in
+* Targets up to a640 (a618, a630 and a640) need to check for a
+* microcode version that is patched to support the whereami opcode or
+* one that is new enough to include it by default.
 */
-   if (((buf[0] & 0xf) == 0xa) && (buf[2] & 0xf) >= 1)
-   a6xx_gpu->has_whereami = true;
-   else if ((buf[0] & 0xfff) > 0x190)
-   a6xx_gpu->has_whereami = true;
+   if (adreno_is_a618(adreno_gpu) || adreno_is_a630(adreno_gpu) ||
+   adreno_is_a640(adreno_gpu)) {
+   /*
+* If the lowest nibble is 0xa that is an indication that this
+* microcode has been patched. The actual version is in dword
+* [3] but we only care about the patchlevel which is the lowest
+* nibble of dword [3]
+*
+* Otherwise check that the firmware is greater than or equal
+* to 1.90 which was the first version that had this fix built
+* in
+*/
+   if buf[0] & 0xf) == 0xa) && (buf[2] & 0xf) >= 1) ||
+   (buf[0] & 0xfff) >= 0x190) {
+   a6xx_gpu->has_whereami = true;
+   ret = true;
+   goto out;
+   }
 
+   DRM_DEV_ERROR(&gpu->pdev->dev,
+   "a630 SQE ucode is too old. Have version %x need at 
least %x\n",
+   buf[0] & 0xfff, 0x190);
+   }  else {
+   /*
+* a650 tier targets don't need whereami but still need to be
+* equal to or newer than 1.95 for other security fixes
+*/
+   if (adreno_is_a650(adreno_gpu)) {
+   if ((buf[0] & 0xfff) >= 0x195) {
+   ret = true;
+   goto out;
+   }
+
+   DRM_DEV_ERROR(&gpu->pdev->dev,
+   "a650 SQE ucode is too old. Have version %x 
need at least %x\n",
+   buf[0] & 0xfff, 0x195);
+   }
+
+   /*
+* When a660 is added those targets should return true here
+* since those have all the critical security fixes built in
+* from the start
+*/
+   }
+out:
msm_gem_put_vaddr(obj);
+   return ret;
 }
 
 static int a6xx_ucode_init(struct msm_gpu *gpu)
@@ -566,7 +611,13 @@ static int a6xx_ucode_init(struct msm_gpu *gpu)
}
 
msm_gem_object_set_name(a6xx_gpu->sqe_bo, "sqefw");
-   a6xx_ucode_check_version(a6xx_gpu, a6xx_gpu->sqe_bo);
+   if (!a6xx_ucode_check_version(a6xx_gpu, a6xx_gpu->sqe_bo)) {
+   msm_gem_unpin_iova(a6xx_gpu->sqe_bo, gpu->aspace);
+   drm_gem_object_put(a6xx_gpu->sqe_bo);
+
+   a6xx_gpu->sqe_bo = NULL;
+   return -EPERM;
+   }
}
 
gpu_write64(gpu, REG_A6XX_CP_SQE_INSTR_BASE_LO,
-- 
2.25.1

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

Re: [RFC][PATCH v6 1/7] drm: Add a sharable drm page-pool implementation

2021-02-09 Thread Suren Baghdasaryan

On Tue, Feb 9, 2021 at 12:03 PM Daniel Vetter  wrote:
>
> On Tue, Feb 9, 2021 at 6:46 PM Christian König  
> wrote:
> >
> >
> >
> > Am 09.02.21 um 18:33 schrieb Suren Baghdasaryan:
> > > On Tue, Feb 9, 2021 at 4:57 AM Christian König  
> > > wrote:
> > >> Am 09.02.21 um 13:11 schrieb Christian König:
> > >>> [SNIP]
> > >> +void drm_page_pool_add(struct drm_page_pool *pool, struct page 
> > >> *page)
> > >> +{
> > >> + spin_lock(&pool->lock);
> > >> + list_add_tail(&page->lru, &pool->items);
> > >> + pool->count++;
> > >> + atomic_long_add(1 << pool->order, &total_pages);
> > >> + spin_unlock(&pool->lock);
> > >> +
> > >> + mod_node_page_state(page_pgdat(page),
> > >> NR_KERNEL_MISC_RECLAIMABLE,
> > >> + 1 << pool->order);
> > > Hui what? What should that be good for?
> >  This is a carryover from the ION page pool implementation:
> >  https://nam11.safelinks.protection.outlook.com/?url=https%3A%2F%2Fgit.kernel.org%2Fpub%2Fscm%2Flinux%2Fkernel%2Fgit%2Ftorvalds%2Flinux.git%2Ftree%2Fdrivers%2Fstaging%2Fandroid%2Fion%2Fion_page_pool.c%3Fh%3Dv5.10%23n28&data=04%7C01%7Cchristian.koenig%40amd.com%7Cdff8edcd4d147a5b08d8cd20cff2%7C3dd8961fe4884e608e11a82d994e183d%7C0%7C0%7C637484888114923580%7CUnknown%7CTWFpbGZsb3d8eyJWIjoiMC4wLjAwMDAiLCJQIjoiV2luMzIiLCJBTiI6Ik1haWwiLCJXVCI6Mn0%3D%7C1000&sdata=9%2BIBC0tezSV6Ci4S3kWfW%2BQvJm4mdunn3dF6C0kyfCw%3D&reserved=0
> > 
> > 
> >  My sense is it helps with the vmstat/meminfo accounting so folks can
> >  see the cached pages are shrinkable/freeable. This maybe falls under
> >  other dmabuf accounting/stats discussions, so I'm happy to remove it
> >  for now, or let the drivers using the shared page pool logic handle
> >  the accounting themselves?
> > >> Intentionally separated the discussion for that here.
> > >>
> > >> As far as I can see this is just bluntly incorrect.
> > >>
> > >> Either the page is reclaimable or it is part of our pool and freeable
> > >> through the shrinker, but never ever both.
> > > IIRC the original motivation for counting ION pooled pages as
> > > reclaimable was to include them into /proc/meminfo's MemAvailable
> > > calculations. NR_KERNEL_MISC_RECLAIMABLE defined as "reclaimable
> > > non-slab kernel pages" seems like a good place to account for them but
> > > I might be wrong.
> >
> > Yeah, that's what I see here as well. But exactly that is utterly nonsense.
> >
> > Those pages are not "free" in the sense that get_free_page could return
> > them directly.
>
> Well on Android that is kinda true, because Android has it's
> oom-killer (way back it was just a shrinker callback, not sure how it
> works now), which just shot down all the background apps. So at least
> some of that (everything used by background apps) is indeed
> reclaimable on Android.
>
> But that doesn't hold on Linux in general, so we can't really do this
> for common code.
>
> Also I had a long meeting with Suren, John and other googles
> yesterday, and the aim is now to try and support all the Android gpu
> memory accounting needs with cgroups. That should work, and it will
> allow Android to handle all the Android-ism in a clean way in upstream
> code. Or that's at least the plan.
>
> I think the only thing we identified that Android still needs on top
> is the dma-buf sysfs stuff, so that shared buffers (which on Android
> are always dma-buf, and always stay around as dma-buf fd throughout
> their lifetime) can be listed/analyzed with full detail.
>
> But aside from this the plan for all the per-process or per-heap
> account, oom-killer integration and everything else is planned to be
> done with cgroups.

Until cgroups are ready we probably will need to add a sysfs node to
report the total dmabuf pool size and I think that would cover our
current accounting need here.
As I mentioned, not including dmabuf pools into MemAvailable would
affect that stat and I'm wondering if pools should be considered as
part of MemAvailable or not. Since MemAvailable includes SReclaimable
I think it makes sense to include them but maybe there are other
considerations that I'm missing?

> Android (for now) only needs to account overall gpu
> memory since none of it is swappable on android drivers anyway, plus
> no vram, so not much needed.
>
> Cheers, Daniel
>
> >
> > Regards,
> > Christian.
> >
> > >
> > >> In the best case this just messes up the accounting, in the worst case
> > >> it can cause memory corruption.
> > >>
> > >> Christian.
> >
>
>
> --
> 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

[PATCH] Revert "drm/atomic: document and enforce rules around "spurious" EBUSY"

2021-02-09 Thread Manasi Navare

This reverts commit fb6473a48b635c55d04eb94e579eede52ef39550.

These additional checks added to avoid EBUSY give unnecessary WARN_ON
in case of big joiner used in i915 in which case even if the modeset
is requested on a single pipe, internally another consecutive
pipe is stolen and used to drive half of the transcoder timings.
So in this case it is expected that requested crtc and affected crtcs
do not match. Hence the added WARN ON becomes irrelevant.
But there is no easy solution to get the bigjoiner information
here at drm level. So for now revert this until we work out
a better solution.

Cc: Ville Syrjala 
Cc: Daniel Vetter 
Signed-off-by: Manasi Navare 
---
 drivers/gpu/drm/drm_atomic.c | 29 -
 1 file changed, 29 deletions(-)

diff --git a/drivers/gpu/drm/drm_atomic.c b/drivers/gpu/drm/drm_atomic.c
index b1efa9322be2..48b2262d69f6 100644
--- a/drivers/gpu/drm/drm_atomic.c
+++ b/drivers/gpu/drm/drm_atomic.c
@@ -320,10 +320,6 @@ EXPORT_SYMBOL(__drm_atomic_state_free);
  * needed. It will also grab the relevant CRTC lock to make sure that the state
  * is consistent.
  *
- * WARNING: Drivers may only add new CRTC states to a @state if
- * drm_atomic_state.allow_modeset is set, or if it's a driver-internal commit
- * not created by userspace through an IOCTL call.
- *
  * Returns:
  *
  * Either the allocated state or the error code encoded into the pointer. When
@@ -1306,15 +1302,10 @@ int drm_atomic_check_only(struct drm_atomic_state 
*state)
struct drm_crtc_state *new_crtc_state;
struct drm_connector *conn;
struct drm_connector_state *conn_state;
-   unsigned requested_crtc = 0;
-   unsigned affected_crtc = 0;
int i, ret = 0;
 
DRM_DEBUG_ATOMIC("checking %p\n", state);
 
-   for_each_new_crtc_in_state(state, crtc, new_crtc_state, i)
-   requested_crtc |= drm_crtc_mask(crtc);
-
for_each_oldnew_plane_in_state(state, plane, old_plane_state, 
new_plane_state, i) {
ret = drm_atomic_plane_check(old_plane_state, new_plane_state);
if (ret) {
@@ -1362,26 +1353,6 @@ int drm_atomic_check_only(struct drm_atomic_state *state)
}
}
 
-   for_each_new_crtc_in_state(state, crtc, new_crtc_state, i)
-   affected_crtc |= drm_crtc_mask(crtc);
-
-   /*
-* For commits that allow modesets drivers can add other CRTCs to the
-* atomic commit, e.g. when they need to reallocate global resources.
-* This can cause spurious EBUSY, which robs compositors of a very
-* effective sanity check for their drawing loop. Therefor only allow
-* drivers to add unrelated CRTC states for modeset commits.
-*
-* FIXME: Should add affected_crtc mask to the ATOMIC IOCTL as an output
-* so compositors know what's going on.
-*/
-   if (affected_crtc != requested_crtc) {
-   DRM_DEBUG_ATOMIC("driver added CRTC to commit: requested 0x%x, 
affected 0x%0x\n",
-requested_crtc, affected_crtc);
-   WARN(!state->allow_modeset, "adding CRTC not allowed without 
modesets: requested 0x%x, affected 0x%0x\n",
-requested_crtc, affected_crtc);
-   }
-
return 0;
 }
 EXPORT_SYMBOL(drm_atomic_check_only);
-- 
2.19.1

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

[PATCH 1/3] drm/amdgpu/ttm: constify static vm_operations_struct

The only usage of amdgpu_ttm_vm_ops is to assign its address to the
vm_ops field in the vm_area_struct struct. Make it const to allow the
compiler to put it in read-only memory.

Signed-off-by: Rikard Falkeborn 
---
 drivers/gpu/drm/amd/amdgpu/amdgpu_ttm.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_ttm.c 
b/drivers/gpu/drm/amd/amdgpu/amdgpu_ttm.c
index 29cfb0809634..a785acc09f20 100644
--- a/drivers/gpu/drm/amd/amdgpu/amdgpu_ttm.c
+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_ttm.c
@@ -2002,7 +2002,7 @@ static vm_fault_t amdgpu_ttm_fault(struct vm_fault *vmf)
return ret;
 }
 
-static struct vm_operations_struct amdgpu_ttm_vm_ops = {
+static const struct vm_operations_struct amdgpu_ttm_vm_ops = {
.fault = amdgpu_ttm_fault,
.open = ttm_bo_vm_open,
.close = ttm_bo_vm_close,
-- 
2.30.0

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

[PATCH 2/3] drm/radeon/ttm: constify static vm_operations_struct

The only usage of radeon_ttm_vm_ops is to assign its address to the
vm_ops field in the vm_area_struct struct. Make it const to allow the
compiler to put it in read-only memory

Signed-off-by: Rikard Falkeborn 
---
 drivers/gpu/drm/radeon/radeon_ttm.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/gpu/drm/radeon/radeon_ttm.c 
b/drivers/gpu/drm/radeon/radeon_ttm.c
index c45e919e03c5..5fc8bae401af 100644
--- a/drivers/gpu/drm/radeon/radeon_ttm.c
+++ b/drivers/gpu/drm/radeon/radeon_ttm.c
@@ -835,7 +835,7 @@ static vm_fault_t radeon_ttm_fault(struct vm_fault *vmf)
return ret;
 }
 
-static struct vm_operations_struct radeon_ttm_vm_ops = {
+static const struct vm_operations_struct radeon_ttm_vm_ops = {
.fault = radeon_ttm_fault,
.open = ttm_bo_vm_open,
.close = ttm_bo_vm_close,
-- 
2.30.0

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

[PATCH 3/3] drm/nouveau/ttm: constify static vm_operations_struct

The only usage of nouveau_ttm_vm_ops is to assign its address to the
vm_ops field in the vm_area_struct struct. Make it const to allow the
compiler to put it in read-only memory

Signed-off-by: Rikard Falkeborn 
---
 drivers/gpu/drm/nouveau/nouveau_ttm.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/gpu/drm/nouveau/nouveau_ttm.c 
b/drivers/gpu/drm/nouveau/nouveau_ttm.c
index 495288182c2d..b81ae90b8449 100644
--- a/drivers/gpu/drm/nouveau/nouveau_ttm.c
+++ b/drivers/gpu/drm/nouveau/nouveau_ttm.c
@@ -154,7 +154,7 @@ static vm_fault_t nouveau_ttm_fault(struct vm_fault *vmf)
return ret;
 }
 
-static struct vm_operations_struct nouveau_ttm_vm_ops = {
+static const struct vm_operations_struct nouveau_ttm_vm_ops = {
.fault = nouveau_ttm_fault,
.open = ttm_bo_vm_open,
.close = ttm_bo_vm_close,
-- 
2.30.0

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

Re: [PATCH v3 1/2] drm/aspeed: Look up syscon by phandle

2021-02-09 Thread Andrew Jeffery




On Tue, 9 Feb 2021, at 23:07, Joel Stanley wrote:
> This scales better to multiple families of SoC. The lookup by compatible
> can be removed in a future change.
> 
> The fallback path is for the ast2500 platform only. Other platforms will
> be added with the new style, so they won't need fallback paths.
> 
> Signed-off-by: Joel Stanley 

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

[PATCH 0/3] drm/ttm: constify static vm_operations_structs

Constify a few static vm_operations_struct that are never modified. Their
only usage is to assign their address to the vm_ops field in the
vm_area_struct, which is a pointer to const vm_operations_struct. Make them
const to allow the compiler to put them in read-only memory.

With this series applied, all static struct vm_operations_struct in the
kernel tree are const.

Rikard Falkeborn (3):
  drm/amdgpu/ttm: constify static vm_operations_struct
  drm/radeon/ttm: constify static vm_operations_struct
  drm/nouveau/ttm: constify static vm_operations_struct

 drivers/gpu/drm/amd/amdgpu/amdgpu_ttm.c | 2 +-
 drivers/gpu/drm/nouveau/nouveau_ttm.c   | 2 +-
 drivers/gpu/drm/radeon/radeon_ttm.c | 2 +-
 3 files changed, 3 insertions(+), 3 deletions(-)

-- 
2.30.0

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

[PATCH] drm/msm: a6xx: Make sure the SQE microcode is safe

2021-02-09 Thread Jordan Crouse

Most a6xx targets have security issues that were fixed with new versions
of the microcode(s). Make sure that we are booting with a safe version of
the microcode for the target and print a message and error if not.

Signed-off-by: Jordan Crouse 
---

 drivers/gpu/drm/msm/adreno/a6xx_gpu.c | 67 +--
 1 file changed, 54 insertions(+), 13 deletions(-)

diff --git a/drivers/gpu/drm/msm/adreno/a6xx_gpu.c 
b/drivers/gpu/drm/msm/adreno/a6xx_gpu.c
index ba8e9d3cf0fe..cfb0d5f63784 100644
--- a/drivers/gpu/drm/msm/adreno/a6xx_gpu.c
+++ b/drivers/gpu/drm/msm/adreno/a6xx_gpu.c
@@ -522,28 +522,61 @@ static int a6xx_cp_init(struct msm_gpu *gpu)
return a6xx_idle(gpu, ring) ? 0 : -EINVAL;
 }
 
-static void a6xx_ucode_check_version(struct a6xx_gpu *a6xx_gpu,
+/*
+ * Check that the microcode version is new enough to include several key
+ * security fixes. Return true if the ucode is safe.
+ */
+static bool a6xx_ucode_check_version(struct a6xx_gpu *a6xx_gpu,
struct drm_gem_object *obj)
 {
+   struct adreno_gpu *adreno_gpu = &a6xx_gpu->base;
u32 *buf = msm_gem_get_vaddr(obj);
+   bool ret = false;
 
if (IS_ERR(buf))
-   return;
+   return false;
 
/*
-* If the lowest nibble is 0xa that is an indication that this microcode
-* has been patched. The actual version is in dword [3] but we only care
-* about the patchlevel which is the lowest nibble of dword [3]
-*
-* Otherwise check that the firmware is greater than or equal to 1.90
-* which was the first version that had this fix built in
+* Targets up to a640 (a618, a630 and a640) need to check for a
+* microcode version that is patched to support the whereami opcode or
+* one that is new enough to include it by default.
 */
-   if (((buf[0] & 0xf) == 0xa) && (buf[2] & 0xf) >= 1)
-   a6xx_gpu->has_whereami = true;
-   else if ((buf[0] & 0xfff) > 0x190)
-   a6xx_gpu->has_whereami = true;
+   if (adreno_is_a618(adreno_gpu) || adreno_is_a630(adreno_gpu) ||
+   adreno_is_a640(adreno_gpu)) {
+   /*
+* If the lowest nibble is 0xa that is an indication that this
+* microcode has been patched. The actual version is in dword
+* [3] but we only care about the patchlevel which is the lowest
+* nibble of dword [3]
+*
+* Otherwise check that the firmware is greater than or equal
+* to 1.90 which was the first version that had this fix built
+* in
+*/
+   if buf[0] & 0xf) == 0xa) && (buf[2] & 0xf) >= 1) ||
+   (buf[0] & 0xfff) > 0x190) {
+   a6xx_gpu->has_whereami = true;
+   ret = true;
+   }
+   }  else {
+   /*
+* a650 tier targets don't need whereami but still need to be
+* equal to or newer than 1.95 for other security fixes
+*/
+   if (adreno_is_a640(adreno_gpu)) {
+   if ((buf[0] & 0xfff) >= 0x195)
+   ret = true;
+   }
+
+   /*
+* When a660 is added those targets should return true here
+* since those have all the critiical security fixes built in
+* from the start
+*/
+   }
 
msm_gem_put_vaddr(obj);
+   return ret;
 }
 
 static int a6xx_ucode_init(struct msm_gpu *gpu)
@@ -566,7 +599,15 @@ static int a6xx_ucode_init(struct msm_gpu *gpu)
}
 
msm_gem_object_set_name(a6xx_gpu->sqe_bo, "sqefw");
-   a6xx_ucode_check_version(a6xx_gpu, a6xx_gpu->sqe_bo);
+   if (!a6xx_ucode_check_version(a6xx_gpu, a6xx_gpu->sqe_bo)) {
+   DRM_DEV_ERROR(&gpu->pdev->dev,
+   "SQE ucode version is too old. It is likely 
unsafe\n");
+   msm_gem_unpin_iova(a6xx_gpu->sqe_bo, gpu->aspace);
+   drm_gem_object_put(a6xx_gpu->sqe_bo);
+
+   a6xx_gpu->sqe_bo = NULL;
+   return -EPERM;
+   }
}
 
gpu_write64(gpu, REG_A6XX_CP_SQE_INSTR_BASE_LO,
-- 
2.25.1

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

Re: [RFC][PATCH v6 1/7] drm: Add a sharable drm page-pool implementation

2021-02-09 Thread Suren Baghdasaryan

On Tue, Feb 9, 2021 at 9:46 AM Christian König  wrote:
>
>
>
> Am 09.02.21 um 18:33 schrieb Suren Baghdasaryan:
> > On Tue, Feb 9, 2021 at 4:57 AM Christian König  
> > wrote:
> >> Am 09.02.21 um 13:11 schrieb Christian König:
> >>> [SNIP]
> >> +void drm_page_pool_add(struct drm_page_pool *pool, struct page *page)
> >> +{
> >> + spin_lock(&pool->lock);
> >> + list_add_tail(&page->lru, &pool->items);
> >> + pool->count++;
> >> + atomic_long_add(1 << pool->order, &total_pages);
> >> + spin_unlock(&pool->lock);
> >> +
> >> + mod_node_page_state(page_pgdat(page),
> >> NR_KERNEL_MISC_RECLAIMABLE,
> >> + 1 << pool->order);
> > Hui what? What should that be good for?
>  This is a carryover from the ION page pool implementation:
>  https://nam11.safelinks.protection.outlook.com/?url=https%3A%2F%2Fgit.kernel.org%2Fpub%2Fscm%2Flinux%2Fkernel%2Fgit%2Ftorvalds%2Flinux.git%2Ftree%2Fdrivers%2Fstaging%2Fandroid%2Fion%2Fion_page_pool.c%3Fh%3Dv5.10%23n28&data=04%7C01%7Cchristian.koenig%40amd.com%7Cdff8edcd4d147a5b08d8cd20cff2%7C3dd8961fe4884e608e11a82d994e183d%7C0%7C0%7C637484888114923580%7CUnknown%7CTWFpbGZsb3d8eyJWIjoiMC4wLjAwMDAiLCJQIjoiV2luMzIiLCJBTiI6Ik1haWwiLCJXVCI6Mn0%3D%7C1000&sdata=9%2BIBC0tezSV6Ci4S3kWfW%2BQvJm4mdunn3dF6C0kyfCw%3D&reserved=0
> 
> 
>  My sense is it helps with the vmstat/meminfo accounting so folks can
>  see the cached pages are shrinkable/freeable. This maybe falls under
>  other dmabuf accounting/stats discussions, so I'm happy to remove it
>  for now, or let the drivers using the shared page pool logic handle
>  the accounting themselves?
> >> Intentionally separated the discussion for that here.
> >>
> >> As far as I can see this is just bluntly incorrect.
> >>
> >> Either the page is reclaimable or it is part of our pool and freeable
> >> through the shrinker, but never ever both.
> > IIRC the original motivation for counting ION pooled pages as
> > reclaimable was to include them into /proc/meminfo's MemAvailable
> > calculations. NR_KERNEL_MISC_RECLAIMABLE defined as "reclaimable
> > non-slab kernel pages" seems like a good place to account for them but
> > I might be wrong.
>
> Yeah, that's what I see here as well. But exactly that is utterly nonsense.
>
> Those pages are not "free" in the sense that get_free_page could return
> them directly.

Any ideas where these pages would fit better? We do want to know that
under memory pressure these pages can be made available (which is I
think what MemAvailable means).

>
> Regards,
> Christian.
>
> >
> >> In the best case this just messes up the accounting, in the worst case
> >> it can cause memory corruption.
> >>
> >> Christian.
>
___
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

[PATCH] drm: use getter/setter functions

2021-02-09 Thread Julia Lawall

Use getter and setter functions, for platform_device structures and a
mipi_dsi_device structure.

Signed-off-by: Julia Lawall 

---
 drivers/gpu/drm/aspeed/aspeed_gfx_drv.c |2 +-
 drivers/gpu/drm/bridge/cadence/cdns-mhdp8546-core.c |2 +-
 drivers/gpu/drm/panel/panel-lvds.c  |2 +-
 drivers/gpu/drm/panel/panel-seiko-43wvf1g.c |4 ++--
 drivers/gpu/drm/panel/panel-simple.c|2 +-
 drivers/gpu/drm/rockchip/rockchip_lvds.c|2 +-
 6 files changed, 7 insertions(+), 7 deletions(-)

diff --git a/drivers/gpu/drm/panel/panel-simple.c 
b/drivers/gpu/drm/panel/panel-simple.c
index 4e2dad314c79..9858079f9e14 100644
--- a/drivers/gpu/drm/panel/panel-simple.c
+++ b/drivers/gpu/drm/panel/panel-simple.c
@@ -4800,7 +4800,7 @@ static int panel_simple_dsi_probe(struct mipi_dsi_device 
*dsi)
 
err = mipi_dsi_attach(dsi);
if (err) {
-   struct panel_simple *panel = dev_get_drvdata(&dsi->dev);
+   struct panel_simple *panel = mipi_dsi_get_drvdata(dsi);
 
drm_panel_remove(&panel->base);
}
diff --git a/drivers/gpu/drm/panel/panel-seiko-43wvf1g.c 
b/drivers/gpu/drm/panel/panel-seiko-43wvf1g.c
index 0ee508576231..3939b25e 100644
--- a/drivers/gpu/drm/panel/panel-seiko-43wvf1g.c
+++ b/drivers/gpu/drm/panel/panel-seiko-43wvf1g.c
@@ -267,7 +267,7 @@ static int seiko_panel_probe(struct device *dev,
 
 static int seiko_panel_remove(struct platform_device *pdev)
 {
-   struct seiko_panel *panel = dev_get_drvdata(&pdev->dev);
+   struct seiko_panel *panel = platform_get_drvdata(pdev);
 
drm_panel_remove(&panel->base);
drm_panel_disable(&panel->base);
@@ -277,7 +277,7 @@ static int seiko_panel_remove(struct platform_device *pdev)
 
 static void seiko_panel_shutdown(struct platform_device *pdev)
 {
-   struct seiko_panel *panel = dev_get_drvdata(&pdev->dev);
+   struct seiko_panel *panel = platform_get_drvdata(pdev);
 
drm_panel_disable(&panel->base);
 }
diff --git a/drivers/gpu/drm/rockchip/rockchip_lvds.c 
b/drivers/gpu/drm/rockchip/rockchip_lvds.c
index 654bc52d9ff3..bd5ba10822c2 100644
--- a/drivers/gpu/drm/rockchip/rockchip_lvds.c
+++ b/drivers/gpu/drm/rockchip/rockchip_lvds.c
@@ -725,7 +725,7 @@ static int rockchip_lvds_probe(struct platform_device *pdev)
 
 static int rockchip_lvds_remove(struct platform_device *pdev)
 {
-   struct rockchip_lvds *lvds = dev_get_drvdata(&pdev->dev);
+   struct rockchip_lvds *lvds = platform_get_drvdata(pdev);
 
component_del(&pdev->dev, &rockchip_lvds_component_ops);
clk_unprepare(lvds->pclk);
diff --git a/drivers/gpu/drm/aspeed/aspeed_gfx_drv.c 
b/drivers/gpu/drm/aspeed/aspeed_gfx_drv.c
index 457ec04950f7..c7707338bfdb 100644
--- a/drivers/gpu/drm/aspeed/aspeed_gfx_drv.c
+++ b/drivers/gpu/drm/aspeed/aspeed_gfx_drv.c
@@ -284,7 +284,7 @@ static int aspeed_gfx_probe(struct platform_device *pdev)
if (ret)
return ret;
 
-   dev_set_drvdata(&pdev->dev, priv);
+   platform_set_drvdata(pdev, priv);
 
ret = sysfs_create_group(&pdev->dev.kobj, &aspeed_sysfs_attr_group);
if (ret)
diff --git a/drivers/gpu/drm/bridge/cadence/cdns-mhdp8546-core.c 
b/drivers/gpu/drm/bridge/cadence/cdns-mhdp8546-core.c
index d0c65610ebb5..989a05bc8197 100644
--- a/drivers/gpu/drm/bridge/cadence/cdns-mhdp8546-core.c
+++ b/drivers/gpu/drm/bridge/cadence/cdns-mhdp8546-core.c
@@ -2457,7 +2457,7 @@ static int cdns_mhdp_probe(struct platform_device *pdev)
 
 static int cdns_mhdp_remove(struct platform_device *pdev)
 {
-   struct cdns_mhdp_device *mhdp = dev_get_drvdata(&pdev->dev);
+   struct cdns_mhdp_device *mhdp = platform_get_drvdata(pdev);
unsigned long timeout = msecs_to_jiffies(100);
bool stop_fw = false;
int ret;
diff --git a/drivers/gpu/drm/panel/panel-lvds.c 
b/drivers/gpu/drm/panel/panel-lvds.c
index 66c7d765b8f7..59a8d99e777d 100644
--- a/drivers/gpu/drm/panel/panel-lvds.c
+++ b/drivers/gpu/drm/panel/panel-lvds.c
@@ -244,7 +244,7 @@ static int panel_lvds_probe(struct platform_device *pdev)
 
 static int panel_lvds_remove(struct platform_device *pdev)
 {
-   struct panel_lvds *lvds = dev_get_drvdata(&pdev->dev);
+   struct panel_lvds *lvds = platform_get_drvdata(pdev);
 
drm_panel_remove(&lvds->panel);
 

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

[PATCH] video: use getter/setter functions

2021-02-09 Thread Julia Lawall

Use getter and setter functions, for platform_device structures and a
spi_device structure.

Signed-off-by: Julia Lawall 

---
 drivers/video/backlight/qcom-wled.c  |2 +-
 drivers/video/fbdev/amifb.c  |4 
++--
 drivers/video/fbdev/da8xx-fb.c   |4 
++--
 drivers/video/fbdev/imxfb.c  |2 +-
 drivers/video/fbdev/omap2/omapfb/displays/panel-lgphilips-lb035q02.c |6 
+++---
 drivers/video/fbdev/omap2/omapfb/dss/dpi.c   |4 
++--
 drivers/video/fbdev/omap2/omapfb/dss/dsi.c   |4 
++--
 drivers/video/fbdev/omap2/omapfb/dss/hdmi4.c |2 +-
 drivers/video/fbdev/omap2/omapfb/dss/hdmi5.c |2 +-
 drivers/video/fbdev/xilinxfb.c   |2 +-
 10 files changed, 16 insertions(+), 16 deletions(-)

diff --git a/drivers/video/backlight/qcom-wled.c 
b/drivers/video/backlight/qcom-wled.c
index 3bc7800eb0a9..091f07e7c145 100644
--- a/drivers/video/backlight/qcom-wled.c
+++ b/drivers/video/backlight/qcom-wled.c
@@ -1692,7 +1692,7 @@ static int wled_probe(struct platform_device *pdev)
 
 static int wled_remove(struct platform_device *pdev)
 {
-   struct wled *wled = dev_get_drvdata(&pdev->dev);
+   struct wled *wled = platform_get_drvdata(pdev);
 
mutex_destroy(&wled->lock);
cancel_delayed_work_sync(&wled->ovp_work);
diff --git a/drivers/video/fbdev/xilinxfb.c b/drivers/video/fbdev/xilinxfb.c
index ca4ff658cad0..ffbf900648d9 100644
--- a/drivers/video/fbdev/xilinxfb.c
+++ b/drivers/video/fbdev/xilinxfb.c
@@ -472,7 +472,7 @@ static int xilinxfb_of_probe(struct platform_device *pdev)
if (of_find_property(pdev->dev.of_node, "rotate-display", NULL))
pdata.rotate_screen = 1;
 
-   dev_set_drvdata(&pdev->dev, drvdata);
+   platform_set_drvdata(pdev, drvdata);
return xilinxfb_assign(pdev, drvdata, &pdata);
 }
 
diff --git a/drivers/video/fbdev/imxfb.c b/drivers/video/fbdev/imxfb.c
index 884b16efa7e8..7f8debd2da06 100644
--- a/drivers/video/fbdev/imxfb.c
+++ b/drivers/video/fbdev/imxfb.c
@@ -657,7 +657,7 @@ static int imxfb_activate_var(struct fb_var_screeninfo 
*var, struct fb_info *inf
 static int imxfb_init_fbinfo(struct platform_device *pdev)
 {
struct imx_fb_platform_data *pdata = dev_get_platdata(&pdev->dev);
-   struct fb_info *info = dev_get_drvdata(&pdev->dev);
+   struct fb_info *info = platform_get_drvdata(pdev);
struct imxfb_info *fbi = info->par;
struct device_node *np;
 
diff --git a/drivers/video/fbdev/omap2/omapfb/dss/hdmi5.c 
b/drivers/video/fbdev/omap2/omapfb/dss/hdmi5.c
index e3d441ade241..2c03608addcd 100644
--- a/drivers/video/fbdev/omap2/omapfb/dss/hdmi5.c
+++ b/drivers/video/fbdev/omap2/omapfb/dss/hdmi5.c
@@ -713,7 +713,7 @@ static int hdmi5_bind(struct device *dev, struct device 
*master, void *data)
int irq;
 
hdmi.pdev = pdev;
-   dev_set_drvdata(&pdev->dev, &hdmi);
+   platform_set_drvdata(pdev, &hdmi);
 
mutex_init(&hdmi.lock);
spin_lock_init(&hdmi.audio_playing_lock);
diff --git a/drivers/video/fbdev/omap2/omapfb/dss/hdmi4.c 
b/drivers/video/fbdev/omap2/omapfb/dss/hdmi4.c
index 496b43bdad21..800bd108e834 100644
--- a/drivers/video/fbdev/omap2/omapfb/dss/hdmi4.c
+++ b/drivers/video/fbdev/omap2/omapfb/dss/hdmi4.c
@@ -672,7 +672,7 @@ static int hdmi4_bind(struct device *dev, struct device 
*master, void *data)
int irq;
 
hdmi.pdev = pdev;
-   dev_set_drvdata(&pdev->dev, &hdmi);
+   platform_set_drvdata(pdev, &hdmi);
 
mutex_init(&hdmi.lock);
spin_lock_init(&hdmi.audio_playing_lock);
diff --git a/drivers/video/fbdev/omap2/omapfb/dss/dsi.c 
b/drivers/video/fbdev/omap2/omapfb/dss/dsi.c
index 58c7aa279ab1..daa313f14335 100644
--- a/drivers/video/fbdev/omap2/omapfb/dss/dsi.c
+++ b/drivers/video/fbdev/omap2/omapfb/dss/dsi.c
@@ -399,7 +399,7 @@ module_param(dsi_perf, bool, 0644);
 
 static inline struct dsi_data *dsi_get_dsidrv_data(struct platform_device 
*dsidev)
 {
-   return dev_get_drvdata(&dsidev->dev);
+   return platform_get_drvdata(dsidev);
 }
 
 static inline struct platform_device *dsi_get_dsidev_from_dssdev(struct 
omap_dss_device *dssdev)
@@ -5266,7 +5266,7 @@ static int dsi_bind(struct device *dev, struct device 
*master, void *data)
return -ENOMEM;
 
dsi->pdev = dsidev;
-   dev_set_drvdata(&dsidev->dev, dsi);
+   platform_set_drvdata(dsidev, dsi);
 
spin_lock_init(&dsi->irq_lock);
spin_lock_init(&dsi->errors_lock);
diff --git a/drivers/video/fbdev/omap2/omapfb/dss/dpi.c 
b/drivers/video/fbdev/omap2/omapfb/dss/dpi.c
index e2e7fe6f89ee..99ce6e955a46 100644
--- a/drivers/video/fbdev/omap2/omapfb/dss/dpi.c
+++ b/drivers/video/fbdev/omap2/omapfb/dss/dpi.c
@@ -55,7 +55,7 @@ static

Re: [RESEND][PATCH] drm/tilcdc: send vblank event when disabling crtc

2021-02-09 Thread Jyri Sarha


On 2021-02-09 10:24, quanyang.w...@windriver.com wrote:

From: Quanyang Wang 

When run xrandr to change resolution on Beaglebone Black board, it will
print the error information:

root@beaglebone:~# xrandr -display :0 --output HDMI-1 --mode 720x400
[drm:drm_crtc_commit_wait] *ERROR* flip_done timed out
[drm:drm_atomic_helper_wait_for_dependencies] *ERROR* [CRTC:32:tilcdc
crtc] commit wait timed out
[drm:drm_crtc_commit_wait] *ERROR* flip_done timed out
[drm:drm_atomic_helper_wait_for_dependencies] *ERROR*
[CONNECTOR:34:HDMI-A-1] commit wait timed out
[drm:drm_crtc_commit_wait] *ERROR* flip_done timed out
[drm:drm_atomic_helper_wait_for_dependencies] *ERROR*
[PLANE:31:plane-0] commit wait timed out
tilcdc 4830e000.lcdc: already pending page flip!

This is because there is operation sequence as below:

drm_atomic_connector_commit_dpms(mode is DRM_MODE_DPMS_OFF):
...
drm_atomic_helper_setup_commit <- 
init_completion(commit_A->flip_done)

drm_atomic_helper_commit_tail
tilcdc_crtc_atomic_disable
tilcdc_plane_atomic_update <- drm_crtc_send_vblank_event in
tilcdc_crtc_irq
  is skipped since 
tilcdc_crtc->enabled is 0

tilcdc_crtc_atomic_flush   <- drm_crtc_send_vblank_event is
skipped since
  crtc->state->event is set to be 
NULL in

  tilcdc_plane_atomic_update
drm_mode_setcrtc:
...
drm_atomic_helper_setup_commit <- 
init_completion(commit_B->flip_done)

drm_atomic_helper_wait_for_dependencies
drm_crtc_commit_wait   <- wait for commit_A->flip_done 
completing


Just as shown above, the steps which could complete commit_A->flip_done
are all skipped and commit_A->flip_done will never be completed. This 
will
result a time-out ERROR when drm_crtc_commit_wait check the 
commit_A->flip_done.

So add drm_crtc_send_vblank_event in tilcdc_crtc_atomic_disable to
complete commit_A->flip_done.

Fixes: cb345decb4d2 ("drm/tilcdc: Use standard 
drm_atomic_helper_commit")

Signed-off-by: Quanyang Wang 


Reviewed-by: Jyri Sarha 
Tested-by: Jyri Sarha 

Thanks a lot! I think I have bumbed into this once or twice, but latelu 
I have had time to look into this. I'll merge this to drm-misc-next 
soon.


Best regards,
Jyri


---
 drivers/gpu/drm/tilcdc/tilcdc_crtc.c | 9 +
 1 file changed, 9 insertions(+)

diff --git a/drivers/gpu/drm/tilcdc/tilcdc_crtc.c
b/drivers/gpu/drm/tilcdc/tilcdc_crtc.c
index 30213708fc99..d99afd19ca08 100644
--- a/drivers/gpu/drm/tilcdc/tilcdc_crtc.c
+++ b/drivers/gpu/drm/tilcdc/tilcdc_crtc.c
@@ -515,6 +515,15 @@ static void tilcdc_crtc_off(struct drm_crtc
*crtc, bool shutdown)

drm_crtc_vblank_off(crtc);

+   spin_lock_irq(&crtc->dev->event_lock);
+
+   if (crtc->state->event) {
+   drm_crtc_send_vblank_event(crtc, crtc->state->event);
+   crtc->state->event = NULL;
+   }
+
+   spin_unlock_irq(&crtc->dev->event_lock);
+
tilcdc_crtc_disable_irqs(dev);

pm_runtime_put_sync(dev->dev);

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

Re: [RFC][PATCH v6 1/7] drm: Add a sharable drm page-pool implementation

2021-02-09 Thread Suren Baghdasaryan

On Tue, Feb 9, 2021 at 4:57 AM Christian König  wrote:
>
> Am 09.02.21 um 13:11 schrieb Christian König:
> > [SNIP]
>  +void drm_page_pool_add(struct drm_page_pool *pool, struct page *page)
>  +{
>  + spin_lock(&pool->lock);
>  + list_add_tail(&page->lru, &pool->items);
>  + pool->count++;
>  + atomic_long_add(1 << pool->order, &total_pages);
>  + spin_unlock(&pool->lock);
>  +
>  + mod_node_page_state(page_pgdat(page),
>  NR_KERNEL_MISC_RECLAIMABLE,
>  + 1 << pool->order);
> >>> Hui what? What should that be good for?
> >> This is a carryover from the ION page pool implementation:
> >> https://nam11.safelinks.protection.outlook.com/?url=https%3A%2F%2Fgit.kernel.org%2Fpub%2Fscm%2Flinux%2Fkernel%2Fgit%2Ftorvalds%2Flinux.git%2Ftree%2Fdrivers%2Fstaging%2Fandroid%2Fion%2Fion_page_pool.c%3Fh%3Dv5.10%23n28&data=04%7C01%7Cchristian.koenig%40amd.com%7Cc4eadb0a9cf6491d99ba08d8ca173457%7C3dd8961fe4884e608e11a82d994e183d%7C0%7C0%7C637481548325174885%7CUnknown%7CTWFpbGZsb3d8eyJWIjoiMC4wLjAwMDAiLCJQIjoiV2luMzIiLCJBTiI6Ik1haWwiLCJXVCI6Mn0%3D%7C1000&sdata=FUjZK5NSDMUYfU7vGeE4fDU2HCF%2FYyNBwc30aoLLPQ4%3D&reserved=0
> >>
> >>
> >> My sense is it helps with the vmstat/meminfo accounting so folks can
> >> see the cached pages are shrinkable/freeable. This maybe falls under
> >> other dmabuf accounting/stats discussions, so I'm happy to remove it
> >> for now, or let the drivers using the shared page pool logic handle
> >> the accounting themselves?
>
> Intentionally separated the discussion for that here.
>
> As far as I can see this is just bluntly incorrect.
>
> Either the page is reclaimable or it is part of our pool and freeable
> through the shrinker, but never ever both.

IIRC the original motivation for counting ION pooled pages as
reclaimable was to include them into /proc/meminfo's MemAvailable
calculations. NR_KERNEL_MISC_RECLAIMABLE defined as "reclaimable
non-slab kernel pages" seems like a good place to account for them but
I might be wrong.

>
> In the best case this just messes up the accounting, in the worst case
> it can cause memory corruption.
>
> Christian.
___
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

[PATCH v5 4/4] drm/i915/gen9_bc: Add W/A for missing STRAP config on TGP PCH + CML combos

Apparently the new gen9_bc platforms that Intel has introduced don't
provide us with a STRAP config register to read from for initializing DDI
B, C, and D detection. So, workaround this by hard-coding our strap config
in intel_setup_outputs().

Changes since v4:
* Split this into it's own commit

Cc: Matt Roper 
Cc: Jani Nikula 
Cc: Ville Syrjala 
[originally from Tejas's work]
Signed-off-by: Tejas Upadhyay 
Signed-off-by: Lyude Paul 
---
 drivers/gpu/drm/i915/display/intel_display.c | 9 -
 1 file changed, 8 insertions(+), 1 deletion(-)

diff --git a/drivers/gpu/drm/i915/display/intel_display.c 
b/drivers/gpu/drm/i915/display/intel_display.c
index beed08c00b6c..4dee37f8659d 100644
--- a/drivers/gpu/drm/i915/display/intel_display.c
+++ b/drivers/gpu/drm/i915/display/intel_display.c
@@ -11943,7 +11943,14 @@ static void intel_setup_outputs(struct 
drm_i915_private *dev_priv)
 
/* DDI B, C, D, and F detection is indicated by the SFUSE_STRAP
 * register */
-   found = intel_de_read(dev_priv, SFUSE_STRAP);
+   if (HAS_PCH_TGP(dev_priv)) {
+   /* W/A due to lack of STRAP config on TGP PCH*/
+   found = (SFUSE_STRAP_DDIB_DETECTED |
+SFUSE_STRAP_DDIC_DETECTED |
+SFUSE_STRAP_DDID_DETECTED);
+   } else {
+   found = intel_de_read(dev_priv, SFUSE_STRAP);
+   }
 
if (found & SFUSE_STRAP_DDIB_DETECTED)
intel_ddi_init(dev_priv, PORT_B);
-- 
2.29.2

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

[PATCH v5 3/4] drm/i915/gen9_bc: Introduce HPD pin mappings for TGP PCH + CML combos

Next, let's start introducing the HPD pin mappings for Intel's new gen9_bc
platform in order to make hotplugging display connectors work. Since
gen9_bc is just a TGP PCH along with a CML CPU, except with the same HPD
mappings as ICL, we simply add a skl_hpd_pin function that is shared
between gen9 and gen9_bc which handles both the traditional gen9 HPD pin
mappings and the Icelake HPD pin mappings that gen9_bc uses.

Changes since v4:
* Split this into its own commit
* Introduce skl_hpd_pin() like vsyrjala suggested and use that instead of
  sticking our HPD pin mappings in TGP code

Cc: Matt Roper 
Cc: Jani Nikula 
Cc: Ville Syrjala 
[originally from Tejas's work]
Signed-off-by: Tejas Upadhyay 
Signed-off-by: Lyude Paul 
---
 drivers/gpu/drm/i915/display/intel_ddi.c | 10 ++
 1 file changed, 10 insertions(+)

diff --git a/drivers/gpu/drm/i915/display/intel_ddi.c 
b/drivers/gpu/drm/i915/display/intel_ddi.c
index 3c4003605f93..01b171f52694 100644
--- a/drivers/gpu/drm/i915/display/intel_ddi.c
+++ b/drivers/gpu/drm/i915/display/intel_ddi.c
@@ -3954,6 +3954,14 @@ static enum hpd_pin cnl_hpd_pin(struct drm_i915_private 
*dev_priv,
return HPD_PORT_A + port - PORT_A;
 }
 
+static enum hpd_pin skl_hpd_pin(struct drm_i915_private *dev_priv, enum port 
port)
+{
+   if (HAS_PCH_TGP(dev_priv))
+   return icl_hpd_pin(dev_priv, port);
+
+   return HPD_PORT_A + port - PORT_A;
+}
+
 #define port_tc_name(port) ((port) - PORT_TC1 + '1')
 #define tc_port_name(tc_port) ((tc_port) - TC_PORT_1 + '1')
 
@@ -4070,6 +4078,8 @@ void intel_ddi_init(struct drm_i915_private *dev_priv, 
enum port port)
encoder->hpd_pin = icl_hpd_pin(dev_priv, port);
else if (IS_GEN(dev_priv, 10))
encoder->hpd_pin = cnl_hpd_pin(dev_priv, port);
+   else if (IS_GEN(dev_priv, 9))
+   encoder->hpd_pin = skl_hpd_pin(dev_priv, port);
else
encoder->hpd_pin = intel_hpd_pin_default(dev_priv, port);
 
-- 
2.29.2

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

[PATCH v5 2/4] drm/i915/gen9_bc: Introduce TGP PCH DDC pin mappings

With the introduction of gen9_bc, where Intel combines Cometlake CPUs with
a Tigerpoint PCH, we'll need to introduce new DDC pin mappings for this
platform in order to make all of the display connectors work. So, let's do
that.

Changes since v4:
* Split this into it's own patch - vsyrjala

Cc: Matt Roper 
Cc: Jani Nikula 
Cc: Ville Syrjala 
[originally from Tejas's work]
Signed-off-by: Tejas Upadhyay 
Signed-off-by: Lyude Paul 
---
 drivers/gpu/drm/i915/display/intel_bios.c |  9 +
 drivers/gpu/drm/i915/display/intel_hdmi.c | 20 
 2 files changed, 29 insertions(+)

diff --git a/drivers/gpu/drm/i915/display/intel_bios.c 
b/drivers/gpu/drm/i915/display/intel_bios.c
index 7118530a1c38..1289f9d437e4 100644
--- a/drivers/gpu/drm/i915/display/intel_bios.c
+++ b/drivers/gpu/drm/i915/display/intel_bios.c
@@ -1638,6 +1638,12 @@ static const u8 adls_ddc_pin_map[] = {
[ADLS_DDC_BUS_PORT_TC4] = GMBUS_PIN_12_TC4_ICP,
 };
 
+static const u8 gen9bc_tgp_ddc_pin_map[] = {
+   [DDC_BUS_DDI_B] = GMBUS_PIN_2_BXT,
+   [DDC_BUS_DDI_C] = GMBUS_PIN_9_TC1_ICP,
+   [DDC_BUS_DDI_D] = GMBUS_PIN_10_TC2_ICP,
+};
+
 static u8 map_ddc_pin(struct drm_i915_private *dev_priv, u8 vbt_pin)
 {
const u8 *ddc_pin_map;
@@ -1651,6 +1657,9 @@ static u8 map_ddc_pin(struct drm_i915_private *dev_priv, 
u8 vbt_pin)
} else if (IS_ROCKETLAKE(dev_priv) && INTEL_PCH_TYPE(dev_priv) == 
PCH_TGP) {
ddc_pin_map = rkl_pch_tgp_ddc_pin_map;
n_entries = ARRAY_SIZE(rkl_pch_tgp_ddc_pin_map);
+   } else if (HAS_PCH_TGP(dev_priv) && IS_GEN9_BC(dev_priv)) {
+   ddc_pin_map = gen9bc_tgp_ddc_pin_map;
+   n_entries = ARRAY_SIZE(gen9bc_tgp_ddc_pin_map);
} else if (INTEL_PCH_TYPE(dev_priv) >= PCH_ICP) {
ddc_pin_map = icp_ddc_pin_map;
n_entries = ARRAY_SIZE(icp_ddc_pin_map);
diff --git a/drivers/gpu/drm/i915/display/intel_hdmi.c 
b/drivers/gpu/drm/i915/display/intel_hdmi.c
index dad54e116bc4..49528d07c7f3 100644
--- a/drivers/gpu/drm/i915/display/intel_hdmi.c
+++ b/drivers/gpu/drm/i915/display/intel_hdmi.c
@@ -3138,6 +3138,24 @@ static u8 rkl_port_to_ddc_pin(struct drm_i915_private 
*dev_priv, enum port port)
return GMBUS_PIN_1_BXT + phy;
 }
 
+static u8 gen9bc_port_to_ddc_pin(struct drm_i915_private *i915, enum port port)
+{
+   enum phy phy = intel_port_to_phy(i915, port);
+
+   drm_WARN_ON(&i915->drm, port == PORT_A);
+
+   /*
+* Pin mapping for GEN9 BC depends on which PCH is present.  With TGP,
+* final two outputs use type-c pins, even though they're actually
+* combo outputs.  With CMP, the traditional DDI A-D pins are used for
+* all outputs.
+*/
+   if (INTEL_PCH_TYPE(i915) >= PCH_TGP && phy >= PHY_C)
+   return GMBUS_PIN_9_TC1_ICP + phy - PHY_C;
+
+   return GMBUS_PIN_1_BXT + phy;
+}
+
 static u8 dg1_port_to_ddc_pin(struct drm_i915_private *dev_priv, enum port 
port)
 {
return intel_port_to_phy(dev_priv, port) + 1;
@@ -3202,6 +3220,8 @@ static u8 intel_hdmi_ddc_pin(struct intel_encoder 
*encoder)
ddc_pin = dg1_port_to_ddc_pin(dev_priv, port);
else if (IS_ROCKETLAKE(dev_priv))
ddc_pin = rkl_port_to_ddc_pin(dev_priv, port);
+   else if (IS_GEN9_BC(dev_priv) && HAS_PCH_TGP(dev_priv))
+   ddc_pin = gen9bc_port_to_ddc_pin(dev_priv, port);
else if (HAS_PCH_MCC(dev_priv))
ddc_pin = mcc_port_to_ddc_pin(dev_priv, port);
else if (INTEL_PCH_TYPE(dev_priv) >= PCH_ICP)
-- 
2.29.2

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

[PATCH v5 1/4] drm/i915/gen9_bc: Recognize TGP PCH + CML combos

Since Intel has introduced the gen9_bc platform, a combination of
Tigerpoint PCHs and CML CPUs, let's recognize such platforms as valid and
avoid WARNing on them.

Changes since v4:
* Split this into it's own patch - vsyrjala

Cc: Matt Roper 
Cc: Jani Nikula 
Cc: Ville Syrjala 
[originally from Tejas's work]
Signed-off-by: Tejas Upadhyay 
Signed-off-by: Lyude Paul 
---
 drivers/gpu/drm/i915/intel_pch.c | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

diff --git a/drivers/gpu/drm/i915/intel_pch.c b/drivers/gpu/drm/i915/intel_pch.c
index 4813207fc053..7476f0e063c6 100644
--- a/drivers/gpu/drm/i915/intel_pch.c
+++ b/drivers/gpu/drm/i915/intel_pch.c
@@ -121,7 +121,8 @@ intel_pch_type(const struct drm_i915_private *dev_priv, 
unsigned short id)
case INTEL_PCH_TGP2_DEVICE_ID_TYPE:
drm_dbg_kms(&dev_priv->drm, "Found Tiger Lake LP PCH\n");
drm_WARN_ON(&dev_priv->drm, !IS_TIGERLAKE(dev_priv) &&
-   !IS_ROCKETLAKE(dev_priv));
+   !IS_ROCKETLAKE(dev_priv) &&
+   !IS_GEN9_BC(dev_priv));
return PCH_TGP;
case INTEL_PCH_JSP_DEVICE_ID_TYPE:
case INTEL_PCH_JSP2_DEVICE_ID_TYPE:
-- 
2.29.2

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

Re: [PATCH 0/9] Add support for SVM atomics in Nouveau

2021-02-09 Thread Jerome Glisse

On Tue, Feb 09, 2021 at 09:35:20AM -0400, Jason Gunthorpe wrote:
> On Tue, Feb 09, 2021 at 11:57:28PM +1100, Alistair Popple wrote:
> > On Tuesday, 9 February 2021 9:27:05 PM AEDT Daniel Vetter wrote:
> > > >
> > > > Recent changes to pin_user_pages() prevent the creation of pinned pages 
> > > > in
> > > > ZONE_MOVABLE. This series allows pinned pages to be created in 
> > ZONE_MOVABLE
> > > > as attempts to migrate may fail which would be fatal to userspace.
> > > >
> > > > In this case migration of the pinned page is unnecessary as the page 
> > > > can 
> > be
> > > > unpinned at anytime by having the driver revoke atomic permission as it
> > > > does for the migrate_to_ram() callback. However a method of calling this
> > > > when memory needs to be moved has yet to be resolved so any discussion 
> > > > is
> > > > welcome.
> > > 
> > > Why do we need to pin for gpu atomics? You still have the callback for
> > > cpu faults, so you
> > > can move the page as needed, and hence a long-term pin sounds like the
> > > wrong approach.
> > 
> > Technically a real long term unmoveable pin isn't required, because as you 
> > say 
> > the page can be moved as needed at any time. However I needed some way of 
> > stopping the CPU page from being freed once the userspace mappings for it 
> > had 
> > been removed. 
> 
> The issue is you took the page out of the PTE it belongs to, which
> makes it orphaned and unlocatable by the rest of the mm?
> 
> Ideally this would leave the PTE in place so everything continues to
> work, just disable CPU access to it.
> 
> Maybe some kind of special swap entry?
> 
> I also don't much like the use of ZONE_DEVICE here, that should only
> be used for actual device memory, not as a temporary proxy for CPU
> pages.. Having two struct pages refer to the same physical memory is
> pretty ugly.
> 
> > The normal solution of registering an MMU notifier to unpin the page when 
> > it 
> > needs to be moved also doesn't work as the CPU page tables now point to the
> > device-private page and hence the migration code won't call any invalidate 
> > notifiers for the CPU page.
> 
> The fact the page is lost from the MM seems to be the main issue here.
> 
> > Yes, I would like to avoid the long term pin constraints as well if 
> > possible I 
> > just haven't found a solution yet. Are you suggesting it might be possible 
> > to 
> > add a callback in the page migration logic to specially deal with moving 
> > these 
> > pages?
> 
> How would migration even find the page?

Migration can scan memory from physical address (isolate_migratepages_range())
So the CPU mapping is not the only path to get to a page.

Cheers,
Jérôme

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

[Bug 211425] [drm:atom_op_jump] ERROR atombios stuck in loop for more than 20secs aborting

2021-02-09 Thread bugzilla-daemon

https://bugzilla.kernel.org/show_bug.cgi?id=211425

--- Comment #6 from Andreas (icedragon...@web.de) ---
(In reply to Alex Deucher from comment #5)
> If this is a regression can you bisect?
I can try to bisect at next weekend.

-- 
You may reply to this email to add a comment.

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 2/6] dt-bindings: display: rockchip-dsi: add optional #phy-cells property

On Tue, 02 Feb 2021 15:56:28 +0100, Heiko Stuebner wrote:
> From: Heiko Stuebner 
> 
> The Rockchip DSI controller on some SoCs also controls a bidrectional
> dphy, which would be connected to an Image Signal Processor as a phy
> in the rx configuration.
> 
> So allow a #phy-cells property for the dsi controller.
> 
> Signed-off-by: Heiko Stuebner 
> ---
>  .../bindings/display/rockchip/dw_mipi_dsi_rockchip.txt   | 1 +
>  1 file changed, 1 insertion(+)
> 

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

Re: [PATCH v3 1/2] dt-bindings: panel: Add Samsung S6E3FA2 panel

On Mon, 01 Feb 2021 18:53:05 +0200, Iskren Chernev wrote:
> The Samsung S6E3FA2 AMOLED cmd LCD panel is used on the Samsung Galaxy
> S5 (klte).
> 
> Signed-off-by: Iskren Chernev 
> ---
> Add a simple generated panel driver that supports on/off and the corresponding
> binding documentation.
> 
> Changes in v3:
> - fix dt_binding_check issue with missing include
> - fix panel type (cmd) in kconfig description
> 
> Changes in v2:
> - move bindings to separate file, add 2 regulators
> - add standalone panel driver
> 
> v1: https://lkml.org/lkml/2020/12/30/293
> v2: https://lkml.org/lkml/2021/2/1/313
> 
>  .../display/panel/samsung,s6e3fa2.yaml| 64 +++
>  1 file changed, 64 insertions(+)
>  create mode 100644 
> Documentation/devicetree/bindings/display/panel/samsung,s6e3fa2.yaml
> 

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

Re: [PATCH 0/9] Add support for SVM atomics in Nouveau

2021-02-09 Thread John Hubbard


On 2/9/21 5:37 AM, Daniel Vetter wrote:

On Tue, Feb 9, 2021 at 1:57 PM Alistair Popple  wrote:


On Tuesday, 9 February 2021 9:27:05 PM AEDT Daniel Vetter wrote:


Recent changes to pin_user_pages() prevent the creation of pinned pages in
ZONE_MOVABLE. This series allows pinned pages to be created in

ZONE_MOVABLE

as attempts to migrate may fail which would be fatal to userspace.

In this case migration of the pinned page is unnecessary as the page can

be

unpinned at anytime by having the driver revoke atomic permission as it
does for the migrate_to_ram() callback. However a method of calling this
when memory needs to be moved has yet to be resolved so any discussion is
welcome.


Why do we need to pin for gpu atomics? You still have the callback for
cpu faults, so you
can move the page as needed, and hence a long-term pin sounds like the
wrong approach.


Technically a real long term unmoveable pin isn't required, because as you say
the page can be moved as needed at any time. However I needed some way of
stopping the CPU page from being freed once the userspace mappings for it had
been removed. Obviously I could have just used get_page() but from the
perspective of page migration the result is much the same as a pin - a page
which can't be moved because of the extra refcount.


long term pin vs short term page reference aren't fully fleshed out.
But the rule more or less is:
- short term page reference: _must_ get released in finite time for
migration and other things, either because you have a callback, or
because it's just for direct I/O, which will complete. This means
short term pins will delay migration, but not foul it complete



GPU atomic operations to sysmem are hard to categorize, because because 
application
programmers could easily write programs that do a long series of atomic 
operations.
Such a program would be a little weird, but it's hard to rule out.




- long term pin: the page cannot be moved, all migration must fail.
Also this will have an impact on COW behaviour for fork (but not sure
where those patches are, John Hubbard will know).



That would be Jason's commit 57efa1fe59576 ("mm/gup: prevent gup_fast from 
racing
with COW during fork"), which is in linux-next 20201216.




So I think for your use case here you want a) short term page
reference to make sure it doesn't disappear plus b) callback to make
sure migrate isn't blocked.

Breaking ZONE_MOVEABLE with either allowing long term pins or failing
migrations because you don't release your short term page reference
isn't good.


The normal solution of registering an MMU notifier to unpin the page when it
needs to be moved also doesn't work as the CPU page tables now point to the
device-private page and hence the migration code won't call any invalidate
notifiers for the CPU page.


Yeah you need some other callback for migration on the page directly.
it's a bit awkward since there is one already for struct
address_space, but that's own by the address_space/page cache, not
HMM. So I think we need something else, maybe something for each
ZONE_DEVICE?



This direction sounds at least...possible. Using MMU notifiers instead of pins
is definitely appealing. I'm not quite clear on the callback idea above, but
overall it seems like taking advantage of the ZONE_DEVICE tracking of pages
(without having to put anything additional in each struct page), could work.

Additional notes or ideas here are definitely welcome.



thanks,
--
John Hubbard
NVIDIA
___
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

Re: [PATCH 2/3] dt-bindings: Add DT bindings for Displaytech DT050TFT-PTS

On Sat, 30 Jan 2021 19:11:13 +0100, Marek Vasut wrote:
> Add DT bindings for Displaytech DT050TFT-PTS 5.0" (800x480)
> color TFT LCD panel, connected over DPI.
> 
> Signed-off-by: Marek Vasut 
> To: dri-devel@lists.freedesktop.org
> Cc: Rob Herring 
> Cc: Sam Ravnborg 
> Cc: devicet...@vger.kernel.org
> ---
>  .../devicetree/bindings/display/panel/panel-simple.yaml | 2 ++
>  1 file changed, 2 insertions(+)
> 

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

Re: [PATCH 1/3] dt-bindings: Add vendor prefix for Displaytech

On Sat, 30 Jan 2021 19:11:12 +0100, Marek Vasut wrote:
> The Displaytech Ltd. is an LCD panel manufacturer.
> 
> Signed-off-by: Marek Vasut 
> To: dri-devel@lists.freedesktop.org
> Cc: Eric Anholt 
> Cc: Rob Herring 
> Cc: Sam Ravnborg 
> Cc: devicet...@vger.kernel.org
> ---
>  Documentation/devicetree/bindings/vendor-prefixes.yaml | 2 ++
>  1 file changed, 2 insertions(+)
> 

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

Re: [PATCH 1/2] dt-bindings: drm/bridge: ti-sn65dsi83: Add TI SN65DSI83 bindings

On Sat, Jan 30, 2021 at 07:10:13PM +0100, Marek Vasut wrote:
> Add DT binding document for TI SN65DSI83 DSI to LVDS bridge.
> 
> Signed-off-by: Marek Vasut 
> Cc: Douglas Anderson 
> Cc: Laurent Pinchart 
> Cc: Linus Walleij 
> Cc: Sam Ravnborg 
> Cc: Stephen Boyd 
> Cc: devicet...@vger.kernel.org
> To: dri-devel@lists.freedesktop.org
> ---
>  .../bindings/display/bridge/ti,sn65dsi83.yaml | 128 ++
>  1 file changed, 128 insertions(+)
>  create mode 100644 
> Documentation/devicetree/bindings/display/bridge/ti,sn65dsi83.yaml
> 
> diff --git 
> a/Documentation/devicetree/bindings/display/bridge/ti,sn65dsi83.yaml 
> b/Documentation/devicetree/bindings/display/bridge/ti,sn65dsi83.yaml
> new file mode 100644
> index ..77e1bafd8cd8
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/display/bridge/ti,sn65dsi83.yaml
> @@ -0,0 +1,128 @@
> +# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause)
> +%YAML 1.2
> +---
> +$id: http://devicetree.org/schemas/display/bridge/ti,sn65dsi83.yaml#
> +$schema: http://devicetree.org/meta-schemas/core.yaml#
> +
> +title: SN65DSI83 DSI to LVDS bridge chip
> +
> +maintainers:
> +  - Marek Vasut 
> +
> +description: |
> +  The Texas Instruments SN65DSI83 bridge takes MIPI DSI in and outputs LVDS.
> +  
> https://www.ti.com/general/docs/lit/getliterature.tsp?genericPartNumber=sn65dsi83&fileType=pdf
> +
> +properties:
> +  compatible:
> +const: ti,sn65dsi83
> +
> +  reg:
> +const: 0x2d
> +
> +  enable-gpios:
> +maxItems: 1
> +description: GPIO specifier for bridge_en pin (active high).
> +
> +  ports:
> +type: object
> +additionalProperties: false
> +
> +properties:
> +  "#address-cells":
> +const: 1
> +
> +  "#size-cells":
> +const: 0
> +
> +  port@0:
> +type: object
> +additionalProperties: false
> +
> +description:
> +  Video port for MIPI DSI input
> +
> +properties:
> +  reg:
> +const: 0
> +
> +  endpoint:
> +type: object
> +additionalProperties: false
> +properties:
> +  remote-endpoint: true
> +  data-lanes:
> +description: array of physical DSI data lane indexes.

This all needs to use graph.yaml and video-interfaces.yaml. The latter 
is in the media tree. See examples there for what to do. It will have to 
wait for rc1 to apply to drm-misc.

For data-lanes, you need to specify how many lanes are valid. If there's 
only 1 possible setting (in the h/w, not driver), then it doesn't need 
to be in DT.

I agree with Doug on adding the regulators. Hard to get wrong in the 
binding. You or someone can add them to the driver when you can test.

> +
> +required:
> +  - reg
> +
> +  port@1:
> +type: object
> +additionalProperties: false
> +
> +description:
> +  Video port for LVDS output (panel or bridge).
> +
> +properties:
> +  reg:
> +const: 1
> +
> +  endpoint:
> +type: object
> +additionalProperties: false
> +properties:
> +  remote-endpoint: true
> +
> +required:
> +  - reg
> +
> +required:
> +  - "#address-cells"
> +  - "#size-cells"
> +  - port@0
> +  - port@1
> +
> +required:
> +  - compatible
> +  - reg
> +  - enable-gpios
> +  - ports
> +
> +additionalProperties: false
> +
> +examples:
> +  - |
> +#include 
> +
> +i2c {
> +  #address-cells = <1>;
> +  #size-cells = <0>;
> +
> +  bridge@2d {
> +compatible = "ti,sn65dsi83";
> +reg = <0x2d>;
> +
> +enable-gpios = <&gpio2 1 GPIO_ACTIVE_HIGH>;
> +
> +ports {
> +  #address-cells = <1>;
> +  #size-cells = <0>;
> +
> +  port@0 {
> +reg = <0>;
> +endpoint {
> +  remote-endpoint = <&dsi0_out>;
> +  data-lanes = <1 2 3 4>;
> +};
> +  };
> +
> +  port@1 {
> +reg = <1>;
> +endpoint {
> +  remote-endpoint = <&panel_in_lvds>;
> +};
> +  };
> +};
> +  };
> +};
> -- 
> 2.29.2
> 
___
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

Re: [RFC][PATCH v6 1/7] drm: Add a sharable drm page-pool implementation

On Tue, Feb 9, 2021 at 6:46 PM Christian König  wrote:
>
>
>
> Am 09.02.21 um 18:33 schrieb Suren Baghdasaryan:
> > On Tue, Feb 9, 2021 at 4:57 AM Christian König  
> > wrote:
> >> Am 09.02.21 um 13:11 schrieb Christian König:
> >>> [SNIP]
> >> +void drm_page_pool_add(struct drm_page_pool *pool, struct page *page)
> >> +{
> >> + spin_lock(&pool->lock);
> >> + list_add_tail(&page->lru, &pool->items);
> >> + pool->count++;
> >> + atomic_long_add(1 << pool->order, &total_pages);
> >> + spin_unlock(&pool->lock);
> >> +
> >> + mod_node_page_state(page_pgdat(page),
> >> NR_KERNEL_MISC_RECLAIMABLE,
> >> + 1 << pool->order);
> > Hui what? What should that be good for?
>  This is a carryover from the ION page pool implementation:
>  https://nam11.safelinks.protection.outlook.com/?url=https%3A%2F%2Fgit.kernel.org%2Fpub%2Fscm%2Flinux%2Fkernel%2Fgit%2Ftorvalds%2Flinux.git%2Ftree%2Fdrivers%2Fstaging%2Fandroid%2Fion%2Fion_page_pool.c%3Fh%3Dv5.10%23n28&data=04%7C01%7Cchristian.koenig%40amd.com%7Cdff8edcd4d147a5b08d8cd20cff2%7C3dd8961fe4884e608e11a82d994e183d%7C0%7C0%7C637484888114923580%7CUnknown%7CTWFpbGZsb3d8eyJWIjoiMC4wLjAwMDAiLCJQIjoiV2luMzIiLCJBTiI6Ik1haWwiLCJXVCI6Mn0%3D%7C1000&sdata=9%2BIBC0tezSV6Ci4S3kWfW%2BQvJm4mdunn3dF6C0kyfCw%3D&reserved=0
> 
> 
>  My sense is it helps with the vmstat/meminfo accounting so folks can
>  see the cached pages are shrinkable/freeable. This maybe falls under
>  other dmabuf accounting/stats discussions, so I'm happy to remove it
>  for now, or let the drivers using the shared page pool logic handle
>  the accounting themselves?
> >> Intentionally separated the discussion for that here.
> >>
> >> As far as I can see this is just bluntly incorrect.
> >>
> >> Either the page is reclaimable or it is part of our pool and freeable
> >> through the shrinker, but never ever both.
> > IIRC the original motivation for counting ION pooled pages as
> > reclaimable was to include them into /proc/meminfo's MemAvailable
> > calculations. NR_KERNEL_MISC_RECLAIMABLE defined as "reclaimable
> > non-slab kernel pages" seems like a good place to account for them but
> > I might be wrong.
>
> Yeah, that's what I see here as well. But exactly that is utterly nonsense.
>
> Those pages are not "free" in the sense that get_free_page could return
> them directly.

Well on Android that is kinda true, because Android has it's
oom-killer (way back it was just a shrinker callback, not sure how it
works now), which just shot down all the background apps. So at least
some of that (everything used by background apps) is indeed
reclaimable on Android.

But that doesn't hold on Linux in general, so we can't really do this
for common code.

Also I had a long meeting with Suren, John and other googles
yesterday, and the aim is now to try and support all the Android gpu
memory accounting needs with cgroups. That should work, and it will
allow Android to handle all the Android-ism in a clean way in upstream
code. Or that's at least the plan.

I think the only thing we identified that Android still needs on top
is the dma-buf sysfs stuff, so that shared buffers (which on Android
are always dma-buf, and always stay around as dma-buf fd throughout
their lifetime) can be listed/analyzed with full detail.

But aside from this the plan for all the per-process or per-heap
account, oom-killer integration and everything else is planned to be
done with cgroups. Android (for now) only needs to account overall gpu
memory since none of it is swappable on android drivers anyway, plus
no vram, so not much needed.

Cheers, Daniel

>
> Regards,
> Christian.
>
> >
> >> In the best case this just messes up the accounting, in the worst case
> >> it can cause memory corruption.
> >>
> >> Christian.
>

-- 
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

[PATCH v2 2/2] dma-buf: heaps: Fix the name used when exporting dmabufs to be the actual heap name

By default dma_buf_export() sets the exporter name to be
KBUILD_MODNAME. Unfortunately this may not be identical to the
string used as the heap name (ie: "system" vs "system_heap").

This can cause some minor confusion with tooling, and there is
the future potential where multiple heap types may be exported
by the same module (but would all have the same name).

So to avoid all this, set the exporter exp_name to the heap name.

Cc: Daniel Vetter 
Cc: Sumit Semwal 
Cc: Liam Mark 
Cc: Chris Goldsworthy 
Cc: Laura Abbott 
Cc: Brian Starkey 
Cc: Hridya Valsaraju 
Cc: Suren Baghdasaryan 
Cc: Sandeep Patil 
Cc: Daniel Mentz 
Cc: Ørjan Eide 
Cc: Robin Murphy 
Cc: Ezequiel Garcia 
Cc: Simon Ser 
Cc: James Jones 
Cc: linux-me...@vger.kernel.org
Cc: dri-devel@lists.freedesktop.org
Signed-off-by: John Stultz 
---
 drivers/dma-buf/heaps/cma_heap.c| 1 +
 drivers/dma-buf/heaps/system_heap.c | 1 +
 2 files changed, 2 insertions(+)

diff --git a/drivers/dma-buf/heaps/cma_heap.c b/drivers/dma-buf/heaps/cma_heap.c
index 364fc2f3e499..62465d61ccc7 100644
--- a/drivers/dma-buf/heaps/cma_heap.c
+++ b/drivers/dma-buf/heaps/cma_heap.c
@@ -339,6 +339,7 @@ static int cma_heap_allocate(struct dma_heap *heap,
buffer->pagecount = pagecount;
 
/* create the dmabuf */
+   exp_info.exp_name = dma_heap_get_name(heap);
exp_info.ops = &cma_heap_buf_ops;
exp_info.size = buffer->len;
exp_info.flags = fd_flags;
diff --git a/drivers/dma-buf/heaps/system_heap.c 
b/drivers/dma-buf/heaps/system_heap.c
index 17e0e9a68baf..2d4afc79c700 100644
--- a/drivers/dma-buf/heaps/system_heap.c
+++ b/drivers/dma-buf/heaps/system_heap.c
@@ -388,6 +388,7 @@ static int system_heap_allocate(struct dma_heap *heap,
}
 
/* create the dmabuf */
+   exp_info.exp_name = dma_heap_get_name(heap);
exp_info.ops = &system_heap_buf_ops;
exp_info.size = buffer->len;
exp_info.flags = fd_flags;
-- 
2.25.1

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

[PATCH v2 1/2] dma-buf: dma-heap: Provide accessor to get heap name

It can be useful to access the name for the heap,
so provide an accessor to do so.

Cc: Daniel Vetter 
Cc: Sumit Semwal 
Cc: Liam Mark 
Cc: Chris Goldsworthy 
Cc: Laura Abbott 
Cc: Brian Starkey 
Cc: Hridya Valsaraju 
Cc: Suren Baghdasaryan 
Cc: Sandeep Patil 
Cc: Daniel Mentz 
Cc: Ørjan Eide 
Cc: Robin Murphy 
Cc: Ezequiel Garcia 
Cc: Simon Ser 
Cc: James Jones 
Cc: linux-me...@vger.kernel.org
Cc: dri-devel@lists.freedesktop.org
Signed-off-by: John Stultz 
---
v2:
* Make sure to use "const char *" as Reported-by: kernel test robot 

---
 drivers/dma-buf/dma-heap.c | 12 
 include/linux/dma-heap.h   |  9 +
 2 files changed, 21 insertions(+)

diff --git a/drivers/dma-buf/dma-heap.c b/drivers/dma-buf/dma-heap.c
index afd22c9dbdcf..70e410c64c1c 100644
--- a/drivers/dma-buf/dma-heap.c
+++ b/drivers/dma-buf/dma-heap.c
@@ -190,6 +190,18 @@ void *dma_heap_get_drvdata(struct dma_heap *heap)
return heap->priv;
 }
 
+/**
+ * dma_heap_get_name() - get heap name
+ * @heap: DMA-Heap to retrieve private data for
+ *
+ * Returns:
+ * The char* for the heap name.
+ */
+const char *dma_heap_get_name(struct dma_heap *heap)
+{
+   return heap->name;
+}
+
 struct dma_heap *dma_heap_add(const struct dma_heap_export_info *exp_info)
 {
struct dma_heap *heap, *h, *err_ret;
diff --git a/include/linux/dma-heap.h b/include/linux/dma-heap.h
index 454e354d1ffb..83b8cfb2d760 100644
--- a/include/linux/dma-heap.h
+++ b/include/linux/dma-heap.h
@@ -50,6 +50,15 @@ struct dma_heap_export_info {
  */
 void *dma_heap_get_drvdata(struct dma_heap *heap);
 
+/**
+ * dma_heap_get_name() - get heap name
+ * @heap: DMA-Heap to retrieve private data for
+ *
+ * Returns:
+ * The char* for the heap name.
+ */
+const char *dma_heap_get_name(struct dma_heap *heap);
+
 /**
  * dma_heap_add - adds a heap to dmabuf heaps
  * @exp_info:  information needed to register this heap
-- 
2.25.1

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

Re: [PATCH] drm/radeon: Simplify bool comparison

Applied.  Thanks!

Alex

On Mon, Feb 8, 2021 at 5:17 AM Jiapeng Chong
 wrote:
>
> Fix the following coccicheck warning:
>
> ./drivers/gpu/drm/radeon/rs690.c:190:6-35: WARNING: Comparison to bool.
>
> Reported-by: Abaci Robot 
> Signed-off-by: Jiapeng Chong 
> ---
>  drivers/gpu/drm/radeon/rs690.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/drivers/gpu/drm/radeon/rs690.c b/drivers/gpu/drm/radeon/rs690.c
> index c296f94..7bc302a 100644
> --- a/drivers/gpu/drm/radeon/rs690.c
> +++ b/drivers/gpu/drm/radeon/rs690.c
> @@ -187,7 +187,7 @@ static void rs690_mc_init(struct radeon_device *rdev)
> /* FastFB shall be used with UMA memory. Here it is simply 
> disabled when sideport
>  * memory is present.
>  */
> -   if (rdev->mc.igp_sideport_enabled == false && radeon_fastfb 
> == 1) {
> +   if (!rdev->mc.igp_sideport_enabled && radeon_fastfb == 1) {
> DRM_INFO("Direct mapping: aper base at 0x%llx, 
> replaced by direct mapping base 0x%llx.\n",
> (unsigned long 
> long)rdev->mc.aper_base, k8_addr);
> rdev->mc.aper_base = (resource_size_t)k8_addr;
> --
> 1.8.3.1
>
> ___
> dri-devel mailing list
> dri-devel@lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/dri-devel
___
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

Re: [PATCH] drm/amd/display: Simplify bool comparison

Applied.  Thanks!

Alex

On Mon, Feb 8, 2021 at 5:29 AM Jiapeng Chong
 wrote:
>
> Fix the following coccicheck warning:
>
> ./drivers/gpu/drm/amd/display/dc/inc/hw/clk_mgr_internal.h:319:11-23:
> WARNING: Comparison to bool.
>
> Reported-by: Abaci Robot 
> Signed-off-by: Jiapeng Chong 
> ---
>  drivers/gpu/drm/amd/display/dc/inc/hw/clk_mgr_internal.h | 4 ++--
>  1 file changed, 2 insertions(+), 2 deletions(-)
>
> diff --git a/drivers/gpu/drm/amd/display/dc/inc/hw/clk_mgr_internal.h 
> b/drivers/gpu/drm/amd/display/dc/inc/hw/clk_mgr_internal.h
> index ffd3769..316301f 100644
> --- a/drivers/gpu/drm/amd/display/dc/inc/hw/clk_mgr_internal.h
> +++ b/drivers/gpu/drm/amd/display/dc/inc/hw/clk_mgr_internal.h
> @@ -309,9 +309,9 @@ static inline bool should_set_clock(bool safe_to_lower, 
> int calc_clk, int cur_cl
>  static inline bool should_update_pstate_support(bool safe_to_lower, bool 
> calc_support, bool cur_support)
>  {
> if (cur_support != calc_support) {
> -   if (calc_support == true && safe_to_lower)
> +   if (calc_support && safe_to_lower)
> return true;
> -   else if (calc_support == false && !safe_to_lower)
> +   else if (!calc_support && !safe_to_lower)
> return true;
> }
>
> --
> 1.8.3.1
>
> ___
> dri-devel mailing list
> dri-devel@lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/dri-devel
___
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

Re: [PATCH] drm/amdgpu: fix unnecessary NULL check warnings

On Tue, Feb 9, 2021 at 3:44 AM Tian Tao  wrote:
>
> Remove NULL checks before vfree() to fix these warnings:
> drivers/gpu/drm/amd/amdgpu/amdgpu_cs.c:102:2-8: WARNING: NULL
> check before some freeing functions is not needed.
>
> Signed-off-by: Tian Tao 

Applied.  Thanks!

Alex


> ---
>  drivers/gpu/drm/amd/amdgpu/amdgpu_cs.c | 3 +--
>  1 file changed, 1 insertion(+), 2 deletions(-)
>
> diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_cs.c 
> b/drivers/gpu/drm/amd/amdgpu/amdgpu_cs.c
> index 594a010..3e240b9 100644
> --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_cs.c
> +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_cs.c
> @@ -98,8 +98,7 @@ static int amdgpu_cs_bo_handles_chunk(struct 
> amdgpu_cs_parser *p,
> return 0;
>
>  error_free:
> -   if (info)
> -   kvfree(info);
> +   kvfree(info);
>
> return r;
>  }
> --
> 2.7.4
>
> ___
> dri-devel mailing list
> dri-devel@lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/dri-devel
___
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

Re: [PATCH v4 1/3] dt-bindings:drm/bridge:anx7625:add vendor define flags

On Thu, Jan 28, 2021 at 11:08:26AM +0800, Xin Ji wrote:
> Add 'bus-type' and 'data-lanes' define for port0, add HDCP support
> flag and DP tx lane0 and lane1 swing register array define.
> 
> Signed-off-by: Xin Ji 
> ---
>  .../bindings/display/bridge/analogix,anx7625.yaml  | 54 
> +-
>  1 file changed, 53 insertions(+), 1 deletion(-)
> 
> diff --git 
> a/Documentation/devicetree/bindings/display/bridge/analogix,anx7625.yaml 
> b/Documentation/devicetree/bindings/display/bridge/analogix,anx7625.yaml
> index c789784..048deec 100644
> --- a/Documentation/devicetree/bindings/display/bridge/analogix,anx7625.yaml
> +++ b/Documentation/devicetree/bindings/display/bridge/analogix,anx7625.yaml
> @@ -34,6 +34,24 @@ properties:
>  description: used for reset chip control, RESET_N pin B7.
>  maxItems: 1
>  
> +  analogix,lane0-swing:
> +$ref: /schemas/types.yaml#/definitions/uint32-array
> +maxItems: 20
> +description:
> +  an array of swing register setting for DP tx lane0 PHY, please don't
> +  add this property, or contact vendor.
> +
> +  analogix,lane1-swing:
> +$ref: /schemas/types.yaml#/definitions/uint32-array
> +maxItems: 20
> +description:
> +  an array of swing register setting for DP tx lane1 PHY, please don't
> +  add this property, or contact vendor.
> +
> +  analogix,hdcp-support:
> +type: boolean
> +description: indicate the DP tx HDCP support or not.

Please show the new properties in the example.

> +
>ports:
>  $ref: /schemas/graph.yaml#/properties/ports
>  
> @@ -41,13 +59,45 @@ properties:
>port@0:
>  $ref: /schemas/graph.yaml#/properties/port
>  description:
> -  Video port for MIPI DSI input.
> +  Video port for MIPI input.
> +
> +properties:
> +  endpoint:
> +type: object
> +additionalProperties: false
> +
> +# Properties described in
> +# Documentation/devicetree/bindings/media/video-interfaces.txt

Now video-interfaces.yaml which should have a $ref here. It's currently 
in media tree and linux-next. Follow the examples there. You'll also 
have to wait for 5.12-rc1 to apply to drm-misc.

> +properties:
> +  remote-endpoint: true
> +  bus-type: true
> +  data-lanes: true
> +
> +required:
> +  - remote-endpoint
> +
> +required:
> +  - endpoint
> +
>  
>port@1:
>  $ref: /schemas/graph.yaml#/properties/port
>  description:
>Video port for panel or connector.
>  
> +properties:
> +  endpoint:
> +type: object
> +additionalProperties: false
> +
> +# Properties described in
> +# Documentation/devicetree/bindings/media/video-interfaces.txt
> +properties:
> +  remote-endpoint: true
> +
> +required:
> +  - remote-endpoint
> +
>  required:
>- port@0
>- port@1
> @@ -81,6 +131,8 @@ examples:
>  reg = <0>;
>  anx7625_in: endpoint {
>  remote-endpoint = <&mipi_dsi>;
> +bus-type = <5>;
> +data-lanes = <0 1 2 3>;
>  };
>  };
>  
> -- 
> 2.7.4
> 
___
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

Re: [PATCH] dt-bindings: mxsfb: Add interconnect bindings for LCDIF path

On Wed, 27 Jan 2021 12:49:01 +0100, Martin Kepplinger wrote:
> Add optional interconnect properties for the dram path requests.
> 
> Signed-off-by: Martin Kepplinger 
> ---
>  Documentation/devicetree/bindings/display/fsl,lcdif.yaml | 8 
>  1 file changed, 8 insertions(+)
> 

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

Re: [PATCH v3 11/14] dt-bindings: display: bridge: Add i.MX8qm/qxp LVDS display bridge binding

On Wed, 27 Jan 2021 16:51:25 +0800, Liu Ying wrote:
> This patch adds bindings for i.MX8qm/qxp LVDS display bridge(LDB).
> 
> Signed-off-by: Liu Ying 
> ---
> v2->v3:
> * Drop 'fsl,syscon' property. (Rob)
> * Mention the CSR module controls LDB.
> 
> v1->v2:
> * Use graph schema. (Laurent)
> * Side note i.MX8qxp LDB official name 'pixel mapper'. (Laurent)
> 
>  .../bindings/display/bridge/fsl,imx8qxp-ldb.yaml   | 173 
> +
>  1 file changed, 173 insertions(+)
>  create mode 100644 
> Documentation/devicetree/bindings/display/bridge/fsl,imx8qxp-ldb.yaml
> 

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

Re: [PATCH v3 09/14] drm/bridge: imx: Add i.MX8qxp pixel link to DPI support

On Wed, Jan 27, 2021 at 04:51:23PM +0800, Liu Ying wrote:
> This patch adds a drm bridge driver for i.MX8qxp pixel link to display
> pixel interface(PXL2DPI).  The PXL2DPI interfaces the pixel link 36-bit
> data output and the DSI controller’s MIPI-DPI 24-bit data input, and
> inputs of LVDS Display Bridge(LDB) module used in LVDS mode, to remap
> the pixel color codings between those modules. The PXL2DPI is purely
> combinatorial.
> 
> Signed-off-by: Liu Ying 
> ---
> v2->v3:
> * Call syscon_node_to_regmap() to get regmap instead of
>   syscon_regmap_lookup_by_phandle().
> 
> v1->v2:
> * Drop unnecessary port availability check.
> 
>  drivers/gpu/drm/bridge/imx/Kconfig   |   8 +
>  drivers/gpu/drm/bridge/imx/Makefile  |   1 +
>  drivers/gpu/drm/bridge/imx/imx8qxp-pxl2dpi.c | 488 
> +++
>  3 files changed, 497 insertions(+)
>  create mode 100644 drivers/gpu/drm/bridge/imx/imx8qxp-pxl2dpi.c

> + p2d->regmap = syscon_node_to_regmap(np->parent);
> + if (IS_ERR(p2d->regmap)) {
> + ret = PTR_ERR(p2d->regmap);
> + if (ret != -EPROBE_DEFER)
> + DRM_DEV_ERROR(dev, "failed to get regmap: %d\n", ret);
> + return ret;
> + }
> +
> + p2d->id = of_alias_get_id(np, "pxl2dpi");

Don't add random aliases. I'd rather see a property in this node as long 
as it is specific to what this is used for (and not a generic index).

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

[PATCH v3 5/5] drm/sun4i: dw-hdmi: Fix max. frequency for H6

It turns out that reasoning for lowering max. supported frequency is
wrong. Scrambling works just fine. Several now fixed bugs prevented
proper functioning, even with rates lower than 340 MHz. Issues were just
more pronounced with higher frequencies.

Fix that by allowing max. supported frequency in HW and fix the comment.

Fixes: cd9063757a22 ("drm/sun4i: DW HDMI: Lower max. supported rate for H6")
Reviewed-by: Chen-Yu Tsai 
Tested-by: Andre Heider 
Signed-off-by: Jernej Skrabec 
---
 drivers/gpu/drm/sun4i/sun8i_dw_hdmi.c | 6 ++
 1 file changed, 2 insertions(+), 4 deletions(-)

diff --git a/drivers/gpu/drm/sun4i/sun8i_dw_hdmi.c 
b/drivers/gpu/drm/sun4i/sun8i_dw_hdmi.c
index 23773a5e0650..bbdfd5e26ec8 100644
--- a/drivers/gpu/drm/sun4i/sun8i_dw_hdmi.c
+++ b/drivers/gpu/drm/sun4i/sun8i_dw_hdmi.c
@@ -47,11 +47,9 @@ sun8i_dw_hdmi_mode_valid_h6(struct dw_hdmi *hdmi, void *data,
 {
/*
 * Controller support maximum of 594 MHz, which correlates to
-* 4K@60Hz 4:4:4 or RGB. However, for frequencies greater than
-* 340 MHz scrambling has to be enabled. Because scrambling is
-* not yet implemented, just limit to 340 MHz for now.
+* 4K@60Hz 4:4:4 or RGB.
 */
-   if (mode->clock > 34)
+   if (mode->clock > 594000)
return MODE_CLOCK_HIGH;
 
return MODE_OK;
-- 
2.30.0

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

[PATCH v3 4/5] drm/sun4i: Fix H6 HDMI PHY configuration

As it turns out, vendor HDMI PHY driver for H6 has a pretty big table
of predefined values for various pixel clocks. However, most of them are
not useful/tested because they come from reference driver code. Vendor
PHY driver is concerned with only few of those, namely 27 MHz, 74.25
MHz, 148.5 MHz, 297 MHz and 594 MHz. These are all frequencies for
standard CEA modes.

Fix sun50i_h6_cur_ctr and sun50i_h6_phy_config with the values only for
aforementioned frequencies.

Table sun50i_h6_mpll_cfg doesn't need to be changed because values are
actually frequency dependant and not so much SoC dependant. See i.MX6
documentation for explanation of those values for similar PHY.

Fixes: c71c9b2fee17 ("drm/sun4i: Add support for Synopsys HDMI PHY")
Tested-by: Andre Heider 
Signed-off-by: Jernej Skrabec 
---
 drivers/gpu/drm/sun4i/sun8i_hdmi_phy.c | 26 +-
 1 file changed, 9 insertions(+), 17 deletions(-)

diff --git a/drivers/gpu/drm/sun4i/sun8i_hdmi_phy.c 
b/drivers/gpu/drm/sun4i/sun8i_hdmi_phy.c
index 35c2133724e2..9994edf67509 100644
--- a/drivers/gpu/drm/sun4i/sun8i_hdmi_phy.c
+++ b/drivers/gpu/drm/sun4i/sun8i_hdmi_phy.c
@@ -104,29 +104,21 @@ static const struct dw_hdmi_mpll_config 
sun50i_h6_mpll_cfg[] = {
 
 static const struct dw_hdmi_curr_ctrl sun50i_h6_cur_ctr[] = {
/* pixelclkbpp8bpp10   bpp12 */
-   { 25175000,  { 0x, 0x, 0x }, },
{ 2700,  { 0x0012, 0x, 0x }, },
-   { 5940,  { 0x0008, 0x0008, 0x0008 }, },
-   { 7200,  { 0x0008, 0x0008, 0x001b }, },
-   { 7425,  { 0x0013, 0x0013, 0x0013 }, },
-   { 9000,  { 0x0008, 0x001a, 0x001b }, },
-   { 11880, { 0x001b, 0x001a, 0x001b }, },
-   { 14400, { 0x001b, 0x001a, 0x0034 }, },
-   { 18000, { 0x001b, 0x0033, 0x0034 }, },
-   { 21600, { 0x0036, 0x0033, 0x0034 }, },
-   { 23760, { 0x0036, 0x0033, 0x001b }, },
-   { 28800, { 0x0036, 0x001b, 0x001b }, },
-   { 29700, { 0x0019, 0x001b, 0x0019 }, },
-   { 33000, { 0x0036, 0x001b, 0x001b }, },
-   { 59400, { 0x003f, 0x001b, 0x001b }, },
+   { 7425,  { 0x0013, 0x001a, 0x001b }, },
+   { 14850, { 0x0019, 0x0033, 0x0034 }, },
+   { 29700, { 0x0019, 0x001b, 0x001b }, },
+   { 59400, { 0x0010, 0x001b, 0x001b }, },
{ ~0UL,  { 0x, 0x, 0x }, }
 };
 
 static const struct dw_hdmi_phy_config sun50i_h6_phy_config[] = {
/*pixelclk   symbol   term   vlev*/
-   { 7425,  0x8009, 0x0004, 0x0232},
-   { 14850, 0x8029, 0x0004, 0x0273},
-   { 59400, 0x8039, 0x0004, 0x014a},
+   { 2700,  0x8009, 0x0007, 0x02b0 },
+   { 7425,  0x8009, 0x0006, 0x022d },
+   { 14850, 0x8029, 0x0006, 0x0270 },
+   { 29700, 0x8039, 0x0005, 0x01ab },
+   { 59400, 0x8029, 0x, 0x008a },
{ ~0UL,  0x, 0x, 0x}
 };
 
-- 
2.30.0

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

[PATCH v3 3/5] drm/sun4i: dw-hdmi: always set clock rate

As expected, HDMI controller clock should always match pixel clock. In
the past, changing HDMI controller rate would seemingly worsen
situation. However, that was the result of other bugs which are now
fixed.

Fix that by removing set_rate quirk and always set clock rate.

Fixes: 40bb9d3147b2 ("drm/sun4i: Add support for H6 DW HDMI controller")
Reviewed-by: Chen-Yu Tsai 
Tested-by: Andre Heider 
Signed-off-by: Jernej Skrabec 
---
 drivers/gpu/drm/sun4i/sun8i_dw_hdmi.c | 4 +---
 drivers/gpu/drm/sun4i/sun8i_dw_hdmi.h | 1 -
 2 files changed, 1 insertion(+), 4 deletions(-)

diff --git a/drivers/gpu/drm/sun4i/sun8i_dw_hdmi.c 
b/drivers/gpu/drm/sun4i/sun8i_dw_hdmi.c
index 92add2cef2e7..23773a5e0650 100644
--- a/drivers/gpu/drm/sun4i/sun8i_dw_hdmi.c
+++ b/drivers/gpu/drm/sun4i/sun8i_dw_hdmi.c
@@ -21,8 +21,7 @@ static void sun8i_dw_hdmi_encoder_mode_set(struct drm_encoder 
*encoder,
 {
struct sun8i_dw_hdmi *hdmi = encoder_to_sun8i_dw_hdmi(encoder);
 
-   if (hdmi->quirks->set_rate)
-   clk_set_rate(hdmi->clk_tmds, mode->crtc_clock * 1000);
+   clk_set_rate(hdmi->clk_tmds, mode->crtc_clock * 1000);
 }
 
 static const struct drm_encoder_helper_funcs
@@ -295,7 +294,6 @@ static int sun8i_dw_hdmi_remove(struct platform_device 
*pdev)
 
 static const struct sun8i_dw_hdmi_quirks sun8i_a83t_quirks = {
.mode_valid = sun8i_dw_hdmi_mode_valid_a83t,
-   .set_rate = true,
 };
 
 static const struct sun8i_dw_hdmi_quirks sun50i_h6_quirks = {
diff --git a/drivers/gpu/drm/sun4i/sun8i_dw_hdmi.h 
b/drivers/gpu/drm/sun4i/sun8i_dw_hdmi.h
index d983746fa194..d4b55af0592f 100644
--- a/drivers/gpu/drm/sun4i/sun8i_dw_hdmi.h
+++ b/drivers/gpu/drm/sun4i/sun8i_dw_hdmi.h
@@ -179,7 +179,6 @@ struct sun8i_dw_hdmi_quirks {
enum drm_mode_status (*mode_valid)(struct dw_hdmi *hdmi, void *data,
   const struct drm_display_info *info,
   const struct drm_display_mode *mode);
-   unsigned int set_rate : 1;
unsigned int use_drm_infoframe : 1;
 };
 
-- 
2.30.0

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

[PATCH v3 2/5] drm/sun4i: tcon: set sync polarity for tcon1 channel

Channel 1 has polarity bits for vsync and hsync signals but driver never
sets them. It turns out that with pre-HDMI2 controllers seemingly there
is no issue if polarity is not set. However, with HDMI2 controllers
(H6) there often comes to de-synchronization due to phase shift. This
causes flickering screen. It's safe to assume that similar issues might
happen also with pre-HDMI2 controllers.

Solve issue with setting vsync and hsync polarity. Note that display
stacks with tcon top have polarity bits actually in tcon0 polarity
register.

Fixes: 9026e0d122ac ("drm: Add Allwinner A10 Display Engine support")
Reviewed-by: Chen-Yu Tsai 
Tested-by: Andre Heider 
Signed-off-by: Jernej Skrabec 
---
 drivers/gpu/drm/sun4i/sun4i_tcon.c | 25 +
 drivers/gpu/drm/sun4i/sun4i_tcon.h |  6 ++
 2 files changed, 31 insertions(+)

diff --git a/drivers/gpu/drm/sun4i/sun4i_tcon.c 
b/drivers/gpu/drm/sun4i/sun4i_tcon.c
index 6b9af4c08cd6..9f06dec0fc61 100644
--- a/drivers/gpu/drm/sun4i/sun4i_tcon.c
+++ b/drivers/gpu/drm/sun4i/sun4i_tcon.c
@@ -672,6 +672,30 @@ static void sun4i_tcon1_mode_set(struct sun4i_tcon *tcon,
 SUN4I_TCON1_BASIC5_V_SYNC(vsync) |
 SUN4I_TCON1_BASIC5_H_SYNC(hsync));
 
+   /* Setup the polarity of multiple signals */
+   if (tcon->quirks->polarity_in_ch0) {
+   val = 0;
+
+   if (mode->flags & DRM_MODE_FLAG_PHSYNC)
+   val |= SUN4I_TCON0_IO_POL_HSYNC_POSITIVE;
+
+   if (mode->flags & DRM_MODE_FLAG_PVSYNC)
+   val |= SUN4I_TCON0_IO_POL_VSYNC_POSITIVE;
+
+   regmap_write(tcon->regs, SUN4I_TCON0_IO_POL_REG, val);
+   } else {
+   /* according to vendor driver, this bit must be always set */
+   val = SUN4I_TCON1_IO_POL_UNKNOWN;
+
+   if (mode->flags & DRM_MODE_FLAG_PHSYNC)
+   val |= SUN4I_TCON1_IO_POL_HSYNC_POSITIVE;
+
+   if (mode->flags & DRM_MODE_FLAG_PVSYNC)
+   val |= SUN4I_TCON1_IO_POL_VSYNC_POSITIVE;
+
+   regmap_write(tcon->regs, SUN4I_TCON1_IO_POL_REG, val);
+   }
+
/* Map output pins to channel 1 */
regmap_update_bits(tcon->regs, SUN4I_TCON_GCTL_REG,
   SUN4I_TCON_GCTL_IOMAP_MASK,
@@ -1500,6 +1524,7 @@ static const struct sun4i_tcon_quirks 
sun8i_a83t_tv_quirks = {
 
 static const struct sun4i_tcon_quirks sun8i_r40_tv_quirks = {
.has_channel_1  = true,
+   .polarity_in_ch0= true,
.set_mux= sun8i_r40_tcon_tv_set_mux,
 };
 
diff --git a/drivers/gpu/drm/sun4i/sun4i_tcon.h 
b/drivers/gpu/drm/sun4i/sun4i_tcon.h
index c5ac1b02482c..e624f6977eb8 100644
--- a/drivers/gpu/drm/sun4i/sun4i_tcon.h
+++ b/drivers/gpu/drm/sun4i/sun4i_tcon.h
@@ -154,6 +154,11 @@
 #define SUN4I_TCON1_BASIC5_V_SYNC(height)  (((height) - 1) & 0x3ff)
 
 #define SUN4I_TCON1_IO_POL_REG 0xf0
+/* there is no documentation about this bit */
+#define SUN4I_TCON1_IO_POL_UNKNOWN BIT(26)
+#define SUN4I_TCON1_IO_POL_HSYNC_POSITIVE  BIT(25)
+#define SUN4I_TCON1_IO_POL_VSYNC_POSITIVE  BIT(24)
+
 #define SUN4I_TCON1_IO_TRI_REG 0xf4
 
 #define SUN4I_TCON_ECC_FIFO_REG0xf8
@@ -236,6 +241,7 @@ struct sun4i_tcon_quirks {
boolneeds_de_be_mux; /* sun6i needs mux to select backend */
boolneeds_edp_reset; /* a80 edp reset needed for tcon0 access */
boolsupports_lvds;   /* Does the TCON support an LVDS output? */
+   boolpolarity_in_ch0; /* some tcon1 channels have polarity bits in 
tcon0 pol register */
u8  dclk_min_div;   /* minimum divider for TCON0 DCLK */
 
/* callback to handle tcon muxing options */
-- 
2.30.0

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

[PATCH v3 1/5] clk: sunxi-ng: mp: fix parent rate change flag check

CLK_SET_RATE_PARENT flag is checked on parent clock instead of current
one. Fix that.

Fixes: 3f790433c3cb ("clk: sunxi-ng: Adjust MP clock parent rate when allowed")
Reviewed-by: Chen-Yu Tsai 
Tested-by: Andre Heider 
Signed-off-by: Jernej Skrabec 
---
 drivers/clk/sunxi-ng/ccu_mp.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/clk/sunxi-ng/ccu_mp.c b/drivers/clk/sunxi-ng/ccu_mp.c
index fa4ecb915590..9d3a76604d94 100644
--- a/drivers/clk/sunxi-ng/ccu_mp.c
+++ b/drivers/clk/sunxi-ng/ccu_mp.c
@@ -108,7 +108,7 @@ static unsigned long ccu_mp_round_rate(struct 
ccu_mux_internal *mux,
max_m = cmp->m.max ?: 1 << cmp->m.width;
max_p = cmp->p.max ?: 1 << ((1 << cmp->p.width) - 1);
 
-   if (!(clk_hw_get_flags(hw) & CLK_SET_RATE_PARENT)) {
+   if (!clk_hw_can_set_rate_parent(&cmp->common.hw)) {
ccu_mp_find_best(*parent_rate, rate, max_m, max_p, &m, &p);
rate = *parent_rate / p / m;
} else {
-- 
2.30.0

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

[PATCH v3 0/5] sunxi: fix H6 HDMI related issues

Over the year I got plenty of reports of troubles with H6 HDMI signal.
Sometimes monitor flickers, sometimes there was no image at all and
sometimes it didn't play well with AVR.

It turns out there are multiple issues. Patch 1 fixes clock issue,
which didn't adjust parent rate, even if it is allowed to do so. Patch 2
adds polarity config in tcon1. This is seemingly not needed for pre-HDMI2
controllers, although BSP drivers set it accordingly every time. It
turns out that HDMI2 controllers often don't work with monitors if
polarity is not set correctly. Patch 3 always set clock rate for HDMI
controller. Patch 4 fixes H6 HDMI PHY settings. Patch 5 fixes comment and
clock rate limit (wrong reasoning).

Please take a look.

Best regards,
Jernej

Changes from v2:
- use clk_hw_can_set_rate_parent() directly instead of checking flags
Changes from v1:
- collected Chen-Yu tags (except on replaced patch 4)
- Added some comments in patch 2
- Replaced patch 4 (see commit log for explanation)

Jernej Skrabec (5):
  clk: sunxi-ng: mp: fix parent rate change flag check
  drm/sun4i: tcon: set sync polarity for tcon1 channel
  drm/sun4i: dw-hdmi: always set clock rate
  drm/sun4i: Fix H6 HDMI PHY configuration
  drm/sun4i: dw-hdmi: Fix max. frequency for H6

 drivers/clk/sunxi-ng/ccu_mp.c  |  2 +-
 drivers/gpu/drm/sun4i/sun4i_tcon.c | 25 +
 drivers/gpu/drm/sun4i/sun4i_tcon.h |  6 ++
 drivers/gpu/drm/sun4i/sun8i_dw_hdmi.c  | 10 +++---
 drivers/gpu/drm/sun4i/sun8i_dw_hdmi.h  |  1 -
 drivers/gpu/drm/sun4i/sun8i_hdmi_phy.c | 26 +-
 6 files changed, 44 insertions(+), 26 deletions(-)

--
2.30.0

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

Re: [RFC][PATCH v6 1/7] drm: Add a sharable drm page-pool implementation

On Tue, Feb 9, 2021 at 4:11 AM Christian König  wrote:
>
>
>
> Am 05.02.21 um 21:46 schrieb John Stultz:
> > On Fri, Feb 5, 2021 at 12:47 AM Christian König
> >  wrote:
> >> Am 05.02.21 um 09:06 schrieb John Stultz:
> >>> diff --git a/drivers/gpu/drm/page_pool.c b/drivers/gpu/drm/page_pool.c
> >>> new file mode 100644
> >>> index ..2139f86e6ca7
> >>> --- /dev/null
> >>> +++ b/drivers/gpu/drm/page_pool.c
> >>> @@ -0,0 +1,220 @@
> >>> +// SPDX-License-Identifier: GPL-2.0
> >> Please use a BSD/MIT compatible license if you want to copy this from
> >> the TTM code.
> > Hrm. This may be problematic, as it's not just TTM code, but some of
> > the TTM logic integrated into a page-pool implementation I wrote based
> > on logic from the ION code (which was GPL-2.0 before it was dropped).
> > So I don't think I can just make it MIT.  Any extra context on the
> > need for MIT, or suggestions on how to best resolve this?
>
> Most of the DRM/TTM code is also license able under the BSD/MIT style
> license since we want to enable the BSD guys to port it over as well.
>
> What stuff do you need from the ION code that you can't just code
> yourself? As far as I have seen this is like 99% code from the TTM pool.

Yea, it evolved into being mostly logic from the TTM pool (or code
that was very similar to begin with), but it's not where it started.
My old days at IBM makes me wary of claiming it's no longer the Ship
of Theseus.

So instead I think I'll have to just throw out my patch and rewrite it
in full (so apologies in advance for any review issues I
introduce/reintroduce).

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

Re: [PATCH v6 1/3] lib/vsprintf: Add support for printing V4L2 and DRM fourccs

2021-02-09 Thread Sakari Ailus

Hi Andy,

On Tue, Feb 09, 2021 at 11:58:40AM +0200, Andy Shevchenko wrote:
> On Tue, Feb 09, 2021 at 11:20:32AM +0200, Sakari Ailus wrote:
> > On Mon, Feb 08, 2021 at 10:43:30PM +0200, Andy Shevchenko wrote:
> > > On Mon, Feb 8, 2021 at 10:11 PM Sakari Ailus
> > >  wrote:
> 
> ...
> 
> > > > +   %p4cc   BG12 little-endian (0x32314742)
> > > 
> > > This misses examples of the (strange) escaping cases and wiped
> > > whitespaces to make sure everybody understands that 'D 12' will be the
> > > same as 'D1 2' (side note: which I disagree on, perhaps something
> > > should be added into documentation why).
> > 
> > The spaces are expected to be at the end only. I can add such example if
> > you like. There are no fourcc codes with spaces in the middle in neither
> > V4L2 nor DRM, and I don't think the expectation is to have them either.
> 
> But then the code suggests otherwise. Perhaps we need to extract
> skip_trailing_spaces() from strim() and use it here.

But this wouldn't affect the result in this case, would it?

> 
> ...
> 
> > > > +static noinline_for_stack
> > > > +char *fourcc_string(char *buf, char *end, const u32 *fourcc,
> > > > +   struct printf_spec spec, const char *fmt)
> > > > +{
> > > 
> > > > +   char output[sizeof("(xx)(xx)(xx)(xx) little-endian 
> > > > (0x01234567)")];
> > > 
> > > Do we have any evidence / document / standard that the above format is
> > > what people would find good? From existing practices (I consider other
> > > printings elsewhere and users in this series) I find '(xx)' form for
> > > hex numbers is weird. The standard practice is to use \xHH (without
> > > parentheses).
> > 
> > Earlier in the review it was proposed that special handling of codes below
> > 32 should be added, which I did. Using the parentheses is apparently an
> > existing practice elsewhere.
> 
> Where? \xHH is quite well established format for escaping. Never heard about
> '(xx)' variant before this very series.

Mauro referred to FourCC codes while reviewing an earlier version of this,
such as RGB(15).

Does \× imply only the next two characters are hexadecimal? I have to admit
I don't remember seeting that, nor \x notation is common.

> 
> > Note that neither DRM nor V4L2 currently has such fourcc codes currently.
> 
> ...
> 
> > > > +   p = special_hex_number(p, output + sizeof(output) - 2, *fourcc,
> > > > +  sizeof(u32));
> > > 
> > > This is perfectly one line (in this file we have even longer lines).
> > 
> > Sure, you can do that, and I can then review your patch and point to the
> > coding style documentation. :-)
> 
> Yes, you can. The problem is that we agreed with others to improve readability
> by letting some lines to be longer, so the code can lie on one line rather be
> broken on two or more.

Making the end of the line invisible without scrolling vertically doesn't
improve readability for me. It does depend on window width though. I'd
simply only use that if 80 isn't enough.

-- 
Regards,

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

Re: [RFC][PATCH v6 1/7] drm: Add a sharable drm page-pool implementation




Am 09.02.21 um 18:33 schrieb Suren Baghdasaryan:

On Tue, Feb 9, 2021 at 4:57 AM Christian König  wrote:

Am 09.02.21 um 13:11 schrieb Christian König:

[SNIP]

+void drm_page_pool_add(struct drm_page_pool *pool, struct page *page)
+{
+ spin_lock(&pool->lock);
+ list_add_tail(&page->lru, &pool->items);
+ pool->count++;
+ atomic_long_add(1 << pool->order, &total_pages);
+ spin_unlock(&pool->lock);
+
+ mod_node_page_state(page_pgdat(page),
NR_KERNEL_MISC_RECLAIMABLE,
+ 1 << pool->order);

Hui what? What should that be good for?

This is a carryover from the ION page pool implementation:
https://nam11.safelinks.protection.outlook.com/?url=https%3A%2F%2Fgit.kernel.org%2Fpub%2Fscm%2Flinux%2Fkernel%2Fgit%2Ftorvalds%2Flinux.git%2Ftree%2Fdrivers%2Fstaging%2Fandroid%2Fion%2Fion_page_pool.c%3Fh%3Dv5.10%23n28&data=04%7C01%7Cchristian.koenig%40amd.com%7Cdff8edcd4d147a5b08d8cd20cff2%7C3dd8961fe4884e608e11a82d994e183d%7C0%7C0%7C637484888114923580%7CUnknown%7CTWFpbGZsb3d8eyJWIjoiMC4wLjAwMDAiLCJQIjoiV2luMzIiLCJBTiI6Ik1haWwiLCJXVCI6Mn0%3D%7C1000&sdata=9%2BIBC0tezSV6Ci4S3kWfW%2BQvJm4mdunn3dF6C0kyfCw%3D&reserved=0


My sense is it helps with the vmstat/meminfo accounting so folks can
see the cached pages are shrinkable/freeable. This maybe falls under
other dmabuf accounting/stats discussions, so I'm happy to remove it
for now, or let the drivers using the shared page pool logic handle
the accounting themselves?

Intentionally separated the discussion for that here.

As far as I can see this is just bluntly incorrect.

Either the page is reclaimable or it is part of our pool and freeable
through the shrinker, but never ever both.

IIRC the original motivation for counting ION pooled pages as
reclaimable was to include them into /proc/meminfo's MemAvailable
calculations. NR_KERNEL_MISC_RECLAIMABLE defined as "reclaimable
non-slab kernel pages" seems like a good place to account for them but
I might be wrong.


Yeah, that's what I see here as well. But exactly that is utterly nonsense.

Those pages are not "free" in the sense that get_free_page could return 
them directly.


Regards,
Christian.




In the best case this just messes up the accounting, in the worst case
it can cause memory corruption.

Christian.


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

Re: [PATCH v2 0/3] drm/amdgpu: Remove in_interrupt() usage.


Hi Sebastian,

to be honest I'm thinking about that for quite some time now and I don't 
think that this is possible without a severe rewrite of the driver.


The problem is simply that we have a lot of functions which deal with 
hardware handling independent of the context. But how registers are 
accessed needs to be different depending if your are in the interrupt 
handler or not.


You would need to push the information if we are coming in from the 
interrupt handler through a > 10 function calls.


I don't think that this is feasible nor good design.

Regards,
Christian.

Am 09.02.21 um 17:53 schrieb Sebastian Andrzej Siewior:

On 2021-02-09 13:50:31 [+0100], Christian König wrote:

Reviewed-by: Christian König  for the series.

Thank you.
Any chance you could give me a hand with the remaining three users
within the amdgpu driver? I don't know if the in_interrupt() check can
be limited to certain callers.
What I noticed while tracing v5.10 is this:

| Xorg-2257[007] d... 57261.620043: amdgpu_device_wreg: 0x699f, 
0x1bcf, 0x0100
|  => trace_event_raw_event_amdgpu_device_wreg
|  => amdgpu_device_wreg.part.0
|  => dce110_arm_vert_intr
|  => dce110_vblank_set
|  => dm_enable_vblank
|  => drm_vblank_enable
|  => drm_vblank_get
|  => drm_wait_vblank_ioctl
|  => drm_ioctl_kernel
|  => drm_ioctl
|  => amdgpu_drm_ioctl
|  => __x64_sys_ioctl
|  => do_syscall_64
|  => entry_SYSCALL_64_after_hwframe

I think that amdgpu_device_wreg() -> amdgpu_kiq_wreg() could be invoked.
It doesn't here because amdgpu_sriov_runtime() is false.
The trace says `d' which means interrupts are disabled but
in_interrupt() will return false in this case (no IRQ/softirq).

Sebastian


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

Re: [RFC][PATCH v6 3/7] drm: ttm_pool: Rework ttm_pool_free_page to allow us to use it as a function pointer

On Tue, Feb 9, 2021 at 4:14 AM Christian König  wrote:
> Am 05.02.21 um 20:47 schrieb John Stultz:
> > On Fri, Feb 5, 2021 at 12:28 AM Christian König
> >  wrote:
> >> Adding this to all pages would increase the memory footprint drastically.
> > Yea, that's a good point!  Hrm... bummer. I'll have to see if there's
> > some other way I can get the needed context for the free from the
> > generic page-pool side.
>
> What exactly is the problem here?

Me, usually. :)

> As far as I can see we just have the
> lru entry (list_head) and the pool.

Yea, I reworked it to an embedded drm_page_pool struct, but that is
mostly a list_head.

> How the lru is cast to the page can be completely pool implementation
> specific.

Yea, I had it do container_of(), just haven't gotten around to sending
it out yet.

Thanks so much for the feedback and ideas!

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

[Bug 211425] [drm:atom_op_jump] ERROR atombios stuck in loop for more than 20secs aborting

2021-02-09 Thread bugzilla-daemon

https://bugzilla.kernel.org/show_bug.cgi?id=211425

Alex Deucher (alexdeuc...@gmail.com) changed:

   What|Removed |Added

 CC||alexdeuc...@gmail.com

--- Comment #5 from Alex Deucher (alexdeuc...@gmail.com) ---
If this is a regression can you bisect?

-- 
You may reply to this email to add a comment.

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 0/3] drm/amdgpu: Remove in_interrupt() usage.

2021-02-09 Thread Sebastian Andrzej Siewior

On 2021-02-09 13:50:31 [+0100], Christian König wrote:
> Reviewed-by: Christian König  for the series.

Thank you.
Any chance you could give me a hand with the remaining three users
within the amdgpu driver? I don't know if the in_interrupt() check can
be limited to certain callers.
What I noticed while tracing v5.10 is this:

| Xorg-2257[007] d... 57261.620043: amdgpu_device_wreg: 0x699f, 
0x1bcf, 0x0100
|  => trace_event_raw_event_amdgpu_device_wreg
|  => amdgpu_device_wreg.part.0
|  => dce110_arm_vert_intr
|  => dce110_vblank_set
|  => dm_enable_vblank
|  => drm_vblank_enable
|  => drm_vblank_get
|  => drm_wait_vblank_ioctl
|  => drm_ioctl_kernel
|  => drm_ioctl
|  => amdgpu_drm_ioctl
|  => __x64_sys_ioctl
|  => do_syscall_64
|  => entry_SYSCALL_64_after_hwframe

I think that amdgpu_device_wreg() -> amdgpu_kiq_wreg() could be invoked.
It doesn't here because amdgpu_sriov_runtime() is false.
The trace says `d' which means interrupts are disabled but
in_interrupt() will return false in this case (no IRQ/softirq).

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

Re: [PATCH] radeon: added support for 2560x1080 resolution

On Sun, Feb 7, 2021 at 1:13 PM Marcin Raszka  wrote:
>
> I was wondering why I can't set the resolution to 2560x1080,
>  while in windows 7 I can without a problem. I looked at the radeon driver
>  code and found it doesn't support this resolution. So I made some changes. I
>  added the hdmi_mhz parameter. In cmdline I set radeon.hdmi_mhz=190
>  Only tested on the Radeon HD 5830
>
> ---
>  drivers/gpu/drm/radeon/radeon_benchmark.c  |  5 +++--
>  drivers/gpu/drm/radeon/radeon_connectors.c | 25 +-
>  drivers/gpu/drm/radeon/radeon_drv.c|  5 +
>  drivers/gpu/drm/radeon/radeon_encoders.c   |  6 --
>  4 files changed, 27 insertions(+), 14 deletions(-)
>
> diff --git a/drivers/gpu/drm/radeon/radeon_benchmark.c 
> b/drivers/gpu/drm/radeon/radeon_benchmark.c
> index ac9a5ec481c3..c283b6b15925 100644
> --- a/drivers/gpu/drm/radeon/radeon_benchmark.c
> +++ b/drivers/gpu/drm/radeon/radeon_benchmark.c
> @@ -30,7 +30,7 @@
>  #define RADEON_BENCHMARK_COPY_DMA  0
>
>  #define RADEON_BENCHMARK_ITERATIONS 1024
> -#define RADEON_BENCHMARK_COMMON_MODES_N 17
> +#define RADEON_BENCHMARK_COMMON_MODES_N 18
>
>  static int radeon_benchmark_do_move(struct radeon_device *rdev, unsigned 
> size,
> uint64_t saddr, uint64_t daddr,
> @@ -184,7 +184,8 @@ void radeon_benchmark(struct radeon_device *rdev, int 
> test_number)
> 1680 * 1050 * 4,
> 1600 * 1200 * 4,
> 1920 * 1080 * 4,
> -   1920 * 1200 * 4
> +   1920 * 1200 * 4,
> +   2560 * 1080 * 4
> };
>

This change is unrelated.

> switch (test_number) {
> diff --git a/drivers/gpu/drm/radeon/radeon_connectors.c 
> b/drivers/gpu/drm/radeon/radeon_connectors.c
> index 607ad5620bd9..182f1e364cbd 100644
> --- a/drivers/gpu/drm/radeon/radeon_connectors.c
> +++ b/drivers/gpu/drm/radeon/radeon_connectors.c
> @@ -37,6 +37,8 @@
>  #include 
>  #include 
>
> +extern int hdmimhz;
> +
>  static int radeon_dp_handle_hpd(struct drm_connector *connector)
>  {
> struct radeon_connector *radeon_connector = 
> to_radeon_connector(connector);
> @@ -503,7 +505,7 @@ static void radeon_add_common_modes(struct drm_encoder 
> *encoder, struct drm_conn
> struct mode_size {
> int w;
> int h;
> -   } common_modes[17] = {
> +   } common_modes[] = {
> { 640,  480},
> { 720,  480},
> { 800,  600},
> @@ -520,10 +522,11 @@ static void radeon_add_common_modes(struct drm_encoder 
> *encoder, struct drm_conn
> {1680, 1050},
> {1600, 1200},
> {1920, 1080},
> -   {1920, 1200}
> +   {1920, 1200},
> +   {2560, 1080}
> };
>
> -   for (i = 0; i < 17; i++) {
> +   for (i = 0; i < ARRAY_SIZE(common_modes); i++) {
> if (radeon_encoder->devices & (ATOM_DEVICE_TV_SUPPORT)) {
> if (common_modes[i].w > 1024 ||
> common_modes[i].h > 768)

Same with these changes.

> @@ -1491,25 +1494,27 @@ static enum drm_mode_status 
> radeon_dvi_mode_valid(struct drm_connector *connecto
> (mode->clock > 135000))
> return MODE_CLOCK_HIGH;
>
> -   if (radeon_connector->use_digital && (mode->clock > 165000)) {
> +   if (radeon_connector->use_digital && (mode->clock > (hdmimhz * 
> 1000))) {
> if ((radeon_connector->connector_object_id == 
> CONNECTOR_OBJECT_ID_DUAL_LINK_DVI_I) ||
> (radeon_connector->connector_object_id == 
> CONNECTOR_OBJECT_ID_DUAL_LINK_DVI_D) ||
> -   (radeon_connector->connector_object_id == 
> CONNECTOR_OBJECT_ID_HDMI_TYPE_B))
> +   (radeon_connector->connector_object_id == 
> CONNECTOR_OBJECT_ID_HDMI_TYPE_B)){
> return MODE_OK;
> -   else if (ASIC_IS_DCE6(rdev) && 
> drm_detect_hdmi_monitor(radeon_connector_edid(connector))) {
> +   }else if (ASIC_IS_DCE6(rdev) && 
> drm_detect_hdmi_monitor(radeon_connector_edid(connector))) {
> /* HDMI 1.3+ supports max clock of 340 Mhz */
> -   if (mode->clock > 34)
> +   if (mode->clock > 34){
> return MODE_CLOCK_HIGH;
> -   else
> +   }else{
> return MODE_OK;
> +   }
> } else {
> return MODE_CLOCK_HIGH;
> }
> }
>
> /* check against the max pixel clock */
> -   if ((mode->clock / 10) > rdev->clock.max_pixel_clock)
> +   if ((mode->clock / 10) > rdev->clock.max_pixel_clock){
> return MODE_CLOCK_HIGH;
> +   }

Unrelated whitespace change.

>
> return MODE_OK;
>  }
> @@ -1809,7 +1814,7 @@ static enum drm_mode_status

Re: [PATCH] drm/vblank: Avoid storing a timestamp for the same frame twice

On Tue, Feb 9, 2021 at 4:41 PM Ville Syrjälä
 wrote:
> On Tue, Feb 09, 2021 at 11:07:53AM +0100, Daniel Vetter wrote:
> > On Thu, Feb 04, 2021 at 04:04:00AM +0200, Ville Syrjala wrote:
> > > From: Ville Syrjälä 
> > >
> > > drm_vblank_restore() exists because certain power saving states
> > > can clobber the hardware frame counter. The way it does this is
> > > by guesstimating how many frames were missed purely based on
> > > the difference between the last stored timestamp vs. a newly
> > > sampled timestamp.
> > >
> > > If we should call this function before a full frame has
> > > elapsed since we sampled the last timestamp we would end up
> > > with a possibly slightly different timestamp value for the
> > > same frame. Currently we will happily overwrite the already
> > > stored timestamp for the frame with the new value. This
> > > could cause userspace to observe two different timestamps
> > > for the same frame (and the timestamp could even go
> > > backwards depending on how much error we introduce when
> > > correcting the timestamp based on the scanout position).
> > >
> > > To avoid that let's not update the stored timestamp unless we're
> > > also incrementing the sequence counter. We do still want to update
> > > vblank->last with the freshly sampled hw frame counter value so
> > > that subsequent vblank irqs/queries can actually use the hw frame
> > > counter to determine how many frames have elapsed.
> > >
> > > Cc: Dhinakaran Pandiyan 
> > > Cc: Rodrigo Vivi 
> > > Cc: Daniel Vetter 
> > > Signed-off-by: Ville Syrjälä 
> >
> > Ok, top-posting because lol I got confused. I mixed up the guesstimation
> > work we do for when we don't have a vblank counter with the precise vblank
> > timestamp stuff.
> >
> > I think it'd still be good to maybe lock down/document a bit better the
> > requirements for drm_crtc_vblank_restore, but I convinced myself now that
> > your patch looks correct.
> >
> > Reviewed-by: Daniel Vetter 
>
> Ta.
>
> Though I wonder if we should just do something like this instead:
> -   store_vblank(dev, pipe, diff, t_vblank, cur_vblank);
> +   vblank->last = (cur_vblank - diff) & max_vblank_count;
>
> to make it entirely obvious that this exists only to fix up
> the stored hw counter value?
>
> Would also avoid the problem the original patch tries to fix
> because we'd simply never store a new timestamp here.

Hm yeah, I think that would nicely limit the impact. But need to check
overflow/underflow math is all correct. And I think that would neatly
implement the trick I proposed to address the bug that wasn't there
:-)

The only thing that I've thought of as issue is that we might have
more wrap-around of the hw vblank counter, but that shouldn't be worse
than without this - anytime we have the vblank on for long enough we
fix the entire thing, and I think our wrap handling is now consistent
enough (there was some "let's just add a large bump" stuff for dri1
userspace iirc) that this shouldn't be any problem.

Plus the comment about _restore being very special would be in the
restore function, so this would also be rather tidy. If you go with
this maybe extend the kerneldoc for ->last to mention that
drm_vblank_restore() adjusts it?

The more I ponder this, the more I like it ... which probably means
I'm missing something, because this is drm_vblank.c?

Cheers, Daniel

>
> >
> > > ---
> > >  drivers/gpu/drm/drm_vblank.c | 11 +++
> > >  1 file changed, 11 insertions(+)
> > >
> > > diff --git a/drivers/gpu/drm/drm_vblank.c b/drivers/gpu/drm/drm_vblank.c
> > > index 893165eeddf3..e127a7db2088 100644
> > > --- a/drivers/gpu/drm/drm_vblank.c
> > > +++ b/drivers/gpu/drm/drm_vblank.c
> > > @@ -176,6 +176,17 @@ static void store_vblank(struct drm_device *dev, 
> > > unsigned int pipe,
> > >
> > > vblank->last = last;
> > >
> > > +   /*
> > > +* drm_vblank_restore() wants to always update
> > > +* vblank->last since we can't trust the frame counter
> > > +* across power saving states. But we don't want to alter
> > > +* the stored timestamp for the same frame number since
> > > +* that would cause userspace to potentially observe two
> > > +* different timestamps for the same frame.
> > > +*/
> > > +   if (vblank_count_inc == 0)
> > > +   return;
> > > +
> > > write_seqlock(&vblank->seqlock);
> > > vblank->time = t_vblank;
> > > atomic64_add(vblank_count_inc, &vblank->count);
> > > --
> > > 2.26.2
> > >
> >
> > --
> > Daniel Vetter
> > Software Engineer, Intel Corporation
> > http://blog.ffwll.ch
>
> --
> Ville Syrjälä
> Intel



-- 
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 1/2] dt-bindings: display: bridge: Add documentation for LT8912

On Sun, Jan 24, 2021 at 04:08:34PM +0100, Adrien Grassein wrote:
> Lontium LT8912 is a DSI to HDMI bridge.
> 
> Signed-off-by: Adrien Grassein 
> ---
>  .../display/bridge/lontium,lt8912.yaml| 92 +++
>  MAINTAINERS   |  5 +
>  2 files changed, 97 insertions(+)
>  create mode 100644 
> Documentation/devicetree/bindings/display/bridge/lontium,lt8912.yaml
> 
> diff --git 
> a/Documentation/devicetree/bindings/display/bridge/lontium,lt8912.yaml 
> b/Documentation/devicetree/bindings/display/bridge/lontium,lt8912.yaml
> new file mode 100644
> index ..ed1a6ddaab2f
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/display/bridge/lontium,lt8912.yaml
> @@ -0,0 +1,92 @@
> +# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause)
> +%YAML 1.2
> +---
> +$id: http://devicetree.org/schemas/display/bridge/lontium,lt8912.yaml#
> +$schema: http://devicetree.org/meta-schemas/core.yaml#
> +
> +title: Lontium LT8912 MIPI to HDMI Bridge
> +
> +maintainers:
> +  - Adrien Grassein 
> +
> +description: |
> +  The LT8912 is a bridge device which convert DSI to HDMI
> +
> +properties:
> +  compatible:
> +enum:
> +  - lontium,lt8912
> +
> +  reg:
> +maxItems: 1
> +
> +  ddc-i2c-bus:

This belongs in an hdmi-connector node.

> +maxItems: 1
> +description: i2c bus used to read EDID of the connected display.
> +
> +  dsi-lanes:
> +maxItems: 1
> +description: dsi lanes count interconnected with lt8912.

'data-lanes' in the graph is the standard way to do this.

You'll need video-interfaces.yaml which is pending in the media tree.

> +
> +  reset-gpios:
> +maxItems: 1
> +description: GPIO connected to active high RESET pin.
> +
> +  ports:
> +$ref: /schemas/graph.yaml#/properties/ports
> +
> +properties:
> +  port@0:
> +$ref: /schemas/graph.yaml#/properties/port
> +description:
> +  Primary MIPI port-1 for MIPI input

You're going to need a port for the connector.

> +
> +required:
> +  - port@0
> +
> +required:
> +  - compatible
> +  - ddc-i2c-bus
> +  - dsi-lanes
> +  - reg
> +  - reset-gpios
> +  - ports
> +
> +additionalProperties: false
> +
> +examples:
> +  - |
> +#include 
> +
> +i2c4 {
> +  #address-cells = <1>;
> +  #size-cells = <0>;
> +
> +  hdmi-bridge@48 {
> +compatible = "lontium,lt8912";
> +reg = <0x48>;
> +reset-gpios = <&max7323 0 GPIO_ACTIVE_LOW>;
> +dsi-lanes = <4>;
> +ddc-i2c-bus = <&ddc_i2c_bus>;
> +
> +ports {
> +  #address-cells = <1>;
> +  #size-cells = <0>;
> +
> +  port@0 {
> +reg = <0>;
> +
> +hdmi_out_in: endpoint {
> +  remote-endpoint = <&mipi_dsi_out>;
> +};
> +  };
> +};
> +  };
> +};
> +
> +ddc_i2c_bus: i2c5 {
> +  #address-cells = <1>;
> +  #size-cells = <0>;
> +};
> +
> +...
> diff --git a/MAINTAINERS b/MAINTAINERS
> index 5aa18cbfb883..01e7e356bfac 100644
> --- a/MAINTAINERS
> +++ b/MAINTAINERS
> @@ -10472,6 +10472,11 @@ S:   Maintained
>  T:   git git://git.kernel.org/pub/scm/linux/kernel/git/hid/hid.git
>  F:   drivers/hid/hid-lg-g15.c
>  
> +LONTIUM LT8912 MIPI TO HDMI BRIDGE
> +M:   Adrien Grassein 
> +S:   Maintained
> +F:   Documentation/devicetree/bindings/display/bridge/lontium,lt8912.yaml
> +
>  LSILOGIC MPT FUSION DRIVERS (FC/SAS/SPI)
>  M:   Sathya Prakash 
>  M:   Sreekanth Reddy 
> -- 
> 2.25.1
> 
___
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

Re: [PATCH v6 2/8] drm/mediatek: add component POSTMASK

2021-02-09 Thread Guenter Roeck

On Tue, Feb 02, 2021 at 04:12:31PM +0800, Hsin-Yi Wang wrote:
> From: Yongqiang Niu 
> 
> This patch add component POSTMASK.
> 
> Signed-off-by: Yongqiang Niu 
> Signed-off-by: Hsin-Yi Wang 
> Reviewed-by: CK Hu 
> ---
[ ... ]
>  
> +void mtk_postmask_config(struct device *dev, unsigned int w,

static

> + unsigned int h, unsigned int vrefresh,
> + unsigned int bpc, struct cmdq_pkt *cmdq_pkt)
> +{
> + struct mtk_ddp_comp_dev *priv = dev_get_drvdata(dev);
> +
> + mtk_ddp_write(cmdq_pkt, w << 16 | h, &priv->cmdq_reg, priv->regs,
> +   DISP_POSTMASK_SIZE);
> + mtk_ddp_write(cmdq_pkt, POSTMASK_RELAY_MODE, &priv->cmdq_reg,
> +   priv->regs, DISP_POSTMASK_CFG);
> +}
> +
> +void mtk_postmask_start(struct device *dev)

static

> +{
> + struct mtk_ddp_comp_dev *priv = dev_get_drvdata(dev);
> +
> + writel(POSTMASK_EN, priv->regs + DISP_POSTMASK_EN);
> +}
> +
> +void mtk_postmask_stop(struct device *dev)

static

> +{
> + struct mtk_ddp_comp_dev *priv = dev_get_drvdata(dev);
> +
> + writel_relaxed(0x0, priv->regs + DISP_POSTMASK_EN);
> +}
> +
>  static void mtk_aal_config(struct device *dev, unsigned int w,
>  unsigned int h, unsigned int vrefresh,
>  unsigned int bpc, struct cmdq_pkt *cmdq_pkt)
> @@ -413,6 +445,14 @@ static const struct mtk_ddp_comp_funcs ddp_ovl = {
>   .bgclr_in_off = mtk_ovl_bgclr_in_off,
>  };
>  
> +static const struct mtk_ddp_comp_funcs ddp_postmask = {
> + .clk_enable = mtk_ddp_clk_enable,
> + .clk_disable = mtk_ddp_clk_disable,
> + .config = mtk_postmask_config,
> + .start = mtk_postmask_start,
> + .stop = mtk_postmask_stop,
> +};
> +
>  static const struct mtk_ddp_comp_funcs ddp_rdma = {
>   .clk_enable = mtk_rdma_clk_enable,
>   .clk_disable = mtk_rdma_clk_disable,
> @@ -448,6 +488,7 @@ static const char * const 
> mtk_ddp_comp_stem[MTK_DDP_COMP_TYPE_MAX] = {
>   [MTK_DISP_MUTEX] = "mutex",
>   [MTK_DISP_OD] = "od",
>   [MTK_DISP_BLS] = "bls",
> + [MTK_DISP_POSTMASK] = "postmask",
>  };
>  
>  struct mtk_ddp_comp_match {
> @@ -457,36 +498,37 @@ struct mtk_ddp_comp_match {
>  };
>  
>  static const struct mtk_ddp_comp_match mtk_ddp_matches[DDP_COMPONENT_ID_MAX] 
> = {
> - [DDP_COMPONENT_AAL0]= { MTK_DISP_AAL,   0, &ddp_aal },
> - [DDP_COMPONENT_AAL1]= { MTK_DISP_AAL,   1, &ddp_aal },
> - [DDP_COMPONENT_BLS] = { MTK_DISP_BLS,   0, NULL },
> - [DDP_COMPONENT_CCORR]   = { MTK_DISP_CCORR, 0, &ddp_ccorr },
> - [DDP_COMPONENT_COLOR0]  = { MTK_DISP_COLOR, 0, &ddp_color },
> - [DDP_COMPONENT_COLOR1]  = { MTK_DISP_COLOR, 1, &ddp_color },
> - [DDP_COMPONENT_DITHER]  = { MTK_DISP_DITHER,0, &ddp_dither },
> - [DDP_COMPONENT_DPI0]= { MTK_DPI,0, &ddp_dpi },
> - [DDP_COMPONENT_DPI1]= { MTK_DPI,1, &ddp_dpi },
> - [DDP_COMPONENT_DSI0]= { MTK_DSI,0, &ddp_dsi },
> - [DDP_COMPONENT_DSI1]= { MTK_DSI,1, &ddp_dsi },
> - [DDP_COMPONENT_DSI2]= { MTK_DSI,2, &ddp_dsi },
> - [DDP_COMPONENT_DSI3]= { MTK_DSI,3, &ddp_dsi },
> - [DDP_COMPONENT_GAMMA]   = { MTK_DISP_GAMMA, 0, &ddp_gamma },
> - [DDP_COMPONENT_OD0] = { MTK_DISP_OD,0, &ddp_od },
> - [DDP_COMPONENT_OD1] = { MTK_DISP_OD,1, &ddp_od },
> - [DDP_COMPONENT_OVL0]= { MTK_DISP_OVL,   0, &ddp_ovl },
> - [DDP_COMPONENT_OVL1]= { MTK_DISP_OVL,   1, &ddp_ovl },
> - [DDP_COMPONENT_OVL_2L0] = { MTK_DISP_OVL_2L,0, &ddp_ovl },
> - [DDP_COMPONENT_OVL_2L1] = { MTK_DISP_OVL_2L,1, &ddp_ovl },
> - [DDP_COMPONENT_OVL_2L2] = { MTK_DISP_OVL_2L,2, &ddp_ovl },
> - [DDP_COMPONENT_PWM0]= { MTK_DISP_PWM,   0, NULL },
> - [DDP_COMPONENT_PWM1]= { MTK_DISP_PWM,   1, NULL },
> - [DDP_COMPONENT_PWM2]= { MTK_DISP_PWM,   2, NULL },
> - [DDP_COMPONENT_RDMA0]   = { MTK_DISP_RDMA,  0, &ddp_rdma },
> - [DDP_COMPONENT_RDMA1]   = { MTK_DISP_RDMA,  1, &ddp_rdma },
> - [DDP_COMPONENT_RDMA2]   = { MTK_DISP_RDMA,  2, &ddp_rdma },
> - [DDP_COMPONENT_UFOE]= { MTK_DISP_UFOE,  0, &ddp_ufoe },
> - [DDP_COMPONENT_WDMA0]   = { MTK_DISP_WDMA,  0, NULL },
> - [DDP_COMPONENT_WDMA1]   = { MTK_DISP_WDMA,  1, NULL },
> + [DDP_COMPONENT_AAL0]= { MTK_DISP_AAL,   0, &ddp_aal },
> + [DDP_COMPONENT_AAL1]= { MTK_DISP_AAL,   1, &ddp_aal },
> + [DDP_COMPONENT_BLS] = { MTK_DISP_BLS,   0, NULL },
> + [DDP_COMPONENT_CCORR]   = { MTK_DISP_CCORR, 0, &ddp_ccorr },
> + [DDP_COMPONENT_COLOR0]  = { MTK_DISP_COLOR, 0, &ddp_color },
> + [DDP_COMPONENT_COLOR1]  = { MTK_DISP_COLOR, 1, &ddp_color },
> + [DDP_COMPONENT_DITHER]  = { MTK_DISP_DITHER,0, &ddp_dither 
> },
>

Re: [PATCH] drm/qxl: properly handle device init failures

2021-02-09 Thread Tong Zhang

Hi Gerd,
I tested the patch on drm-misc-next and it fixed the issue.
Thanks,
- Tong

> On Feb 9, 2021, at 7:16 AM, Gerd Hoffmann  wrote:
> 
> On Mon, Feb 08, 2021 at 12:07:01PM -0500, Tong Zhang wrote:
>> Does this patch fix an issue raised previously? Or should they be used 
>> together?
>> https://www.mail-archive.com/linux-kernel@vger.kernel.org/msg2466541.html 
>> 
>> IMHO using this patch alone won’t fix the issue
> 
> This patch on top of drm-misc-next fixes the initialization error issue
> reported by you in my testing.
> 
> take care,
>  Gerd
> 

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

[PATCH 2/2] drm/vmwgfx: Remove pointless code

2021-02-09 Thread Zack Rusin

There's no need to check for the presence of the hotplug
property just to return because this is the end of the function
so we're returning either way.

Signed-off-by: Zack Rusin 
Reviewed-by: Martin Krastev 
Reviewed-by: Roland Scheidegger 
---
 drivers/gpu/drm/vmwgfx/vmwgfx_kms.c | 4 
 1 file changed, 4 deletions(-)

diff --git a/drivers/gpu/drm/vmwgfx/vmwgfx_kms.c 
b/drivers/gpu/drm/vmwgfx/vmwgfx_kms.c
index daec9d07c696..c7a939743f7c 100644
--- a/drivers/gpu/drm/vmwgfx/vmwgfx_kms.c
+++ b/drivers/gpu/drm/vmwgfx/vmwgfx_kms.c
@@ -1779,10 +1779,6 @@ vmw_kms_create_hotplug_mode_update_property(struct 
vmw_private *dev_priv)
drm_property_create_range(&dev_priv->drm,
  DRM_MODE_PROP_IMMUTABLE,
  "hotplug_mode_update", 0, 1);
-
-   if (!dev_priv->hotplug_mode_update_property)
-   return;
-
 }
 
 int vmw_kms_init(struct vmw_private *dev_priv)
-- 
2.27.0

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

[PATCH 1/2] drm/vmwgfx: Correctly set the name of the preferred mode

2021-02-09 Thread Zack Rusin

Our sysfs "modes" entries were broken because our preffered mode
never had its name set correctly. This resulted in the first
entry simply being called "preferred" followed by a list of
other resolutions. Lets fix it by actually setting the name of
mode (which is its resolution). This allows one to quickly
validate the modes set by the open-vm-tools.

Signed-off-by: Zack Rusin 
Reviewed-by: Martin Krastev 
Reviewed-by: Roland Scheidegger 
---
 drivers/gpu/drm/vmwgfx/vmwgfx_kms.c | 1 +
 1 file changed, 1 insertion(+)

diff --git a/drivers/gpu/drm/vmwgfx/vmwgfx_kms.c 
b/drivers/gpu/drm/vmwgfx/vmwgfx_kms.c
index 84143b707cd3..daec9d07c696 100644
--- a/drivers/gpu/drm/vmwgfx/vmwgfx_kms.c
+++ b/drivers/gpu/drm/vmwgfx/vmwgfx_kms.c
@@ -2176,6 +2176,7 @@ int vmw_du_connector_fill_modes(struct drm_connector 
*connector,
mode->hdisplay = du->pref_width;
mode->vdisplay = du->pref_height;
vmw_guess_mode_timing(mode);
+   drm_mode_set_name(mode);
 
if (vmw_kms_validate_mode_vram(dev_priv,
mode->hdisplay * assumed_bpp,
-- 
2.27.0

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

Re: [PATCH] drm/vblank: Avoid storing a timestamp for the same frame twice

2021-02-09 Thread Ville Syrjälä

On Tue, Feb 09, 2021 at 11:07:53AM +0100, Daniel Vetter wrote:
> On Thu, Feb 04, 2021 at 04:04:00AM +0200, Ville Syrjala wrote:
> > From: Ville Syrjälä 
> > 
> > drm_vblank_restore() exists because certain power saving states
> > can clobber the hardware frame counter. The way it does this is
> > by guesstimating how many frames were missed purely based on
> > the difference between the last stored timestamp vs. a newly
> > sampled timestamp.
> > 
> > If we should call this function before a full frame has
> > elapsed since we sampled the last timestamp we would end up
> > with a possibly slightly different timestamp value for the
> > same frame. Currently we will happily overwrite the already
> > stored timestamp for the frame with the new value. This
> > could cause userspace to observe two different timestamps
> > for the same frame (and the timestamp could even go
> > backwards depending on how much error we introduce when
> > correcting the timestamp based on the scanout position).
> > 
> > To avoid that let's not update the stored timestamp unless we're
> > also incrementing the sequence counter. We do still want to update
> > vblank->last with the freshly sampled hw frame counter value so
> > that subsequent vblank irqs/queries can actually use the hw frame
> > counter to determine how many frames have elapsed.
> > 
> > Cc: Dhinakaran Pandiyan 
> > Cc: Rodrigo Vivi 
> > Cc: Daniel Vetter 
> > Signed-off-by: Ville Syrjälä 
> 
> Ok, top-posting because lol I got confused. I mixed up the guesstimation
> work we do for when we don't have a vblank counter with the precise vblank
> timestamp stuff.
> 
> I think it'd still be good to maybe lock down/document a bit better the
> requirements for drm_crtc_vblank_restore, but I convinced myself now that
> your patch looks correct.
> 
> Reviewed-by: Daniel Vetter 

Ta.

Though I wonder if we should just do something like this instead:
-   store_vblank(dev, pipe, diff, t_vblank, cur_vblank);
+   vblank->last = (cur_vblank - diff) & max_vblank_count;

to make it entirely obvious that this exists only to fix up
the stored hw counter value?

Would also avoid the problem the original patch tries to fix
because we'd simply never store a new timestamp here.

> 
> > ---
> >  drivers/gpu/drm/drm_vblank.c | 11 +++
> >  1 file changed, 11 insertions(+)
> > 
> > diff --git a/drivers/gpu/drm/drm_vblank.c b/drivers/gpu/drm/drm_vblank.c
> > index 893165eeddf3..e127a7db2088 100644
> > --- a/drivers/gpu/drm/drm_vblank.c
> > +++ b/drivers/gpu/drm/drm_vblank.c
> > @@ -176,6 +176,17 @@ static void store_vblank(struct drm_device *dev, 
> > unsigned int pipe,
> >  
> > vblank->last = last;
> >  
> > +   /*
> > +* drm_vblank_restore() wants to always update
> > +* vblank->last since we can't trust the frame counter
> > +* across power saving states. But we don't want to alter
> > +* the stored timestamp for the same frame number since
> > +* that would cause userspace to potentially observe two
> > +* different timestamps for the same frame.
> > +*/
> > +   if (vblank_count_inc == 0)
> > +   return;
> > +
> > write_seqlock(&vblank->seqlock);
> > vblank->time = t_vblank;
> > atomic64_add(vblank_count_inc, &vblank->count);
> > -- 
> > 2.26.2
> > 
> 
> -- 
> Daniel Vetter
> Software Engineer, Intel Corporation
> http://blog.ffwll.ch

-- 
Ville Syrjälä
Intel
___
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

Re: [PATCH v4 11/14] drm/amdgpu: Guard against write accesses after device removal


Am 09.02.21 um 15:30 schrieb Andrey Grodzovsky:

[SNIP]
Question - Why can't we just set those PTEs to point to system 
memory (another RO dummy page)

filled with 1s ?



Then writes are not discarded. E.g. the 1s would change to something 
else.


Christian.



I see but, what about marking the mappings as RO and discarding the 
write access page faults continuously until the device is finalized ?
Regarding using an unused range behind the upper bridge as Daniel 
suggested, I wonder will this interfere with
the upcoming feature to support BARs movement  during hot plug - 
https://www.spinics.net/lists/linux-pci/msg103195.html ?


In the picture in my head the address will never be used.

But it doesn't even needs to be an unused range of the root bridge. That 
one is usually stuffed full by the BIOS.


For my BAR resize work I looked at quite a bunch of NB documentation. At 
least for AMD CPUs we should always have an MMIO address which is never 
ever used. That makes this platform/CPU dependent, but the code is just 
minimal.


The really really nice thing about this approach is that you could unit 
test and audit the code for problems even without *real* hotplug 
hardware. E.g. we can swap the kernel PTEs and see how the whole power 
and display code reacts to that etc


Christian.



Andrey







Andrey





Christian.


It's a nifty idea indeed otherwise ...
-Daniel


Regards,
Christian.



But ugh ...

Otoh validating an entire driver like amdgpu without such a 
trick
against 0xff reads is practically impossible. So maybe you 
need to add

this as one of the tasks here?
Or I could just for validation purposes return ~0 from all 
reg reads in the code
and ignore writes if drm_dev_unplugged, this could already 
easily validate a big

portion of the code flow under such scenario.
Hm yeah if your really wrap them all, that should work too. 
Since
iommappings have __iomem pointer type, as long as amdgpu is 
sparse

warning free, should be doable to guarantee this.

Problem is that ~0 is not always a valid register value.

You would need to audit every register read that it doesn't 
use the returned

value blindly as index or similar. That is quite a bit of work.

Yeah that's the entire crux here :-/
-Daniel




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

Re: [PATCH v4, 01/10] soc: mediatek: mmsys: create mmsys folder

2021-02-09 Thread Enric Balletbo Serra

Hi Yongqiang Niu,

Thank you for your patch.

Missatge de Yongqiang Niu  del dia dt., 5
de gen. 2021 a les 4:07:
>
> the mmsys will more and more complicated after support
> more and more SoCs, add an independent folder will be
> more clear
>
> Signed-off-by: Yongqiang Niu 
> ---
>  drivers/soc/mediatek/Makefile  |   2 +-

It will not apply cleanly anymore after the below commit that is
already queued. Maybe you could rebase the patches and resend them
again?

commit e1e4f7fea37572f0ccf3887430e52c491e9accb6
Author: CK Hu 
Date:   Tue Jul 21 15:46:06 2020 +0800

soc / drm: mediatek: Move mtk mutex driver to soc folder

mtk mutex is used by DRM and MDP driver, and its function is SoC-specific,
so move it to soc folder.

With that fixed,

Reviewed-by: Enric Balletbo i Serra 

Thanks,
  Enric

>  drivers/soc/mediatek/mmsys/Makefile|   2 +
>  drivers/soc/mediatek/mmsys/mtk-mmsys.c | 373 
> +
>  drivers/soc/mediatek/mtk-mmsys.c   | 373 
> -
>  4 files changed, 376 insertions(+), 374 deletions(-)
>  create mode 100644 drivers/soc/mediatek/mmsys/Makefile
>  create mode 100644 drivers/soc/mediatek/mmsys/mtk-mmsys.c
>  delete mode 100644 drivers/soc/mediatek/mtk-mmsys.c
>
> diff --git a/drivers/soc/mediatek/Makefile b/drivers/soc/mediatek/Makefile
> index b6908db..eca9774 100644
> --- a/drivers/soc/mediatek/Makefile
> +++ b/drivers/soc/mediatek/Makefile
> @@ -5,4 +5,4 @@ obj-$(CONFIG_MTK_INFRACFG) += mtk-infracfg.o
>  obj-$(CONFIG_MTK_PMIC_WRAP) += mtk-pmic-wrap.o
>  obj-$(CONFIG_MTK_SCPSYS) += mtk-scpsys.o
>  obj-$(CONFIG_MTK_SCPSYS_PM_DOMAINS) += mtk-pm-domains.o
> -obj-$(CONFIG_MTK_MMSYS) += mtk-mmsys.o
> +obj-$(CONFIG_MTK_MMSYS) += mmsys/
> diff --git a/drivers/soc/mediatek/mmsys/Makefile 
> b/drivers/soc/mediatek/mmsys/Makefile
> new file mode 100644
> index 000..f44eadc
> --- /dev/null
> +++ b/drivers/soc/mediatek/mmsys/Makefile
> @@ -0,0 +1,2 @@
> +# SPDX-License-Identifier: GPL-2.0-only
> +obj-$(CONFIG_MTK_MMSYS) += mtk-mmsys.o
> diff --git a/drivers/soc/mediatek/mmsys/mtk-mmsys.c 
> b/drivers/soc/mediatek/mmsys/mtk-mmsys.c
> new file mode 100644
> index 000..18f9397
> --- /dev/null
> +++ b/drivers/soc/mediatek/mmsys/mtk-mmsys.c
> @@ -0,0 +1,373 @@
> +// SPDX-License-Identifier: GPL-2.0-only
> +/*
> + * Copyright (c) 2014 MediaTek Inc.
> + * Author: James Liao 
> + */
> +
> +#include 
> +#include 
> +#include 
> +#include 
> +#include 
> +
> +#define DISP_REG_CONFIG_DISP_OVL0_MOUT_EN  0x040
> +#define DISP_REG_CONFIG_DISP_OVL1_MOUT_EN  0x044
> +#define DISP_REG_CONFIG_DISP_OD_MOUT_EN0x048
> +#define DISP_REG_CONFIG_DISP_GAMMA_MOUT_EN 0x04c
> +#define DISP_REG_CONFIG_DISP_UFOE_MOUT_EN  0x050
> +#define DISP_REG_CONFIG_DISP_COLOR0_SEL_IN 0x084
> +#define DISP_REG_CONFIG_DISP_COLOR1_SEL_IN 0x088
> +#define DISP_REG_CONFIG_DSIE_SEL_IN0x0a4
> +#define DISP_REG_CONFIG_DSIO_SEL_IN0x0a8
> +#define DISP_REG_CONFIG_DPI_SEL_IN 0x0ac
> +#define DISP_REG_CONFIG_DISP_RDMA2_SOUT0x0b8
> +#define DISP_REG_CONFIG_DISP_RDMA0_SOUT_EN 0x0c4
> +#define DISP_REG_CONFIG_DISP_RDMA1_SOUT_EN 0x0c8
> +#define DISP_REG_CONFIG_MMSYS_CG_CON0  0x100
> +
> +#define DISP_REG_CONFIG_DISP_OVL_MOUT_EN   0x030
> +#define DISP_REG_CONFIG_OUT_SEL0x04c
> +#define DISP_REG_CONFIG_DSI_SEL0x050
> +#define DISP_REG_CONFIG_DPI_SEL0x064
> +
> +#define OVL0_MOUT_EN_COLOR00x1
> +#define OD_MOUT_EN_RDMA0   0x1
> +#define OD1_MOUT_EN_RDMA1  BIT(16)
> +#define UFOE_MOUT_EN_DSI0  0x1
> +#define COLOR0_SEL_IN_OVL0 0x1
> +#define OVL1_MOUT_EN_COLOR10x1
> +#define GAMMA_MOUT_EN_RDMA10x1
> +#define RDMA0_SOUT_DPI00x2
> +#define RDMA0_SOUT_DPI10x3
> +#define RDMA0_SOUT_DSI10x1
> +#define RDMA0_SOUT_DSI20x4
> +#define RDMA0_SOUT_DSI30x5
> +#define RDMA1_SOUT_DPI00x2
> +#define RDMA1_SOUT_DPI10x3
> +#define RDMA1_SOUT_DSI10x1
> +#define RDMA1_SOUT_DSI20x4
> +#define RDMA1_SOUT_DSI30x5
> +#define RDMA2_SOUT_DPI00x2
> +#define RDMA2_SOUT_DPI10x3
> +#define RDMA2_SOUT_DSI10x1
> +#define RDMA2_SOUT_DSI20x4
> +#define RDMA2_SOUT_DSI30x5
> +#define DPI0_SEL_IN_RDMA1  0x1
> +#define DPI0_SEL_IN_RDMA2  0x3
> +#define DPI1_SEL_IN_RDMA1

Re: [PATCH] drm/vblank: Document drm_crtc_vblank_restore constraints

2021-02-09 Thread Ville Syrjälä

On Tue, Feb 09, 2021 at 11:15:23AM +0100, Daniel Vetter wrote:
> I got real badly confused when trying to review a fix from Ville for
> this. Let's try to document better what's required for this, and check
> the minimal settings at runtime - we can't check ofc that there's
> indeed no races in the driver callback.
> 
> Also noticed that the drm_vblank_restore version is unused, so lets
> unexport that while at it.
> 
> Cc: Ville Syrjala 
> Signed-off-by: Daniel Vetter 
> ---
>  drivers/gpu/drm/drm_vblank.c | 25 ++---
>  include/drm/drm_vblank.h |  1 -
>  2 files changed, 10 insertions(+), 16 deletions(-)
> 
> diff --git a/drivers/gpu/drm/drm_vblank.c b/drivers/gpu/drm/drm_vblank.c
> index c914b14cfb43..05f4d4c078fd 100644
> --- a/drivers/gpu/drm/drm_vblank.c
> +++ b/drivers/gpu/drm/drm_vblank.c
> @@ -1471,20 +1471,7 @@ void drm_crtc_vblank_on(struct drm_crtc *crtc)
>  }
>  EXPORT_SYMBOL(drm_crtc_vblank_on);
>  
> -/**
> - * drm_vblank_restore - estimate missed vblanks and update vblank count.
> - * @dev: DRM device
> - * @pipe: CRTC index
> - *
> - * Power manamement features can cause frame counter resets between vblank
> - * disable and enable. Drivers can use this function in their
> - * &drm_crtc_funcs.enable_vblank implementation to estimate missed vblanks 
> since
> - * the last &drm_crtc_funcs.disable_vblank using timestamps and update the
> - * vblank counter.
> - *
> - * This function is the legacy version of drm_crtc_vblank_restore().
> - */
> -void drm_vblank_restore(struct drm_device *dev, unsigned int pipe)
> +static void drm_vblank_restore(struct drm_device *dev, unsigned int pipe)
>  {
>   ktime_t t_vblank;
>   struct drm_vblank_crtc *vblank;
> @@ -1520,7 +1507,6 @@ void drm_vblank_restore(struct drm_device *dev, 
> unsigned int pipe)
>   diff, diff_ns, framedur_ns, cur_vblank - vblank->last);
>   store_vblank(dev, pipe, diff, t_vblank, cur_vblank);
>  }
> -EXPORT_SYMBOL(drm_vblank_restore);
>  
>  /**
>   * drm_crtc_vblank_restore - estimate missed vblanks and update vblank count.
> @@ -1531,9 +1517,18 @@ EXPORT_SYMBOL(drm_vblank_restore);
>   * &drm_crtc_funcs.enable_vblank implementation to estimate missed vblanks 
> since
>   * the last &drm_crtc_funcs.disable_vblank using timestamps and update the
>   * vblank counter.
> + *
> + * Note that drivers must have race-free high-precision timestamping support,
> + * i.e.  &drm_crtc_funcs.get_vblank_timestamp must be hooked up and
> + * &drm_driver.vblank_disable_immediate must be set to indicate the
> + * time-stamping functions are race-free against vblank hardware counter
> + * increments.

Looks good. Might prevent someone from shooting themselves in
the foot.

Reviewed-by: Ville Syrjälä 

>   */
>  void drm_crtc_vblank_restore(struct drm_crtc *crtc)
>  {
> + WARN_ON_ONCE(!crtc->funcs->get_vblank_timestamp);
> + WARN_ON_ONCE(!crtc->dev->vblank_disable_immediate);
> +
>   drm_vblank_restore(crtc->dev, drm_crtc_index(crtc));
>  }
>  EXPORT_SYMBOL(drm_crtc_vblank_restore);
> diff --git a/include/drm/drm_vblank.h b/include/drm/drm_vblank.h
> index dd125f8c766c..733a3e2d1d10 100644
> --- a/include/drm/drm_vblank.h
> +++ b/include/drm/drm_vblank.h
> @@ -247,7 +247,6 @@ void drm_crtc_vblank_off(struct drm_crtc *crtc);
>  void drm_crtc_vblank_reset(struct drm_crtc *crtc);
>  void drm_crtc_vblank_on(struct drm_crtc *crtc);
>  u64 drm_crtc_accurate_vblank_count(struct drm_crtc *crtc);
> -void drm_vblank_restore(struct drm_device *dev, unsigned int pipe);
>  void drm_crtc_vblank_restore(struct drm_crtc *crtc);
>  
>  void drm_calc_timestamping_constants(struct drm_crtc *crtc,
> -- 
> 2.30.0

-- 
Ville Syrjälä
Intel
___
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

Re: [PATCH v4 00/14] RFC Support hot device unplug in amdgpu

2021-02-09 Thread Andrey Grodzovsky




On 2/9/21 4:50 AM, Daniel Vetter wrote:

On Mon, Feb 08, 2021 at 11:01:14PM -0500, Andrey Grodzovsky wrote:

On 2/8/21 2:27 AM, Daniel Vetter wrote:

On Mon, Feb 8, 2021 at 6:59 AM Andrey Grodzovsky
 wrote:

On 1/20/21 10:59 AM, Daniel Vetter wrote:

On Wed, Jan 20, 2021 at 3:20 PM Andrey Grodzovsky
 wrote:

On 1/20/21 4:05 AM, Daniel Vetter wrote:

On Tue, Jan 19, 2021 at 01:18:15PM -0500, Andrey Grodzovsky wrote:

On 1/19/21 1:08 PM, Daniel Vetter wrote:

On Tue, Jan 19, 2021 at 6:31 PM Andrey Grodzovsky
 wrote:

On 1/19/21 9:16 AM, Daniel Vetter wrote:

On Mon, Jan 18, 2021 at 04:01:09PM -0500, Andrey Grodzovsky wrote:

Until now extracting a card either by physical extraction (e.g. eGPU with
thunderbolt connection or by emulation through  syfs -> 
/sys/bus/pci/devices/device_id/remove)
would cause random crashes in user apps. The random crashes in apps were
mostly due to the app having mapped a device backed BO into its address
space was still trying to access the BO while the backing device was gone.
To answer this first problem Christian suggested to fix the handling of mapped
memory in the clients when the device goes away by forcibly unmap all buffers 
the
user processes has by clearing their respective VMAs mapping the device BOs.
Then when the VMAs try to fill in the page tables again we check in the fault
handlerif the device is removed and if so, return an error. This will generate a
SIGBUS to the application which can then cleanly terminate.This indeed was done
but this in turn created a problem of kernel OOPs were the OOPSes were due to 
the
fact that while the app was terminating because of the SIGBUSit would trigger 
use
after free in the driver by calling to accesses device structures that were 
already
released from the pci remove sequence.This was handled by introducing a 'flush'
sequence during device removal were we wait for drm file reference to drop to 0
meaning all user clients directly using this device terminated.

v2:
Based on discussions in the mailing list with Daniel and Pekka [1] and based on 
the document
produced by Pekka from those discussions [2] the whole approach with returning 
SIGBUS and
waiting for all user clients having CPU mapping of device BOs to die was 
dropped.
Instead as per the document suggestion the device structures are kept alive 
until
the last reference to the device is dropped by user client and in the meanwhile 
all existing and new CPU mappings of the BOs
belonging to the device directly or by dma-buf import are rerouted to per user
process dummy rw page.Also, I skipped the 'Requirements for KMS UAPI' section 
of [2]
since i am trying to get the minimal set of requirements that still give useful 
solution
to work and this is the'Requirements for Render and Cross-Device UAPI' section 
and so my
test case is removing a secondary device, which is render only and is not 
involved
in KMS.

v3:
More updates following comments from v2 such as removing loop to find DRM file 
when rerouting
page faults to dummy page,getting rid of unnecessary sysfs handling refactoring 
and moving
prevention of GPU recovery post device unplug from amdgpu to scheduler layer.
On top of that added unplug support for the IOMMU enabled system.

v4:
Drop last sysfs hack and use sysfs default attribute.
Guard against write accesses after device removal to avoid modifying released 
memory.
Update dummy pages handling to on demand allocation and release through drm 
managed framework.
Add return value to scheduler job TO handler (by Luben Tuikov) and use this in 
amdgpu for prevention
of GPU recovery post device unplug
Also rebase on top of drm-misc-mext instead of amd-staging-drm-next

With these patches I am able to gracefully remove the secondary card using 
sysfs remove hook while glxgears
is running off of secondary card (DRI_PRIME=1) without kernel oopses or hangs 
and keep working
with the primary card or soft reset the device without hangs or oopses

TODOs for followup work:
Convert AMDGPU code to use devm (for hw stuff) and drmm (for sw stuff and 
allocations) (Daniel)
Support plugging the secondary device back after unplug - currently still 
experiencing HW error on plugging back.
Add support for 'Requirements for KMS UAPI' section of [2] - unplugging 
primary, display connected card.

[1] - Discussions during v3 of the patchset 
https://nam11.safelinks.protection.outlook.com/?url=https%3A%2F%2Fwww.spinics.net%2Flists%2Famd-gfx%2Fmsg55576.html&data=04%7C01%7CAndrey.Grodzovsky%40amd.com%7C49a8be1691aca3ce08d8cce01c88%7C3dd8961fe4884e608e11a82d994e183d%7C0%7C0%7C637484610239738569%7CUnknown%7CTWFpbGZsb3d8eyJWIjoiMC4wLjAwMDAiLCJQIjoiV2luMzIiLCJBTiI6Ik1haWwiLCJXVCI6Mn0%3D%7C1000&sdata=PkvEkJt2zQ9G0dpiGYtSuVLpziwQ0FklkjdMLV2ZOnE%3D&reserved=0
[2] - drm/doc: device hot-unplug for userspace 
https://nam11.safelinks.protection.outlook.com/?url=https%3A%2F%2Fwww.spinics.net%2Flists%2Fdri-devel%2Fmsg259755.html&data=04%7C01%7CAndrey.Grodzovsky%40amd.com%7C49a8be1691aca3ce0

Re: [PATCH v2 3/3] drm/panfrost: Stay in the threaded MMU IRQ handler until we've handled all IRQs

On Fri, Feb 5, 2021 at 5:18 AM Boris Brezillon
 wrote:
>
> Doing a hw-irq -> threaded-irq round-trip is counter-productive, stay
> in the threaded irq handler as long as we can.
>
> v2:
> * Rework the loop to avoid a goto
>
> Signed-off-by: Boris Brezillon 
> ---
>  drivers/gpu/drm/panfrost/panfrost_mmu.c | 26 +
>  1 file changed, 14 insertions(+), 12 deletions(-)

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

[PATCH v6] dt-bindings: display: panel: one file of all simple LVDS panels with dual ports

2021-02-09 Thread Liu Ying

To complement panel-simple.yaml, create panel-simple-lvds-dual-ports.yaml.
panel-simple-lvds-dual-ports.yaml is for all simple LVDS panels that
have dual LVDS ports and require only a single power-supply.
The first port receives odd pixels, and the second port receives even pixels.
Optionally, a backlight and an enable GPIO can be specified as properties.

Panels with swapped pixel order, if any, need dedicated bindings.

Migrate 'auo,g133han01', 'auo,g185han01', 'auo,g190ean01',
'koe,tx26d202vm0bwa' and 'nlt,nl192108ac18-02d' over to the new file.

The objectives with one file for all the simple LVDS panels with dual ports are:
- Make it simpler to add bindings for this kind of LVDS panels
- Keep the number of bindings file lower
- Keep the binding documentation for this kind of LVDS panels more consistent
- Make it possible for drivers to get pixel order via
  drm_of_lvds_get_dual_link_pixel_order(), as the 'ports' property is required

Suggested-by: Sam Ravnborg 
Cc: Thierry Reding 
Cc: Sam Ravnborg 
Cc: David Airlie 
Cc: Daniel Vetter 
Cc: Rob Herring 
Cc: Lucas Stach 
Cc: Sebastian Reichel 
Signed-off-by: Liu Ying 
---
v5->v6:
* Use OF graph schema.
* Drop Rob's R-b tag, as review is needed.

v4->v5:
* Require the 'ports' property and update commit message accordingly. (Rob)
* Add Rob's R-b tag.

v3->v4:
* Add type and descriptions for dual-lvds-{odd,even}-pixels properties.
  Also, update descriptions for port@0 and port@1 properties accordingly. (Rob)

v2->v3:
* Do not allow 'port' property. (Rob)
* Define port number. (Rob)
* Specify 'dual-lvds-odd-pixels' and 'dual-lvds-even-pixels' properties. (Rob)

v1->v2:
* Correct pixel order in example LVDS panel node.

 .../panel/panel-simple-lvds-dual-ports.yaml| 116 +
 .../bindings/display/panel/panel-simple.yaml   |  10 --
 2 files changed, 116 insertions(+), 10 deletions(-)
 create mode 100644 
Documentation/devicetree/bindings/display/panel/panel-simple-lvds-dual-ports.yaml

diff --git 
a/Documentation/devicetree/bindings/display/panel/panel-simple-lvds-dual-ports.yaml
 
b/Documentation/devicetree/bindings/display/panel/panel-simple-lvds-dual-ports.yaml
new file mode 100644
index ..274e89b
--- /dev/null
+++ 
b/Documentation/devicetree/bindings/display/panel/panel-simple-lvds-dual-ports.yaml
@@ -0,0 +1,116 @@
+# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause)
+%YAML 1.2
+---
+$id: 
http://devicetree.org/schemas/display/panel/panel-simple-lvds-dual-ports.yaml#
+$schema: http://devicetree.org/meta-schemas/core.yaml#
+
+title: Simple LVDS panels with one power supply and dual LVDS ports
+
+maintainers:
+  - Liu Ying 
+  - Thierry Reding 
+  - Sam Ravnborg 
+
+description: |
+  This binding file is a collection of the LVDS panels that
+  has dual LVDS ports and requires only a single power-supply.
+  The first port receives odd pixels, and the second port receives even pixels.
+  There are optionally a backlight and an enable GPIO.
+  The panel may use an OF graph binding for the association to the display,
+  or it may be a direct child node of the display.
+
+  If the panel is more advanced a dedicated binding file is required.
+
+allOf:
+  - $ref: panel-common.yaml#
+
+properties:
+
+  compatible:
+enum:
+# compatible must be listed in alphabetical order, ordered by compatible.
+# The description in the comment is mandatory for each compatible.
+
+# AU Optronics Corporation 13.3" FHD (1920x1080) TFT LCD panel
+  - auo,g133han01
+# AU Optronics Corporation 18.5" FHD (1920x1080) TFT LCD panel
+  - auo,g185han01
+# AU Optronics Corporation 19.0" (1280x1024) TFT LCD panel
+  - auo,g190ean01
+# Kaohsiung Opto-Electronics Inc. 10.1" WUXGA (1920 x 1200) LVDS TFT 
LCD panel
+  - koe,tx26d202vm0bwa
+# NLT Technologies, Ltd. 15.6" FHD (1920x1080) LVDS TFT LCD panel
+  - nlt,nl192108ac18-02d
+
+  ports:
+$ref: /schemas/graph.yaml#/properties/ports
+
+properties:
+  port@0:
+$ref: /schemas/graph.yaml#/properties/port
+description: The first sink port.
+
+properties:
+  dual-lvds-odd-pixels:
+type: boolean
+description: The first sink port for odd pixels.
+
+required:
+  - dual-lvds-odd-pixels
+
+  port@1:
+$ref: /schemas/graph.yaml#/properties/port
+description: The second sink port.
+
+properties:
+  dual-lvds-even-pixels:
+type: boolean
+description: The second sink port for even pixels.
+
+required:
+  - dual-lvds-even-pixels
+
+required:
+  - port@0
+  - port@1
+
+  backlight: true
+  enable-gpios: true
+  power-supply: true
+
+additionalProperties: false
+
+required:
+  - compatible
+  - ports
+  - power-supply
+
+examples:
+  - |
+panel: panel-lvds {
+  compatible = "koe,tx26d202vm0bwa";
+  power-supply = <&vdd_lcd_reg>;
+
+  ports {
+#address-cells

Re: [PATCH v13 7/8] soc: mediatek: add mtk mutex support for MT8183

2021-02-09 Thread Enric Balletbo Serra

Hi Hsin-Yi,

Thank you for your patch.

Missatge de Hsin-Yi Wang  del dia dv., 29 de gen.
2021 a les 10:23:
>
> From: Yongqiang Niu 
>
> Add mtk mutex support for MT8183 SoC.
>
> Signed-off-by: Yongqiang Niu 
> Signed-off-by: Hsin-Yi Wang 
> Reviewed-by: CK Hu 

Reviewed-by: Enric Balletbo i Serra 

FWIW this patch is required to have the display working on the
Chromebook IdeaPad Duet, so

Tested-by: Enric Balletbo i Serra 

Matthias, If I am not wrong, this patch is the only one that is not
applied for this series. I know that is too late for 5.12, but If
you're fine with it, could you pick this patch directly or do you
prefer a resend of this patch alone once you will start to accept
patches for the next release?

Thanks,
  Enric

> ---
>  drivers/soc/mediatek/mtk-mutex.c | 50 
>  1 file changed, 50 insertions(+)
>
> diff --git a/drivers/soc/mediatek/mtk-mutex.c 
> b/drivers/soc/mediatek/mtk-mutex.c
> index f531b119da7a9..718a41beb6afb 100644
> --- a/drivers/soc/mediatek/mtk-mutex.c
> +++ b/drivers/soc/mediatek/mtk-mutex.c
> @@ -14,6 +14,8 @@
>
>  #define MT2701_MUTEX0_MOD0 0x2c
>  #define MT2701_MUTEX0_SOF0 0x30
> +#define MT8183_MUTEX0_MOD0 0x30
> +#define MT8183_MUTEX0_SOF0 0x2c
>
>  #define DISP_REG_MUTEX_EN(n)   (0x20 + 0x20 * (n))
>  #define DISP_REG_MUTEX(n)  (0x24 + 0x20 * (n))
> @@ -37,6 +39,18 @@
>  #define MT8167_MUTEX_MOD_DISP_DITHER   15
>  #define MT8167_MUTEX_MOD_DISP_UFOE 16
>
> +#define MT8183_MUTEX_MOD_DISP_RDMA00
> +#define MT8183_MUTEX_MOD_DISP_RDMA11
> +#define MT8183_MUTEX_MOD_DISP_OVL0 9
> +#define MT8183_MUTEX_MOD_DISP_OVL0_2L  10
> +#define MT8183_MUTEX_MOD_DISP_OVL1_2L  11
> +#define MT8183_MUTEX_MOD_DISP_WDMA012
> +#define MT8183_MUTEX_MOD_DISP_COLOR0   13
> +#define MT8183_MUTEX_MOD_DISP_CCORR0   14
> +#define MT8183_MUTEX_MOD_DISP_AAL0 15
> +#define MT8183_MUTEX_MOD_DISP_GAMMA0   16
> +#define MT8183_MUTEX_MOD_DISP_DITHER0  17
> +
>  #define MT8173_MUTEX_MOD_DISP_OVL0 11
>  #define MT8173_MUTEX_MOD_DISP_OVL1 12
>  #define MT8173_MUTEX_MOD_DISP_RDMA013
> @@ -87,6 +101,11 @@
>  #define MT2712_MUTEX_SOF_DSI3  6
>  #define MT8167_MUTEX_SOF_DPI0  2
>  #define MT8167_MUTEX_SOF_DPI1  3
> +#define MT8183_MUTEX_SOF_DSI0  1
> +#define MT8183_MUTEX_SOF_DPI0  2
> +
> +#define MT8183_MUTEX_EOF_DSI0  (MT8183_MUTEX_SOF_DSI0 << 6)
> +#define MT8183_MUTEX_EOF_DPI0  (MT8183_MUTEX_SOF_DPI0 << 6)
>
>  struct mtk_mutex {
> int id;
> @@ -181,6 +200,20 @@ static const unsigned int 
> mt8173_mutex_mod[DDP_COMPONENT_ID_MAX] = {
> [DDP_COMPONENT_WDMA1] = MT8173_MUTEX_MOD_DISP_WDMA1,
>  };
>
> +static const unsigned int mt8183_mutex_mod[DDP_COMPONENT_ID_MAX] = {
> +   [DDP_COMPONENT_AAL0] = MT8183_MUTEX_MOD_DISP_AAL0,
> +   [DDP_COMPONENT_CCORR] = MT8183_MUTEX_MOD_DISP_CCORR0,
> +   [DDP_COMPONENT_COLOR0] = MT8183_MUTEX_MOD_DISP_COLOR0,
> +   [DDP_COMPONENT_DITHER] = MT8183_MUTEX_MOD_DISP_DITHER0,
> +   [DDP_COMPONENT_GAMMA] = MT8183_MUTEX_MOD_DISP_GAMMA0,
> +   [DDP_COMPONENT_OVL0] = MT8183_MUTEX_MOD_DISP_OVL0,
> +   [DDP_COMPONENT_OVL_2L0] = MT8183_MUTEX_MOD_DISP_OVL0_2L,
> +   [DDP_COMPONENT_OVL_2L1] = MT8183_MUTEX_MOD_DISP_OVL1_2L,
> +   [DDP_COMPONENT_RDMA0] = MT8183_MUTEX_MOD_DISP_RDMA0,
> +   [DDP_COMPONENT_RDMA1] = MT8183_MUTEX_MOD_DISP_RDMA1,
> +   [DDP_COMPONENT_WDMA0] = MT8183_MUTEX_MOD_DISP_WDMA0,
> +};
> +
>  static const unsigned int mt2712_mutex_sof[MUTEX_SOF_DSI3 + 1] = {
> [MUTEX_SOF_SINGLE_MODE] = MUTEX_SOF_SINGLE_MODE,
> [MUTEX_SOF_DSI0] = MUTEX_SOF_DSI0,
> @@ -198,6 +231,13 @@ static const unsigned int 
> mt8167_mutex_sof[MUTEX_SOF_DSI3 + 1] = {
> [MUTEX_SOF_DPI1] = MT8167_MUTEX_SOF_DPI1,
>  };
>
> +/* Add EOF setting so overlay hardware can receive frame done irq */
> +static const unsigned int mt8183_mutex_sof[MUTEX_SOF_DSI3 + 1] = {
> +   [MUTEX_SOF_SINGLE_MODE] = MUTEX_SOF_SINGLE_MODE,
> +   [MUTEX_SOF_DSI0] = MUTEX_SOF_DSI0 | MT8183_MUTEX_EOF_DSI0,
> +   [MUTEX_SOF_DPI0] = MT8183_MUTEX_SOF_DPI0 | MT8183_MUTEX_EOF_DPI0,
> +};
> +
>  static const struct mtk_mutex_data mt2701_mutex_driver_data = {
> .mutex_mod = mt2701_mutex_mod,
> .mutex_sof = mt2712_mutex_sof,
> @@ -227,6 +267,14 @@ static const struct mtk_mutex_data 
> mt8173_mutex_driver_data = {
> .mutex_sof_reg = MT2701_MUTEX0_SOF0,
>  };
>
> +static const struct mtk_mutex_data mt8183_mutex_driver_data = {
> +   .mutex_mod = mt8183_mutex_mod,
> +   .mutex_sof = mt8183_mutex_sof,
> +   .mutex_mod_reg = MT8183_MUTEX0_MOD0,
> +   .mutex_sof_reg = MT8183_MUTEX0_SOF0,
> +   .n

Re: [PATCH v4 11/14] drm/amdgpu: Guard against write accesses after device removal

2021-02-09 Thread Andrey Grodzovsky

On 2/9/21 2:58 AM, Christian König wrote:

Am 08.02.21 um 23:15 schrieb Andrey Grodzovsky:

On 2/8/21 11:23 AM, Daniel Vetter wrote:
On Mon, Feb 8, 2021 at 3:00 PM Christian König
wrote:

Am 08.02.21 um 11:11 schrieb Daniel Vetter:

On Mon, Feb 08, 2021 at 11:03:15AM +0100, Christian König wrote:

Am 08.02.21 um 10:48 schrieb Daniel Vetter:

On Mon, Feb 08, 2021 at 10:37:19AM +0100, Christian König wrote:

Am 07.02.21 um 22:50 schrieb Daniel Vetter:

[SNIP]
Clarification - as far as I know there are no page fault handlers for
kernel
mappings. And we are talking about kernel mappings here, right ? If
there were

I could solve all those issues the same as I do for user mappings, by
invalidating all existing mappings in the kernel (both kmaps and
ioreamps)and

insert dummy zero or ~0 filled page instead.
Also, I assume forcefully remapping the IO BAR to ~0 filled page
would involve
ioremap API and it's not something that I think can be easily done
according to

am answer i got to a related topic a few weeks ago
https://nam11.safelinks.protection.outlook.com/?url=https%3A%2F%2Fwww.spinics.net%2Flists%2Flinux-pci%2Fmsg103396.html&data=04%7C01%7CAndrey.Grodzovsky%40amd.com%7C9d1bdf4cee504cd71b4908d8cc4df310%7C3dd8961fe4884e608e11a82d994e183d%7C0%7C0%7C637483982454608249%7CUnknown%7CTWFpbGZsb3d8eyJWIjoiMC4wLjAwMDAiLCJQIjoiV2luMzIiLCJBTiI6Ik1haWwiLCJXVCI6Mn0%3D%7C1000&sdata=Anw%2BOwJ%2B5tvjW3tmkVNdz13%2BZ18vdpfOLWqsUZL7D2I%3D&reserved=0
(that was the only reply

i got)

mmiotrace can, but only for debug, and only on x86 platforms:

https://nam11.safelinks.protection.outlook.com/?url=https%3A%2F%2Fwww.kernel.org%2Fdoc%2Fhtml%2Flatest%2Ftrace%2Fmmiotrace.html&data=04%7C01%7CAndrey.Grodzovsky%40amd.com%7C9d1bdf4cee504cd71b4908d8cc4df310%7C3dd8961fe4884e608e11a82d994e183d%7C0%7C0%7C637483982454608249%7CUnknown%7CTWFpbGZsb3d8eyJWIjoiMC4wLjAwMDAiLCJQIjoiV2luMzIiLCJBTiI6Ik1haWwiLCJXVCI6Mn0%3D%7C1000&sdata=Wa7BFNySVQJLyD6WY4pZHTP1QfeZwD7F5ydBrXuxppQ%3D&reserved=0

Should be feasible (but maybe not worth the effort) to extend this to
support fake unplug.

Mhm, interesting idea you guys brought up here.

We don't need a page fault for this to work, all we need to do is to
insert

dummy PTEs into the kernels page table at the place where previously the
MMIO mapping has been.

Simply pte trick isn't enough, because we need:
- drop all writes silently
- all reads return 0xff

ptes can't do that themselves, we minimally need write protection and then
silently proceed on each write fault without restarting the instruction.
Better would be to only catch reads, but x86 doesn't do write-only pte
permissions afaik.

You are not thinking far enough :)

The dummy PTE is point to a dummy MMIO page which is just never used.

That hast the exact same properties than our removed MMIO space just doesn't
goes bananas when a new device is MMIO mapped into that and our driver still
tries to write there.

Hm, but where do we get such a "guaranteed never used" mmio page from?

Well we have tons of unused IO space on 64bit systems these days.

Doesn't really needs to be PCIe address space, doesn't it?

That sounds very trusting to modern systems not decoding random
ranges. E.g. the pci code stopped extending the host bridge windows on
its own, entirely relying on the acpi provided ranges, to avoid
stomping on stuff that's the but not listed anywhere.

I guess if we have a range behind a pci bridge, which isn't used by
any device, but decoded by the bridge, then that should be safe
enough. Maybe could even have an option in upstream to do that on
unplug, if a certain flag is set, or a cmdline option.
-Daniel

Question - Why can't we just set those PTEs to point to system memory
(another RO dummy page)

filled with 1s ?

Then writes are not discarded. E.g. the 1s would change to something else.

Christian.

I see but, what about marking the mappings as RO and discarding the write access
page faults continuously until the device is finalized ?
Regarding using an unused range behind the upper bridge as Daniel suggested, I
wonder will this interfere with
the upcoming feature to support BARs movement during hot plug -
https://www.spinics.net/lists/linux-pci/msg103195.html ?

Andrey

Christian.

It's a nifty idea indeed otherwise ...
-Daniel

Regards,
Christian.

But ugh ...

Otoh validating an entire driver like amdgpu without such a trick
against 0xff reads is practically impossible. So maybe you need to add
this as one of the tasks here?
Or I could just for validation purposes return ~0 from all reg reads
in the code
and ignore writes if drm_dev_unplugged, this could already easily
validate a big

portion of the code flow under such scenario.

Hm yeah if your really wrap them all, that should work too. Since
iommappings have __iomem pointer type, as long as amdgpu is sparse
warning free, should be doable to guarantee this.

Problem is that ~0 is not always a valid register value.

You would need t

Re: [Linaro-mm-sig] [PATCH] RFC: dma-fence: Document recoverable page fault implications

2021-02-09 Thread Felix Kuehling

Am 2021-02-09 um 9:08 a.m. schrieb Daniel Vetter:
> On Tue, Feb 9, 2021 at 12:15 PM Felix Kuehling  wrote:
>> Am 2021-02-09 um 1:37 a.m. schrieb Daniel Vetter:
>>> On Tue, Feb 9, 2021 at 4:13 AM Bas Nieuwenhuizen
>>>  wrote:
 On Thu, Jan 28, 2021 at 4:40 PM Felix Kuehling  
 wrote:
> Am 2021-01-28 um 2:39 a.m. schrieb Christian König:
>> Am 27.01.21 um 23:00 schrieb Felix Kuehling:
>>> Am 2021-01-27 um 7:16 a.m. schrieb Christian König:
 Am 27.01.21 um 13:11 schrieb Maarten Lankhorst:
> Op 27-01-2021 om 01:22 schreef Felix Kuehling:
>> Am 2021-01-21 um 2:40 p.m. schrieb Daniel Vetter:
>>> Recently there was a fairly long thread about recoreable hardware
>>> page
>>> faults, how they can deadlock, and what to do about that.
>>>
>>> While the discussion is still fresh I figured good time to try and
>>> document the conclusions a bit.
>>>
>>> References:
>>> https://lore.kernel.org/dri-devel/20210107030127.20393-1-felix.kuehl...@amd.com/
>>>
>>> Cc: Maarten Lankhorst 
>>> Cc: Thomas Hellström 
>>> Cc: "Christian König" 
>>> Cc: Jerome Glisse 
>>> Cc: Felix Kuehling 
>>> Signed-off-by: Daniel Vetter 
>>> Cc: Sumit Semwal 
>>> Cc: linux-me...@vger.kernel.org
>>> Cc: linaro-mm-...@lists.linaro.org
>>> --
>>> I'll be away next week, but figured I'll type this up quickly for
>>> some
>>> comments and to check whether I got this all roughly right.
>>>
>>> Critique very much wanted on this, so that we can make sure hw which
>>> can't preempt (with pagefaults pending) like gfx10 has a clear
>>> path to
>>> support page faults in upstream. So anything I missed, got wrong or
>>> like that would be good.
>>> -Daniel
>>> ---
>>>Documentation/driver-api/dma-buf.rst | 66
>>> 
>>>1 file changed, 66 insertions(+)
>>>
>>> diff --git a/Documentation/driver-api/dma-buf.rst
>>> b/Documentation/driver-api/dma-buf.rst
>>> index a2133d69872c..e924c1e4f7a3 100644
>>> --- a/Documentation/driver-api/dma-buf.rst
>>> +++ b/Documentation/driver-api/dma-buf.rst
>>> @@ -257,3 +257,69 @@ fences in the kernel. This means:
>>>  userspace is allowed to use userspace fencing or long running
>>> compute
>>>  workloads. This also means no implicit fencing for shared
>>> buffers in these
>>>  cases.
>>> +
>>> +Recoverable Hardware Page Faults Implications
>>> +~
>>> +
>>> +Modern hardware supports recoverable page faults, which has a
>>> lot of
>>> +implications for DMA fences.
>>> +
>>> +First, a pending page fault obviously holds up the work that's
>>> running on the
>>> +accelerator and a memory allocation is usually required to resolve
>>> the fault.
>>> +But memory allocations are not allowed to gate completion of DMA
>>> fences, which
>>> +means any workload using recoverable page faults cannot use DMA
>>> fences for
>>> +synchronization. Synchronization fences controlled by userspace
>>> must be used
>>> +instead.
>>> +
>>> +On GPUs this poses a problem, because current desktop compositor
>>> protocols on
>>> +Linus rely on DMA fences, which means without an entirely new
>>> userspace stack
>>> +built on top of userspace fences, they cannot benefit from
>>> recoverable page
>>> +faults. The exception is when page faults are only used as
>>> migration hints and
>>> +never to on-demand fill a memory request. For now this means
>>> recoverable page
>>> +faults on GPUs are limited to pure compute workloads.
>>> +
>>> +Furthermore GPUs usually have shared resources between the 3D
>>> rendering and
>>> +compute side, like compute units or command submission engines. If
>>> both a 3D
>>> +job with a DMA fence and a compute workload using recoverable page
>>> faults are
>>> +pending they could deadlock:
>>> +
>>> +- The 3D workload might need to wait for the compute job to finish
>>> and release
>>> +  hardware resources first.
>>> +
>>> +- The compute workload might be stuck in a page fault, because the
>>> memory
>>> +  allocation is waiting for the DMA fence of the 3D workload to
>>> complete.
>>> +
>>> +There are a few ways to prevent this problem:
>>> +
>>> +- Compute workloads can always be preempted, even when a page
>>> fault is p

Re: [PATCH] backlight: ktd253: Bring up in a known state

2021-02-09 Thread Lee Jones

On Tue, 26 Jan 2021, Linus Walleij wrote:

> The KTD253 backlight might already be on when the driver
> is probed: then we don't really know what the current
> ratio is and all light intensity settings will be off
> relative to what it was at boot.
> 
> To fix this, bring up the backlight OFF then move it to
> the default backlight from there so we know the state.
> 
> Signed-off-by: Linus Walleij 
> ---
>  drivers/video/backlight/ktd253-backlight.c | 12 +++-
>  1 file changed, 3 insertions(+), 9 deletions(-)

Grudgingly applied, thanks.

-- 
Lee Jones [李琼斯]
Senior Technical Lead - Developer Services
Linaro.org │ Open source software for Arm SoCs
Follow Linaro: Facebook | Twitter | Blog
___
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

Re: [Linaro-mm-sig] [PATCH] RFC: dma-fence: Document recoverable page fault implications

On Tue, Feb 9, 2021 at 12:15 PM Felix Kuehling  wrote:
> Am 2021-02-09 um 1:37 a.m. schrieb Daniel Vetter:
> > On Tue, Feb 9, 2021 at 4:13 AM Bas Nieuwenhuizen
> >  wrote:
> >> On Thu, Jan 28, 2021 at 4:40 PM Felix Kuehling  
> >> wrote:
> >>> Am 2021-01-28 um 2:39 a.m. schrieb Christian König:
>  Am 27.01.21 um 23:00 schrieb Felix Kuehling:
> > Am 2021-01-27 um 7:16 a.m. schrieb Christian König:
> >> Am 27.01.21 um 13:11 schrieb Maarten Lankhorst:
> >>> Op 27-01-2021 om 01:22 schreef Felix Kuehling:
>  Am 2021-01-21 um 2:40 p.m. schrieb Daniel Vetter:
> > Recently there was a fairly long thread about recoreable hardware
> > page
> > faults, how they can deadlock, and what to do about that.
> >
> > While the discussion is still fresh I figured good time to try and
> > document the conclusions a bit.
> >
> > References:
> > https://lore.kernel.org/dri-devel/20210107030127.20393-1-felix.kuehl...@amd.com/
> >
> > Cc: Maarten Lankhorst 
> > Cc: Thomas Hellström 
> > Cc: "Christian König" 
> > Cc: Jerome Glisse 
> > Cc: Felix Kuehling 
> > Signed-off-by: Daniel Vetter 
> > Cc: Sumit Semwal 
> > Cc: linux-me...@vger.kernel.org
> > Cc: linaro-mm-...@lists.linaro.org
> > --
> > I'll be away next week, but figured I'll type this up quickly for
> > some
> > comments and to check whether I got this all roughly right.
> >
> > Critique very much wanted on this, so that we can make sure hw which
> > can't preempt (with pagefaults pending) like gfx10 has a clear
> > path to
> > support page faults in upstream. So anything I missed, got wrong or
> > like that would be good.
> > -Daniel
> > ---
> >Documentation/driver-api/dma-buf.rst | 66
> > 
> >1 file changed, 66 insertions(+)
> >
> > diff --git a/Documentation/driver-api/dma-buf.rst
> > b/Documentation/driver-api/dma-buf.rst
> > index a2133d69872c..e924c1e4f7a3 100644
> > --- a/Documentation/driver-api/dma-buf.rst
> > +++ b/Documentation/driver-api/dma-buf.rst
> > @@ -257,3 +257,69 @@ fences in the kernel. This means:
> >  userspace is allowed to use userspace fencing or long running
> > compute
> >  workloads. This also means no implicit fencing for shared
> > buffers in these
> >  cases.
> > +
> > +Recoverable Hardware Page Faults Implications
> > +~
> > +
> > +Modern hardware supports recoverable page faults, which has a
> > lot of
> > +implications for DMA fences.
> > +
> > +First, a pending page fault obviously holds up the work that's
> > running on the
> > +accelerator and a memory allocation is usually required to resolve
> > the fault.
> > +But memory allocations are not allowed to gate completion of DMA
> > fences, which
> > +means any workload using recoverable page faults cannot use DMA
> > fences for
> > +synchronization. Synchronization fences controlled by userspace
> > must be used
> > +instead.
> > +
> > +On GPUs this poses a problem, because current desktop compositor
> > protocols on
> > +Linus rely on DMA fences, which means without an entirely new
> > userspace stack
> > +built on top of userspace fences, they cannot benefit from
> > recoverable page
> > +faults. The exception is when page faults are only used as
> > migration hints and
> > +never to on-demand fill a memory request. For now this means
> > recoverable page
> > +faults on GPUs are limited to pure compute workloads.
> > +
> > +Furthermore GPUs usually have shared resources between the 3D
> > rendering and
> > +compute side, like compute units or command submission engines. If
> > both a 3D
> > +job with a DMA fence and a compute workload using recoverable page
> > faults are
> > +pending they could deadlock:
> > +
> > +- The 3D workload might need to wait for the compute job to finish
> > and release
> > +  hardware resources first.
> > +
> > +- The compute workload might be stuck in a page fault, because the
> > memory
> > +  allocation is waiting for the DMA fence of the 3D workload to
> > complete.
> > +
> > +There are a few ways to prevent this problem:
> > +
> > +- Compute workloads can always be preempted, even when a page
> > fault is pending
> > +  and not yet repaired. Not all h

Re: [PATCH] drm/ttm: fix removal of bo_count sysfs file

2021-02-09 Thread Nirmoy


Reviewed-by: Nirmoy Das

On 2/9/21 2:17 PM, Christian König wrote:

Only a zombie leftover from rebasing.

Signed-off-by: Christian König 
Fixes: 3763d635deaa ("drm/ttm: add debugfs directory v2")
---
  drivers/gpu/drm/ttm/ttm_device.c | 2 --
  include/drm/ttm/ttm_device.h | 1 -
  2 files changed, 3 deletions(-)

diff --git a/drivers/gpu/drm/ttm/ttm_device.c b/drivers/gpu/drm/ttm/ttm_device.c
index ac0903c9e60a..8bb61dd26437 100644
--- a/drivers/gpu/drm/ttm/ttm_device.c
+++ b/drivers/gpu/drm/ttm/ttm_device.c
@@ -49,8 +49,6 @@ static void ttm_global_release(void)
if (--ttm_glob_use_count > 0)
goto out;
  
-	kobject_del(&glob->kobj);

-   kobject_put(&glob->kobj);
ttm_mem_global_release(&ttm_mem_glob);
__free_page(glob->dummy_read_page);
memset(glob, 0, sizeof(*glob));
diff --git a/include/drm/ttm/ttm_device.h b/include/drm/ttm/ttm_device.h
index 7bc8bb797161..035bbc044a3b 100644
--- a/include/drm/ttm/ttm_device.h
+++ b/include/drm/ttm/ttm_device.h
@@ -55,7 +55,6 @@ extern struct ttm_global {
 * Constant after init.
 */
  
-	struct kobject kobj;

struct page *dummy_read_page;
spinlock_t lru_lock;
  

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

[Bug 211425] [drm:atom_op_jump] ERROR atombios stuck in loop for more than 20secs aborting

2021-02-09 Thread bugzilla-daemon

https://bugzilla.kernel.org/show_bug.cgi?id=211425

Andreas (icedragon...@web.de) changed:

   What|Removed |Added

 Kernel Version|5.10.13 |5.10.14

--- Comment #4 from Andreas (icedragon...@web.de) ---
Hi, it is still reproducibility with mainline kernel 5.10.14:

[Di Feb  9 12:51:04 2021] [drm:atom_op_jump] *ERROR* atombios stuck in loop for
more than 20secs aborting
[Di Feb  9 12:51:04 2021] [drm:amdgpu_atom_execute_table_locked] *ERROR*
atombios stuck executing B200 (len 3615, WS 8, PS 0) @ 0xB34E
[Di Feb  9 12:51:04 2021] [drm:amdgpu_atom_execute_table_locked] *ERROR*
atombios stuck executing B0F4 (len 268, WS 4, PS 0) @ 0xB147
[Di Feb  9 12:51:04 2021] [drm:dcn10_link_encoder_enable_dp_output] *ERROR*
dcn10_link_encoder_enable_dp_output: Failed to execute VBIOS command table!
[Di Feb  9 12:51:06 2021] [drm] amdgpu_dm_irq_schedule_work FAILED src 2
...
[Di Feb  9 12:51:26 2021] [drm:atom_op_jump] *ERROR* atombios stuck in loop for
more than 20secs aborting
[Di Feb  9 12:51:26 2021] [drm:amdgpu_atom_execute_table_locked] *ERROR*
atombios stuck executing B200 (len 3615, WS 8, PS 0) @ 0xB6EA
[Di Feb  9 12:51:26 2021] [drm:amdgpu_atom_execute_table_locked] *ERROR*
atombios stuck executing B0F4 (len 268, WS 4, PS 0) @ 0xB147
[Di Feb  9 12:51:26 2021] [drm:dcn10_link_encoder_enable_dp_output] *ERROR*
dcn10_link_encoder_enable_dp_output: Failed to execute VBIOS command table!

-- 
You may reply to this email to add a comment.

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] drm/ttm: fix removal of bo_count sysfs file

On Tue, Feb 9, 2021 at 2:34 PM Christian König
 wrote:
>
>
>
> Am 09.02.21 um 14:32 schrieb Daniel Vetter:
> > On Tue, Feb 9, 2021 at 2:18 PM Christian König
> >  wrote:
> >> Only a zombie leftover from rebasing.
> >>
> >> Signed-off-by: Christian König 
> >> Fixes: 3763d635deaa ("drm/ttm: add debugfs directory v2")
> > My drm-misc-next still uses this in places,
>
> Hui? Where?

Duh, I mixed up ttm_mem_global with ttm_global and didn't fitler my
grep output correctly ...

>
> >   is this just fallout from
> > the move of the sysfs files into vmwgfx? Your Fixes: line here
> > confuses me a lot, and on the current baseline I don't think this
> > applies ...
>
> As far as I can see this was just an error during rebasing.
>
> Commit 3763d635deaa ("drm/ttm: add debugfs directory v2") removed the
> kobj, but while rebasing the ttm_device rename I accidentally brought it
> back.

Your patch is fine. Reviewed-by: Daniel Vetter 

>
> Christian.
>
> > -Daniel
> >
> >> ---
> >>   drivers/gpu/drm/ttm/ttm_device.c | 2 --
> >>   include/drm/ttm/ttm_device.h | 1 -
> >>   2 files changed, 3 deletions(-)
> >>
> >> diff --git a/drivers/gpu/drm/ttm/ttm_device.c 
> >> b/drivers/gpu/drm/ttm/ttm_device.c
> >> index ac0903c9e60a..8bb61dd26437 100644
> >> --- a/drivers/gpu/drm/ttm/ttm_device.c
> >> +++ b/drivers/gpu/drm/ttm/ttm_device.c
> >> @@ -49,8 +49,6 @@ static void ttm_global_release(void)
> >>  if (--ttm_glob_use_count > 0)
> >>  goto out;
> >>
> >> -   kobject_del(&glob->kobj);
> >> -   kobject_put(&glob->kobj);
> >>  ttm_mem_global_release(&ttm_mem_glob);
> >>  __free_page(glob->dummy_read_page);
> >>  memset(glob, 0, sizeof(*glob));
> >> diff --git a/include/drm/ttm/ttm_device.h b/include/drm/ttm/ttm_device.h
> >> index 7bc8bb797161..035bbc044a3b 100644
> >> --- a/include/drm/ttm/ttm_device.h
> >> +++ b/include/drm/ttm/ttm_device.h
> >> @@ -55,7 +55,6 @@ extern struct ttm_global {
> >>   * Constant after init.
> >>   */
> >>
> >> -   struct kobject kobj;
> >>  struct page *dummy_read_page;
> >>  spinlock_t lru_lock;
> >>
> >> --
> >> 2.25.1
> >>
> >> ___
> >> dri-devel mailing list
> >> dri-devel@lists.freedesktop.org
> >> https://lists.freedesktop.org/mailman/listinfo/dri-devel
> >
> >
>


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

[PATCH v2 10/10] drm/ast: Move all of the cursor-update functionality to atomic_update

We used to update the cursor image in prepare_fb. Move all this code to
atomic_update (where it belongs). The generic helper for shadow-buffered
planes now implement the cursor plane's prepare_fb.

Signed-off-by: Thomas Zimmermann 
---
 drivers/gpu/drm/ast/ast_mode.c | 66 --
 1 file changed, 22 insertions(+), 44 deletions(-)

diff --git a/drivers/gpu/drm/ast/ast_mode.c b/drivers/gpu/drm/ast/ast_mode.c
index e8de86d23805..617b4c4af1a5 100644
--- a/drivers/gpu/drm/ast/ast_mode.c
+++ b/drivers/gpu/drm/ast/ast_mode.c
@@ -37,6 +37,7 @@
 #include 
 #include 
 #include 
+#include 
 #include 
 #include 
 #include 
@@ -753,40 +754,6 @@ static const uint32_t ast_cursor_plane_formats[] = {
DRM_FORMAT_ARGB,
 };
 
-static int
-ast_cursor_plane_helper_prepare_fb(struct drm_plane *plane,
-  struct drm_plane_state *new_state)
-{
-   struct ast_cursor_plane *ast_cursor_plane = to_ast_cursor_plane(plane);
-   struct drm_framebuffer *fb = new_state->fb;
-   struct dma_buf_map dst_map =
-   ast_cursor_plane->hwc[ast_cursor_plane->next_hwc_index].map;
-   struct drm_gem_vram_object *src_gbo;
-   struct dma_buf_map src_map;
-   void __iomem *dst;
-   void *src;
-   int ret;
-
-   if (!fb)
-   return 0;
-
-   src_gbo = drm_gem_vram_of_gem(fb->obj[0]);
-
-   ret = drm_gem_vram_vmap(src_gbo, &src_map);
-   if (ret)
-   return ret;
-   src = src_map.vaddr; /* TODO: Use mapping abstraction properly */
-
-   dst = dst_map.vaddr_iomem; /* TODO: Use mapping abstraction properly */
-
-   /* do data transfer to cursor BO */
-   ast_update_cursor_image(dst, src, fb->width, fb->height);
-
-   drm_gem_vram_vunmap(src_gbo, &src_map);
-
-   return 0;
-}
-
 static int ast_cursor_plane_helper_atomic_check(struct drm_plane *plane,
struct drm_plane_state *state)
 {
@@ -821,18 +788,31 @@ ast_cursor_plane_helper_atomic_update(struct drm_plane 
*plane,
 {
struct ast_cursor_plane *ast_cursor_plane = to_ast_cursor_plane(plane);
struct drm_plane_state *state = plane->state;
+   struct drm_shadow_plane_state *shadow_plane_state = 
to_drm_shadow_plane_state(state);
struct drm_framebuffer *fb = state->fb;
struct ast_private *ast = to_ast_private(plane->dev);
+   struct dma_buf_map dst_map =
+   ast_cursor_plane->hwc[ast_cursor_plane->next_hwc_index].map;
u64 dst_off =
ast_cursor_plane->hwc[ast_cursor_plane->next_hwc_index].off;
+   struct dma_buf_map src_map = shadow_plane_state->map[0];
unsigned int offset_x, offset_y;
-   struct dma_buf_map map;
u16 x, y;
u8 x_offset, y_offset;
u8 __iomem *dst;
u8 __iomem *sig;
+   const u8 *src;
+
+   src = src_map.vaddr; /* TODO: Use mapping abstraction properly */
+   dst = dst_map.vaddr_iomem; /* TODO: Use mapping abstraction properly */
+   sig = dst + AST_HWC_SIZE; /* TODO: Use mapping abstraction properly */
 
-   map = ast_cursor_plane->hwc[ast_cursor_plane->next_hwc_index].map;
+   /*
+* Do data transfer to HW cursor BO. If a new cursor image was 
installed,
+* point the scanout engine to dst_gbo's offset and page-flip the HWC 
buffers.
+*/
+
+   ast_update_cursor_image(dst, src, fb->width, fb->height);
 
if (state->fb != old_state->fb) {
ast_set_cursor_base(ast, dst_off);
@@ -841,9 +821,10 @@ ast_cursor_plane_helper_atomic_update(struct drm_plane 
*plane,
ast_cursor_plane->next_hwc_index %= 
ARRAY_SIZE(ast_cursor_plane->hwc);
}
 
-   dst = map.vaddr_iomem; /* TODO: Use mapping abstraction properly */
+   /*
+* Update location in HWC signature and registers.
+*/
 
-   sig = dst + AST_HWC_SIZE;
writel(state->crtc_x, sig + AST_HWC_SIGNATURE_X);
writel(state->crtc_y, sig + AST_HWC_SIGNATURE_Y);
 
@@ -867,7 +848,7 @@ ast_cursor_plane_helper_atomic_update(struct drm_plane 
*plane,
 
ast_set_cursor_location(ast, x, y, x_offset, y_offset);
 
-   /* dummy write to fire HWC */
+   /* Dummy write to enable HWC and make the HW pick-up the changes. */
ast_set_cursor_enabled(ast, true);
 }
 
@@ -881,8 +862,7 @@ ast_cursor_plane_helper_atomic_disable(struct drm_plane 
*plane,
 }
 
 static const struct drm_plane_helper_funcs ast_cursor_plane_helper_funcs = {
-   .prepare_fb = ast_cursor_plane_helper_prepare_fb,
-   .cleanup_fb = NULL, /* not required for cursor plane */
+   DRM_GEM_SHADOW_PLANE_HELPER_FUNCS,
.atomic_check = ast_cursor_plane_helper_atomic_check,
.atomic_update = ast_cursor_plane_helper_atomic_update,
.atomic_disable = ast_cursor_plane_helper_atomic_disable,
@@ -910,9 +890,7 @@ static const struct drm_plane_funcs ast_cursor_plane_funcs 
= {
.update_plane

[PATCH v2 03/10] drm/ast: Initialize planes in helper functions

This change will help with inlining cursor functions into modesetting
code. The primary plane's field used to be cleared with memset(). This
has been dropped as the memory is always allocated with kzalloc().

Signed-off-by: Thomas Zimmermann 
---
 drivers/gpu/drm/ast/ast_mode.c | 66 +++---
 1 file changed, 45 insertions(+), 21 deletions(-)

diff --git a/drivers/gpu/drm/ast/ast_mode.c b/drivers/gpu/drm/ast/ast_mode.c
index 758c69aa7232..f86773a869f0 100644
--- a/drivers/gpu/drm/ast/ast_mode.c
+++ b/drivers/gpu/drm/ast/ast_mode.c
@@ -621,6 +621,26 @@ static const struct drm_plane_funcs 
ast_primary_plane_funcs = {
.atomic_destroy_state = drm_atomic_helper_plane_destroy_state,
 };
 
+static int ast_primary_plane_init(struct ast_private *ast)
+{
+   struct drm_device *dev = &ast->base;
+   struct drm_plane *primary_plane = &ast->primary_plane;
+   int ret;
+
+   ret = drm_universal_plane_init(dev, primary_plane, 0x01,
+  &ast_primary_plane_funcs,
+  ast_primary_plane_formats,
+  ARRAY_SIZE(ast_primary_plane_formats),
+  NULL, DRM_PLANE_TYPE_PRIMARY, NULL);
+   if (ret) {
+   drm_err(dev, "drm_universal_plane_init() failed: %d\n", ret);
+   return ret;
+   }
+   drm_plane_helper_add(primary_plane, &ast_primary_plane_helper_funcs);
+
+   return 0;
+}
+
 /*
  * Cursor plane
  */
@@ -725,6 +745,26 @@ static const struct drm_plane_funcs ast_cursor_plane_funcs 
= {
.atomic_destroy_state = drm_atomic_helper_plane_destroy_state,
 };
 
+static int ast_cursor_plane_init(struct ast_private *ast)
+{
+   struct drm_device *dev = &ast->base;
+   struct drm_plane *cursor_plane = &ast->cursor_plane;
+   int ret;
+
+   ret = drm_universal_plane_init(dev, cursor_plane, 0x01,
+  &ast_cursor_plane_funcs,
+  ast_cursor_plane_formats,
+  ARRAY_SIZE(ast_cursor_plane_formats),
+  NULL, DRM_PLANE_TYPE_CURSOR, NULL);
+   if (ret) {
+   drm_err(dev, "drm_universal_plane_failed(): %d\n", ret);
+   return ret;
+   }
+   drm_plane_helper_add(cursor_plane, &ast_cursor_plane_helper_funcs);
+
+   return 0;
+}
+
 /*
  * CRTC
  */
@@ -1138,30 +1178,14 @@ int ast_mode_config_init(struct ast_private *ast)
 
dev->mode_config.helper_private = &ast_mode_config_helper_funcs;
 
-   memset(&ast->primary_plane, 0, sizeof(ast->primary_plane));
-   ret = drm_universal_plane_init(dev, &ast->primary_plane, 0x01,
-  &ast_primary_plane_funcs,
-  ast_primary_plane_formats,
-  ARRAY_SIZE(ast_primary_plane_formats),
-  NULL, DRM_PLANE_TYPE_PRIMARY, NULL);
-   if (ret) {
-   drm_err(dev, "ast: drm_universal_plane_init() failed: %d\n", 
ret);
+
+   ret = ast_primary_plane_init(ast);
+   if (ret)
return ret;
-   }
-   drm_plane_helper_add(&ast->primary_plane,
-&ast_primary_plane_helper_funcs);
 
-   ret = drm_universal_plane_init(dev, &ast->cursor_plane, 0x01,
-  &ast_cursor_plane_funcs,
-  ast_cursor_plane_formats,
-  ARRAY_SIZE(ast_cursor_plane_formats),
-  NULL, DRM_PLANE_TYPE_CURSOR, NULL);
-   if (ret) {
-   drm_err(dev, "drm_universal_plane_failed(): %d\n", ret);
+   ret = ast_cursor_plane_init(ast);
+   if (ret)
return ret;
-   }
-   drm_plane_helper_add(&ast->cursor_plane,
-&ast_cursor_plane_helper_funcs);
 
ast_crtc_init(dev);
ast_encoder_init(dev);
-- 
2.30.0

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

[PATCH v2 07/10] drm/ast: Store cursor BOs in cursor plane

The cursor uses two BOs in video RAM to implement double buffering. Store
both in struct ast_cursor_plane.

Signed-off-by: Thomas Zimmermann 
---
 drivers/gpu/drm/ast/ast_drv.h  | 11 ++-
 drivers/gpu/drm/ast/ast_mode.c | 27 +++
 2 files changed, 21 insertions(+), 17 deletions(-)

diff --git a/drivers/gpu/drm/ast/ast_drv.h b/drivers/gpu/drm/ast/ast_drv.h
index 9eefd3f01f4c..4117c49096d4 100644
--- a/drivers/gpu/drm/ast/ast_drv.h
+++ b/drivers/gpu/drm/ast/ast_drv.h
@@ -104,6 +104,12 @@ enum ast_tx_chip {
 
 struct ast_cursor_plane {
struct drm_plane base;
+
+   struct {
+   struct drm_gem_vram_object *gbo;
+   } hwc[AST_DEFAULT_HWC_NUM];
+
+   unsigned int next_hwc_index;
 };
 
 static inline struct ast_cursor_plane *
@@ -151,11 +157,6 @@ struct ast_private {
 
int fb_mtrr;
 
-   struct {
-   struct drm_gem_vram_object *gbo[AST_DEFAULT_HWC_NUM];
-   unsigned int next_index;
-   } cursor;
-
struct drm_plane primary_plane;
struct ast_cursor_plane cursor_plane;
struct drm_crtc crtc;
diff --git a/drivers/gpu/drm/ast/ast_mode.c b/drivers/gpu/drm/ast/ast_mode.c
index 9dc70aa62fef..dfff30e3d411 100644
--- a/drivers/gpu/drm/ast/ast_mode.c
+++ b/drivers/gpu/drm/ast/ast_mode.c
@@ -757,9 +757,10 @@ static int
 ast_cursor_plane_helper_prepare_fb(struct drm_plane *plane,
   struct drm_plane_state *new_state)
 {
+   struct ast_cursor_plane *ast_cursor_plane = to_ast_cursor_plane(plane);
struct drm_framebuffer *fb = new_state->fb;
-   struct ast_private *ast = to_ast_private(plane->dev);
-   struct drm_gem_vram_object *dst_gbo = 
ast->cursor.gbo[ast->cursor.next_index];
+   struct drm_gem_vram_object *dst_gbo =
+   ast_cursor_plane->hwc[ast_cursor_plane->next_hwc_index].gbo;
struct drm_gem_vram_object *src_gbo;
struct dma_buf_map src_map, dst_map;
void __iomem *dst;
@@ -826,11 +827,13 @@ static void
 ast_cursor_plane_helper_atomic_update(struct drm_plane *plane,
  struct drm_plane_state *old_state)
 {
+   struct ast_cursor_plane *ast_cursor_plane = to_ast_cursor_plane(plane);
struct drm_plane_state *state = plane->state;
struct drm_framebuffer *fb = state->fb;
struct ast_private *ast = to_ast_private(plane->dev);
struct drm_device *dev = &ast->base;
-   struct drm_gem_vram_object *gbo = 
ast->cursor.gbo[ast->cursor.next_index];
+   struct drm_gem_vram_object *gbo =
+   ast_cursor_plane->hwc[ast_cursor_plane->next_hwc_index].gbo;
unsigned int offset_x, offset_y;
s64 off;
struct dma_buf_map map;
@@ -840,7 +843,7 @@ ast_cursor_plane_helper_atomic_update(struct drm_plane 
*plane,
u8 __iomem *sig;
int ret;
 
-   gbo = ast->cursor.gbo[ast->cursor.next_index];
+   gbo = ast_cursor_plane->hwc[ast_cursor_plane->next_hwc_index].gbo;
 
if (state->fb != old_state->fb) {
/* A new cursor image was installed. */
@@ -849,8 +852,8 @@ ast_cursor_plane_helper_atomic_update(struct drm_plane 
*plane,
return; /* Bug: we didn't pin the cursor HW BO to VRAM. 
*/
ast_set_cursor_base(ast, off);
 
-   ++ast->cursor.next_index;
-   ast->cursor.next_index %= ARRAY_SIZE(ast->cursor.gbo);
+   ++ast_cursor_plane->next_hwc_index;
+   ast_cursor_plane->next_hwc_index %= 
ARRAY_SIZE(ast_cursor_plane->hwc);
}
 
ret = drm_gem_vram_vmap(gbo, &map);
@@ -907,12 +910,12 @@ static const struct drm_plane_helper_funcs 
ast_cursor_plane_helper_funcs = {
 
 static void ast_cursor_plane_destroy(struct drm_plane *plane)
 {
-   struct ast_private *ast = to_ast_private(plane->dev);
+   struct ast_cursor_plane *ast_cursor_plane = to_ast_cursor_plane(plane);
size_t i;
struct drm_gem_vram_object *gbo;
 
-   for (i = 0; i < ARRAY_SIZE(ast->cursor.gbo); ++i) {
-   gbo = ast->cursor.gbo[i];
+   for (i = 0; i < ARRAY_SIZE(ast_cursor_plane->hwc); ++i) {
+   gbo = ast_cursor_plane->hwc[i].gbo;
drm_gem_vram_unpin(gbo);
drm_gem_vram_put(gbo);
}
@@ -945,7 +948,7 @@ static int ast_cursor_plane_init(struct ast_private *ast)
 
size = roundup(AST_HWC_SIZE + AST_HWC_SIGNATURE_SIZE, PAGE_SIZE);
 
-   for (i = 0; i < ARRAY_SIZE(ast->cursor.gbo); ++i) {
+   for (i = 0; i < ARRAY_SIZE(ast_cursor_plane->hwc); ++i) {
gbo = drm_gem_vram_create(dev, size, 0);
if (IS_ERR(gbo)) {
ret = PTR_ERR(gbo);
@@ -955,7 +958,7 @@ static int ast_cursor_plane_init(struct ast_private *ast)
DRM_GEM_VRAM_PL_FLAG_TOPDOWN);
if (ret)
goto err_drm_gem_vram_put;
-   ast->cursor.gbo

[PATCH v2 06/10] drm/ast: Add cursor-plane data structure

Cursor state is currently located throughout struct ast_private. Having
struct ast_cursor_plane as dedicated data structure for cursors helps to
organize the modesetting code.

Signed-off-by: Thomas Zimmermann 
---
 drivers/gpu/drm/ast/ast_drv.h  | 23 ++-
 drivers/gpu/drm/ast/ast_mode.c |  5 +++--
 2 files changed, 25 insertions(+), 3 deletions(-)

diff --git a/drivers/gpu/drm/ast/ast_drv.h b/drivers/gpu/drm/ast/ast_drv.h
index 2761fa547c35..9eefd3f01f4c 100644
--- a/drivers/gpu/drm/ast/ast_drv.h
+++ b/drivers/gpu/drm/ast/ast_drv.h
@@ -81,6 +81,9 @@ enum ast_tx_chip {
 #define AST_DRAM_4Gx16   7
 #define AST_DRAM_8Gx16   8
 
+/*
+ * Cursor plane
+ */
 
 #define AST_MAX_HWC_WIDTH  64
 #define AST_MAX_HWC_HEIGHT 64
@@ -99,6 +102,20 @@ enum ast_tx_chip {
 #define AST_HWC_SIGNATURE_HOTSPOTX 0x14
 #define AST_HWC_SIGNATURE_HOTSPOTY 0x18
 
+struct ast_cursor_plane {
+   struct drm_plane base;
+};
+
+static inline struct ast_cursor_plane *
+to_ast_cursor_plane(struct drm_plane *plane)
+{
+   return container_of(plane, struct ast_cursor_plane, base);
+}
+
+/*
+ * Connector with i2c channel
+ */
+
 struct ast_i2c_chan {
struct i2c_adapter adapter;
struct drm_device *dev;
@@ -116,6 +133,10 @@ to_ast_connector(struct drm_connector *connector)
return container_of(connector, struct ast_connector, base);
 }
 
+/*
+ * Device
+ */
+
 struct ast_private {
struct drm_device base;
 
@@ -136,7 +157,7 @@ struct ast_private {
} cursor;
 
struct drm_plane primary_plane;
-   struct drm_plane cursor_plane;
+   struct ast_cursor_plane cursor_plane;
struct drm_crtc crtc;
struct drm_encoder encoder;
struct ast_connector connector;
diff --git a/drivers/gpu/drm/ast/ast_mode.c b/drivers/gpu/drm/ast/ast_mode.c
index 968ee0c69ec3..9dc70aa62fef 100644
--- a/drivers/gpu/drm/ast/ast_mode.c
+++ b/drivers/gpu/drm/ast/ast_mode.c
@@ -932,7 +932,8 @@ static const struct drm_plane_funcs ast_cursor_plane_funcs 
= {
 static int ast_cursor_plane_init(struct ast_private *ast)
 {
struct drm_device *dev = &ast->base;
-   struct drm_plane *cursor_plane = &ast->cursor_plane;
+   struct ast_cursor_plane *ast_cursor_plane = &ast->cursor_plane;
+   struct drm_plane *cursor_plane = &ast_cursor_plane->base;
size_t size, i;
struct drm_gem_vram_object *gbo;
int ret;
@@ -1178,7 +1179,7 @@ static int ast_crtc_init(struct drm_device *dev)
int ret;
 
ret = drm_crtc_init_with_planes(dev, crtc, &ast->primary_plane,
-   &ast->cursor_plane, &ast_crtc_funcs,
+   &ast->cursor_plane.base, 
&ast_crtc_funcs,
NULL);
if (ret)
return ret;
-- 
2.30.0

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

[PATCH v2 04/10] drm/ast: Allocate HW cursor BOs during cursor-plane initialization

The BOs are eventually released by the cursor plane's destroy callback.

Signed-off-by: Thomas Zimmermann 
---
 drivers/gpu/drm/ast/ast_cursor.c | 58 --
 drivers/gpu/drm/ast/ast_drv.h|  1 -
 drivers/gpu/drm/ast/ast_mode.c   | 62 
 3 files changed, 55 insertions(+), 66 deletions(-)

diff --git a/drivers/gpu/drm/ast/ast_cursor.c b/drivers/gpu/drm/ast/ast_cursor.c
index 024858371f64..31c35296a107 100644
--- a/drivers/gpu/drm/ast/ast_cursor.c
+++ b/drivers/gpu/drm/ast/ast_cursor.c
@@ -32,64 +32,6 @@
 
 #include "ast_drv.h"
 
-static void ast_cursor_fini(struct ast_private *ast)
-{
-   size_t i;
-   struct drm_gem_vram_object *gbo;
-
-   for (i = 0; i < ARRAY_SIZE(ast->cursor.gbo); ++i) {
-   gbo = ast->cursor.gbo[i];
-   drm_gem_vram_unpin(gbo);
-   drm_gem_vram_put(gbo);
-   }
-}
-
-static void ast_cursor_release(struct drm_device *dev, void *ptr)
-{
-   struct ast_private *ast = to_ast_private(dev);
-
-   ast_cursor_fini(ast);
-}
-
-/*
- * Allocate cursor BOs and pin them at the end of VRAM.
- */
-int ast_cursor_init(struct ast_private *ast)
-{
-   struct drm_device *dev = &ast->base;
-   size_t size, i;
-   struct drm_gem_vram_object *gbo;
-   int ret;
-
-   size = roundup(AST_HWC_SIZE + AST_HWC_SIGNATURE_SIZE, PAGE_SIZE);
-
-   for (i = 0; i < ARRAY_SIZE(ast->cursor.gbo); ++i) {
-   gbo = drm_gem_vram_create(dev, size, 0);
-   if (IS_ERR(gbo)) {
-   ret = PTR_ERR(gbo);
-   goto err_drm_gem_vram_put;
-   }
-   ret = drm_gem_vram_pin(gbo, DRM_GEM_VRAM_PL_FLAG_VRAM |
-   DRM_GEM_VRAM_PL_FLAG_TOPDOWN);
-   if (ret) {
-   drm_gem_vram_put(gbo);
-   goto err_drm_gem_vram_put;
-   }
-   ast->cursor.gbo[i] = gbo;
-   }
-
-   return drmm_add_action_or_reset(dev, ast_cursor_release, NULL);
-
-err_drm_gem_vram_put:
-   while (i) {
-   --i;
-   gbo = ast->cursor.gbo[i];
-   drm_gem_vram_unpin(gbo);
-   drm_gem_vram_put(gbo);
-   }
-   return ret;
-}
-
 static void update_cursor_image(u8 __iomem *dst, const u8 *src, int width, int 
height)
 {
union {
diff --git a/drivers/gpu/drm/ast/ast_drv.h b/drivers/gpu/drm/ast/ast_drv.h
index 1575e8e636d7..c9c3950449b5 100644
--- a/drivers/gpu/drm/ast/ast_drv.h
+++ b/drivers/gpu/drm/ast/ast_drv.h
@@ -318,7 +318,6 @@ u8 ast_get_dp501_max_clk(struct drm_device *dev);
 void ast_init_3rdtx(struct drm_device *dev);
 
 /* ast_cursor.c */
-int ast_cursor_init(struct ast_private *ast);
 int ast_cursor_blit(struct ast_private *ast, struct drm_framebuffer *fb);
 void ast_cursor_page_flip(struct ast_private *ast);
 void ast_cursor_show(struct ast_private *ast, int x, int y,
diff --git a/drivers/gpu/drm/ast/ast_mode.c b/drivers/gpu/drm/ast/ast_mode.c
index f86773a869f0..99b6f7c5cb2f 100644
--- a/drivers/gpu/drm/ast/ast_mode.c
+++ b/drivers/gpu/drm/ast/ast_mode.c
@@ -736,10 +736,25 @@ static const struct drm_plane_helper_funcs 
ast_cursor_plane_helper_funcs = {
.atomic_disable = ast_cursor_plane_helper_atomic_disable,
 };
 
+static void ast_cursor_plane_destroy(struct drm_plane *plane)
+{
+   struct ast_private *ast = to_ast_private(plane->dev);
+   size_t i;
+   struct drm_gem_vram_object *gbo;
+
+   for (i = 0; i < ARRAY_SIZE(ast->cursor.gbo); ++i) {
+   gbo = ast->cursor.gbo[i];
+   drm_gem_vram_unpin(gbo);
+   drm_gem_vram_put(gbo);
+   }
+
+   drm_plane_cleanup(plane);
+}
+
 static const struct drm_plane_funcs ast_cursor_plane_funcs = {
.update_plane = drm_atomic_helper_update_plane,
.disable_plane = drm_atomic_helper_disable_plane,
-   .destroy = drm_plane_cleanup,
+   .destroy = ast_cursor_plane_destroy,
.reset = drm_atomic_helper_plane_reset,
.atomic_duplicate_state = drm_atomic_helper_plane_duplicate_state,
.atomic_destroy_state = drm_atomic_helper_plane_destroy_state,
@@ -749,20 +764,57 @@ static int ast_cursor_plane_init(struct ast_private *ast)
 {
struct drm_device *dev = &ast->base;
struct drm_plane *cursor_plane = &ast->cursor_plane;
+   size_t size, i;
+   struct drm_gem_vram_object *gbo;
int ret;
 
+   /*
+* Allocate backing storage for cursors. The BOs are permanently
+* pinned to the top end of the VRAM.
+*/
+
+   size = roundup(AST_HWC_SIZE + AST_HWC_SIGNATURE_SIZE, PAGE_SIZE);
+
+   for (i = 0; i < ARRAY_SIZE(ast->cursor.gbo); ++i) {
+   gbo = drm_gem_vram_create(dev, size, 0);
+   if (IS_ERR(gbo)) {
+   ret = PTR_ERR(gbo);
+   goto err_hwc;
+   }
+   ret = drm_gem_vram_pin(gbo, DRM_GEM_V

[PATCH v2 08/10] drm/ast: Map HW cursor BOs permanently

The BOs of the hardware cursor are now mapped permanently while the
cursor plane is being used. This reduces the CPU overhead of the cursor
plane's atomic_update function.

The change also resolves a problem with the vmap call in the commit tail.
The vmap implementation could acquire the DMA reservation lock on the
BO, which is not allowed that late in the atomic update. Removing the
vmap call from atomic_update fixes the issue.

Signed-off-by: Thomas Zimmermann 
---
 drivers/gpu/drm/ast/ast_drv.h  |  1 +
 drivers/gpu/drm/ast/ast_mode.c | 32 +++-
 2 files changed, 16 insertions(+), 17 deletions(-)

diff --git a/drivers/gpu/drm/ast/ast_drv.h b/drivers/gpu/drm/ast/ast_drv.h
index 4117c49096d4..22193cfde255 100644
--- a/drivers/gpu/drm/ast/ast_drv.h
+++ b/drivers/gpu/drm/ast/ast_drv.h
@@ -107,6 +107,7 @@ struct ast_cursor_plane {
 
struct {
struct drm_gem_vram_object *gbo;
+   struct dma_buf_map map;
} hwc[AST_DEFAULT_HWC_NUM];
 
unsigned int next_hwc_index;
diff --git a/drivers/gpu/drm/ast/ast_mode.c b/drivers/gpu/drm/ast/ast_mode.c
index dfff30e3d411..b9b9badcee00 100644
--- a/drivers/gpu/drm/ast/ast_mode.c
+++ b/drivers/gpu/drm/ast/ast_mode.c
@@ -759,10 +759,10 @@ ast_cursor_plane_helper_prepare_fb(struct drm_plane 
*plane,
 {
struct ast_cursor_plane *ast_cursor_plane = to_ast_cursor_plane(plane);
struct drm_framebuffer *fb = new_state->fb;
-   struct drm_gem_vram_object *dst_gbo =
-   ast_cursor_plane->hwc[ast_cursor_plane->next_hwc_index].gbo;
+   struct dma_buf_map dst_map =
+   ast_cursor_plane->hwc[ast_cursor_plane->next_hwc_index].map;
struct drm_gem_vram_object *src_gbo;
-   struct dma_buf_map src_map, dst_map;
+   struct dma_buf_map src_map;
void __iomem *dst;
void *src;
int ret;
@@ -777,22 +777,14 @@ ast_cursor_plane_helper_prepare_fb(struct drm_plane 
*plane,
return ret;
src = src_map.vaddr; /* TODO: Use mapping abstraction properly */
 
-   ret = drm_gem_vram_vmap(dst_gbo, &dst_map);
-   if (ret)
-   goto err_drm_gem_vram_vunmap;
dst = dst_map.vaddr_iomem; /* TODO: Use mapping abstraction properly */
 
/* do data transfer to cursor BO */
ast_update_cursor_image(dst, src, fb->width, fb->height);
 
-   drm_gem_vram_vunmap(dst_gbo, &dst_map);
drm_gem_vram_vunmap(src_gbo, &src_map);
 
return 0;
-
-err_drm_gem_vram_vunmap:
-   drm_gem_vram_vunmap(src_gbo, &src_map);
-   return ret;
 }
 
 static int ast_cursor_plane_helper_atomic_check(struct drm_plane *plane,
@@ -841,9 +833,9 @@ ast_cursor_plane_helper_atomic_update(struct drm_plane 
*plane,
u8 x_offset, y_offset;
u8 __iomem *dst;
u8 __iomem *sig;
-   int ret;
 
gbo = ast_cursor_plane->hwc[ast_cursor_plane->next_hwc_index].gbo;
+   map = ast_cursor_plane->hwc[ast_cursor_plane->next_hwc_index].map;
 
if (state->fb != old_state->fb) {
/* A new cursor image was installed. */
@@ -856,17 +848,12 @@ ast_cursor_plane_helper_atomic_update(struct drm_plane 
*plane,
ast_cursor_plane->next_hwc_index %= 
ARRAY_SIZE(ast_cursor_plane->hwc);
}
 
-   ret = drm_gem_vram_vmap(gbo, &map);
-   if (drm_WARN_ONCE(dev, ret, "drm_gem_vram_vmap() failed, ret=%d\n", 
ret))
-   return;
dst = map.vaddr_iomem; /* TODO: Use mapping abstraction properly */
 
sig = dst + AST_HWC_SIZE;
writel(state->crtc_x, sig + AST_HWC_SIGNATURE_X);
writel(state->crtc_y, sig + AST_HWC_SIGNATURE_Y);
 
-   drm_gem_vram_vunmap(gbo, &map);
-
offset_x = AST_MAX_HWC_WIDTH - fb->width;
offset_y = AST_MAX_HWC_HEIGHT - fb->height;
 
@@ -913,9 +900,12 @@ static void ast_cursor_plane_destroy(struct drm_plane 
*plane)
struct ast_cursor_plane *ast_cursor_plane = to_ast_cursor_plane(plane);
size_t i;
struct drm_gem_vram_object *gbo;
+   struct dma_buf_map map;
 
for (i = 0; i < ARRAY_SIZE(ast_cursor_plane->hwc); ++i) {
gbo = ast_cursor_plane->hwc[i].gbo;
+   map = ast_cursor_plane->hwc[i].map;
+   drm_gem_vram_vunmap(gbo, &map);
drm_gem_vram_unpin(gbo);
drm_gem_vram_put(gbo);
}
@@ -939,6 +929,7 @@ static int ast_cursor_plane_init(struct ast_private *ast)
struct drm_plane *cursor_plane = &ast_cursor_plane->base;
size_t size, i;
struct drm_gem_vram_object *gbo;
+   struct dma_buf_map map;
int ret;
 
/*
@@ -958,7 +949,11 @@ static int ast_cursor_plane_init(struct ast_private *ast)
DRM_GEM_VRAM_PL_FLAG_TOPDOWN);
if (ret)
goto err_drm_gem_vram_put;
+   ret = drm_gem_vram_vmap(gbo, &map);
+   if (ret)
+   goto err_drm_

[PATCH v2 09/10] drm/ast: Store each HW cursor offset after pinning the rsp BO

As HW cursor BOs never move, we can store the offset in VRAM in the
cursor-plane's HWC state. This removes the last possible source of
runtime errors from atomic_update.

Signed-off-by: Thomas Zimmermann 
---
 drivers/gpu/drm/ast/ast_drv.h  |  1 +
 drivers/gpu/drm/ast/ast_mode.c | 21 +++--
 2 files changed, 12 insertions(+), 10 deletions(-)

diff --git a/drivers/gpu/drm/ast/ast_drv.h b/drivers/gpu/drm/ast/ast_drv.h
index 22193cfde255..e82ab8628770 100644
--- a/drivers/gpu/drm/ast/ast_drv.h
+++ b/drivers/gpu/drm/ast/ast_drv.h
@@ -108,6 +108,7 @@ struct ast_cursor_plane {
struct {
struct drm_gem_vram_object *gbo;
struct dma_buf_map map;
+   u64 off;
} hwc[AST_DEFAULT_HWC_NUM];
 
unsigned int next_hwc_index;
diff --git a/drivers/gpu/drm/ast/ast_mode.c b/drivers/gpu/drm/ast/ast_mode.c
index b9b9badcee00..e8de86d23805 100644
--- a/drivers/gpu/drm/ast/ast_mode.c
+++ b/drivers/gpu/drm/ast/ast_mode.c
@@ -823,26 +823,19 @@ ast_cursor_plane_helper_atomic_update(struct drm_plane 
*plane,
struct drm_plane_state *state = plane->state;
struct drm_framebuffer *fb = state->fb;
struct ast_private *ast = to_ast_private(plane->dev);
-   struct drm_device *dev = &ast->base;
-   struct drm_gem_vram_object *gbo =
-   ast_cursor_plane->hwc[ast_cursor_plane->next_hwc_index].gbo;
+   u64 dst_off =
+   ast_cursor_plane->hwc[ast_cursor_plane->next_hwc_index].off;
unsigned int offset_x, offset_y;
-   s64 off;
struct dma_buf_map map;
u16 x, y;
u8 x_offset, y_offset;
u8 __iomem *dst;
u8 __iomem *sig;
 
-   gbo = ast_cursor_plane->hwc[ast_cursor_plane->next_hwc_index].gbo;
map = ast_cursor_plane->hwc[ast_cursor_plane->next_hwc_index].map;
 
if (state->fb != old_state->fb) {
-   /* A new cursor image was installed. */
-   off = drm_gem_vram_offset(gbo);
-   if (drm_WARN_ON_ONCE(dev, off < 0))
-   return; /* Bug: we didn't pin the cursor HW BO to VRAM. 
*/
-   ast_set_cursor_base(ast, off);
+   ast_set_cursor_base(ast, dst_off);
 
++ast_cursor_plane->next_hwc_index;
ast_cursor_plane->next_hwc_index %= 
ARRAY_SIZE(ast_cursor_plane->hwc);
@@ -931,6 +924,7 @@ static int ast_cursor_plane_init(struct ast_private *ast)
struct drm_gem_vram_object *gbo;
struct dma_buf_map map;
int ret;
+   s64 off;
 
/*
 * Allocate backing storage for cursors. The BOs are permanently
@@ -952,8 +946,14 @@ static int ast_cursor_plane_init(struct ast_private *ast)
ret = drm_gem_vram_vmap(gbo, &map);
if (ret)
goto err_drm_gem_vram_unpin;
+   off = drm_gem_vram_offset(gbo);
+   if (off < 0) {
+   ret = off;
+   goto err_drm_gem_vram_vunmap;
+   }
ast_cursor_plane->hwc[i].gbo = gbo;
ast_cursor_plane->hwc[i].map = map;
+   ast_cursor_plane->hwc[i].off = off;
}
 
/*
@@ -979,6 +979,7 @@ static int ast_cursor_plane_init(struct ast_private *ast)
--i;
gbo = ast_cursor_plane->hwc[i].gbo;
map = ast_cursor_plane->hwc[i].map;
+err_drm_gem_vram_vunmap:
drm_gem_vram_vunmap(gbo, &map);
 err_drm_gem_vram_unpin:
drm_gem_vram_unpin(gbo);
-- 
2.30.0

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

[PATCH v2 05/10] drm/ast: Inline ast cursor-update functions into modesetting code

The logic for cursor updates is now located in the cursor plane's
modesetting code. A number of helper functions remain to modify the
rsp registers and image.

Signed-off-by: Thomas Zimmermann 
---
 drivers/gpu/drm/ast/Makefile |   3 +-
 drivers/gpu/drm/ast/ast_cursor.c | 237 ---
 drivers/gpu/drm/ast/ast_drv.h|   7 -
 drivers/gpu/drm/ast/ast_mode.c   | 191 +++--
 4 files changed, 181 insertions(+), 257 deletions(-)
 delete mode 100644 drivers/gpu/drm/ast/ast_cursor.c

diff --git a/drivers/gpu/drm/ast/Makefile b/drivers/gpu/drm/ast/Makefile
index 2265a8a624dd..438a2d05b115 100644
--- a/drivers/gpu/drm/ast/Makefile
+++ b/drivers/gpu/drm/ast/Makefile
@@ -3,7 +3,6 @@
 # Makefile for the drm device driver.  This driver provides support for the
 # Direct Rendering Infrastructure (DRI) in XFree86 4.1.0 and higher.
 
-ast-y := ast_cursor.o ast_drv.o ast_main.o ast_mm.o ast_mode.o ast_post.o \
-ast_dp501.o
+ast-y := ast_drv.o ast_main.o ast_mm.o ast_mode.o ast_post.o ast_dp501.o
 
 obj-$(CONFIG_DRM_AST) := ast.o
diff --git a/drivers/gpu/drm/ast/ast_cursor.c b/drivers/gpu/drm/ast/ast_cursor.c
deleted file mode 100644
index 31c35296a107..
--- a/drivers/gpu/drm/ast/ast_cursor.c
+++ /dev/null
@@ -1,237 +0,0 @@
-/*
- * Copyright 2012 Red Hat Inc.
- * Parts based on xf86-video-ast
- * Copyright (c) 2005 ASPEED Technology Inc.
- *
- * Permission is hereby granted, free of charge, to any person obtaining a
- * copy of this software and associated documentation files (the
- * "Software"), to deal in the Software without restriction, including
- * without limitation the rights to use, copy, modify, merge, publish,
- * distribute, sub license, and/or sell copies of the Software, and to
- * permit persons to whom the Software is furnished to do so, subject to
- * the following conditions:
- *
- * THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND, EXPRESS OR
- * IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF MERCHANTABILITY,
- * FITNESS FOR A PARTICULAR PURPOSE AND NON-INFRINGEMENT. IN NO EVENT SHALL
- * THE COPYRIGHT HOLDERS, AUTHORS AND/OR ITS SUPPLIERS BE LIABLE FOR ANY CLAIM,
- * DAMAGES OR OTHER LIABILITY, WHETHER IN AN ACTION OF CONTRACT, TORT OR
- * OTHERWISE, ARISING FROM, OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE
- * USE OR OTHER DEALINGS IN THE SOFTWARE.
- *
- * The above copyright notice and this permission notice (including the
- * next paragraph) shall be included in all copies or substantial portions
- * of the Software.
- */
-/*
- * Authors: Dave Airlie 
- */
-
-#include 
-#include 
-
-#include "ast_drv.h"
-
-static void update_cursor_image(u8 __iomem *dst, const u8 *src, int width, int 
height)
-{
-   union {
-   u32 ul;
-   u8 b[4];
-   } srcdata32[2], data32;
-   union {
-   u16 us;
-   u8 b[2];
-   } data16;
-   u32 csum = 0;
-   s32 alpha_dst_delta, last_alpha_dst_delta;
-   u8 __iomem *dstxor;
-   const u8 *srcxor;
-   int i, j;
-   u32 per_pixel_copy, two_pixel_copy;
-
-   alpha_dst_delta = AST_MAX_HWC_WIDTH << 1;
-   last_alpha_dst_delta = alpha_dst_delta - (width << 1);
-
-   srcxor = src;
-   dstxor = (u8 *)dst + last_alpha_dst_delta + (AST_MAX_HWC_HEIGHT - 
height) * alpha_dst_delta;
-   per_pixel_copy = width & 1;
-   two_pixel_copy = width >> 1;
-
-   for (j = 0; j < height; j++) {
-   for (i = 0; i < two_pixel_copy; i++) {
-   srcdata32[0].ul = *((u32 *)srcxor) & 0xf0f0f0f0;
-   srcdata32[1].ul = *((u32 *)(srcxor + 4)) & 0xf0f0f0f0;
-   data32.b[0] = srcdata32[0].b[1] | (srcdata32[0].b[0] >> 
4);
-   data32.b[1] = srcdata32[0].b[3] | (srcdata32[0].b[2] >> 
4);
-   data32.b[2] = srcdata32[1].b[1] | (srcdata32[1].b[0] >> 
4);
-   data32.b[3] = srcdata32[1].b[3] | (srcdata32[1].b[2] >> 
4);
-
-   writel(data32.ul, dstxor);
-   csum += data32.ul;
-
-   dstxor += 4;
-   srcxor += 8;
-
-   }
-
-   for (i = 0; i < per_pixel_copy; i++) {
-   srcdata32[0].ul = *((u32 *)srcxor) & 0xf0f0f0f0;
-   data16.b[0] = srcdata32[0].b[1] | (srcdata32[0].b[0] >> 
4);
-   data16.b[1] = srcdata32[0].b[3] | (srcdata32[0].b[2] >> 
4);
-   writew(data16.us, dstxor);
-   csum += (u32)data16.us;
-
-   dstxor += 2;
-   srcxor += 4;
-   }
-   dstxor += last_alpha_dst_delta;
-   }
-
-   /* write checksum + signature */
-   dst += AST_HWC_SIZE;
-   writel(csum, dst);
-   writel(width, dst + AST_HWC_SIGNATURE_SizeX);
-   writel(height, dst + AST_HWC_SIGNATURE_SizeY);
-   writel(0, dst + AST_HWC_S

[PATCH v2 02/10] drm/ast: Fix invalid usage of AST_MAX_HWC_WIDTH in cursor atomic_check

Use AST_MAX_HWC_HEIGHT for setting offset_y in the cursor plane's
atomic_check. The code used AST_MAX_HWC_WIDTH instead. This worked
because both constants has the same value.

Signed-off-by: Thomas Zimmermann 
---
 drivers/gpu/drm/ast/ast_mode.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/gpu/drm/ast/ast_mode.c b/drivers/gpu/drm/ast/ast_mode.c
index 988b270fea5e..758c69aa7232 100644
--- a/drivers/gpu/drm/ast/ast_mode.c
+++ b/drivers/gpu/drm/ast/ast_mode.c
@@ -688,7 +688,7 @@ ast_cursor_plane_helper_atomic_update(struct drm_plane 
*plane,
unsigned int offset_x, offset_y;
 
offset_x = AST_MAX_HWC_WIDTH - fb->width;
-   offset_y = AST_MAX_HWC_WIDTH - fb->height;
+   offset_y = AST_MAX_HWC_HEIGHT - fb->height;
 
if (state->fb != old_state->fb) {
/* A new cursor image was installed. */
-- 
2.30.0

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

[PATCH v2 01/10] drm/ast: Add constants for VGACRCB register bits

Set the bits in VGACRCB with constants. Alo move the rsp code into a
helper function.

Signed-off-by: Thomas Zimmermann 
---
 drivers/gpu/drm/ast/ast_cursor.c | 21 +++--
 drivers/gpu/drm/ast/ast_drv.h|  3 +++
 2 files changed, 18 insertions(+), 6 deletions(-)

diff --git a/drivers/gpu/drm/ast/ast_cursor.c b/drivers/gpu/drm/ast/ast_cursor.c
index fac1ee79c372..024858371f64 100644
--- a/drivers/gpu/drm/ast/ast_cursor.c
+++ b/drivers/gpu/drm/ast/ast_cursor.c
@@ -236,6 +236,19 @@ static void ast_cursor_set_location(struct ast_private 
*ast, u16 x, u16 y,
ast_set_index_reg(ast, AST_IO_CRTC_PORT, 0xc7, y1);
 }
 
+static void ast_set_cursor_enabled(struct ast_private *ast, bool enabled)
+{
+   static const u8 mask = (u8)~(AST_IO_VGACRCB_HWC_16BPP |
+AST_IO_VGACRCB_HWC_ENABLED);
+
+   u8 vgacrcb = AST_IO_VGACRCB_HWC_16BPP;
+
+   if (enabled)
+   vgacrcb |= AST_IO_VGACRCB_HWC_ENABLED;
+
+   ast_set_index_reg_mask(ast, AST_IO_CRTC_PORT, 0xcb, mask, vgacrcb);
+}
+
 void ast_cursor_show(struct ast_private *ast, int x, int y,
 unsigned int offset_x, unsigned int offset_y)
 {
@@ -245,7 +258,6 @@ void ast_cursor_show(struct ast_private *ast, int x, int y,
u8 x_offset, y_offset;
u8 __iomem *dst;
u8 __iomem *sig;
-   u8 jreg;
int ret;
 
ret = drm_gem_vram_vmap(gbo, &map);
@@ -274,13 +286,10 @@ void ast_cursor_show(struct ast_private *ast, int x, int 
y,
 
ast_cursor_set_location(ast, x, y, x_offset, y_offset);
 
-   /* dummy write to fire HWC */
-   jreg = 0x02 |
-  0x01; /* enable ARGB cursor */
-   ast_set_index_reg_mask(ast, AST_IO_CRTC_PORT, 0xcb, 0xfc, jreg);
+   ast_set_cursor_enabled(ast, true); /* dummy write to fire HWC */
 }
 
 void ast_cursor_hide(struct ast_private *ast)
 {
-   ast_set_index_reg_mask(ast, AST_IO_CRTC_PORT, 0xcb, 0xfc, 0x00);
+   ast_set_cursor_enabled(ast, false);
 }
diff --git a/drivers/gpu/drm/ast/ast_drv.h b/drivers/gpu/drm/ast/ast_drv.h
index f871fc36c2f7..1575e8e636d7 100644
--- a/drivers/gpu/drm/ast/ast_drv.h
+++ b/drivers/gpu/drm/ast/ast_drv.h
@@ -179,6 +179,9 @@ struct ast_private *ast_device_create(const struct 
drm_driver *drv,
 
 #define AST_IO_VGAIR1_VREFRESH BIT(3)
 
+#define AST_IO_VGACRCB_HWC_ENABLED BIT(1)
+#define AST_IO_VGACRCB_HWC_16BPP   BIT(0) /* set: ARGB, cleared: 2bpp 
palette */
+
 #define __ast_read(x) \
 static inline u##x ast_read##x(struct ast_private *ast, u32 reg) { \
 u##x val = 0;\
-- 
2.30.0

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

[PATCH v2 00/10] drm/ast: Clean-up cursor-plane updates

(was: drm/ast: Move cursor vmap calls out of commit tail)

Ast has vmap calls in its cursor's atomic_update function. This is not
supported as vmap might aquire the dma reservation lock. While at it,
cleanup the whole cursor code: the patchset removes all possible runtime
errors from the atomic_update function and reduces the overhead from
vmap calls on the HW cursor BOs to a minimum.

Patches 1 to 3 update the cursor code and prepare before the refactoring.

Patch 4 and 5 inline the cursor update logic into the rsp cursor-plane
functions. This is mostly about moving code around.

Patches 6 to 9 add a dedicated cursor plane that maintains the two BOs
for HW cursors. The HW cursor BOs are permanently pinned and vmapped
while the cursor plane is initialized. Thus removing the related vmap
operations from atomic_update.

Finally patch 10 converts ast cursors to struct drm_shadow_plane_state.
BOs with cursor image data from userspace are vmapped in prepare_fb and
vunampped in cleanup_fb. The actual update of the cursor image is being
moved from prepare_fb to atomic_update.

With the patchset applied, all cursor preparation is performed before
the commit-tail functions; while the actual update is performed within.

Tested by running X11 and Weston on ast hardware.

v2:
* convert to drm_shadow_plane_state helpers

Thomas Zimmermann (10):
  drm/ast: Add constants for VGACRCB register bits
  drm/ast: Fix invalid usage of AST_MAX_HWC_WIDTH in cursor atomic_check
  drm/ast: Initialize planes in helper functions
  drm/ast: Allocate HW cursor BOs during cursor-plane initialization
  drm/ast: Inline ast cursor-update functions into modesetting code
  drm/ast: Add cursor-plane data structure
  drm/ast: Store cursor BOs in cursor plane
  drm/ast: Map HW cursor BOs permanently
  drm/ast: Store each HW cursor offset after pinning the rsp BO
  drm/ast: Move all of the cursor-update functionality to atomic_update

 drivers/gpu/drm/ast/Makefile |   3 +-
 drivers/gpu/drm/ast/ast_cursor.c | 286 --
 drivers/gpu/drm/ast/ast_drv.h|  47 +++--
 drivers/gpu/drm/ast/ast_mode.c   | 334 +--
 4 files changed, 312 insertions(+), 358 deletions(-)
 delete mode 100644 drivers/gpu/drm/ast/ast_cursor.c

--
2.30.0

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

Re: [PATCH 0/9] Add support for SVM atomics in Nouveau

2021-02-09 Thread Jason Gunthorpe

On Tue, Feb 09, 2021 at 02:39:51PM +0100, Daniel Vetter wrote:

> Either way ZONE_DEVICE for not vram/device memory sounds wrong. Is
> that really going on here?

My read was this was doing non-coherent atomics on CPU memory.

Atomics on GPU memory is just called migration to GPU memory, it
doesn't need to be special for atomics. In that case it can free the
CPU struct page completely as the data now lives in the ZONE_DEVICE
page so no need for a pin, no problem with movable

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

Re: [PATCH 0/9] Add support for SVM atomics in Nouveau

On Tue, Feb 9, 2021 at 2:35 PM Jason Gunthorpe  wrote:
>
> On Tue, Feb 09, 2021 at 11:57:28PM +1100, Alistair Popple wrote:
> > On Tuesday, 9 February 2021 9:27:05 PM AEDT Daniel Vetter wrote:
> > > >
> > > > Recent changes to pin_user_pages() prevent the creation of pinned pages 
> > > > in
> > > > ZONE_MOVABLE. This series allows pinned pages to be created in
> > ZONE_MOVABLE
> > > > as attempts to migrate may fail which would be fatal to userspace.
> > > >
> > > > In this case migration of the pinned page is unnecessary as the page can
> > be
> > > > unpinned at anytime by having the driver revoke atomic permission as it
> > > > does for the migrate_to_ram() callback. However a method of calling this
> > > > when memory needs to be moved has yet to be resolved so any discussion 
> > > > is
> > > > welcome.
> > >
> > > Why do we need to pin for gpu atomics? You still have the callback for
> > > cpu faults, so you
> > > can move the page as needed, and hence a long-term pin sounds like the
> > > wrong approach.
> >
> > Technically a real long term unmoveable pin isn't required, because as you 
> > say
> > the page can be moved as needed at any time. However I needed some way of
> > stopping the CPU page from being freed once the userspace mappings for it 
> > had
> > been removed.
>
> The issue is you took the page out of the PTE it belongs to, which
> makes it orphaned and unlocatable by the rest of the mm?
>
> Ideally this would leave the PTE in place so everything continues to
> work, just disable CPU access to it.
>
> Maybe some kind of special swap entry?

I probably should have read the patches more in detail, I was assuming
the ZONE_DEVICE is only for vram. At least I thought the requirement
for gpu atomics was that the page is in vram, but maybe I'm mixing up
how this works on nvidia with how it works in other places. Iirc we
had a long discussion about this at lpc19 that ended with the
conclusion that we must be able to migrate, and sometimes migration is
blocked. But the details ellude me now.

Either way ZONE_DEVICE for not vram/device memory sounds wrong. Is
that really going on here?
-Daniel

>
> I also don't much like the use of ZONE_DEVICE here, that should only
> be used for actual device memory, not as a temporary proxy for CPU
> pages.. Having two struct pages refer to the same physical memory is
> pretty ugly.
>
> > The normal solution of registering an MMU notifier to unpin the page when it
> > needs to be moved also doesn't work as the CPU page tables now point to the
> > device-private page and hence the migration code won't call any invalidate
> > notifiers for the CPU page.
>
> The fact the page is lost from the MM seems to be the main issue here.
>
> > Yes, I would like to avoid the long term pin constraints as well if 
> > possible I
> > just haven't found a solution yet. Are you suggesting it might be possible 
> > to
> > add a callback in the page migration logic to specially deal with moving 
> > these
> > pages?
>
> How would migration even find the page?
>
> Jason



-- 
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 1/9] mm/migrate.c: Always allow device private pages to migrate

2021-02-09 Thread Jason Gunthorpe

On Tue, Feb 09, 2021 at 12:07:14PM +1100, Alistair Popple wrote:
> Device private pages are used to represent device memory that is not
> directly accessible from the CPU. Extra references to a device private
> page are only used to ensure the struct page itself remains valid whilst
> waiting for migration entries. Therefore extra references should not
> prevent device private page migration as this can lead to failures to
> migrate pages back to the CPU which are fatal to the user process.

This should identify the extra references in expected_count, just
disabling this protection seems unsafe, ZONE_DEVICE is not so special
that the refcount means nothing

Is this a side effect of the extra refcounts that Ralph was trying to
get rid of? I'd rather see that work finished :)

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

Re: [PATCH 0/9] Add support for SVM atomics in Nouveau