date:20220517

So I think you forgot to update the subject of the patch. If you can send a
respin of this patch with a corrected patch title, then you can consider this:

Reviewed-by: Lyude Paul 

I'll push once you get the respin out

On Mon, 2022-05-16 at 15:31 +0200, Mark Menzynski wrote:
> Resources needed for output poll workers are destroyed in
> nouveau_fbcon_fini() before output poll workers are cleared in
> nouveau_display_fini(). This means there is a time between fbcon_fini()
> and display_fini(), where if output poll happens, it crashes.
> 
> This makes output poll workers cleared right before fbcon resources are
> destroyed.
> 
> BUG: KASAN: use-after-free in
> __drm_fb_helper_initial_config_and_unlock.cold+0x1f3/0x291
> [drm_kms_helper]
> 
> Cc: Ben Skeggs 
> Cc: Karol Herbst 
> Cc: Lyude Paul 
> Cc: David Airlie 
> Cc: Daniel Vetter 
> Cc: dri-devel@lists.freedesktop.org
> Cc: nouv...@lists.freedesktop.org
> Cc: linux-ker...@vger.kernel.org
> Signed-off-by: Mark Menzynski 
> ---
>  drivers/gpu/drm/nouveau/nouveau_fbcon.c | 2 ++
>  1 file changed, 2 insertions(+)
> 
> diff --git a/drivers/gpu/drm/nouveau/nouveau_fbcon.c
> b/drivers/gpu/drm/nouveau/nouveau_fbcon.c
> index 4f9b3aa5deda..5226323e55d3 100644
> --- a/drivers/gpu/drm/nouveau/nouveau_fbcon.c
> +++ b/drivers/gpu/drm/nouveau/nouveau_fbcon.c
> @@ -39,6 +39,7 @@
>  
>  #include 
>  #include 
> +#include 
>  #include 
>  #include 
>  #include 
> @@ -605,6 +606,7 @@ nouveau_fbcon_fini(struct drm_device *dev)
> if (!drm->fbcon)
> return;
>  
> +   drm_kms_helper_poll_fini(dev);
> nouveau_fbcon_accel_fini(dev);
> nouveau_fbcon_destroy(dev, drm->fbcon);
> kfree(drm->fbcon);

-- 
Cheers,
 Lyude Paul (she/her)
 Software Engineer at Red Hat

Re: [PATCH v2] drm/nouveau: reorder nouveau_drm_device_fini

So I think you forgot to update the subject of the patch. If you can send a
respin of this patch with a corrected patch title, then you can consider this:

Reviewed-by: Lyude Paul 

I'll push once you get the respin out

On Mon, 2022-05-16 at 15:31 +0200, Mark Menzynski wrote:
> Resources needed for output poll workers are destroyed in
> nouveau_fbcon_fini() before output poll workers are cleared in
> nouveau_display_fini(). This means there is a time between fbcon_fini()
> and display_fini(), where if output poll happens, it crashes.
> 
> This makes output poll workers cleared right before fbcon resources are
> destroyed.
> 
> BUG: KASAN: use-after-free in
> __drm_fb_helper_initial_config_and_unlock.cold+0x1f3/0x291
> [drm_kms_helper]
> 
> Cc: Ben Skeggs 
> Cc: Karol Herbst 
> Cc: Lyude Paul 
> Cc: David Airlie 
> Cc: Daniel Vetter 
> Cc: dri-devel@lists.freedesktop.org
> Cc: nouv...@lists.freedesktop.org
> Cc: linux-ker...@vger.kernel.org
> Signed-off-by: Mark Menzynski 
> ---
>  drivers/gpu/drm/nouveau/nouveau_fbcon.c | 2 ++
>  1 file changed, 2 insertions(+)
> 
> diff --git a/drivers/gpu/drm/nouveau/nouveau_fbcon.c
> b/drivers/gpu/drm/nouveau/nouveau_fbcon.c
> index 4f9b3aa5deda..5226323e55d3 100644
> --- a/drivers/gpu/drm/nouveau/nouveau_fbcon.c
> +++ b/drivers/gpu/drm/nouveau/nouveau_fbcon.c
> @@ -39,6 +39,7 @@
>  
>  #include 
>  #include 
> +#include 
>  #include 
>  #include 
>  #include 
> @@ -605,6 +606,7 @@ nouveau_fbcon_fini(struct drm_device *dev)
> if (!drm->fbcon)
> return;
>  
> +   drm_kms_helper_poll_fini(dev);
> nouveau_fbcon_accel_fini(dev);
> nouveau_fbcon_destroy(dev, drm->fbcon);
> kfree(drm->fbcon);

-- 
Cheers,
 Lyude Paul (she/her)
 Software Engineer at Red Hat

Re: nouveau lockdep deadlock report with 5.18-rc6

YEah I saw this as well, will try to bisect soon

On Tue, 2022-05-17 at 13:10 +0200, Hans de Goede wrote:
> Hi All,
> 
> I just noticed the below lockdep possible deadlock report with a 5.18-rc6
> kernel on a Dell Latitude E6430 laptop with the following nvidia GPU:
> 
> 01:00.0 VGA compatible controller [0300]: NVIDIA Corporation GF108GLM [NVS
> 5200M] [10de:0dfc] (rev a1)
> 01:00.1 Audio device [0403]: NVIDIA Corporation GF108 High Definition Audio
> Controller [10de:0bea] (rev a1)
> 
> This is with the laptop in Optimus mode, so with the Intel integrated
> gfx from the i5-3320M CPU driving the LCD panel and with nothing connected
> to the HDMI connector, which is always routed to the NVIDIA GPU on this
> laptop.
> 
> The lockdep possible deadlock warning seems to happen when the NVIDIA GPU
> is runtime suspended shortly after gdm has loaded:
> 
> [   24.859171] ==
> [   24.859173] WARNING: possible circular locking dependency detected
> [   24.859175] 5.18.0-rc6+ #34 Tainted: G    E    
> [   24.859178] --
> [   24.859179] kworker/1:1/46 is trying to acquire lock:
> [   24.859181] 92b0c0ee0518 (>mutex){+.+.}-{3:3}, at:
> nouveau_vga_lastclose+0x910/0x1030 [nouveau]
> [   24.859231] 
>    but task is already holding lock:
> [   24.859233] 92b0c4bf35a0 (reservation_ww_class_mutex){+.+.}-{3:3},
> at: ttm_bo_wait+0x7d/0x140 [ttm]
> [   24.859243] 
>    which lock already depends on the new lock.
> 
> [   24.859244] 
>    the existing dependency chain (in reverse order) is:
> [   24.859246] 
>    -> #1 (reservation_ww_class_mutex){+.+.}-{3:3}:
> [   24.859249]    __ww_mutex_lock.constprop.0+0xb3/0xfb0
> [   24.859256]    ww_mutex_lock+0x38/0xa0
> [   24.859259]    nouveau_bo_pin+0x30/0x380 [nouveau]
> [   24.859297]    nouveau_channel_del+0x1d7/0x3e0 [nouveau]
> [   24.859328]    nouveau_channel_new+0x48/0x730 [nouveau]
> [   24.859358]    nouveau_abi16_ioctl_channel_alloc+0x113/0x360
> [nouveau]
> [   24.859389]    drm_ioctl_kernel+0xa1/0x150
> [   24.859392]    drm_ioctl+0x21c/0x410
> [   24.859395]    nouveau_drm_ioctl+0x56/0x1820 [nouveau]
> [   24.859431]    __x64_sys_ioctl+0x8d/0xc0
> [   24.859436]    do_syscall_64+0x5b/0x80
> [   24.859440]    entry_SYSCALL_64_after_hwframe+0x44/0xae
> [   24.859443] 
>    -> #0 (>mutex){+.+.}-{3:3}:
> [   24.859446]    __lock_acquire+0x12e2/0x1f90
> [   24.859450]    lock_acquire+0xad/0x290
> [   24.859453]    __mutex_lock+0x90/0x830
> [   24.859456]    nouveau_vga_lastclose+0x910/0x1030 [nouveau]
> [   24.859493]    ttm_bo_move_to_lru_tail+0x32c/0x980 [ttm]
> [   24.859498]    ttm_mem_evict_first+0x25c/0x4b0 [ttm]
> [   24.859503]    ttm_resource_manager_evict_all+0x93/0x1b0 [ttm]
> [   24.859509]    nouveau_debugfs_fini+0x161/0x260 [nouveau]
> [   24.859545]    nouveau_drm_ioctl+0xa4a/0x1820 [nouveau]
> [   24.859582]    pci_pm_runtime_suspend+0x5c/0x180
> [   24.859585]    __rpm_callback+0x48/0x1b0
> [   24.859589]    rpm_callback+0x5a/0x70
> [   24.859591]    rpm_suspend+0x10a/0x6f0
> [   24.859594]    pm_runtime_work+0xa0/0xb0
> [   24.859596]    process_one_work+0x254/0x560
> [   24.859601]    worker_thread+0x4f/0x390
> [   24.859604]    kthread+0xe6/0x110
> [   24.859607]    ret_from_fork+0x22/0x30
> [   24.859611] 
>    other info that might help us debug this:
> 
> [   24.859612]  Possible unsafe locking scenario:
> 
> [   24.859613]    CPU0    CPU1
> [   24.859615]        
> [   24.859616]   lock(reservation_ww_class_mutex);
> [   24.859618]    lock(>mutex);
> [   24.859620]   
> lock(reservation_ww_class_mutex);
> [   24.859622]   lock(>mutex);
> [   24.859624] 
>     *** DEADLOCK ***
> 
> [   24.859625] 3 locks held by kworker/1:1/46:
> [   24.859627]  #0: 92b0c0bb4338 ((wq_completion)pm){+.+.}-{0:0}, at:
> process_one_work+0x1d0/0x560
> [   24.859634]  #1: a8ffc02dfe80 ((work_completion)(
> >power.work)){+.+.}-{0:0}, at: process_one_work+0x1d0/0x560
> [   24.859641]  #2: 92b0c4bf35a0 (reservation_ww_class_mutex){+.+.}-
> {3:3}, at: ttm_bo_wait+0x7d/0x140 [ttm]
> [   24.859649] 
>    stack backtrace:
> [   24.859651] CPU: 1 PID: 46 Comm: kworker/1:1 Tainted: G    E
> 5.18.0-rc6+ #34
> [   24.859654] Hardware name: Dell Inc. Latitude E6430/0H3MT5, BIOS A21
> 05/08/2017
> [   24.859656] Workqueue: pm pm_runtime_work
> [   24.859660] Call Trace:
> [   24.859662]  
> [   24.859665]  dump_stack_lvl+0x5b/0x74
> [   24.859669]  check_noncircular+0xdf/0x100
> [   24.859672]  ? register_lock_class+0x38/0x470
> [   24.859678]  __lock_acquire+0x12e2/0x1f90
> [   24.859683]  lock_acquire+0xad/0x290
> [

Re: [PATCH] drm: dp: fix a possible memory leak in fetch_monitor_name()

Reviewed-by: Lyude Paul 

Will push to the appropriate branch in a bit

On Mon, 2022-05-16 at 11:20 +0800, Hangyu Hua wrote:
> drm_dp_mst_get_edid call kmemdup to create mst_edid. So mst_edid need to be
> freed after use.
> 
> Signed-off-by: Hangyu Hua 
> ---
>  drivers/gpu/drm/dp/drm_dp_mst_topology.c | 1 +
>  1 file changed, 1 insertion(+)
> 
> diff --git a/drivers/gpu/drm/dp/drm_dp_mst_topology.c
> b/drivers/gpu/drm/dp/drm_dp_mst_topology.c
> index 11300b53d24f..7a7cc44686f9 100644
> --- a/drivers/gpu/drm/dp/drm_dp_mst_topology.c
> +++ b/drivers/gpu/drm/dp/drm_dp_mst_topology.c
> @@ -4852,6 +4852,7 @@ static void fetch_monitor_name(struct
> drm_dp_mst_topology_mgr *mgr,
>  
> mst_edid = drm_dp_mst_get_edid(port->connector, mgr, port);
> drm_edid_get_monitor_name(mst_edid, name, namelen);
> +   kfree(mst_edid);
>  }
>  
>  /**

-- 
Cheers,
 Lyude Paul (she/her)
 Software Engineer at Red Hat

[RFC v3 3/3] drm/doc/rfc: VM_BIND uapi definition

VM_BIND and related uapi definitions

v2: Ensure proper kernel-doc formatting with cross references.
Also add new uapi and documentation as per review comments
from Daniel.

Signed-off-by: Niranjana Vishwanathapura 
---
 Documentation/gpu/rfc/i915_vm_bind.h | 399 +++
 1 file changed, 399 insertions(+)
 create mode 100644 Documentation/gpu/rfc/i915_vm_bind.h

diff --git a/Documentation/gpu/rfc/i915_vm_bind.h 
b/Documentation/gpu/rfc/i915_vm_bind.h
new file mode 100644
index ..589c0a009107
--- /dev/null
+++ b/Documentation/gpu/rfc/i915_vm_bind.h
@@ -0,0 +1,399 @@
+/* SPDX-License-Identifier: MIT */
+/*
+ * Copyright © 2022 Intel Corporation
+ */
+
+/**
+ * DOC: I915_PARAM_HAS_VM_BIND
+ *
+ * VM_BIND feature availability.
+ * See typedef drm_i915_getparam_t param.
+ */
+#define I915_PARAM_HAS_VM_BIND 57
+
+/**
+ * DOC: I915_VM_CREATE_FLAGS_USE_VM_BIND
+ *
+ * Flag to opt-in for VM_BIND mode of binding during VM creation.
+ * See struct drm_i915_gem_vm_control flags.
+ *
+ * A VM in VM_BIND mode will not support the older execbuff mode of binding.
+ * In VM_BIND mode, execbuff ioctl will not accept any execlist (ie., the
+ * _i915_gem_execbuffer2.buffer_count must be 0).
+ * Also, _i915_gem_execbuffer2.batch_start_offset and
+ * _i915_gem_execbuffer2.batch_len must be 0.
+ * DRM_I915_GEM_EXECBUFFER_EXT_BATCH_ADDRESSES extension must be provided
+ * to pass in the batch buffer addresses.
+ *
+ * Additionally, I915_EXEC_NO_RELOC, I915_EXEC_HANDLE_LUT and
+ * I915_EXEC_BATCH_FIRST of _i915_gem_execbuffer2.flags must be 0
+ * (not used) in VM_BIND mode. I915_EXEC_USE_EXTENSIONS flag must always be
+ * set (See struct drm_i915_gem_execbuffer_ext_batch_addresses).
+ * The buffers_ptr, buffer_count, batch_start_offset and batch_len fields
+ * of struct drm_i915_gem_execbuffer2 are also not used and must be 0.
+ */
+#define I915_VM_CREATE_FLAGS_USE_VM_BIND   (1 << 0)
+
+/**
+ * DOC: I915_CONTEXT_CREATE_FLAGS_LONG_RUNNING
+ *
+ * Flag to declare context as long running.
+ * See struct drm_i915_gem_context_create_ext flags.
+ *
+ * Usage of dma-fence expects that they complete in reasonable amount of time.
+ * Compute on the other hand can be long running. Hence it is not appropriate
+ * for compute contexts to export request completion dma-fence to user.
+ * The dma-fence usage will be limited to in-kernel consumption only.
+ * Compute contexts need to use user/memory fence.
+ *
+ * So, long running contexts do not support output fences. Hence,
+ * I915_EXEC_FENCE_OUT (See _i915_gem_execbuffer2.flags and
+ * I915_EXEC_FENCE_SIGNAL (See _i915_gem_exec_fence.flags) are expected
+ * to be not used.
+ *
+ * DRM_I915_GEM_WAIT ioctl call is also not supported for objects mapped
+ * to long running contexts.
+ */
+#define I915_CONTEXT_CREATE_FLAGS_LONG_RUNNING   (1u << 2)
+
+/* VM_BIND related ioctls */
+#define DRM_I915_GEM_VM_BIND   0x3d
+#define DRM_I915_GEM_VM_UNBIND 0x3e
+#define DRM_I915_GEM_WAIT_USER_FENCE   0x3f
+
+#define DRM_IOCTL_I915_GEM_VM_BIND DRM_IOWR(DRM_COMMAND_BASE + 
DRM_I915_GEM_VM_BIND, struct drm_i915_gem_vm_bind)
+#define DRM_IOCTL_I915_GEM_VM_UNBIND   DRM_IOWR(DRM_COMMAND_BASE + 
DRM_I915_GEM_VM_UNBIND, struct drm_i915_gem_vm_bind)
+#define DRM_IOCTL_I915_GEM_WAIT_USER_FENCE DRM_IOWR(DRM_COMMAND_BASE + 
DRM_I915_GEM_WAIT_USER_FENCE, struct drm_i915_gem_wait_user_fence)
+
+/**
+ * struct drm_i915_gem_vm_bind - VA to object mapping to bind.
+ *
+ * This structure is passed to VM_BIND ioctl and specifies the mapping of GPU
+ * virtual address (VA) range to the section of an object that should be bound
+ * in the device page table of the specified address space (VM).
+ * The VA range specified must be unique (ie., not currently bound) and can
+ * be mapped to whole object or a section of the object (partial binding).
+ * Multiple VA mappings can be created to the same section of the object
+ * (aliasing).
+ */
+struct drm_i915_gem_vm_bind {
+   /** @vm_id: VM (address space) id to bind */
+   __u32 vm_id;
+
+   /** @handle: Object handle */
+   __u32 handle;
+
+   /** @start: Virtual Address start to bind */
+   __u64 start;
+
+   /** @offset: Offset in object to bind */
+   __u64 offset;
+
+   /** @length: Length of mapping to bind */
+   __u64 length;
+
+   /**
+* @flags: Supported flags are,
+*
+* I915_GEM_VM_BIND_READONLY:
+* Mapping is read-only.
+*
+* I915_GEM_VM_BIND_CAPTURE:
+* Capture this mapping in the dump upon GPU error.
+*/
+   __u64 flags;
+#define I915_GEM_VM_BIND_READONLY(1 << 0)
+#define I915_GEM_VM_BIND_CAPTURE (1 << 1)
+
+   /** @extensions: 0-terminated chain of extensions for this mapping. */
+   __u64 extensions;
+};
+
+/**
+ * struct drm_i915_gem_vm_unbind - VA to object mapping to unbind.
+ *
+ * This structure is passed to VM_UNBIND ioctl and specifies the GPU

[RFC v3 0/3] drm/doc/rfc: i915 VM_BIND feature design + uapi

This is the i915 driver VM_BIND feature design RFC patch series along
with the required uapi definition and description of intended use cases.

v2: Updated design and uapi, more documentation.
v3: Add more documentation and proper kernel-doc formatting with cross
references (including missing i915_drm uapi kernel-docs which are
required) as per review comments from Daniel.

Signed-off-by: Niranjana Vishwanathapura 

Niranjana Vishwanathapura (3):
  drm/doc/rfc: VM_BIND feature design document
  drm/i915: Update i915 uapi documentation
  drm/doc/rfc: VM_BIND uapi definition

 Documentation/driver-api/dma-buf.rst   |   2 +
 Documentation/gpu/rfc/i915_vm_bind.h   | 399 +
 Documentation/gpu/rfc/i915_vm_bind.rst | 304 +++
 Documentation/gpu/rfc/index.rst|   4 +
 include/uapi/drm/i915_drm.h| 153 +++---
 5 files changed, 825 insertions(+), 37 deletions(-)
 create mode 100644 Documentation/gpu/rfc/i915_vm_bind.h
 create mode 100644 Documentation/gpu/rfc/i915_vm_bind.rst

-- 
2.21.0.rc0.32.g243a4c7e27

[RFC v3 2/3] drm/i915: Update i915 uapi documentation

Add some missing i915 upai documentation which the new
i915 VM_BIND feature documentation will be refer to.

Signed-off-by: Niranjana Vishwanathapura 
---
 include/uapi/drm/i915_drm.h | 153 +++-
 1 file changed, 116 insertions(+), 37 deletions(-)

diff --git a/include/uapi/drm/i915_drm.h b/include/uapi/drm/i915_drm.h
index a2def7b27009..8c834a31b56f 100644
--- a/include/uapi/drm/i915_drm.h
+++ b/include/uapi/drm/i915_drm.h
@@ -751,9 +751,16 @@ typedef struct drm_i915_irq_wait {
 
 /* Must be kept compact -- no holes and well documented */
 
+/**
+ * typedef drm_i915_getparam_t - Driver parameter query structure.
+ */
 typedef struct drm_i915_getparam {
+   /** @param: Driver parameter to query. */
__s32 param;
-   /*
+
+   /**
+* @value: Address of memory where queried value should be put.
+*
 * WARNING: Using pointers instead of fixed-size u64 means we need to 
write
 * compat32 code. Don't repeat this mistake.
 */
@@ -1239,76 +1246,114 @@ struct drm_i915_gem_exec_object2 {
__u64 rsvd2;
 };
 
+/**
+ * struct drm_i915_gem_exec_fence - An input or output fence for the execbuff
+ * ioctl.
+ *
+ * The request will wait for input fence to signal before submission.
+ *
+ * The returned output fence will be signaled after the completion of the
+ * request.
+ */
 struct drm_i915_gem_exec_fence {
-   /**
-* User's handle for a drm_syncobj to wait on or signal.
-*/
+   /** @handle: User's handle for a drm_syncobj to wait on or signal. */
__u32 handle;
 
+   /**
+* @flags: Supported flags are,
+*
+* I915_EXEC_FENCE_WAIT:
+* Wait for the input fence before request submission.
+*
+* I915_EXEC_FENCE_SIGNAL:
+* Return request completion fence as output
+*/
+   __u32 flags;
 #define I915_EXEC_FENCE_WAIT(1<<0)
 #define I915_EXEC_FENCE_SIGNAL  (1<<1)
 #define __I915_EXEC_FENCE_UNKNOWN_FLAGS (-(I915_EXEC_FENCE_SIGNAL << 1))
-   __u32 flags;
 };
 
-/*
- * See drm_i915_gem_execbuffer_ext_timeline_fences.
- */
-#define DRM_I915_GEM_EXECBUFFER_EXT_TIMELINE_FENCES 0
-
-/*
+/**
+ * struct drm_i915_gem_execbuffer_ext_timeline_fences - Timeline fences
+ * for execbuff.
+ *
  * This structure describes an array of drm_syncobj and associated points for
  * timeline variants of drm_syncobj. It is invalid to append this structure to
  * the execbuf if I915_EXEC_FENCE_ARRAY is set.
  */
 struct drm_i915_gem_execbuffer_ext_timeline_fences {
+#define DRM_I915_GEM_EXECBUFFER_EXT_TIMELINE_FENCES 0
+   /** @base: Extension link. See struct i915_user_extension. */
struct i915_user_extension base;
 
/**
-* Number of element in the handles_ptr & value_ptr arrays.
+* @fence_count: Number of element in the @handles_ptr & @value_ptr
+* arrays.
 */
__u64 fence_count;
 
/**
-* Pointer to an array of struct drm_i915_gem_exec_fence of length
-* fence_count.
+* @handles_ptr: Pointer to an array of struct drm_i915_gem_exec_fence
+* of length @fence_count.
 */
__u64 handles_ptr;
 
/**
-* Pointer to an array of u64 values of length fence_count. Values
-* must be 0 for a binary drm_syncobj. A Value of 0 for a timeline
-* drm_syncobj is invalid as it turns a drm_syncobj into a binary one.
+* @values_ptr: Pointer to an array of u64 values of length
+* @fence_count.
+* Values must be 0 for a binary drm_syncobj. A Value of 0 for a
+* timeline drm_syncobj is invalid as it turns a drm_syncobj into a
+* binary one.
 */
__u64 values_ptr;
 };
 
+/**
+ * struct drm_i915_gem_execbuffer2 - Structure for execbuff submission
+ */
 struct drm_i915_gem_execbuffer2 {
-   /**
-* List of gem_exec_object2 structs
-*/
+   /** @buffers_ptr: Pointer to a list of gem_exec_object2 structs */
__u64 buffers_ptr;
+
+   /** @buffer_count: Number of elements in @buffers_ptr array */
__u32 buffer_count;
 
-   /** Offset in the batchbuffer to start execution from. */
+   /**
+* @batch_start_offset: Offset in the batchbuffer to start execution
+* from.
+*/
__u32 batch_start_offset;
-   /** Bytes used in batchbuffer from batch_start_offset */
+
+   /** @batch_len: Bytes used in batchbuffer from batch_start_offset */
__u32 batch_len;
+
+   /** @DR1: deprecated */
__u32 DR1;
+
+   /** @DR4: deprecated */
__u32 DR4;
+
+   /** @num_cliprects: See @cliprects_ptr */
__u32 num_cliprects;
+
/**
-* This is a struct drm_clip_rect *cliprects if I915_EXEC_FENCE_ARRAY
-* & I915_EXEC_USE_EXTENSIONS are not set.
+* @cliprects_ptr: Kernel clipping was a DRI1 misfeature.
+*
+* It is invalid to use

[RFC v3 1/3] drm/doc/rfc: VM_BIND feature design document

VM_BIND design document with description of intended use cases.

v2: Add more documentation and format as per review comments
from Daniel.

Signed-off-by: Niranjana Vishwanathapura 
---
 Documentation/driver-api/dma-buf.rst   |   2 +
 Documentation/gpu/rfc/i915_vm_bind.rst | 304 +
 Documentation/gpu/rfc/index.rst|   4 +
 3 files changed, 310 insertions(+)
 create mode 100644 Documentation/gpu/rfc/i915_vm_bind.rst

diff --git a/Documentation/driver-api/dma-buf.rst 
b/Documentation/driver-api/dma-buf.rst
index 36a76cbe9095..64cb924ec5bb 100644
--- a/Documentation/driver-api/dma-buf.rst
+++ b/Documentation/driver-api/dma-buf.rst
@@ -200,6 +200,8 @@ DMA Fence uABI/Sync File
 .. kernel-doc:: include/linux/sync_file.h
:internal:
 
+.. _indefinite_dma_fences:
+
 Indefinite DMA Fences
 ~
 
diff --git a/Documentation/gpu/rfc/i915_vm_bind.rst 
b/Documentation/gpu/rfc/i915_vm_bind.rst
new file mode 100644
index ..f1be560d313c
--- /dev/null
+++ b/Documentation/gpu/rfc/i915_vm_bind.rst
@@ -0,0 +1,304 @@
+==
+I915 VM_BIND feature design and use cases
+==
+
+VM_BIND feature
+
+DRM_I915_GEM_VM_BIND/UNBIND ioctls allows UMD to bind/unbind GEM buffer
+objects (BOs) or sections of a BOs at specified GPU virtual addresses on a
+specified address space (VM). These mappings (also referred to as persistent
+mappings) will be persistent across multiple GPU submissions (execbuff calls)
+issued by the UMD, without user having to provide a list of all required
+mappings during each submission (as required by older execbuff mode).
+
+VM_BIND/UNBIND ioctls will support 'in' and 'out' fences to allow userpace
+to specify how the binding/unbinding should sync with other operations
+like the GPU job submission. These fences will be timeline 'drm_syncobj's
+for non-Compute contexts (See struct drm_i915_vm_bind_ext_timeline_fences).
+For Compute contexts, they will be user/memory fences (See struct
+drm_i915_vm_bind_ext_user_fence).
+
+VM_BIND feature is advertised to user via I915_PARAM_HAS_VM_BIND.
+User has to opt-in for VM_BIND mode of binding for an address space (VM)
+during VM creation time via I915_VM_CREATE_FLAGS_USE_VM_BIND extension.
+
+VM_BIND/UNBIND ioctl will immediately start binding/unbinding the mapping in an
+async worker. The binding and unbinding will work like a special GPU engine.
+The binding and unbinding operations are serialized and will wait on specified
+input fences before the operation and will signal the output fences upon the
+completion of the operation. Due to serialization, completion of an operation
+will also indicate that all previous operations are also complete.
+
+VM_BIND features include:
+
+* Multiple Virtual Address (VA) mappings can map to the same physical pages
+  of an object (aliasing).
+* VA mapping can map to a partial section of the BO (partial binding).
+* Support capture of persistent mappings in the dump upon GPU error.
+* TLB is flushed upon unbind completion. Batching of TLB flushes in some
+  use cases will be helpful.
+* Asynchronous vm_bind and vm_unbind support with 'in' and 'out' fences.
+* Support for userptr gem objects (no special uapi is required for this).
+
+Execbuff ioctl in VM_BIND mode
+---
+The execbuff ioctl handling in VM_BIND mode differs significantly from the
+older method. A VM in VM_BIND mode will not support older execbuff mode of
+binding. In VM_BIND mode, execbuff ioctl will not accept any execlist. Hence,
+no support for implicit sync. It is expected that the below work will be able
+to support requirements of object dependency setting in all use cases:
+
+"dma-buf: Add an API for exporting sync files"
+(https://lwn.net/Articles/859290/)
+
+This also means, we need an execbuff extension to pass in the batch
+buffer addresses (See struct drm_i915_gem_execbuffer_ext_batch_addresses).
+
+If at all execlist support in execbuff ioctl is deemed necessary for
+implicit sync in certain use cases, then support can be added later.
+
+In VM_BIND mode, VA allocation is completely managed by the user instead of
+the i915 driver. Hence all VA assignment, eviction are not applicable in
+VM_BIND mode. Also, for determining object activeness, VM_BIND mode will not
+be using the i915_vma active reference tracking. It will instead use dma-resv
+object for that (See `VM_BIND dma_resv usage`_).
+
+So, a lot of existing code in the execbuff path like relocations, VA evictions,
+vma lookup table, implicit sync, vma active reference tracking etc., are not
+applicable in VM_BIND mode. Hence, the execbuff path needs to be cleaned up
+by clearly separating out the functionalities where the VM_BIND mode differs
+from older method and they should be moved to separate files.
+
+VM_PRIVATE objects
+---
+By default, BOs can be mapped on multiple VMs and can also be dma-buf

Re: [PATCH v11 00/24] drm/rockchip: RK356x VOP2 support

2022-05-17 Thread Heiko Stuebner

Am Freitag, 22. April 2022, 09:28:17 CEST schrieb Sascha Hauer:
> It's v11 time. There's only one small change to v10. Discussion seems to
> have settled now. Is there anything left that prevents the series from
> being merged? I'd really like to have it in during the next merge
> window.
> 
> This series still depends on:
> drm/rockchip: Refactor IOMMU initialisation 
> (https://lists.freedesktop.org/archives/dri-devel/2022-April/349548.html)
> arm64: dts: rockchip: add basic dts for the radxa rock3 model a
> 
> Sascha

I've now picked up everything except the hdmi-rate stuff in some way.
The driver changes will go into 5.19 and the DT-changes into 5.20.

I'll now move the series out of my focus and would expect further
hdmi rate voodoo to start a new series :-) .

Thanks to all involved for working on this.
Heiko

Re: (subset) [PATCH v11 00/24] drm/rockchip: RK356x VOP2 support

2022-05-17 Thread Heiko Stuebner

On Fri, 22 Apr 2022 09:28:17 +0200, Sascha Hauer wrote:
> It's v11 time. There's only one small change to v10. Discussion seems to
> have settled now. Is there anything left that prevents the series from
> being merged? I'd really like to have it in during the next merge
> window.
> 
> This series still depends on:
> drm/rockchip: Refactor IOMMU initialisation 
> (https://lists.freedesktop.org/archives/dri-devel/2022-April/349548.html)
> arm64: dts: rockchip: add basic dts for the radxa rock3 model a
> 
> [...]

Applied for 5.20.

The vop2 dt-binding header is going through the drm tree and
the reg-names addition is going through drm-misc-next-fixes
into the 5.19-rc kernels and this is a big bunch of dt changes
so I'm leaving them for 5.20.

[16/24] arm64: dts: rockchip: rk356x: Add VOP2 nodes
commit: 9b3c9f6e57ef26b8478da64e3d49d0438291a98a
[17/24] arm64: dts: rockchip: rk356x: Add HDMI nodes
commit: db24799d31e7149bd8495a5f41c408434ca1a654
[18/24] arm64: dts: rockchip: rk3568-evb: Enable VOP2 and hdmi
commit: f2cdaaae9b947970842196ac93d71b0daad3e629
[19/24] arm64: dts: rockchip: enable vop2 and hdmi tx on quartz64a
commit: 97d7309a70b03d01a154c4308f423b020f7d397e
[20/24] arm64: dts: rockchip: enable vop2 and hdmi tx on rock-3a
commit: 281f94d9cbac31c1c112d09883bc42021435f1ad

Best regards,
-- 
Heiko Stuebner

Re: (subset) [PATCH v2 0/3] RK356x VOP2: Change register space names

2022-05-17 Thread Heiko Stuebner

On Wed, 11 May 2022 10:21:06 +0200, Sascha Hauer wrote:
> The VOP2 driver sitting in next uses named register spaces, but the
> binding lacks documentation for that. Add the missing documentation
> and while at it take the opportunity to rename the register spaces
> from too generic "regs" to "vop" and from "gamma_lut" to better looking
> "gamma-lut".
> 
> Changes since v1:
> - Fix dt_binding_check errors
> 
> [...]

Applied, thanks!

[3/3] arm64: dts: rockchip: rk356x: Change VOP2 register space names

applied as fixup to
[16/24] arm64: dts: rockchip: rk356x: Add VOP2 nodes
commit: 9b3c9f6e57ef26b8478da64e3d49d0438291a98a



Best regards,
-- 
Heiko Stuebner

Re: [PATCH v1 1/2] phy/qcom: add regulator_set_load to edp/dp phy

2022-05-17 Thread Doug Anderson

Hi,

On Tue, May 17, 2022 at 10:26 AM Kuogee Hsieh  wrote:
>
> This patch add regulator_set_load() to both eDP and DP phy driver
> to have totally control regulators.
>
> Signed-off-by: Kuogee Hsieh 
> ---
>  drivers/phy/qualcomm/phy-qcom-edp.c | 25 +
>  drivers/phy/qualcomm/phy-qcom-qmp.c | 24 
>  2 files changed, 45 insertions(+), 4 deletions(-)
>
> diff --git a/drivers/phy/qualcomm/phy-qcom-edp.c 
> b/drivers/phy/qualcomm/phy-qcom-edp.c
> index cacd32f..9b55095 100644
> --- a/drivers/phy/qualcomm/phy-qcom-edp.c
> +++ b/drivers/phy/qualcomm/phy-qcom-edp.c
> @@ -87,17 +87,24 @@ struct qcom_edp {
>
> struct clk_bulk_data clks[2];
> struct regulator_bulk_data supplies[2];
> +   int enable_load[2];
> +   int disable_load[2];
>  };
>
>  static int qcom_edp_phy_init(struct phy *phy)
>  {
> struct qcom_edp *edp = phy_get_drvdata(phy);
> int ret;
> +   int num_consumers = ARRAY_SIZE(edp->supplies);
> +   int i;
>
> -   ret = regulator_bulk_enable(ARRAY_SIZE(edp->supplies), edp->supplies);
> +   ret = regulator_bulk_enable(num_consumers, edp->supplies);
> if (ret)
> return ret;
>
> +   for (i = num_consumers - 1; i >= 0; --i)
> +   regulator_set_load(edp->supplies[i].consumer, 
> edp->enable_load[i]);
> +
> ret = clk_bulk_prepare_enable(ARRAY_SIZE(edp->clks), edp->clks);
> if (ret)
> goto out_disable_supplies;
> @@ -425,9 +432,15 @@ static int qcom_edp_phy_power_off(struct phy *phy)
>  static int qcom_edp_phy_exit(struct phy *phy)
>  {
> struct qcom_edp *edp = phy_get_drvdata(phy);
> +   int num_consumers = ARRAY_SIZE(edp->supplies);
> +   int i;
>
> clk_bulk_disable_unprepare(ARRAY_SIZE(edp->clks), edp->clks);
> -   regulator_bulk_disable(ARRAY_SIZE(edp->supplies), edp->supplies);
> +
> +   for (i = num_consumers - 1; i >= 0; --i)
> +   regulator_set_load(edp->supplies[i].consumer, 
> edp->disable_load[i]);
> +
> +   regulator_bulk_disable(num_consumers, edp->supplies);
>
> return 0;
>  }
> @@ -633,8 +646,12 @@ static int qcom_edp_phy_probe(struct platform_device 
> *pdev)
> if (ret)
> return ret;
>
> -   edp->supplies[0].supply = "vdda-phy";
> -   edp->supplies[1].supply = "vdda-pll";
> +   edp->supplies[0].supply = "vdda-1p2";
> +   edp->supplies[1].supply = "vdda-0p9";
> +   edp->enable_load[0] = 21800;/* 1.2 V */
> +   edp->enable_load[1] = 36000;/* 1.2 V */
> +   edp->disable_load[0] = 4;   /* 0.9 V */
> +   edp->disable_load[1] = 4;   /* 10.9V */

10.9V? That's a lot!


> ret = devm_regulator_bulk_get(dev, ARRAY_SIZE(edp->supplies), 
> edp->supplies);
> if (ret)
> return ret;
> diff --git a/drivers/phy/qualcomm/phy-qcom-qmp.c 
> b/drivers/phy/qualcomm/phy-qcom-qmp.c
> index b144ae1..c589231 100644
> --- a/drivers/phy/qualcomm/phy-qcom-qmp.c
> +++ b/drivers/phy/qualcomm/phy-qcom-qmp.c
> @@ -3130,6 +3130,8 @@ struct qmp_phy_cfg {
> int num_resets;
> /* regulators to be requested */
> const char * const *vreg_list;
> +   const unsigned int *vreg_enable_load;
> +   const unsigned int *vreg_disable_load;
> int num_vregs;
>
> /* array of registers with different offsets */
> @@ -3346,6 +3348,14 @@ static const char * const qmp_phy_vreg_l[] = {
> "vdda-phy", "vdda-pll",
>  };
>
> +static const unsigned int qmp_phy_vreg_enable_load[] = {
> +   21800, 36000
> +};
> +
> +static const unsigned int qmp_phy_vreg_disable_load[] = {
> +   4, 32
> +};
> +
>  static const struct qmp_phy_cfg ipq8074_usb3phy_cfg = {
> .type   = PHY_TYPE_USB3,
> .nlanes = 1,
> @@ -4072,6 +4082,8 @@ static const struct qmp_phy_cfg sm8250_usb3phy_cfg = {
> .reset_list = msm8996_usb3phy_reset_l,
> .num_resets = ARRAY_SIZE(msm8996_usb3phy_reset_l),
> .vreg_list  = qmp_phy_vreg_l,
> +   .vreg_enable_load   = qmp_phy_vreg_enable_load,
> +   .vreg_disable_load  = qmp_phy_vreg_disable_load,
> .num_vregs  = ARRAY_SIZE(qmp_phy_vreg_l),
> .regs   = qmp_v4_usb3phy_regs_layout,
>
> @@ -4139,6 +4151,8 @@ static const struct qmp_phy_cfg sm8250_dpphy_cfg = {
> .reset_list = msm8996_usb3phy_reset_l,
> .num_resets = ARRAY_SIZE(msm8996_usb3phy_reset_l),
> .vreg_list  = qmp_phy_vreg_l,
> +   .vreg_enable_load   = qmp_phy_vreg_enable_load,
> +   .vreg_disable_load  = qmp_phy_vreg_disable_load,
> .num_vregs  = ARRAY_SIZE(qmp_phy_vreg_l),
> .regs   = qmp_v4_usb3phy_regs_layout,
>
> @@ -5015,6 +5029,11 @@ static int qcom_qmp_phy_com_init(struct qmp_phy *qphy)
> goto

Re: linux-next: Tree for May 16 (drm/i915/gt/intel_gt_sysfs_pm.c)

2022-05-17 Thread Randy Dunlap




On 5/17/22 00:35, Tvrtko Ursulin wrote:
> 
> Hi,
> 
> On 16/05/2022 22:22, Randy Dunlap wrote:
>>
>>
>> On 5/16/22 03:57, Stephen Rothwell wrote:
>>> Hi all,
>>>
>>> Changes since 20220513:
>>>
>>
>> on i386:
>>
>>    CC  drivers/gpu/drm/i915/gt/intel_gt_sysfs_pm.o
>> ../drivers/gpu/drm/i915/gt/intel_gt_sysfs_pm.c: In function 
>> ‘act_freq_mhz_show’:
>> ../drivers/gpu/drm/i915/gt/intel_gt_sysfs_pm.c:276:20: error: implicit 
>> declaration of function ‘sysfs_gt_attribute_r_max_func’ 
>> [-Werror=implicit-function-declaration]
>>    u32 actual_freq = sysfs_gt_attribute_r_max_func(dev, attr,
>>  ^
>> ../drivers/gpu/drm/i915/gt/intel_gt_sysfs_pm.c: In function 
>> ‘boost_freq_mhz_store’:
>> ../drivers/gpu/drm/i915/gt/intel_gt_sysfs_pm.c:327:9: error: implicit 
>> declaration of function ‘sysfs_gt_attribute_w_func’ 
>> [-Werror=implicit-function-declaration]
>>    return sysfs_gt_attribute_w_func(dev, attr,
>>   ^
>> ../drivers/gpu/drm/i915/gt/intel_gt_sysfs_pm.c: In function 
>> ‘min_freq_mhz_show’:
>> ../drivers/gpu/drm/i915/gt/intel_gt_sysfs_pm.c:416:17: error: implicit 
>> declaration of function ‘sysfs_gt_attribute_r_min_func’ 
>> [-Werror=implicit-function-declaration]
>>    u32 min_freq = sysfs_gt_attribute_r_min_func(dev, attr,
>>   ^
>> cc1: some warnings being treated as errors
>>
>>
>> Full randconfig file is attached.
> 
> There is a fix for this in 09708b6d82ef ("drm/i915/gt: Fix build error 
> without CONFIG_PM") queued up, waiting for the next pull request, which the 
> plan was to send out next week or so. Is that okay?

When will it be in linux-next?

thanks.
-- 
~Randy

RE: [PATCH v2] drm: rcar-du: Fix Alpha blending issue on Gen3

2022-05-17 Thread Biju Das

Hi Kieran,

> Subject: Re: [PATCH v2] drm: rcar-du: Fix Alpha blending issue on Gen3
> 
> Quoting Biju Das (2022-04-26 09:41:57)
> > From: LUU HOAI 
> >
> > As per R-Car-Gen3_Common_OPC_Customer_Notifications_V30.1.pdf,
> > unexpected image output(such as incorrect colors or planes being
> > invisible) can happen on the below conditions, as PnALPHAR register is
> > not initialized by reset.
> >
> > When alpha blending (PpMRm.PpSPIM=0b101) is performed and:
> > •two Planes are enabled on any DUn (n=0,1,2,3)
> > oDSPRn= 0x 0031 or 0x 0013
> > •or DU0 and DU1 is used for display at the same time
> > oDSPR0= 0x 0001 and DSPR1= 0x 0003
> > oDSPR0= 0x 0003 and DSPR1= 0x 0001
> > •or DU2 and DU3(H3 Only) is used for display at the same time
> > oDSPR2= 0x 0001 and DSPR3= 0x 0003
> > oDSPR2= 0x 0003 and DSPR3= 0x 0001
> >
> > This patch set PnALPHAR register to 0 to avoid this issue.
> >
> > Signed-off-by: LUU HOAI 
> > Signed-off-by: Biju Das 
> > ---
> > v1->v2:
> >  * Updated commit description
> >  * Updated the code comments
> > ---
> >  drivers/gpu/drm/rcar-du/rcar_du_plane.c | 12 
> >  1 file changed, 12 insertions(+)
> >
> > diff --git a/drivers/gpu/drm/rcar-du/rcar_du_plane.c
> > b/drivers/gpu/drm/rcar-du/rcar_du_plane.c
> > index f214a8b6cfd3..aa80c44dd8d7 100644
> > --- a/drivers/gpu/drm/rcar-du/rcar_du_plane.c
> > +++ b/drivers/gpu/drm/rcar-du/rcar_du_plane.c
> > @@ -510,6 +510,18 @@ static void
> > rcar_du_plane_setup_format_gen3(struct rcar_du_group *rgrp,
> >
> > rcar_du_plane_write(rgrp, index, PnDDCR4,
> > state->format->edf | PnDDCR4_CODE);
> > +
> > +   /*
> > +* On Gen3, some DU channels have two planes, each being wired to
> a
> > +* separate VSPD instance. The DU can then blend two planes.
> While
> > +* this feature isn't used by the driver, issues related to alpha
> > +* blending (such as incorrect colors or planes being invisible)
> may
> > +* still occur if the PnALPHAR register has a stale value. Set
> the
> > +* register to 0 to avoid this.
> > +*/
> > +
> 
> Sounds reasonable.
> 
> > +   /* TODO: Check if alpha-blending should be disabled in PnMR.
> > + */
> 
> How will this be done? Do you have a specific test in mind to determine if
> it causes issues? Or does it need validating through the hardware teams?

It is TO DO activity and will be investigated later. After investigation, if it 
is no more required
Then will remove the above comment or if it is required will send a patch.

Cheers,
Biju

> 
> 
> 
> > +   rcar_du_plane_write(rgrp, index, PnALPHAR, 0);
> 
> 
> Reviewed-by: Kieran Bingham 
> 
> >  }
> >
> >  static void rcar_du_plane_setup_format(struct rcar_du_group *rgrp,
> > --
> > 2.25.1
> >

RE: [PATCH v4] drm/doc: add rfc section for small BAR uapi

2022-05-17 Thread Abodunrin, Akeem G



> -Original Message-
> From: Tvrtko Ursulin 
> Sent: Tuesday, May 17, 2022 6:49 AM
> To: Auld, Matthew ; intel-...@lists.freedesktop.org
> Cc: dri-devel@lists.freedesktop.org; Thomas Hellström
> ; Landwerlin, Lionel G
> ; Bloomfield, Jon ;
> Daniel Vetter ; Justen, Jordan L
> ; Kenneth Graunke ;
> Abodunrin, Akeem G ; mesa-
> d...@lists.freedesktop.org
> Subject: Re: [PATCH v4] drm/doc: add rfc section for small BAR uapi
> 
> 
> On 17/05/2022 11:52, Matthew Auld wrote:
> > Add an entry for the new uapi needed for small BAR on DG2+.
> >
> > v2:
> >- Some spelling fixes and other small tweaks. (Akeem & Thomas)
> >- Rework error capture interactions, including no longer needing
> >  NEEDS_CPU_ACCESS for objects marked for capture. (Thomas)
> >- Add probed_cpu_visible_size. (Lionel)
> > v3:
> >- Drop the vma query for now.
> >- Add unallocated_cpu_visible_size as part of the region query.
> >- Improve the docs some more, including documenting the expected
> >  behaviour on older kernels, since this came up in some offline
> >  discussion.
> > v4:
> >- Various improvements all over. (Tvrtko)
> 
> You can ignore my previous reply, the clarifications from v4 read good to me.
> 
> Acked-by: Tvrtko Ursulin 

The patch looks good to me as well...

Acked-by: Akeem G Abodunrin 

> 
> Regards,
> 
> Tvrtko
> 
> >
> > Signed-off-by: Matthew Auld 
> > Cc: Thomas Hellström 
> > Cc: Lionel Landwerlin 
> > Cc: Tvrtko Ursulin 
> > Cc: Jon Bloomfield 
> > Cc: Daniel Vetter 
> > Cc: Jon Bloomfield 
> > Cc: Jordan Justen 
> > Cc: Kenneth Graunke 
> > Cc: Akeem G Abodunrin 
> > Cc: mesa-...@lists.freedesktop.org
> > ---
> >   Documentation/gpu/rfc/i915_small_bar.h   | 189
> +++
> >   Documentation/gpu/rfc/i915_small_bar.rst |  47 ++
> >   Documentation/gpu/rfc/index.rst  |   4 +
> >   3 files changed, 240 insertions(+)
> >   create mode 100644 Documentation/gpu/rfc/i915_small_bar.h
> >   create mode 100644 Documentation/gpu/rfc/i915_small_bar.rst
> >
> > diff --git a/Documentation/gpu/rfc/i915_small_bar.h
> > b/Documentation/gpu/rfc/i915_small_bar.h
> > new file mode 100644
> > index ..c676640b23ef
> > --- /dev/null
> > +++ b/Documentation/gpu/rfc/i915_small_bar.h
> > @@ -0,0 +1,189 @@
> > +/**
> > + * struct __drm_i915_memory_region_info - Describes one region as
> > +known to the
> > + * driver.
> > + *
> > + * Note this is using both struct drm_i915_query_item and struct
> drm_i915_query.
> > + * For this new query we are adding the new query id
> > +DRM_I915_QUERY_MEMORY_REGIONS
> > + * at _i915_query_item.query_id.
> > + */
> > +struct __drm_i915_memory_region_info {
> > +   /** @region: The class:instance pair encoding */
> > +   struct drm_i915_gem_memory_class_instance region;
> > +
> > +   /** @rsvd0: MBZ */
> > +   __u32 rsvd0;
> > +
> > +   /**
> > +* @probed_size: Memory probed by the driver (-1 = unknown)
> > +*
> > +* Note that it should not be possible to ever encounter a zero value
> > +* here, also note that no current region type will ever return -1 here.
> > +* Although for future region types, this might be a possibility. The
> > +* same applies to the other size fields.
> > +*/
> > +   __u64 probed_size;
> > +
> > +   /**
> > +* @unallocated_size: Estimate of memory remaining (-1 = unknown)
> > +*
> > +* Requires CAP_PERFMON or CAP_SYS_ADMIN to get reliable
> accounting.
> > +* Without this (or if this is an older kernel) the value here will
> > +* always equal the @probed_size. Note this is only currently tracked
> > +* for I915_MEMORY_CLASS_DEVICE regions (for other types the value
> here
> > +* will always equal the @probed_size).
> > +*/
> > +   __u64 unallocated_size;
> > +
> > +   union {
> > +   /** @rsvd1: MBZ */
> > +   __u64 rsvd1[8];
> > +   struct {
> > +   /**
> > +* @probed_cpu_visible_size: Memory probed by the
> driver
> > +* that is CPU accessible. (-1 = unknown).
> > +*
> > +* This will be always be <= @probed_size, and the
> > +* remainder (if there is any) will not be CPU
> > +* accessible.
> > +*
> > +* On systems without small BAR, the @probed_size will
> > +* always equal the @probed_cpu_visible_size, since all
> > +* of it will be CPU accessible.
> > +*
> > +* Note this is only tracked for
> > +* I915_MEMORY_CLASS_DEVICE regions (for other
> types the
> > +* value here will always equal the @probed_size).
> > +*
> > +* Note that if the value returned here is zero, then
> > +* this must be an old kernel which lacks the relevant
> > +*

[PATCH 1/1] drm/panfrost: Add support for devcoredump

2022-05-17 Thread Adrián Larumbe

In the event of a job timeout, debug dump information will be written into
/sys/class/devcoredump.

Inspired by etnaviv's similar feature.

Signed-off-by: Adrián Larumbe 
---
 drivers/gpu/drm/panfrost/Kconfig |   1 +
 drivers/gpu/drm/panfrost/Makefile|   3 +-
 drivers/gpu/drm/panfrost/panfrost_dump.c | 198 +++
 drivers/gpu/drm/panfrost/panfrost_dump.h |  12 ++
 drivers/gpu/drm/panfrost/panfrost_job.c  |   3 +
 include/uapi/drm/panfrost_drm.h  |  32 
 6 files changed, 248 insertions(+), 1 deletion(-)
 create mode 100644 drivers/gpu/drm/panfrost/panfrost_dump.c
 create mode 100644 drivers/gpu/drm/panfrost/panfrost_dump.h

diff --git a/drivers/gpu/drm/panfrost/Kconfig b/drivers/gpu/drm/panfrost/Kconfig
index 86cdc0ce79e6..079600328be1 100644
--- a/drivers/gpu/drm/panfrost/Kconfig
+++ b/drivers/gpu/drm/panfrost/Kconfig
@@ -11,6 +11,7 @@ config DRM_PANFROST
select DRM_GEM_SHMEM_HELPER
select PM_DEVFREQ
select DEVFREQ_GOV_SIMPLE_ONDEMAND
+   select WANT_DEV_COREDUMP
help
  DRM driver for ARM Mali Midgard (T6xx, T7xx, T8xx) and
  Bifrost (G3x, G5x, G7x) GPUs.
diff --git a/drivers/gpu/drm/panfrost/Makefile 
b/drivers/gpu/drm/panfrost/Makefile
index b71935862417..7da2b3f02ed9 100644
--- a/drivers/gpu/drm/panfrost/Makefile
+++ b/drivers/gpu/drm/panfrost/Makefile
@@ -9,6 +9,7 @@ panfrost-y := \
panfrost_gpu.o \
panfrost_job.o \
panfrost_mmu.o \
-   panfrost_perfcnt.o
+   panfrost_perfcnt.o \
+   panfrost_dump.o
 
 obj-$(CONFIG_DRM_PANFROST) += panfrost.o
diff --git a/drivers/gpu/drm/panfrost/panfrost_dump.c 
b/drivers/gpu/drm/panfrost/panfrost_dump.c
new file mode 100644
index ..a76dcf4acf6f
--- /dev/null
+++ b/drivers/gpu/drm/panfrost/panfrost_dump.c
@@ -0,0 +1,198 @@
+// SPDX-License-Identifier: GPL-2.0
+/* Copyright 2021 Collabora ltd. */
+
+#include 
+#include 
+#include 
+#include 
+#include 
+#include 
+#include 
+
+#include "panfrost_job.h"
+#include "panfrost_gem.h"
+#include "panfrost_regs.h"
+#include "panfrost_dump.h"
+#include "panfrost_device.h"
+
+static bool panfrost_dump_core = true;
+module_param_named(dump_core, panfrost_dump_core, bool, 0600);
+
+struct panfrost_dump_iterator {
+   void *start;
+   struct panfrost_dump_object_header *hdr;
+   void *data;
+};
+
+static const unsigned short panfrost_dump_registers[] = {
+   GPU_ID,
+   GPU_L2_FEATURES,
+   GPU_CORE_FEATURES,
+   GPU_TILER_FEATURES,
+   GPU_MEM_FEATURES,
+   GPU_MMU_FEATURES,
+   GPU_AS_PRESENT,
+   GPU_JS_PRESENT,
+   GPU_INT_RAWSTAT,
+   GPU_INT_CLEAR,
+   GPU_INT_MASK,
+   GPU_INT_STAT,
+};
+
+static void panfrost_core_dump_header(struct panfrost_dump_iterator *iter,
+   u32 type, void *data_end)
+{
+   struct panfrost_dump_object_header *hdr = iter->hdr;
+
+   hdr->magic = cpu_to_le32(PANFROSTDUMP_MAGIC);
+   hdr->type = cpu_to_le32(type);
+   hdr->file_offset = cpu_to_le32(iter->data - iter->start);
+   hdr->file_size = cpu_to_le32(data_end - iter->data);
+
+   iter->hdr++;
+   iter->data += le32_to_cpu(hdr->file_size);
+}
+
+static void
+panfrost_core_dump_registers(struct panfrost_dump_iterator *iter,
+struct panfrost_device *pfdev)
+{
+   struct panfrost_dump_registers *reg = iter->data;
+   unsigned int i;
+
+   for (i = 0; i < ARRAY_SIZE(panfrost_dump_registers); i++, reg++) {
+   reg->reg = cpu_to_le32(panfrost_dump_registers[i]);
+   reg->value = cpu_to_le32(gpu_read(pfdev, 
panfrost_dump_registers[i]));
+   }
+
+   panfrost_core_dump_header(iter, PANFROSTDUMP_BUF_REG, reg);
+}
+
+void panfrost_core_dump(struct panfrost_job *job)
+{
+   struct panfrost_device *pfdev = job->pfdev;
+   struct panfrost_dump_iterator iter;
+   struct drm_gem_object *dbo;
+   unsigned int n_obj, n_bomap_pages;
+   __le64 *bomap, *bomap_start;
+   size_t file_size;
+   int ret, i;
+
+   /* Only catch the first event, or when manually re-armed */
+   if (!panfrost_dump_core)
+   return;
+   panfrost_dump_core = false;
+
+   /* At least, we dump registers and end marker */
+   n_obj = 2;
+   n_bomap_pages = 0;
+   file_size = ARRAY_SIZE(panfrost_dump_registers) *
+   sizeof(struct panfrost_dump_registers);
+
+   /* Add in the active buffer objects */
+   for (i = 0; i < job->bo_count; i++) {
+   dbo = job->bos[i];
+   file_size += dbo->size;
+   n_bomap_pages += dbo->size >> PAGE_SHIFT;
+   n_obj++;
+   }
+
+   /* If we have any buffer objects, add a bomap object */
+   if (n_bomap_pages) {
+   file_size += n_bomap_pages * sizeof(__le64);
+   n_obj++;
+   }
+
+   /* Add the size of the headers */
+   file_size += sizeof(*iter.hdr) * n_obj;
+
+   /*

[PATCH 0/1] devcoredump support for Panfrost GPU driver

2022-05-17 Thread Adrián Larumbe

This patch adds support for devcoredump in the Panfrost driver. Code
structure is heavily inspired by similar functionality in the Etnaviv
driver, but the main goal of the crash dump is feeding it to a userspace
analyser that accesses the BO raw binary data to pass it to the pandecode
library.

Mesa MR under review can be found at:
https://gitlab.freedesktop.org/mesa/mesa/-/merge_requests/14034

Adrián Larumbe (1):
  drm/panfrost: Add support for devcoredump

 drivers/gpu/drm/panfrost/Kconfig |   1 +
 drivers/gpu/drm/panfrost/Makefile|   3 +-
 drivers/gpu/drm/panfrost/panfrost_dump.c | 198 +++
 drivers/gpu/drm/panfrost/panfrost_dump.h |  12 ++
 drivers/gpu/drm/panfrost/panfrost_job.c  |   3 +
 include/uapi/drm/panfrost_drm.h  |  32 
 6 files changed, 248 insertions(+), 1 deletion(-)
 create mode 100644 drivers/gpu/drm/panfrost/panfrost_dump.c
 create mode 100644 drivers/gpu/drm/panfrost/panfrost_dump.h

-- 
2.35.1

[PATCH v1 2/2] drm/msm/dp: delete vdda regulator related functions from eDP/DP controller

Vdda regulators are related to both eDP and DP phy so that it should be
managed at eDP and DP phy driver instead of controller. This patch remove
vdda regulators related functions out of eDP/DP controller.

Signed-off-by: Kuogee Hsieh 
---
 drivers/gpu/drm/msm/dp/dp_parser.c | 14 --
 drivers/gpu/drm/msm/dp/dp_parser.h |  6 ---
 drivers/gpu/drm/msm/dp/dp_power.c  | 95 +-
 3 files changed, 2 insertions(+), 113 deletions(-)

diff --git a/drivers/gpu/drm/msm/dp/dp_parser.c 
b/drivers/gpu/drm/msm/dp/dp_parser.c
index 8f9fed9..4ef2130 100644
--- a/drivers/gpu/drm/msm/dp/dp_parser.c
+++ b/drivers/gpu/drm/msm/dp/dp_parser.c
@@ -22,14 +22,6 @@
 #define DP_DEFAULT_P0_OFFSET   0x1000
 #define DP_DEFAULT_P0_SIZE 0x0400
 
-static const struct dp_regulator_cfg sdm845_dp_reg_cfg = {
-   .num = 2,
-   .regs = {
-   {"vdda-1p2", 21800, 4 },/* 1.2 V */
-   {"vdda-0p9", 36000, 32 },   /* 0.9 V */
-   },
-};
-
 static void __iomem *dp_ioremap(struct platform_device *pdev, int idx, size_t 
*len)
 {
struct resource *res;
@@ -298,12 +290,6 @@ static int dp_parser_parse(struct dp_parser *parser)
if (rc)
return rc;
 
-   /* Map the corresponding regulator information according to
-* version. Currently, since we only have one supported platform,
-* mapping the regulator directly.
-*/
-   parser->regulator_cfg = _dp_reg_cfg;
-
return 0;
 }
 
diff --git a/drivers/gpu/drm/msm/dp/dp_parser.h 
b/drivers/gpu/drm/msm/dp/dp_parser.h
index 3a4d797..b56b4d7 100644
--- a/drivers/gpu/drm/msm/dp/dp_parser.h
+++ b/drivers/gpu/drm/msm/dp/dp_parser.h
@@ -101,11 +101,6 @@ struct dp_reg_entry {
int disable_load;
 };
 
-struct dp_regulator_cfg {
-   int num;
-   struct dp_reg_entry regs[DP_DEV_REGULATOR_MAX];
-};
-
 /**
  * struct dp_parser - DP parser's data exposed to clients
  *
@@ -121,7 +116,6 @@ struct dp_parser {
struct dp_pinctrl pinctrl;
struct dp_io io;
struct dp_display_data disp_data;
-   const struct dp_regulator_cfg *regulator_cfg;
u32 max_dp_lanes;
struct drm_bridge *next_bridge;
 
diff --git a/drivers/gpu/drm/msm/dp/dp_power.c 
b/drivers/gpu/drm/msm/dp/dp_power.c
index d9e0117..b52ac1d 100644
--- a/drivers/gpu/drm/msm/dp/dp_power.c
+++ b/drivers/gpu/drm/msm/dp/dp_power.c
@@ -20,82 +20,10 @@ struct dp_power_private {
struct clk *link_clk_src;
struct clk *pixel_provider;
struct clk *link_provider;
-   struct regulator_bulk_data supplies[DP_DEV_REGULATOR_MAX];
 
struct dp_power dp_power;
 };
 
-static void dp_power_regulator_disable(struct dp_power_private *power)
-{
-   struct regulator_bulk_data *s = power->supplies;
-   const struct dp_reg_entry *regs = power->parser->regulator_cfg->regs;
-   int num = power->parser->regulator_cfg->num;
-   int i;
-
-   DBG("");
-   for (i = num - 1; i >= 0; i--)
-   if (regs[i].disable_load >= 0)
-   regulator_set_load(s[i].consumer,
-  regs[i].disable_load);
-
-   regulator_bulk_disable(num, s);
-}
-
-static int dp_power_regulator_enable(struct dp_power_private *power)
-{
-   struct regulator_bulk_data *s = power->supplies;
-   const struct dp_reg_entry *regs = power->parser->regulator_cfg->regs;
-   int num = power->parser->regulator_cfg->num;
-   int ret, i;
-
-   DBG("");
-   for (i = 0; i < num; i++) {
-   if (regs[i].enable_load >= 0) {
-   ret = regulator_set_load(s[i].consumer,
-regs[i].enable_load);
-   if (ret < 0) {
-   pr_err("regulator %d set op mode failed, %d\n",
-   i, ret);
-   goto fail;
-   }
-   }
-   }
-
-   ret = regulator_bulk_enable(num, s);
-   if (ret < 0) {
-   pr_err("regulator enable failed, %d\n", ret);
-   goto fail;
-   }
-
-   return 0;
-
-fail:
-   for (i--; i >= 0; i--)
-   regulator_set_load(s[i].consumer, regs[i].disable_load);
-   return ret;
-}
-
-static int dp_power_regulator_init(struct dp_power_private *power)
-{
-   struct regulator_bulk_data *s = power->supplies;
-   const struct dp_reg_entry *regs = power->parser->regulator_cfg->regs;
-   struct platform_device *pdev = power->pdev;
-   int num = power->parser->regulator_cfg->num;
-   int i, ret;
-
-   for (i = 0; i < num; i++)
-   s[i].supply = regs[i].name;
-
-   ret = devm_regulator_bulk_get(>dev, num, s);
-   if (ret < 0) {
-   pr_err("%s: failed to init regulator, ret=%d\n",
-   __func__, ret);
-   return ret;
-   }
-
-

[PATCH v1 1/2] phy/qcom: add regulator_set_load to edp/dp phy

This patch add regulator_set_load() to both eDP and DP phy driver
to have totally control regulators.

Signed-off-by: Kuogee Hsieh 
---
 drivers/phy/qualcomm/phy-qcom-edp.c | 25 +
 drivers/phy/qualcomm/phy-qcom-qmp.c | 24 
 2 files changed, 45 insertions(+), 4 deletions(-)

diff --git a/drivers/phy/qualcomm/phy-qcom-edp.c 
b/drivers/phy/qualcomm/phy-qcom-edp.c
index cacd32f..9b55095 100644
--- a/drivers/phy/qualcomm/phy-qcom-edp.c
+++ b/drivers/phy/qualcomm/phy-qcom-edp.c
@@ -87,17 +87,24 @@ struct qcom_edp {
 
struct clk_bulk_data clks[2];
struct regulator_bulk_data supplies[2];
+   int enable_load[2];
+   int disable_load[2];
 };
 
 static int qcom_edp_phy_init(struct phy *phy)
 {
struct qcom_edp *edp = phy_get_drvdata(phy);
int ret;
+   int num_consumers = ARRAY_SIZE(edp->supplies);
+   int i;
 
-   ret = regulator_bulk_enable(ARRAY_SIZE(edp->supplies), edp->supplies);
+   ret = regulator_bulk_enable(num_consumers, edp->supplies);
if (ret)
return ret;
 
+   for (i = num_consumers - 1; i >= 0; --i)
+   regulator_set_load(edp->supplies[i].consumer, 
edp->enable_load[i]);
+
ret = clk_bulk_prepare_enable(ARRAY_SIZE(edp->clks), edp->clks);
if (ret)
goto out_disable_supplies;
@@ -425,9 +432,15 @@ static int qcom_edp_phy_power_off(struct phy *phy)
 static int qcom_edp_phy_exit(struct phy *phy)
 {
struct qcom_edp *edp = phy_get_drvdata(phy);
+   int num_consumers = ARRAY_SIZE(edp->supplies);
+   int i;
 
clk_bulk_disable_unprepare(ARRAY_SIZE(edp->clks), edp->clks);
-   regulator_bulk_disable(ARRAY_SIZE(edp->supplies), edp->supplies);
+
+   for (i = num_consumers - 1; i >= 0; --i)
+   regulator_set_load(edp->supplies[i].consumer, 
edp->disable_load[i]);
+
+   regulator_bulk_disable(num_consumers, edp->supplies);
 
return 0;
 }
@@ -633,8 +646,12 @@ static int qcom_edp_phy_probe(struct platform_device *pdev)
if (ret)
return ret;
 
-   edp->supplies[0].supply = "vdda-phy";
-   edp->supplies[1].supply = "vdda-pll";
+   edp->supplies[0].supply = "vdda-1p2";
+   edp->supplies[1].supply = "vdda-0p9";
+   edp->enable_load[0] = 21800;/* 1.2 V */
+   edp->enable_load[1] = 36000;/* 1.2 V */
+   edp->disable_load[0] = 4;   /* 0.9 V */
+   edp->disable_load[1] = 4;   /* 10.9V */
ret = devm_regulator_bulk_get(dev, ARRAY_SIZE(edp->supplies), 
edp->supplies);
if (ret)
return ret;
diff --git a/drivers/phy/qualcomm/phy-qcom-qmp.c 
b/drivers/phy/qualcomm/phy-qcom-qmp.c
index b144ae1..c589231 100644
--- a/drivers/phy/qualcomm/phy-qcom-qmp.c
+++ b/drivers/phy/qualcomm/phy-qcom-qmp.c
@@ -3130,6 +3130,8 @@ struct qmp_phy_cfg {
int num_resets;
/* regulators to be requested */
const char * const *vreg_list;
+   const unsigned int *vreg_enable_load;
+   const unsigned int *vreg_disable_load;
int num_vregs;
 
/* array of registers with different offsets */
@@ -3346,6 +3348,14 @@ static const char * const qmp_phy_vreg_l[] = {
"vdda-phy", "vdda-pll",
 };
 
+static const unsigned int qmp_phy_vreg_enable_load[] = {
+   21800, 36000
+};
+
+static const unsigned int qmp_phy_vreg_disable_load[] = {
+   4, 32
+};
+
 static const struct qmp_phy_cfg ipq8074_usb3phy_cfg = {
.type   = PHY_TYPE_USB3,
.nlanes = 1,
@@ -4072,6 +4082,8 @@ static const struct qmp_phy_cfg sm8250_usb3phy_cfg = {
.reset_list = msm8996_usb3phy_reset_l,
.num_resets = ARRAY_SIZE(msm8996_usb3phy_reset_l),
.vreg_list  = qmp_phy_vreg_l,
+   .vreg_enable_load   = qmp_phy_vreg_enable_load,
+   .vreg_disable_load  = qmp_phy_vreg_disable_load,
.num_vregs  = ARRAY_SIZE(qmp_phy_vreg_l),
.regs   = qmp_v4_usb3phy_regs_layout,
 
@@ -4139,6 +4151,8 @@ static const struct qmp_phy_cfg sm8250_dpphy_cfg = {
.reset_list = msm8996_usb3phy_reset_l,
.num_resets = ARRAY_SIZE(msm8996_usb3phy_reset_l),
.vreg_list  = qmp_phy_vreg_l,
+   .vreg_enable_load   = qmp_phy_vreg_enable_load,
+   .vreg_disable_load  = qmp_phy_vreg_disable_load,
.num_vregs  = ARRAY_SIZE(qmp_phy_vreg_l),
.regs   = qmp_v4_usb3phy_regs_layout,
 
@@ -5015,6 +5029,11 @@ static int qcom_qmp_phy_com_init(struct qmp_phy *qphy)
goto err_reg_enable;
}
 
+   if (cfg->vreg_enable_load) {
+   for (i = cfg->num_vregs - 1; i >= 0; --i)
+   regulator_set_load(qmp->vregs[i].consumer, 
cfg->vreg_enable_load[i]);
+   }
+
for (i = 0; i < cfg->num_resets; i++) {
ret =

[PATCH v1 0/2] eDP/DP Phy vdda realted function

1) add regulator_set_load() to eDP/DP phy
2) remove vdda related function out of eDP/DP controller

Kuogee Hsieh (2):
  phy/qcom: add regulator_set_load to edp/dp phy
  drm/msm/dp: delete vdda regulator related functions from eDP/DP
controller

 drivers/gpu/drm/msm/dp/dp_parser.c  | 14 --
 drivers/gpu/drm/msm/dp/dp_parser.h  |  6 ---
 drivers/gpu/drm/msm/dp/dp_power.c   | 95 +
 drivers/phy/qualcomm/phy-qcom-edp.c | 25 --
 drivers/phy/qualcomm/phy-qcom-qmp.c | 24 ++
 5 files changed, 47 insertions(+), 117 deletions(-)

-- 
The Qualcomm Innovation Center, Inc. is a member of the Code Aurora Forum,
a Linux Foundation Collaborative Project

[PATCH] drm/i915: check fence to avoid null pointer dereference

2022-05-17 Thread Yongzhi Liu

if drm_syncobj_fence_get return null, we will get a
null pointer. Fix this by adding the null pointer check
on fence.

Signed-off-by: Yongzhi Liu 
---
 drivers/gpu/drm/i915/gem/i915_gem_execbuffer.c | 4 
 1 file changed, 4 insertions(+)

diff --git a/drivers/gpu/drm/i915/gem/i915_gem_execbuffer.c 
b/drivers/gpu/drm/i915/gem/i915_gem_execbuffer.c
index fd0e15d..3a82a62 100644
--- a/drivers/gpu/drm/i915/gem/i915_gem_execbuffer.c
+++ b/drivers/gpu/drm/i915/gem/i915_gem_execbuffer.c
@@ -3184,6 +3184,10 @@ eb_fences_add(struct i915_execbuffer *eb, struct 
i915_request *rq,
struct dma_fence *fence;
 
fence = drm_syncobj_fence_get(eb->gem_context->syncobj);
+   if (!fence) {
+   DRM_DEBUG("Syncobj has no fence\n");
+   return ERR_PTR(-EINVAL);
+   }
err = i915_request_await_dma_fence(rq, fence);
dma_fence_put(fence);
if (err)
-- 
2.7.4

[PATCH v7] drm/msm/dp: Always clear mask bits to disable interrupts at dp_ctrl_reset_irq_ctrl()

dp_catalog_ctrl_reset() will software reset DP controller. But it will
not reset programmable registers to default value. DP driver still have
to clear mask bits to interrupt status registers to disable interrupts
after software reset of controller.

At current implementation, dp_ctrl_reset_irq_ctrl() will software reset dp
controller but did not call dp_catalog_ctrl_enable_irq(false) to clear hpd
related interrupt mask bits to disable hpd related interrupts due to it
mistakenly think hpd related interrupt mask bits will be cleared by software
reset of dp controller automatically. This mistake may cause system to crash
during suspending procedure due to unexpected irq fired and trigger event
thread to access dp controller registers with controller clocks are disabled.

This patch fixes system crash during suspending problem by removing "enable"
flag condition checking at dp_ctrl_reset_irq_ctrl() so that hpd related
interrupt mask bits are cleared to prevent unexpected from happening.

Changes in v2:
-- add more details commit text

Changes in v3:
-- add synchrons_irq()
-- add atomic_t suspended

Changes in v4:
-- correct Fixes's commit ID
-- remove synchrons_irq()

Changes in v5:
-- revise commit text

Changes in v6:
-- add event_lock to protect "suspended"

Changes in v7:
-- delete "suspended" flag

Fixes: 989ebe7bc446 ("drm/msm/dp: do not initialize phy until plugin interrupt 
received")
Signed-off-by: Kuogee Hsieh 
---
 drivers/gpu/drm/msm/dp/dp_ctrl.c | 9 +++--
 1 file changed, 7 insertions(+), 2 deletions(-)

diff --git a/drivers/gpu/drm/msm/dp/dp_ctrl.c b/drivers/gpu/drm/msm/dp/dp_ctrl.c
index 5356856..5ddb4e8 100644
--- a/drivers/gpu/drm/msm/dp/dp_ctrl.c
+++ b/drivers/gpu/drm/msm/dp/dp_ctrl.c
@@ -1380,8 +1380,13 @@ void dp_ctrl_reset_irq_ctrl(struct dp_ctrl *dp_ctrl, 
bool enable)
 
dp_catalog_ctrl_reset(ctrl->catalog);
 
-   if (enable)
-   dp_catalog_ctrl_enable_irq(ctrl->catalog, enable);
+   /*
+* all dp controller programmable registers will not
+* be reset to default value after DP_SW_RESET
+* therefore interrupt mask bits have to be updated
+* to enable/disable interrupts
+*/
+   dp_catalog_ctrl_enable_irq(ctrl->catalog, enable);
 }
 
 void dp_ctrl_phy_init(struct dp_ctrl *dp_ctrl)
-- 
The Qualcomm Innovation Center, Inc. is a member of the Code Aurora Forum,
a Linux Foundation Collaborative Project

Re: [PATCH v2 4/4] drm/bridge: anx7625: Use DPI bus type

2022-05-17 Thread Robert Foss

On Mon, 25 Apr 2022 at 11:14, Xin Ji  wrote:
>
> On Mon, Apr 25, 2022 at 04:24:50PM +0800, Chen-Yu Tsai wrote:
> > On Fri, Apr 22, 2022 at 10:13 PM Robert Foss  wrote:
> > >
> > > On Fri, 22 Apr 2022 at 16:01, Robert Foss  wrote:
> > > >
> > > > On Fri, 22 Apr 2022 at 10:49, Xin Ji  wrote:
> > > > >
> > > > > As V4L2_FWNODE_BUS_TYPE_PARALLEL not properly descript for DPI
> > > > > interface, this patch use new defined V4L2_FWNODE_BUS_TYPE_DPI for it.
> > > > >
> > > > > Fixes: fd0310b6fe7d ("drm/bridge: anx7625: add MIPI DPI input 
> > > > > feature")
> > > > > Signed-off-by: Xin Ji 
> > > > > ---
> > > > >  drivers/gpu/drm/bridge/analogix/anx7625.c | 8 
> > > > >  1 file changed, 4 insertions(+), 4 deletions(-)
> > > > >
> > > > > diff --git a/drivers/gpu/drm/bridge/analogix/anx7625.c 
> > > > > b/drivers/gpu/drm/bridge/analogix/anx7625.c
> > > > > index 376da01243a3..71df977e8f53 100644
> > > > > --- a/drivers/gpu/drm/bridge/analogix/anx7625.c
> > > > > +++ b/drivers/gpu/drm/bridge/analogix/anx7625.c
> > > > > @@ -1623,14 +1623,14 @@ static int anx7625_parse_dt(struct device 
> > > > > *dev,
> > > > >
> > > > > anx7625_get_swing_setting(dev, pdata);
> > > > >
> > > > > -   pdata->is_dpi = 1; /* default dpi mode */
> > > > > +   pdata->is_dpi = 0; /* default dsi mode */
> > > > > pdata->mipi_host_node = of_graph_get_remote_node(np, 0, 0);
> > > > > if (!pdata->mipi_host_node) {
> > > > > DRM_DEV_ERROR(dev, "fail to get internal panel.\n");
> > > > > return -ENODEV;
> > > > > }
> > > > >
> > > > > -   bus_type = V4L2_FWNODE_BUS_TYPE_PARALLEL;
> > > > > +   bus_type = 0;
> > > > > mipi_lanes = MAX_LANES_SUPPORT;
> > > > > ep0 = of_graph_get_endpoint_by_regs(np, 0, 0);
> > > > > if (ep0) {
> > > > > @@ -1640,8 +1640,8 @@ static int anx7625_parse_dt(struct device *dev,
> > > > > mipi_lanes = of_property_count_u32_elems(ep0, 
> > > > > "data-lanes");
> > > > > }
> > > > >
> > > > > -   if (bus_type == V4L2_FWNODE_BUS_TYPE_PARALLEL) /* bus type is 
> > > > > Parallel(DSI) */
> > > > > -   pdata->is_dpi = 0;
> > > > > +   if (bus_type == V4L2_FWNODE_BUS_TYPE_DPI) /* bus type is DPI 
> > > > > */
> > > > > +   pdata->is_dpi = 1;
> > > > >
> > > > > pdata->mipi_lanes = mipi_lanes;
> > > > > if (pdata->mipi_lanes > MAX_LANES_SUPPORT || 
> > > > > pdata->mipi_lanes <= 0)
> > > >
> > > > Reviewed-by: Robert Foss 
> > >
> > > Acked-by: Robert Foss 
> >
> > Tested-by: Chen-Yu Tsai 
> >
> > Confirmed this fixes the display on Juniper (Acer Chromebook Spin 311) on
> > mainline (next-20220422).
> >
> > Xin, in the future, please send the whole series to all recipients of
> > all patches listed by get_maintainers.pl, not just the recipients of
> > each patch. In the case of this series, they should have been sent
> > to all of the mailing lists (media, devicetree, dri-devel) so that
> > everyone has the same, full view of the patches.
> Hi ChenYu, OK, I'll send to all media, devicetree, dri-devel next time.
> Thanks,
> Xin
> >
> > ChenYu

Applied 3/4 + 4/4 to drm-misc-next.

Re: [PATCH v6] drm/msm/dp: Always clear mask bits to disable interrupts at dp_ctrl_reset_irq_ctrl()




On 5/17/2022 1:25 AM, Stephen Boyd wrote:

Quoting Kuogee Hsieh (2022-05-12 12:43:18)

diff --git a/drivers/gpu/drm/msm/dp/dp_ctrl.c b/drivers/gpu/drm/msm/dp/dp_ctrl.c
index af7a80c..f3e333e 100644
--- a/drivers/gpu/drm/msm/dp/dp_ctrl.c
+++ b/drivers/gpu/drm/msm/dp/dp_ctrl.c
@@ -1389,8 +1389,13 @@ void dp_ctrl_reset_irq_ctrl(struct dp_ctrl *dp_ctrl, 
bool enable)

 dp_catalog_ctrl_reset(ctrl->catalog);

-   if (enable)
-   dp_catalog_ctrl_enable_irq(ctrl->catalog, enable);
+   /*
+* all dp controller programmable registers will not
+* be reset to default value after DP_SW_RESET
+* therefore interrupt mask bits have to be updated
+* to enable/disable interrupts
+*/
+   dp_catalog_ctrl_enable_irq(ctrl->catalog, enable);

I think Dmitry requested that this part be split off to a different
patch. It's fixing the call to dp_ctrl_reset_irq_ctrl() for the disable
case.

ok, will do that

  }

  void dp_ctrl_phy_init(struct dp_ctrl *dp_ctrl)
diff --git a/drivers/gpu/drm/msm/dp/dp_display.c 
b/drivers/gpu/drm/msm/dp/dp_display.c
index c388323..ab691aa 100644
--- a/drivers/gpu/drm/msm/dp/dp_display.c
+++ b/drivers/gpu/drm/msm/dp/dp_display.c
@@ -98,6 +98,8 @@ struct dp_display_private {
 struct dp_ctrl*ctrl;
 struct dp_debug   *debug;

+   bool suspended;
+

Can we not have this flag? I also don't understand how this patch waits
for the event queue to drain. There are now multiple places we check to
see if we've suspended, but is it even possible to be in those places
during suspend? What is wrong with moving to an irq thread? Is it
because we want to wait for a modeset (EV_USER_NOTIFICATION)?


dp_pm_suspend happen after display off. therefore event q should be 
emptied when dp_pm_suspend called.


since dp_pm_suspend and event_thread share an mutex, if suspended flag 
set at dp_pm_suspend which guarantee


event thread will not be wakeed up to process since event q is empty.

We used event q is to guarantee all events (irqs) are kept in event q 
with timing order (no any events missed) and executed later in timing 
order too to work the scenario likes dongle plug/unplugged several times 
consecutively.


We used to workqueue but cause many synchronization issues.

I will try thread_irq to see it will yield any advantage over kernel thread.


 struct dp_usbpd_cb usbpd_cb;
 struct dp_display_mode dp_mode;
 struct msm_dp dp_display;

Re: [PATCH v2 2/3] ARM: dts: aspeed: Add USB2.0 device controller node

On 17/05/2022 17:21, Neal Liu wrote:
>
> + udc: udc@1e6a2000 {

 The same as DTS in bindings - generic node name, please.

>>>
>>> Is it possible to use "udc: usb-udc@1e6a2000" to distinguish it between 
>>> "vhub:
>> usb-vhub@1e6a"?
>>
>> Possible yes :), but not recommended and not wanted. Nodes should be generic
>> and prefixes are added only if there is no unit address. You can though use
>> some more descriptive label.
>>
> "udc: usb@1e6a2000" is okay for you?

Yes, it's perfect.


Best regards,
Krzysztof

[PATCH 14/14] drm/radeon: Register ACPI video backlight when skipping radeon backlight registration

Typically the acpi_video driver will initialize before radeon, which
used to cause /sys/class/backlight/acpi_video0 to get registered and then
radeon would register its own radeon_bl# device later. After which
the drivers/acpi/video_detect.c code unregistered the acpi_video0 device
to avoid there being 2 backlight devices.

This means that userspace used to briefly see 2 devices and the
disappearing of acpi_video0 after a brief time confuses the systemd
backlight level save/restore code, see e.g.:
https://bbs.archlinux.org/viewtopic.php?id=269920

To fix this the ACPI video code has been modified to make backlight class
device registration a separate step, relying on the drm/kms driver to
ask for the acpi_video backlight registration after it is done setting up
its native backlight device.

Add a call to the new acpi_video_register_backlight() when radeon skips
registering its own backlight device because of e.g. the firmware_flags
or the acpi_video_get_backlight_type() return value. This ensures that
if the acpi_video backlight device should be used, it will be available
before the radeon drm_device gets registered with userspace.

Signed-off-by: Hans de Goede 
---
 drivers/gpu/drm/radeon/radeon_encoders.c | 11 ++-
 1 file changed, 10 insertions(+), 1 deletion(-)

diff --git a/drivers/gpu/drm/radeon/radeon_encoders.c 
b/drivers/gpu/drm/radeon/radeon_encoders.c
index 46549d5179ee..c1cbebb51be1 100644
--- a/drivers/gpu/drm/radeon/radeon_encoders.c
+++ b/drivers/gpu/drm/radeon/radeon_encoders.c
@@ -30,6 +30,8 @@
 #include 
 #include 
 
+#include 
+
 #include "radeon.h"
 #include "radeon_atombios.h"
 #include "radeon_legacy_encoders.h"
@@ -167,7 +169,7 @@ static void radeon_encoder_add_backlight(struct 
radeon_encoder *radeon_encoder,
return;
 
if (radeon_backlight == 0) {
-   return;
+   use_bl = false;
} else if (radeon_backlight == 1) {
use_bl = true;
} else if (radeon_backlight == -1) {
@@ -193,6 +195,13 @@ static void radeon_encoder_add_backlight(struct 
radeon_encoder *radeon_encoder,
else
radeon_legacy_backlight_init(radeon_encoder, connector);
}
+
+   /*
+* If there is no native backlight device (which may happen even when
+* use_bl==true) try registering an ACPI video backlight device instead.
+*/
+   if (!rdev->mode_info.bl_encoder)
+   acpi_video_register_backlight();
 }
 
 void
-- 
2.36.0

[PATCH 13/14] drm/amdgpu: Register ACPI video backlight when skipping amdgpu backlight registration

Typically the acpi_video driver will initialize before amdgpu, which
used to cause /sys/class/backlight/acpi_video0 to get registered and then
amdgpu would register its own amdgpu_bl# device later. After which
the drivers/acpi/video_detect.c code unregistered the acpi_video0 device
to avoid there being 2 backlight devices.

This means that userspace used to briefly see 2 devices and the
disappearing of acpi_video0 after a brief time confuses the systemd
backlight level save/restore code, see e.g.:
https://bbs.archlinux.org/viewtopic.php?id=269920

To fix this the ACPI video code has been modified to make backlight class
device registration a separate step, relying on the drm/kms driver to
ask for the acpi_video backlight registration after it is done setting up
its native backlight device.

Add a call to the new acpi_video_register_backlight() when amdgpu skips
registering its own backlight device because of either the firmware_flags
or the acpi_video_get_backlight_type() return value. This ensures that
if the acpi_video backlight device should be used, it will be available
before the amdgpu drm_device gets registered with userspace.

Signed-off-by: Hans de Goede 
---
 drivers/gpu/drm/amd/amdgpu/atombios_encoders.c| 9 +++--
 drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c | 2 ++
 2 files changed, 9 insertions(+), 2 deletions(-)

diff --git a/drivers/gpu/drm/amd/amdgpu/atombios_encoders.c 
b/drivers/gpu/drm/amd/amdgpu/atombios_encoders.c
index f9c62cd84a18..26f638ab7a5f 100644
--- a/drivers/gpu/drm/amd/amdgpu/atombios_encoders.c
+++ b/drivers/gpu/drm/amd/amdgpu/atombios_encoders.c
@@ -186,11 +186,11 @@ void amdgpu_atombios_encoder_init_backlight(struct 
amdgpu_encoder *amdgpu_encode
return;
 
if (!(adev->mode_info.firmware_flags & 
ATOM_BIOS_INFO_BL_CONTROLLED_BY_GPU))
-   return;
+   goto register_acpi_backlight;
 
if (acpi_video_get_backlight_type(true) != acpi_backlight_native) {
DRM_INFO("Skipping amdgpu atom DIG backlight registration\n");
-   return;
+   goto register_acpi_backlight;
}
 
pdata = kmalloc(sizeof(struct amdgpu_backlight_privdata), GFP_KERNEL);
@@ -227,6 +227,11 @@ void amdgpu_atombios_encoder_init_backlight(struct 
amdgpu_encoder *amdgpu_encode
 error:
kfree(pdata);
return;
+
+register_acpi_backlight:
+   /* Try registering an ACPI video backlight device instead. */
+   acpi_video_register_backlight();
+   return;
 }
 
 void
diff --git a/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c 
b/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c
index a838c7b5d942..06baa4f27680 100644
--- a/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c
+++ b/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c
@@ -4083,6 +4083,8 @@ amdgpu_dm_register_backlight_device(struct 
amdgpu_display_manager *dm)
 
if (acpi_video_get_backlight_type(true) != acpi_backlight_native) {
DRM_INFO("Skipping amdgpu DM backlight registration\n");
+   /* Try registering an ACPI video backlight device instead. */
+   acpi_video_register_backlight();
return;
}
 
-- 
2.36.0

[PATCH 12/14] drm/nouveau: Register ACPI video backlight when nv_backlight registration fails

Typically the acpi_video driver will initialize before nouveau, which
used to cause /sys/class/backlight/acpi_video0 to get registered and then
nouveau would register its own nv_backlight device later. After which
the drivers/acpi/video_detect.c code unregistered the acpi_video0 device
to avoid there being 2 backlight devices.

This means that userspace used to briefly see 2 devices and the
disappearing of acpi_video0 after a brief time confuses the systemd
backlight level save/restore code, see e.g.:
https://bbs.archlinux.org/viewtopic.php?id=269920

To fix this the ACPI video code has been modified to make backlight class
device registration a separate step, relying on the drm/kms driver to
ask for the acpi_video backlight registration after it is done setting up
its native backlight device.

Add a call to the new acpi_video_register_backlight() when native backlight
device registration has failed / was skipped to ensure that there is a
backlight device available before the drm_device gets registered with
userspace.

Signed-off-by: Hans de Goede 
---
 drivers/gpu/drm/nouveau/nouveau_backlight.c | 7 +++
 1 file changed, 7 insertions(+)

diff --git a/drivers/gpu/drm/nouveau/nouveau_backlight.c 
b/drivers/gpu/drm/nouveau/nouveau_backlight.c
index f56ff797c78c..0ae8793357a4 100644
--- a/drivers/gpu/drm/nouveau/nouveau_backlight.c
+++ b/drivers/gpu/drm/nouveau/nouveau_backlight.c
@@ -436,6 +436,13 @@ nouveau_backlight_init(struct drm_connector *connector)
 
 fail_alloc:
kfree(bl);
+   /*
+* If we get here we have an internal panel, but no nv_backlight,
+* try registering an ACPI video backlight device instead.
+*/
+   if (ret == 0)
+   acpi_video_register_backlight();
+
return ret;
 }
 
-- 
2.36.0

[PATCH 10/14] ACPI: video: Remove code to unregister acpi_video backlight when a native backlight registers

Remove the code to unregister acpi_video backlight devices when
a native backlight device gets registered later.

Now that the acpi_video backlight device registration is a separate step
which runs later, after the drm/kms driver is done setting up its own
native backlight device, it is no longer necessary to monitor for a
native (BACKLIGHT_RAW) device showing up later and to then unregister
the acpi_video backlight device(s).

Signed-off-by: Hans de Goede 
---
 drivers/acpi/acpi_video.c   |  2 --
 drivers/acpi/video_detect.c | 36 
 2 files changed, 38 deletions(-)

diff --git a/drivers/acpi/acpi_video.c b/drivers/acpi/acpi_video.c
index 79e75dc86243..20a2638f0ef1 100644
--- a/drivers/acpi/acpi_video.c
+++ b/drivers/acpi/acpi_video.c
@@ -89,7 +89,6 @@ static void acpi_video_bus_notify(struct acpi_device *device, 
u32 event);
 static void acpi_video_bus_register_backlight_work(struct work_struct 
*ignored);
 static DECLARE_DELAYED_WORK(video_bus_register_backlight_work,
acpi_video_bus_register_backlight_work);
-void acpi_video_detect_exit(void);
 
 /*
  * Indices in the _BCL method response: the first two items are special,
@@ -2345,7 +2344,6 @@ static int __init acpi_video_init(void)
 
 static void __exit acpi_video_exit(void)
 {
-   acpi_video_detect_exit();
acpi_video_unregister();
 }
 
diff --git a/drivers/acpi/video_detect.c b/drivers/acpi/video_detect.c
index 6caabdf189c9..3fa79584981e 100644
--- a/drivers/acpi/video_detect.c
+++ b/drivers/acpi/video_detect.c
@@ -39,10 +39,6 @@
 
 void acpi_video_unregister_backlight(void);
 
-static bool backlight_notifier_registered;
-static struct notifier_block backlight_nb;
-static struct work_struct backlight_notify_work;
-
 static enum acpi_backlight_type acpi_backlight_cmdline = acpi_backlight_undef;
 static enum acpi_backlight_type acpi_backlight_dmi = acpi_backlight_undef;
 
@@ -516,26 +512,6 @@ static const struct dmi_system_id video_detect_dmi_table[] 
= {
{ },
 };
 
-/* This uses a workqueue to avoid various locking ordering issues */
-static void acpi_video_backlight_notify_work(struct work_struct *work)
-{
-   if (acpi_video_get_backlight_type(false) != acpi_backlight_video)
-   acpi_video_unregister_backlight();
-}
-
-static int acpi_video_backlight_notify(struct notifier_block *nb,
-  unsigned long val, void *bd)
-{
-   struct backlight_device *backlight = bd;
-
-   /* A raw bl registering may change video -> native */
-   if (backlight->props.type == BACKLIGHT_RAW &&
-   val == BACKLIGHT_REGISTERED)
-   schedule_work(_notify_work);
-
-   return NOTIFY_OK;
-}
-
 /*
  * Determine which type of backlight interface to use on this system,
  * First check cmdline, then dmi quirks, then do autodetect.
@@ -565,12 +541,6 @@ enum acpi_backlight_type 
acpi_video_get_backlight_type(bool native)
acpi_walk_namespace(ACPI_TYPE_DEVICE, ACPI_ROOT_OBJECT,
ACPI_UINT32_MAX, find_video, NULL,
_caps, NULL);
-   INIT_WORK(_notify_work,
- acpi_video_backlight_notify_work);
-   backlight_nb.notifier_call = acpi_video_backlight_notify;
-   backlight_nb.priority = 0;
-   if (backlight_register_notifier(_nb) == 0)
-   backlight_notifier_registered = true;
init_done = true;
}
if (native)
@@ -606,9 +576,3 @@ void acpi_video_set_dmi_backlight_type(enum 
acpi_backlight_type type)
acpi_video_unregister_backlight();
 }
 EXPORT_SYMBOL(acpi_video_set_dmi_backlight_type);
-
-void __exit acpi_video_detect_exit(void)
-{
-   if (backlight_notifier_registered)
-   backlight_unregister_notifier(_nb);
-}
-- 
2.36.0

[PATCH 11/14] drm/i915: Call acpi_video_register_backlight()

On machins without an i915 opregion the acpi_video driver immediately
probes the ACPI video bus and used to also immediately register
acpi_video# backlight devices when supported.

Once the drm/kms driver then loaded later and possibly registered
a native backlight device then the drivers/acpi/video_detect.c code
unregistered the acpi_video0 device to avoid there being 2 backlight
devices (when acpi_video_get_backlight_type()==native).

This means that userspace used to briefly see 2 devices and the
disappearing of acpi_video0 after a brief time confuses the systemd
backlight level save/restore code, see e.g.:
https://bbs.archlinux.org/viewtopic.php?id=269920

To fix this the ACPI video code has been modified to make backlight class
device registration a separate step, relying on the drm/kms driver to
ask for the acpi_video backlight registration after it is done setting up
its native backlight device.

Add a call to the new acpi_video_register_backlight() after the i915 calls
acpi_video_register() (after setting up the i915 opregion) so that the
acpi_video backlight devices get registered on systems where the i915
native backlight device is not registered.

Signed-off-by: Hans de Goede 
---
 drivers/gpu/drm/i915/display/intel_display.c | 1 +
 1 file changed, 1 insertion(+)

diff --git a/drivers/gpu/drm/i915/display/intel_display.c 
b/drivers/gpu/drm/i915/display/intel_display.c
index 85fbf59e0f58..500659c51e8d 100644
--- a/drivers/gpu/drm/i915/display/intel_display.c
+++ b/drivers/gpu/drm/i915/display/intel_display.c
@@ -10672,6 +10672,7 @@ void intel_display_driver_register(struct 
drm_i915_private *i915)
/* Must be done after probing outputs */
intel_opregion_register(i915);
acpi_video_register();
+   acpi_video_register_backlight();
 
intel_audio_init(i915);
 
-- 
2.36.0

[PATCH 09/14] ACPI: video: Make backlight class device registration a separate step

On x86/ACPI boards the acpi_video driver will usually initializing before
the kms driver (except i915). This causes /sys/class/backlight/acpi_video0
to show up and then the kms driver registers its own native backlight
device after which the drivers/acpi/video_detect.c code unregisters
the acpi_video0 device (when acpi_video_get_backlight_type()==native).

This means that userspace briefly sees 2 devices and the disappearing of
acpi_video0 after a brief time confuses the systemd backlight level
save/restore code, see e.g.:
https://bbs.archlinux.org/viewtopic.php?id=269920

To fix this make backlight class device registration a separate step
done by a new acpi_video_register_backlight() function. The intend is for
this to be called by the drm/kms driver *after* it is done setting up its
own native backlight device. So that acpi_video_get_backlight_type() knows
if a native backlight will be available or not at acpi_video backlight
registration time, avoiding the add + remove dance.

Note the new acpi_video_register_backlight() function is also called from
a delayed work to ensure that the acpi_video backlight devices does get
registered if necessary even if there is no drm/kms driver or when it is
disabled.

Signed-off-by: Hans de Goede 
---
 drivers/acpi/acpi_video.c | 45 ---
 include/acpi/video.h  |  2 ++
 2 files changed, 44 insertions(+), 3 deletions(-)

diff --git a/drivers/acpi/acpi_video.c b/drivers/acpi/acpi_video.c
index 95d4868f6a8c..79e75dc86243 100644
--- a/drivers/acpi/acpi_video.c
+++ b/drivers/acpi/acpi_video.c
@@ -31,6 +31,12 @@
 #define ACPI_VIDEO_BUS_NAME"Video Bus"
 #define ACPI_VIDEO_DEVICE_NAME "Video Device"
 
+/*
+ * Display probing is known to take up to 5 seconds, so delay the fallback
+ * backlight registration by 5 seconds + 3 seconds for some extra margin.
+ */
+#define ACPI_VIDEO_REGISTER_BACKLIGHT_DELAY(8 * HZ)
+
 #define MAX_NAME_LEN   20
 
 MODULE_AUTHOR("Bruno Ducrot");
@@ -80,6 +86,9 @@ static LIST_HEAD(video_bus_head);
 static int acpi_video_bus_add(struct acpi_device *device);
 static int acpi_video_bus_remove(struct acpi_device *device);
 static void acpi_video_bus_notify(struct acpi_device *device, u32 event);
+static void acpi_video_bus_register_backlight_work(struct work_struct 
*ignored);
+static DECLARE_DELAYED_WORK(video_bus_register_backlight_work,
+   acpi_video_bus_register_backlight_work);
 void acpi_video_detect_exit(void);
 
 /*
@@ -1862,8 +1871,6 @@ static int acpi_video_bus_register_backlight(struct 
acpi_video_bus *video)
if (video->backlight_registered)
return 0;
 
-   acpi_video_run_bcl_for_osi(video);
-
if (acpi_video_get_backlight_type(false) != acpi_backlight_video)
return 0;
 
@@ -2089,7 +2096,11 @@ static int acpi_video_bus_add(struct acpi_device *device)
list_add_tail(>entry, _bus_head);
mutex_unlock(_list_lock);
 
-   acpi_video_bus_register_backlight(video);
+   /*
+* The userspace visible backlight_device gets registered separately
+* from acpi_video_register_backlight().
+*/
+   acpi_video_run_bcl_for_osi(video);
acpi_video_bus_add_notify_handler(video);
 
return 0;
@@ -2128,6 +2139,11 @@ static int acpi_video_bus_remove(struct acpi_device 
*device)
return 0;
 }
 
+static void acpi_video_bus_register_backlight_work(struct work_struct *ignored)
+{
+   acpi_video_register_backlight();
+}
+
 static int __init is_i740(struct pci_dev *dev)
 {
if (dev->device == 0x00D1)
@@ -2238,6 +2254,17 @@ int acpi_video_register(void)
 */
register_count = 1;
 
+   /*
+* acpi_video_bus_add() skips registering the userspace visible
+* backlight_device. The intend is for this to be registered by the
+* drm/kms driver calling acpi_video_register_backlight() *after* it is
+* done setting up its own native backlight device. The delayed work
+* ensures that acpi_video_register_backlight() always gets called
+* eventually, in case there is no drm/kms driver or it is disabled.
+*/
+   schedule_delayed_work(_bus_register_backlight_work,
+ ACPI_VIDEO_REGISTER_BACKLIGHT_DELAY);
+
 leave:
mutex_unlock(_count_mutex);
return ret;
@@ -2248,6 +2275,7 @@ void acpi_video_unregister(void)
 {
mutex_lock(_count_mutex);
if (register_count) {
+   cancel_delayed_work_sync(_bus_register_backlight_work);
acpi_bus_unregister_driver(_video_bus);
register_count = 0;
}
@@ -2255,6 +2283,17 @@ void acpi_video_unregister(void)
 }
 EXPORT_SYMBOL(acpi_video_unregister);
 
+void acpi_video_register_backlight(void)
+{
+   struct acpi_video_bus *video;
+
+   mutex_lock(_list_lock);
+   list_for_each_entry(video, _bus_head, entry)
+

[PATCH 08/14] ACPI: video: Simplify acpi_video_unregister_backlight()

When acpi_video_register() has not run yet the video_bus_head will be
empty, so there is no need to check the register_count flag first.

Signed-off-by: Hans de Goede 
---
 drivers/acpi/acpi_video.c | 12 
 1 file changed, 4 insertions(+), 8 deletions(-)

diff --git a/drivers/acpi/acpi_video.c b/drivers/acpi/acpi_video.c
index 7f48352840bb..95d4868f6a8c 100644
--- a/drivers/acpi/acpi_video.c
+++ b/drivers/acpi/acpi_video.c
@@ -2259,14 +2259,10 @@ void acpi_video_unregister_backlight(void)
 {
struct acpi_video_bus *video;
 
-   mutex_lock(_count_mutex);
-   if (register_count) {
-   mutex_lock(_list_lock);
-   list_for_each_entry(video, _bus_head, entry)
-   acpi_video_bus_unregister_backlight(video);
-   mutex_unlock(_list_lock);
-   }
-   mutex_unlock(_count_mutex);
+   mutex_lock(_list_lock);
+   list_for_each_entry(video, _bus_head, entry)
+   acpi_video_bus_unregister_backlight(video);
+   mutex_unlock(_list_lock);
 }
 
 bool acpi_video_handles_brightness_key_presses(void)
-- 
2.36.0

[PATCH 07/14] ACPI: video: Remove acpi_video_bus from list before tearing it down

Move the list_del removing an acpi_video_bus from video_bus_head
on teardown to before the teardown is done, to avoid code iterating
over the video_bus_head list seeing acpi_video_bus objects on there
which are (partly) torn down already.

Signed-off-by: Hans de Goede 
---
 drivers/acpi/acpi_video.c | 8 
 1 file changed, 4 insertions(+), 4 deletions(-)

diff --git a/drivers/acpi/acpi_video.c b/drivers/acpi/acpi_video.c
index cebef3403620..7f48352840bb 100644
--- a/drivers/acpi/acpi_video.c
+++ b/drivers/acpi/acpi_video.c
@@ -2114,14 +2114,14 @@ static int acpi_video_bus_remove(struct acpi_device 
*device)
 
video = acpi_driver_data(device);
 
-   acpi_video_bus_remove_notify_handler(video);
-   acpi_video_bus_unregister_backlight(video);
-   acpi_video_bus_put_devices(video);
-
mutex_lock(_list_lock);
list_del(>entry);
mutex_unlock(_list_lock);
 
+   acpi_video_bus_remove_notify_handler(video);
+   acpi_video_bus_unregister_backlight(video);
+   acpi_video_bus_put_devices(video);
+
kfree(video->attached_array);
kfree(video);
 
-- 
2.36.0

[PATCH 03/14] drm/amdgpu: Don't register backlight when another backlight should be used

Before this commit when we want userspace to use the acpi_video backlight
device we register both the GPU's native backlight device and acpi_video's
firmware acpi_video# backlight device. This relies on userspace preferring
firmware type backlight devices over native ones.

Registering 2 backlight devices for a single display really is
undesirable, don't register the GPU's native backlight device when
another backlight device should be used.

Signed-off-by: Hans de Goede 
---
 drivers/gpu/drm/Kconfig   | 1 +
 drivers/gpu/drm/amd/amdgpu/atombios_encoders.c| 7 +++
 drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c | 7 +++
 3 files changed, 15 insertions(+)

diff --git a/drivers/gpu/drm/Kconfig b/drivers/gpu/drm/Kconfig
index f1422bee3dcc..ddbeb2124df7 100644
--- a/drivers/gpu/drm/Kconfig
+++ b/drivers/gpu/drm/Kconfig
@@ -280,6 +280,7 @@ config DRM_AMDGPU
select HWMON
select BACKLIGHT_CLASS_DEVICE
select INTERVAL_TREE
+   select ACPI_VIDEO if ACPI && X86 && INPUT
help
  Choose this option if you have a recent AMD Radeon graphics card.
 
diff --git a/drivers/gpu/drm/amd/amdgpu/atombios_encoders.c 
b/drivers/gpu/drm/amd/amdgpu/atombios_encoders.c
index a92d86e12718..f9c62cd84a18 100644
--- a/drivers/gpu/drm/amd/amdgpu/atombios_encoders.c
+++ b/drivers/gpu/drm/amd/amdgpu/atombios_encoders.c
@@ -26,6 +26,8 @@
 
 #include 
 
+#include 
+
 #include 
 #include 
 #include "amdgpu.h"
@@ -186,6 +188,11 @@ void amdgpu_atombios_encoder_init_backlight(struct 
amdgpu_encoder *amdgpu_encode
if (!(adev->mode_info.firmware_flags & 
ATOM_BIOS_INFO_BL_CONTROLLED_BY_GPU))
return;
 
+   if (acpi_video_get_backlight_type(true) != acpi_backlight_native) {
+   DRM_INFO("Skipping amdgpu atom DIG backlight registration\n");
+   return;
+   }
+
pdata = kmalloc(sizeof(struct amdgpu_backlight_privdata), GFP_KERNEL);
if (!pdata) {
DRM_ERROR("Memory allocation failed\n");
diff --git a/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c 
b/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c
index 62139ff35476..a838c7b5d942 100644
--- a/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c
+++ b/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c
@@ -83,6 +83,8 @@
 #include 
 #include 
 
+#include 
+
 #if defined(CONFIG_DRM_AMD_DC_DCN)
 #include "ivsrcid/dcn/irqsrcs_dcn_1_0.h"
 
@@ -4079,6 +4081,11 @@ amdgpu_dm_register_backlight_device(struct 
amdgpu_display_manager *dm)
amdgpu_dm_update_backlight_caps(dm, dm->num_of_edps);
dm->brightness[dm->num_of_edps] = AMDGPU_MAX_BL_LEVEL;
 
+   if (acpi_video_get_backlight_type(true) != acpi_backlight_native) {
+   DRM_INFO("Skipping amdgpu DM backlight registration\n");
+   return;
+   }
+
props.max_brightness = AMDGPU_MAX_BL_LEVEL;
props.brightness = AMDGPU_MAX_BL_LEVEL;
props.type = BACKLIGHT_RAW;
-- 
2.36.0

[PATCH 06/14] ACPI: video: Drop backlight_device_get_by_type() call from acpi_video_get_backlight_type()

Now that all kms drivers which register native/BACKLIGHT_RAW type backlight
devices on x86/ACPI boards call acpi_video_get_backlight_type(true), with
the native=true value getting cached, there no longer is a need to call
backlight_device_get_by_type(BACKLIGHT_RAW) to see if a native backlight
device is available.

Relying on the cached native_available value not only is simpler, it will
also work correctly in cases where then native backlight registration was
skipped because of the acpi_video_get_backlight_type() return value.

Signed-off-by: Hans de Goede 
---
 drivers/acpi/video_detect.c | 3 +--
 1 file changed, 1 insertion(+), 2 deletions(-)

diff --git a/drivers/acpi/video_detect.c b/drivers/acpi/video_detect.c
index 0a06f0edd298..6caabdf189c9 100644
--- a/drivers/acpi/video_detect.c
+++ b/drivers/acpi/video_detect.c
@@ -586,8 +586,7 @@ enum acpi_backlight_type acpi_video_get_backlight_type(bool 
native)
if (!(video_caps & ACPI_VIDEO_BACKLIGHT))
return acpi_backlight_vendor;
 
-   if (acpi_osi_is_win8() &&
-   (native_available || backlight_device_get_by_type(BACKLIGHT_RAW)))
+   if (acpi_osi_is_win8() && native_available)
return acpi_backlight_native;
 
return acpi_backlight_video;
-- 
2.36.0

[PATCH 04/14] drm/radeon: Don't register backlight when another backlight should be used

Before this commit when we want userspace to use the acpi_video backlight
device we register both the GPU's native backlight device and acpi_video's
firmware acpi_video# backlight device. This relies on userspace preferring
firmware type backlight devices over native ones.

Registering 2 backlight devices for a single display really is
undesirable, don't register the GPU's native backlight device when
another backlight device should be used.

Signed-off-by: Hans de Goede 
---
 drivers/gpu/drm/Kconfig | 1 +
 drivers/gpu/drm/radeon/atombios_encoders.c  | 7 +++
 drivers/gpu/drm/radeon/radeon_legacy_encoders.c | 7 +++
 3 files changed, 15 insertions(+)

diff --git a/drivers/gpu/drm/Kconfig b/drivers/gpu/drm/Kconfig
index ddbeb2124df7..37205953056b 100644
--- a/drivers/gpu/drm/Kconfig
+++ b/drivers/gpu/drm/Kconfig
@@ -258,6 +258,7 @@ config DRM_RADEON
select HWMON
select BACKLIGHT_CLASS_DEVICE
select INTERVAL_TREE
+   select ACPI_VIDEO if ACPI && X86 && INPUT
help
  Choose this option if you have an ATI Radeon graphics card.  There
  are both PCI and AGP versions.  You don't need to choose this to
diff --git a/drivers/gpu/drm/radeon/atombios_encoders.c 
b/drivers/gpu/drm/radeon/atombios_encoders.c
index 70bd84b7ef2b..f82577dc25e8 100644
--- a/drivers/gpu/drm/radeon/atombios_encoders.c
+++ b/drivers/gpu/drm/radeon/atombios_encoders.c
@@ -32,6 +32,8 @@
 #include 
 #include 
 
+#include 
+
 #include "atom.h"
 #include "radeon_atombios.h"
 #include "radeon.h"
@@ -211,6 +213,11 @@ void radeon_atom_backlight_init(struct radeon_encoder 
*radeon_encoder,
if (!(rdev->mode_info.firmware_flags & 
ATOM_BIOS_INFO_BL_CONTROLLED_BY_GPU))
return;
 
+   if (acpi_video_get_backlight_type(true) != acpi_backlight_native) {
+   DRM_INFO("Skipping radeon atom DIG backlight registration\n");
+   return;
+   }
+
pdata = kmalloc(sizeof(struct radeon_backlight_privdata), GFP_KERNEL);
if (!pdata) {
DRM_ERROR("Memory allocation failed\n");
diff --git a/drivers/gpu/drm/radeon/radeon_legacy_encoders.c 
b/drivers/gpu/drm/radeon/radeon_legacy_encoders.c
index 7fdb77d48d6a..d2180f5c80fa 100644
--- a/drivers/gpu/drm/radeon/radeon_legacy_encoders.c
+++ b/drivers/gpu/drm/radeon/radeon_legacy_encoders.c
@@ -33,6 +33,8 @@
 #include 
 #include 
 
+#include 
+
 #include "radeon.h"
 #include "radeon_asic.h"
 #include "radeon_legacy_encoders.h"
@@ -389,6 +391,11 @@ void radeon_legacy_backlight_init(struct radeon_encoder 
*radeon_encoder,
return;
 #endif
 
+   if (acpi_video_get_backlight_type(true) != acpi_backlight_native) {
+   DRM_INFO("Skipping radeon legacy LVDS backlight 
registration\n");
+   return;
+   }
+
pdata = kmalloc(sizeof(struct radeon_backlight_privdata), GFP_KERNEL);
if (!pdata) {
DRM_ERROR("Memory allocation failed\n");
-- 
2.36.0

[PATCH 05/14] drm/nouveau: Don't register backlight when another backlight should be used

Before this commit when we want userspace to use the acpi_video backlight
device we register both the GPU's native backlight device and acpi_video's
firmware acpi_video# backlight device. This relies on userspace preferring
firmware type backlight devices over native ones.

Registering 2 backlight devices for a single display really is
undesirable, don't register the GPU's native backlight device when
another backlight device should be used.

Signed-off-by: Hans de Goede 
---
 drivers/gpu/drm/nouveau/nouveau_backlight.c | 7 +++
 1 file changed, 7 insertions(+)

diff --git a/drivers/gpu/drm/nouveau/nouveau_backlight.c 
b/drivers/gpu/drm/nouveau/nouveau_backlight.c
index daf9f87477ba..f56ff797c78c 100644
--- a/drivers/gpu/drm/nouveau/nouveau_backlight.c
+++ b/drivers/gpu/drm/nouveau/nouveau_backlight.c
@@ -34,6 +34,8 @@
 #include 
 #include 
 
+#include 
+
 #include "nouveau_drv.h"
 #include "nouveau_reg.h"
 #include "nouveau_encoder.h"
@@ -404,6 +406,11 @@ nouveau_backlight_init(struct drm_connector *connector)
goto fail_alloc;
}
 
+   if (acpi_video_get_backlight_type(true) != acpi_backlight_native) {
+   NV_INFO(drm, "Skipping nv_backlight registration\n");
+   goto fail_alloc;
+   }
+
if (!nouveau_get_backlight_name(backlight_name, bl)) {
NV_ERROR(drm, "Failed to retrieve a unique name for the 
backlight interface\n");
goto fail_alloc;
-- 
2.36.0

[PATCH 01/14] ACPI: video: Add a native function parameter to acpi_video_get_backlight_type()

ATM on x86 laptops where we want userspace to use the acpi_video backlight
device we often register both the GPU's native backlight device and
acpi_video's firmware acpi_video# backlight device. This relies on
userspace preferring firmware type backlight devices over native ones, but
registering 2 backlight devices for a single display really is undesirable.

On x86 laptops where the native GPU backlight device should be used,
the registering of other backlight devices is avoided by their drivers
using acpi_video_get_backlight_type() and only registering their backlight
if the return value matches their type.

acpi_video_get_backlight_type() uses
backlight_device_get_by_type(BACKLIGHT_RAW) to determine if a native
driver is available and will never return native if this returns
false. This means that the GPU's native backlight registering code
cannot just call acpi_video_get_backlight_type() to determine if it
should register its backlight, since acpi_video_get_backlight_type() will
never return native until the native backlight has already registered.

To fix this add a native function parameter to
acpi_video_get_backlight_type(), which when set to true will make
acpi_video_get_backlight_type() behave as if a native backlight has
already been registered.

Note that all current callers are updated to pass false for the new
parameter, so this change in itself causes no functional changes.

Signed-off-by: Hans de Goede 
---
 drivers/acpi/acpi_video.c |  2 +-
 drivers/acpi/video_detect.c   | 20 ---
 drivers/gpu/drm/i915/display/intel_opregion.c |  2 +-
 drivers/platform/x86/acer-wmi.c   |  2 +-
 drivers/platform/x86/asus-laptop.c|  2 +-
 drivers/platform/x86/asus-wmi.c   |  4 ++--
 drivers/platform/x86/compal-laptop.c  |  2 +-
 drivers/platform/x86/dell/dell-laptop.c   |  2 +-
 drivers/platform/x86/eeepc-laptop.c   |  2 +-
 drivers/platform/x86/fujitsu-laptop.c |  4 ++--
 drivers/platform/x86/ideapad-laptop.c |  2 +-
 drivers/platform/x86/intel/oaktrail.c |  2 +-
 drivers/platform/x86/msi-laptop.c |  2 +-
 drivers/platform/x86/msi-wmi.c|  2 +-
 drivers/platform/x86/samsung-laptop.c |  2 +-
 drivers/platform/x86/sony-laptop.c|  2 +-
 drivers/platform/x86/thinkpad_acpi.c  |  4 ++--
 drivers/platform/x86/toshiba_acpi.c   |  2 +-
 include/acpi/video.h  |  6 +++---
 19 files changed, 36 insertions(+), 30 deletions(-)

diff --git a/drivers/acpi/acpi_video.c b/drivers/acpi/acpi_video.c
index 990ff5b0aeb8..cebef3403620 100644
--- a/drivers/acpi/acpi_video.c
+++ b/drivers/acpi/acpi_video.c
@@ -1864,7 +1864,7 @@ static int acpi_video_bus_register_backlight(struct 
acpi_video_bus *video)
 
acpi_video_run_bcl_for_osi(video);
 
-   if (acpi_video_get_backlight_type() != acpi_backlight_video)
+   if (acpi_video_get_backlight_type(false) != acpi_backlight_video)
return 0;
 
mutex_lock(>device_list_lock);
diff --git a/drivers/acpi/video_detect.c b/drivers/acpi/video_detect.c
index becc198e4c22..0a06f0edd298 100644
--- a/drivers/acpi/video_detect.c
+++ b/drivers/acpi/video_detect.c
@@ -17,12 +17,14 @@
  * Otherwise vendor specific drivers like thinkpad_acpi, asus-laptop,
  * sony_acpi,... can take care about backlight brightness.
  *
- * Backlight drivers can use acpi_video_get_backlight_type() to determine
- * which driver should handle the backlight.
+ * Backlight drivers can use acpi_video_get_backlight_type() to determine which
+ * driver should handle the backlight. RAW/GPU-driver backlight drivers must
+ * pass true for the native function argument, other drivers must pass false.
  *
  * If CONFIG_ACPI_VIDEO is neither set as "compiled in" (y) nor as a module (m)
  * this file will not be compiled and acpi_video_get_backlight_type() will
- * always return acpi_backlight_vendor.
+ * return acpi_backlight_native when its native argument is true and
+ * acpi_backlight_vendor when it is false.
  */
 
 #include 
@@ -517,7 +519,7 @@ static const struct dmi_system_id video_detect_dmi_table[] 
= {
 /* This uses a workqueue to avoid various locking ordering issues */
 static void acpi_video_backlight_notify_work(struct work_struct *work)
 {
-   if (acpi_video_get_backlight_type() != acpi_backlight_video)
+   if (acpi_video_get_backlight_type(false) != acpi_backlight_video)
acpi_video_unregister_backlight();
 }
 
@@ -548,9 +550,10 @@ static int acpi_video_backlight_notify(struct 
notifier_block *nb,
  * Arguably the native on win8 check should be done first, but that would
  * be a behavior change, which may causes issues.
  */
-enum acpi_backlight_type acpi_video_get_backlight_type(void)
+enum acpi_backlight_type acpi_video_get_backlight_type(bool native)
 {
static DEFINE_MUTEX(init_mutex);
+   static bool native_available;

[PATCH 02/14] drm/i915: Don't register backlight when another backlight should be used

Before this commit when we want userspace to use the acpi_video backlight
device we register both the GPU's native backlight device and acpi_video's
firmware acpi_video# backlight device. This relies on userspace preferring
firmware type backlight devices over native ones.

Registering 2 backlight devices for a single display really is
undesirable, don't register the GPU's native backlight device when
another backlight device should be used.

Signed-off-by: Hans de Goede 
---
 drivers/gpu/drm/i915/display/intel_backlight.c | 7 +++
 1 file changed, 7 insertions(+)

diff --git a/drivers/gpu/drm/i915/display/intel_backlight.c 
b/drivers/gpu/drm/i915/display/intel_backlight.c
index 98f7ea44042f..582d7f48575d 100644
--- a/drivers/gpu/drm/i915/display/intel_backlight.c
+++ b/drivers/gpu/drm/i915/display/intel_backlight.c
@@ -6,6 +6,8 @@
 #include 
 #include 
 
+#include 
+
 #include "intel_backlight.h"
 #include "intel_connector.h"
 #include "intel_de.h"
@@ -948,6 +950,11 @@ int intel_backlight_device_register(struct intel_connector 
*connector)
 
WARN_ON(panel->backlight.max == 0);
 
+   if (acpi_video_get_backlight_type(true) != acpi_backlight_native) {
+   DRM_INFO("Skipping intel_backlight registration\n");
+   return 0;
+   }
+
memset(, 0, sizeof(props));
props.type = BACKLIGHT_RAW;
 
-- 
2.36.0

[PATCH 00/14] drm/kms: Stop registering multiple /sys/class/backlight devs for a single display

Hi All,

As mentioned in my RFC titled "drm/kms: control display brightness through
drm_connector properties":
https://lore.kernel.org/dri-devel/0d188965-d809-81b5-74ce-7d30c49fe...@redhat.com/

The first step towards this is to deal with some existing technical debt
in backlight handling on x86/ACPI boards, specifically we need to stop
registering multiple /sys/class/backlight devs for a single display.

This series implements my RFC describing my plan for these cleanups:
https://lore.kernel.org/dri-devel/98519ba0-7f18-201a-ea34-652f50343...@redhat.com/

Specifically patches 1-6 implement the "Fixing kms driver unconditionally
register their "native" backlight dev" part.

And patches 7-14 implement the "Fixing acpi_video0 getting registered for
a brief time" time.

Note this series does not deal yet with the "Other issues" part, I plan
to do a follow up series for that.

The changes in this series are good to have regardless of the further
"drm/kms: control display brightness through drm_connector properties"
plans. So I plan to push these upstream once they are ready (once
reviewed). Since this crosses various subsystems / touches multiple
kms drivers my plan is to provide an immutable branch based on say
5.19-rc1 and then have that get merged into all the relevant trees.

Please review.

Regards,

Hans


Hans de Goede (14):
  ACPI: video: Add a native function parameter to
acpi_video_get_backlight_type()
  drm/i915: Don't register backlight when another backlight should be
used
  drm/amdgpu: Don't register backlight when another backlight should be
used
  drm/radeon: Don't register backlight when another backlight should be
used
  drm/nouveau: Don't register backlight when another backlight should be
used
  ACPI: video: Drop backlight_device_get_by_type() call from
acpi_video_get_backlight_type()
  ACPI: video: Remove acpi_video_bus from list before tearing it down
  ACPI: video: Simplify acpi_video_unregister_backlight()
  ACPI: video: Make backlight class device registration a separate step
  ACPI: video: Remove code to unregister acpi_video backlight when a
native backlight registers
  drm/i915: Call acpi_video_register_backlight()
  drm/nouveau: Register ACPI video backlight when nv_backlight
registration fails
  drm/amdgpu: Register ACPI video backlight when skipping amdgpu
backlight registration
  drm/radeon: Register ACPI video backlight when skipping radeon
backlight registration

 drivers/acpi/acpi_video.c | 69 ++-
 drivers/acpi/video_detect.c   | 53 +++---
 drivers/gpu/drm/Kconfig   |  2 +
 .../gpu/drm/amd/amdgpu/atombios_encoders.c| 14 +++-
 .../gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c |  9 +++
 .../gpu/drm/i915/display/intel_backlight.c|  7 ++
 drivers/gpu/drm/i915/display/intel_display.c  |  1 +
 drivers/gpu/drm/i915/display/intel_opregion.c |  2 +-
 drivers/gpu/drm/nouveau/nouveau_backlight.c   | 14 
 drivers/gpu/drm/radeon/atombios_encoders.c|  7 ++
 drivers/gpu/drm/radeon/radeon_encoders.c  | 11 ++-
 .../gpu/drm/radeon/radeon_legacy_encoders.c   |  7 ++
 drivers/platform/x86/acer-wmi.c   |  2 +-
 drivers/platform/x86/asus-laptop.c|  2 +-
 drivers/platform/x86/asus-wmi.c   |  4 +-
 drivers/platform/x86/compal-laptop.c  |  2 +-
 drivers/platform/x86/dell/dell-laptop.c   |  2 +-
 drivers/platform/x86/eeepc-laptop.c   |  2 +-
 drivers/platform/x86/fujitsu-laptop.c |  4 +-
 drivers/platform/x86/ideapad-laptop.c |  2 +-
 drivers/platform/x86/intel/oaktrail.c |  2 +-
 drivers/platform/x86/msi-laptop.c |  2 +-
 drivers/platform/x86/msi-wmi.c|  2 +-
 drivers/platform/x86/samsung-laptop.c |  2 +-
 drivers/platform/x86/sony-laptop.c|  2 +-
 drivers/platform/x86/thinkpad_acpi.c  |  4 +-
 drivers/platform/x86/toshiba_acpi.c   |  2 +-
 include/acpi/video.h  |  8 ++-
 28 files changed, 156 insertions(+), 84 deletions(-)

-- 
2.36.0

[PATCH v3 5/6] drm/i915/sseu: Disassociate internal subslice mask representation from uapi

2022-05-17 Thread Matt Roper

As with EU masks, it's easier to store subslice/DSS masks internally in
a format that's more natural for the driver to work with, and then only
covert into the u8[] uapi form when the query ioctl is invoked.  Since
the hardware design changed significantly with Xe_HP, we'll use a union
to choose between the old "hsw-style" subslice masks or the newer xehp
mask.  HSW-style masks will be stored in an array of u8's, indexed by
slice (there's never more than 6 subslices per slice on older
platforms).  For Xe_HP and beyond where slices no longer exist, we only
need a single bitmask.  However we already know that this mask is
eventually going to grow too large for a simple u64 to hold, so we'll
represent it in a manner that can be operated on by the utilities in
linux/bitmap.h.

v3:
 - Fix typo: BIT(s) -> BIT(ss) in gen9_sseu_device_status()

Cc: Tvrtko Ursulin 
Signed-off-by: Matt Roper 
---
 drivers/gpu/drm/i915/gem/i915_gem_context.c  |   5 +-
 drivers/gpu/drm/i915/gt/intel_engine_cs.c|   4 +-
 drivers/gpu/drm/i915/gt/intel_gt.c   |  12 +-
 drivers/gpu/drm/i915/gt/intel_sseu.c | 246 +++
 drivers/gpu/drm/i915/gt/intel_sseu.h |  78 +++---
 drivers/gpu/drm/i915/gt/intel_sseu_debugfs.c |  30 +--
 drivers/gpu/drm/i915/gt/intel_workarounds.c  |  24 +-
 drivers/gpu/drm/i915/i915_getparam.c |   3 +-
 drivers/gpu/drm/i915/i915_query.c|   8 +-
 9 files changed, 226 insertions(+), 184 deletions(-)

diff --git a/drivers/gpu/drm/i915/gem/i915_gem_context.c 
b/drivers/gpu/drm/i915/gem/i915_gem_context.c
index ab4c5ab28e4d..a3bb73f5d53b 100644
--- a/drivers/gpu/drm/i915/gem/i915_gem_context.c
+++ b/drivers/gpu/drm/i915/gem/i915_gem_context.c
@@ -1875,6 +1875,7 @@ i915_gem_user_to_context_sseu(struct intel_gt *gt,
 {
const struct sseu_dev_info *device = >info.sseu;
struct drm_i915_private *i915 = gt->i915;
+   unsigned int dev_subslice_mask = intel_sseu_get_hsw_subslices(device, 
0);
 
/* No zeros in any field. */
if (!user->slice_mask || !user->subslice_mask ||
@@ -1901,7 +1902,7 @@ i915_gem_user_to_context_sseu(struct intel_gt *gt,
if (user->slice_mask & ~device->slice_mask)
return -EINVAL;
 
-   if (user->subslice_mask & ~device->subslice_mask[0])
+   if (user->subslice_mask & ~dev_subslice_mask)
return -EINVAL;
 
if (user->max_eus_per_subslice > device->max_eus_per_subslice)
@@ -1915,7 +1916,7 @@ i915_gem_user_to_context_sseu(struct intel_gt *gt,
/* Part specific restrictions. */
if (GRAPHICS_VER(i915) == 11) {
unsigned int hw_s = hweight8(device->slice_mask);
-   unsigned int hw_ss_per_s = hweight8(device->subslice_mask[0]);
+   unsigned int hw_ss_per_s = hweight8(dev_subslice_mask);
unsigned int req_s = hweight8(context->slice_mask);
unsigned int req_ss = hweight8(context->subslice_mask);
 
diff --git a/drivers/gpu/drm/i915/gt/intel_engine_cs.c 
b/drivers/gpu/drm/i915/gt/intel_engine_cs.c
index 1adbf34c3632..f0acf8518a51 100644
--- a/drivers/gpu/drm/i915/gt/intel_engine_cs.c
+++ b/drivers/gpu/drm/i915/gt/intel_engine_cs.c
@@ -674,8 +674,8 @@ static void engine_mask_apply_compute_fuses(struct intel_gt 
*gt)
if (GRAPHICS_VER_FULL(i915) < IP_VER(12, 50))
return;
 
-   ccs_mask = 
intel_slicemask_from_dssmask(intel_sseu_get_compute_subslices(>sseu),
-   ss_per_ccs);
+   ccs_mask = 
intel_slicemask_from_xehp_dssmask(info->sseu.compute_subslice_mask,
+ss_per_ccs);
/*
 * If all DSS in a quadrant are fused off, the corresponding CCS
 * engine is not available for use.
diff --git a/drivers/gpu/drm/i915/gt/intel_gt.c 
b/drivers/gpu/drm/i915/gt/intel_gt.c
index 034182f85501..2921f510642f 100644
--- a/drivers/gpu/drm/i915/gt/intel_gt.c
+++ b/drivers/gpu/drm/i915/gt/intel_gt.c
@@ -133,13 +133,6 @@ static const struct intel_mmio_range 
dg2_lncf_steering_table[] = {
{},
 };
 
-static u16 slicemask(struct intel_gt *gt, int count)
-{
-   u64 dss_mask = intel_sseu_get_subslices(>info.sseu, 0);
-
-   return intel_slicemask_from_dssmask(dss_mask, count);
-}
-
 int intel_gt_init_mmio(struct intel_gt *gt)
 {
struct drm_i915_private *i915 = gt->i915;
@@ -155,9 +148,12 @@ int intel_gt_init_mmio(struct intel_gt *gt)
 */
if (HAS_MSLICES(i915)) {
gt->info.mslice_mask =
-   slicemask(gt, GEN_DSS_PER_MSLICE) |
+   
intel_slicemask_from_xehp_dssmask(gt->info.sseu.subslice_mask,
+ GEN_DSS_PER_MSLICE);
+   gt->info.mslice_mask |=
(intel_uncore_read(gt->uncore, GEN10_MIRROR_FUSE3) &
 GEN12_MEML3_EN_MASK);
+
if (!gt->info.mslice_mask) /*

Re: [PATCH v2 2/3] ARM: dts: aspeed: Add USB2.0 device controller node

On 17/05/2022 16:50, Neal Liu wrote:
>> -Original Message-
>> From: Krzysztof Kozlowski 
>> Sent: Tuesday, May 17, 2022 8:00 PM
>> To: Neal Liu ; Greg Kroah-Hartman
>> ; Rob Herring ; Krzysztof
>> Kozlowski ; Joel Stanley ;
>> Andrew Jeffery ; Felipe Balbi ; Sumit
>> Semwal ; Christian König
>> ; Geert Uytterhoeven ;
>> Li Yang 
>> Cc: linux-asp...@lists.ozlabs.org; linux-...@vger.kernel.org;
>> devicet...@vger.kernel.org; linux-arm-ker...@lists.infradead.org;
>> linux-ker...@vger.kernel.org; linux-me...@vger.kernel.org;
>> dri-devel@lists.freedesktop.org; linaro-mm-...@lists.linaro.org
>> Subject: Re: [PATCH v2 2/3] ARM: dts: aspeed: Add USB2.0 device controller
>> node
>>
>> On 17/05/2022 10:25, Neal Liu wrote:
>>> Add USB2.0 device controller(udc) node to device tree for AST2600.
>>>
>>> Signed-off-by: Neal Liu 
>>> ---
>>>  arch/arm/boot/dts/aspeed-g6.dtsi | 10 ++
>>>  1 file changed, 10 insertions(+)
>>>
>>> diff --git a/arch/arm/boot/dts/aspeed-g6.dtsi
>>> b/arch/arm/boot/dts/aspeed-g6.dtsi
>>> index 3d5ce9da42c3..5517313eb2b5 100644
>>> --- a/arch/arm/boot/dts/aspeed-g6.dtsi
>>> +++ b/arch/arm/boot/dts/aspeed-g6.dtsi
>>> @@ -298,6 +298,16 @@ vhub: usb-vhub@1e6a {
>>> status = "disabled";
>>> };
>>>
>>> +   udc: udc@1e6a2000 {
>>
>> The same as DTS in bindings - generic node name, please.
>>
> 
> Is it possible to use "udc: usb-udc@1e6a2000" to distinguish it between 
> "vhub: usb-vhub@1e6a"?

Possible yes :), but not recommended and not wanted. Nodes should be
generic and prefixes are added only if there is no unit address. You can
though use some more descriptive label.


Best regards,
Krzysztof

Re: [PATCH v1] drm/scheduler: Don't kill jobs in interrupt context

2022-05-17 Thread Dmitry Osipenko

On 5/17/22 17:13, Andrey Grodzovsky wrote:
> Done.
> 
> Andrey

Awesome, thank you!

-- 
Best regards,
Dmitry

Re: [PATCH v1] drm/scheduler: Don't kill jobs in interrupt context

2022-05-17 Thread Andrey Grodzovsky


Done.

Andrey

On 2022-05-17 10:03, Andrey Grodzovsky wrote:

Let me push it into drm-misc-next.

Andrey

On 2022-05-17 05:03, Dmitry Osipenko wrote:


On 5/17/22 10:40, Erico Nunes wrote:
On Wed, Apr 13, 2022 at 12:05 PM Steven Price  
wrote:

On 11/04/2022 23:15, Dmitry Osipenko wrote:
Interrupt context can't sleep. Drivers like Panfrost and MSM are 
taking
mutex when job is released, and thus, that code can sleep. This 
results
into "BUG: scheduling while atomic" if locks are contented while 
job is

freed. There is no good reason for releasing scheduler's jobs in IRQ
context, hence use normal context to fix the trouble.

Cc: sta...@vger.kernel.org
Fixes: 542cff7893a3 ("drm/sched: Avoid lockdep spalt on killing a 
processes")

Signed-off-by: Dmitry Osipenko 

Reviewed-by: Steven Price 

Is there something blocking this patch?
Mesa CI is still hitting the issue and I have been waiting for it to
be applied/backported to update CI with it.
Thanks

If this patch won't be picked up anytime soon, then I'll include it into
my "memory shrinker" patchset together with the rest of the fixes, so it
won't get lost.

Re: [PATCH v1] drm/scheduler: Don't kill jobs in interrupt context

2022-05-17 Thread Andrey Grodzovsky


Let me push it into drm-misc-next.

Andrey

On 2022-05-17 05:03, Dmitry Osipenko wrote:


On 5/17/22 10:40, Erico Nunes wrote:

On Wed, Apr 13, 2022 at 12:05 PM Steven Price  wrote:

On 11/04/2022 23:15, Dmitry Osipenko wrote:

Interrupt context can't sleep. Drivers like Panfrost and MSM are taking
mutex when job is released, and thus, that code can sleep. This results
into "BUG: scheduling while atomic" if locks are contented while job is
freed. There is no good reason for releasing scheduler's jobs in IRQ
context, hence use normal context to fix the trouble.

Cc: sta...@vger.kernel.org
Fixes: 542cff7893a3 ("drm/sched: Avoid lockdep spalt on killing a processes")
Signed-off-by: Dmitry Osipenko 

Reviewed-by: Steven Price 

Is there something blocking this patch?
Mesa CI is still hitting the issue and I have been waiting for it to
be applied/backported to update CI with it.
Thanks

If this patch won't be picked up anytime soon, then I'll include it into
my "memory shrinker" patchset together with the rest of the fixes, so it
won't get lost.

Re: [PATCH] drm: bridge: DRM_FSL_LDB should depend on ARCH_MXC

2022-05-17 Thread Robert Foss

On Mon, 9 May 2022 at 16:41, Marek Vasut  wrote:
>
> On 5/9/22 15:52, Geert Uytterhoeven wrote:
> > The Freescale i.MX8MP LDB bridge is only present on Freescale i.MX8MP
> > SoCs.  Hence add a dependency on ARCH_MXC, to prevent asking the user
> > about this driver when configuring a kernel without i.MX SoC support.
> >
> > Fixes: 463db5c2ed4aed01 ("drm: bridge: ldb: Implement simple Freescale 
> > i.MX8MP LDB bridge")
> > Signed-off-by: Geert Uytterhoeven 
>
> Reviewed-by: Marek Vasut 

Applied to drm-misc-next

Re: [PATCH v4] drm/doc: add rfc section for small BAR uapi




On 17/05/2022 11:52, Matthew Auld wrote:

Add an entry for the new uapi needed for small BAR on DG2+.

v2:
   - Some spelling fixes and other small tweaks. (Akeem & Thomas)
   - Rework error capture interactions, including no longer needing
 NEEDS_CPU_ACCESS for objects marked for capture. (Thomas)
   - Add probed_cpu_visible_size. (Lionel)
v3:
   - Drop the vma query for now.
   - Add unallocated_cpu_visible_size as part of the region query.
   - Improve the docs some more, including documenting the expected
 behaviour on older kernels, since this came up in some offline
 discussion.
v4:
   - Various improvements all over. (Tvrtko)


You can ignore my previous reply, the clarifications from v4 read good 
to me.


Acked-by: Tvrtko Ursulin 

Regards,

Tvrtko



Signed-off-by: Matthew Auld 
Cc: Thomas Hellström 
Cc: Lionel Landwerlin 
Cc: Tvrtko Ursulin 
Cc: Jon Bloomfield 
Cc: Daniel Vetter 
Cc: Jon Bloomfield 
Cc: Jordan Justen 
Cc: Kenneth Graunke 
Cc: Akeem G Abodunrin 
Cc: mesa-...@lists.freedesktop.org
---
  Documentation/gpu/rfc/i915_small_bar.h   | 189 +++
  Documentation/gpu/rfc/i915_small_bar.rst |  47 ++
  Documentation/gpu/rfc/index.rst  |   4 +
  3 files changed, 240 insertions(+)
  create mode 100644 Documentation/gpu/rfc/i915_small_bar.h
  create mode 100644 Documentation/gpu/rfc/i915_small_bar.rst

diff --git a/Documentation/gpu/rfc/i915_small_bar.h 
b/Documentation/gpu/rfc/i915_small_bar.h
new file mode 100644
index ..c676640b23ef
--- /dev/null
+++ b/Documentation/gpu/rfc/i915_small_bar.h
@@ -0,0 +1,189 @@
+/**
+ * struct __drm_i915_memory_region_info - Describes one region as known to the
+ * driver.
+ *
+ * Note this is using both struct drm_i915_query_item and struct 
drm_i915_query.
+ * For this new query we are adding the new query id 
DRM_I915_QUERY_MEMORY_REGIONS
+ * at _i915_query_item.query_id.
+ */
+struct __drm_i915_memory_region_info {
+   /** @region: The class:instance pair encoding */
+   struct drm_i915_gem_memory_class_instance region;
+
+   /** @rsvd0: MBZ */
+   __u32 rsvd0;
+
+   /**
+* @probed_size: Memory probed by the driver (-1 = unknown)
+*
+* Note that it should not be possible to ever encounter a zero value
+* here, also note that no current region type will ever return -1 here.
+* Although for future region types, this might be a possibility. The
+* same applies to the other size fields.
+*/
+   __u64 probed_size;
+
+   /**
+* @unallocated_size: Estimate of memory remaining (-1 = unknown)
+*
+* Requires CAP_PERFMON or CAP_SYS_ADMIN to get reliable accounting.
+* Without this (or if this is an older kernel) the value here will
+* always equal the @probed_size. Note this is only currently tracked
+* for I915_MEMORY_CLASS_DEVICE regions (for other types the value here
+* will always equal the @probed_size).
+*/
+   __u64 unallocated_size;
+
+   union {
+   /** @rsvd1: MBZ */
+   __u64 rsvd1[8];
+   struct {
+   /**
+* @probed_cpu_visible_size: Memory probed by the driver
+* that is CPU accessible. (-1 = unknown).
+*
+* This will be always be <= @probed_size, and the
+* remainder (if there is any) will not be CPU
+* accessible.
+*
+* On systems without small BAR, the @probed_size will
+* always equal the @probed_cpu_visible_size, since all
+* of it will be CPU accessible.
+*
+* Note this is only tracked for
+* I915_MEMORY_CLASS_DEVICE regions (for other types the
+* value here will always equal the @probed_size).
+*
+* Note that if the value returned here is zero, then
+* this must be an old kernel which lacks the relevant
+* small-bar uAPI support (including
+* I915_GEM_CREATE_EXT_FLAG_NEEDS_CPU_ACCESS), but on
+* such systems we should never actually end up with a
+* small BAR configuration, assuming we are able to load
+* the kernel module. Hence it should be safe to treat
+* this the same as when @probed_cpu_visible_size ==
+* @probed_size.
+*/
+   __u64 probed_cpu_visible_size;
+
+   /**
+* @unallocated_cpu_visible_size: Estimate of CPU
+* visible memory remaining (-1 = unknown).
+*
+

Re: [PATCH v3] drm/doc: add rfc section for small BAR uapi




On 17/05/2022 10:12, Matthew Auld wrote:

On 17/05/2022 09:29, Tvrtko Ursulin wrote:


On 16/05/2022 19:11, Matthew Auld wrote:

Add an entry for the new uapi needed for small BAR on DG2+.

v2:
   - Some spelling fixes and other small tweaks. (Akeem & Thomas)
   - Rework error capture interactions, including no longer needing
 NEEDS_CPU_ACCESS for objects marked for capture. (Thomas)
   - Add probed_cpu_visible_size. (Lionel)
v3:
   - Drop the vma query for now.
   - Add unallocated_cpu_visible_size as part of the region query.
   - Improve the docs some more, including documenting the expected
 behaviour on older kernels, since this came up in some offline
 discussion.

Signed-off-by: Matthew Auld 
Cc: Thomas Hellström 
Cc: Lionel Landwerlin 
Cc: Tvrtko Ursulin 
Cc: Jon Bloomfield 
Cc: Daniel Vetter 
Cc: Jon Bloomfield 
Cc: Jordan Justen 
Cc: Kenneth Graunke 
Cc: Akeem G Abodunrin 
Cc: mesa-...@lists.freedesktop.org
---
  Documentation/gpu/rfc/i915_small_bar.h   | 164 +++
  Documentation/gpu/rfc/i915_small_bar.rst |  47 +++
  Documentation/gpu/rfc/index.rst  |   4 +
  3 files changed, 215 insertions(+)
  create mode 100644 Documentation/gpu/rfc/i915_small_bar.h
  create mode 100644 Documentation/gpu/rfc/i915_small_bar.rst

diff --git a/Documentation/gpu/rfc/i915_small_bar.h 
b/Documentation/gpu/rfc/i915_small_bar.h

new file mode 100644
index ..4079d287750b
--- /dev/null
+++ b/Documentation/gpu/rfc/i915_small_bar.h
@@ -0,0 +1,164 @@
+/**
+ * struct __drm_i915_memory_region_info - Describes one region as 
known to the

+ * driver.
+ *
+ * Note this is using both struct drm_i915_query_item and struct 
drm_i915_query.
+ * For this new query we are adding the new query id 
DRM_I915_QUERY_MEMORY_REGIONS

+ * at _i915_query_item.query_id.
+ */
+struct __drm_i915_memory_region_info {
+    /** @region: The class:instance pair encoding */
+    struct drm_i915_gem_memory_class_instance region;
+
+    /** @rsvd0: MBZ */
+    __u32 rsvd0;
+
+    /** @probed_size: Memory probed by the driver (-1 = unknown) */
+    __u64 probed_size;


Is -1 possible today or when it will be? For system memory it appears 
zeroes are returned today so that has to stay I think. Does it 
effectively mean userspace has to consider both 0 and -1 as unknown is 
the question.


It looks like it just returns the totalram_pages(). So at the moment 
nothing ever currently returns -1 or 0. Maybe that was a mistake for 
I915_MEMORY_SYSTEM.


Yes I figured out my confusion later.




+
+    /**
+ * @unallocated_size: Estimate of memory remaining (-1 = unknown)
+ *
+ * Note this is only currently tracked for I915_MEMORY_CLASS_DEVICE
+ * regions, and also requires CAP_PERFMON or CAP_SYS_ADMIN to get
+ * reliable accounting. Without this(or if this an older kernel) 
the


s/if this an/if this is an/

Also same question as above about -1.


This should be the same as above, since this will give the same value as 
probed_size, or give the real avail value for lmem.





+ * value here will always match the @probed_size.
+ */
+    __u64 unallocated_size;
+
+    union {
+    /** @rsvd1: MBZ */
+    __u64 rsvd1[8];
+    struct {
+    /**
+ * @probed_cpu_visible_size: Memory probed by the driver
+ * that is CPU accessible. (-1 = unknown).


Also question about -1. In this case this could be done since the 
field is yet to be added but I am curious if it ever can be -1.


I was just going to make this the same as probed_size for system memory. 
But we could use -1 here instead. What do you think? Same for 
unallocated below.


I don't completely follow the instead part, when you already have 
documented it?


-1 will start appearing with changes that require CAP_SYS_PERFMON right?

Regards,

Tvrtko


+ *
+ * This will be always be <= @probed_size, and the
+ * remainder(if there is any) will not be CPU
+ * accessible.
+ *
+ * On systems without small BAR, the @probed_size will
+ * always equal the @probed_cpu_visible_size, since all
+ * of it will be CPU accessible.
+ *
+ * Note that if the value returned here is zero, then
+ * this must be an old kernel which lacks the relevant
+ * small-bar uAPI support(including


I have noticed you prefer no space before parentheses throughout the 
text so I guess it's just my preference to have it. Very nitpicky even 
if I am right so up to you.


Ok, will change :)




+ * I915_GEM_CREATE_EXT_FLAG_NEEDS_CPU_ACCESS), but on
+ * such systems we should never actually end up with a
+ * small BAR configuration, assuming we are able to load
+ * the kernel module. Hence it should be safe to treat
+ * this the same as when @probed_cpu_visible_size ==
+ * @probed_size.
+ */
+

Re: [PATCH v3] drm/doc: add rfc section for small BAR uapi




On 17/05/2022 10:39, Lionel Landwerlin wrote:

On 17/05/2022 12:23, Tvrtko Ursulin wrote:


On 17/05/2022 09:55, Lionel Landwerlin wrote:

On 17/05/2022 11:29, Tvrtko Ursulin wrote:


On 16/05/2022 19:11, Matthew Auld wrote:

Add an entry for the new uapi needed for small BAR on DG2+.

v2:
   - Some spelling fixes and other small tweaks. (Akeem & Thomas)
   - Rework error capture interactions, including no longer needing
 NEEDS_CPU_ACCESS for objects marked for capture. (Thomas)
   - Add probed_cpu_visible_size. (Lionel)
v3:
   - Drop the vma query for now.
   - Add unallocated_cpu_visible_size as part of the region query.
   - Improve the docs some more, including documenting the expected
 behaviour on older kernels, since this came up in some offline
 discussion.

Signed-off-by: Matthew Auld 
Cc: Thomas Hellström 
Cc: Lionel Landwerlin 
Cc: Tvrtko Ursulin 
Cc: Jon Bloomfield 
Cc: Daniel Vetter 
Cc: Jon Bloomfield 
Cc: Jordan Justen 
Cc: Kenneth Graunke 
Cc: Akeem G Abodunrin 
Cc: mesa-...@lists.freedesktop.org
---
  Documentation/gpu/rfc/i915_small_bar.h   | 164 
+++

  Documentation/gpu/rfc/i915_small_bar.rst |  47 +++
  Documentation/gpu/rfc/index.rst  |   4 +
  3 files changed, 215 insertions(+)
  create mode 100644 Documentation/gpu/rfc/i915_small_bar.h
  create mode 100644 Documentation/gpu/rfc/i915_small_bar.rst

diff --git a/Documentation/gpu/rfc/i915_small_bar.h 
b/Documentation/gpu/rfc/i915_small_bar.h

new file mode 100644
index ..4079d287750b
--- /dev/null
+++ b/Documentation/gpu/rfc/i915_small_bar.h
@@ -0,0 +1,164 @@
+/**
+ * struct __drm_i915_memory_region_info - Describes one region as 
known to the

+ * driver.
+ *
+ * Note this is using both struct drm_i915_query_item and struct 
drm_i915_query.
+ * For this new query we are adding the new query id 
DRM_I915_QUERY_MEMORY_REGIONS

+ * at _i915_query_item.query_id.
+ */
+struct __drm_i915_memory_region_info {
+    /** @region: The class:instance pair encoding */
+    struct drm_i915_gem_memory_class_instance region;
+
+    /** @rsvd0: MBZ */
+    __u32 rsvd0;
+
+    /** @probed_size: Memory probed by the driver (-1 = unknown) */
+    __u64 probed_size;


Is -1 possible today or when it will be? For system memory it 
appears zeroes are returned today so that has to stay I think. Does 
it effectively mean userspace has to consider both 0 and -1 as 
unknown is the question.



I raised this on v2. As far as I can tell there are no situation 
where we would get -1.


Is it really probed_size=0 on smem?? It's not the case on the 
internal branch.


My bad, I misread the arguments to intel_memory_region_create while 
grepping:


struct intel_memory_region *i915_gem_shmem_setup(struct 
drm_i915_private *i915,

 u16 type, u16 instance)
{
return intel_memory_region_create(i915, 0,
  totalram_pages() << PAGE_SHIFT,
  PAGE_SIZE, 0, 0,
  type, instance,
  _region_ops);

I saw "0, 0" and wrongly assumed that would be the data, since it 
matched with my mental model and the comment against unallocated_size 
saying it's only tracked for device memory.


Although I'd say it is questionable for i915 to return this data. I 
wonder it use case is possible where it would even be wrong but don't 
know. I guess the cat is out of the bag now.



Not sure how questionable that is. There are a bunch of tools reporting 
the amount of memory available (free, top, htop, etc...).


It might not be totalram_pages() but probably something close to it.


Questionable as it being a resource driver does not own so it reporting 
it is a pure userspace convenience and maintenance burden for the 
driver. :) Not sure I understand why userspace even wants to know? Only 
reliable use could be to decide whether to even try and run a workload? 
But in that case too perhaps users wants to run it with swapping so let 
them.


There is also the question memory accounting and a process could be 
running in a container with a limited view of the world (totalram_pages 
would be wrong). Although granted, we bypass memory accounting at the 
moment. Not sure what was the discussion on that before. Probably 
because one process can create an object and another can instantiate the 
backing store. Whether or not we could, or should, therefore account 
against the real creator is the question. But if we one day did, then 
we'd have to fiddle with the memory query to stop returning 
totalram_pages() and that would potentially be complicated. Well 
"would".. we probably will since this is already shipping.


Regards,

Tvrtko



Having a non 0 & non -1 value is useful.


-Lionel




If the situation is -1 for unknown and some valid size (not zero) I 
don't think there is a problem here.


Regards,

Tvrtko


Anv is not currently handling that case.


I would very much like to not deal with 0 for smem.

It really makes it easier for

Re: [PATCH v1 03/13] arm64: tegra: Add Host1x and VIC on Tegra234

On 17/05/2022 10:38, Mikko Perttunen wrote:
>>>   
>>> +   host1x@13e0 {
>>
>> Generic node names, if that possible. Since the bindings do not exist in
>> the next, I actually cannot figure out what's host1x...
> 
> Host1x is a hardware block that provides programmable DMA channels, HW 
> synchronization primitives, and virtualization support for IP blocks 
> connected to its "host1x bus". So far I haven't found a one or two word 
> way to describe it despite efforts. In any case, considering all the 
> existing documentation and device trees that use this name, I'd prefer 
> not changing it (especially as I don't know what else it could be called).

OK


Best regards,
Krzysztof

Re: [PATCH v1 1/2] dt-bindings: display: add new bus-format property for panel-dpi

2022-05-17 Thread Rob Herring

On Mon, 16 May 2022 18:28:25 +0200, Max Krummenacher wrote:
> From: Max Krummenacher 
> 
> The property is used to set the enum bus_format and infer the bpc
> for a panel defined by 'panel-dpi'.
> This specifies how the panel is connected to the display interface.
> 
> Signed-off-by: Max Krummenacher 
> ---
> 
>  .../bindings/display/panel/panel-dpi.yaml | 11 +
>  .../dt-bindings/display/dt-media-bus-format.h | 23 +++
>  2 files changed, 34 insertions(+)
>  create mode 100644 include/dt-bindings/display/dt-media-bus-format.h
> 

My bot found errors running 'make DT_CHECKER_FLAGS=-m dt_binding_check'
on your patch (DT_CHECKER_FLAGS is new in v5.13):

yamllint warnings/errors:

dtschema/dtc warnings/errors:
Error: 
Documentation/devicetree/bindings/display/panel/panel-dpi.example.dts:20.9-10 
syntax error
FATAL ERROR: Unable to parse input tree
make[1]: *** [scripts/Makefile.lib:364: 
Documentation/devicetree/bindings/display/panel/panel-dpi.example.dtb] Error 1
make[1]: *** Waiting for unfinished jobs
make: *** [Makefile:1401: dt_binding_check] Error 2

doc reference errors (make refcheckdocs):

See https://patchwork.ozlabs.org/patch/

This check can fail if there are any dependencies. The base for a patch
series is generally the most recent rc1.

If you already ran 'make dt_binding_check' and didn't see the above
error(s), then make sure 'yamllint' is installed and dt-schema is up to
date:

pip3 install dtschema --upgrade

Please check and re-submit.

Re: [PATCH v2 3/3] dt-bindings: usb: add documentation for aspeed udc

On 17/05/2022 10:25, Neal Liu wrote:
> +
> +  interrupts:
> +maxItems: 1
> +
> +required:
> +  - compatible
> +  - reg
> +  - clocks
> +  - interrupts
> +
> +additionalProperties: false
> +
> +examples:
> +  - |
> +#include 
> +usb: usb@1e6a2000 {
> +compatible = "aspeed,ast2600-udc";

Sorry for not noticing it earlier - slipped through the cracks. You use
here unusual indentation (not matching the rest of the example - above
is 4 spaces, here is 7 or 8).

Just keep 4 spaces for DTS example.

Reviewed-by: Krzysztof Kozlowski 

Best regards,
Krzysztof

Re: [PATCH v2 2/3] ARM: dts: aspeed: Add USB2.0 device controller node

On 17/05/2022 10:25, Neal Liu wrote:
> Add USB2.0 device controller(udc) node to device tree
> for AST2600.
> 
> Signed-off-by: Neal Liu 
> ---
>  arch/arm/boot/dts/aspeed-g6.dtsi | 10 ++
>  1 file changed, 10 insertions(+)
> 
> diff --git a/arch/arm/boot/dts/aspeed-g6.dtsi 
> b/arch/arm/boot/dts/aspeed-g6.dtsi
> index 3d5ce9da42c3..5517313eb2b5 100644
> --- a/arch/arm/boot/dts/aspeed-g6.dtsi
> +++ b/arch/arm/boot/dts/aspeed-g6.dtsi
> @@ -298,6 +298,16 @@ vhub: usb-vhub@1e6a {
>   status = "disabled";
>   };
>  
> + udc: udc@1e6a2000 {

The same as DTS in bindings - generic node name, please.


Best regards,
Krzysztof

Re: [PATCH v2 0/5] drm: Ignore non-existing color planes in helpers

2022-05-17 Thread Christian König


Am 17.05.22 um 13:33 schrieb Thomas Zimmermann:

Some DRM helpers assume that all potential color planes of a framebuffer
are available; even if the color format didn't specify them. Non-existing
planes are silently ignored. This behavior is inconsistent with other
helpers and apparently leads to subtle bugs with uninitialized GEM buffer
mappings. [1]

Change all affected code to look at the framebuffer format's number of
color planes and only process these planes. The update has been discussed
in [2] before.

Tested with GEM SHMEM helpers on simpledrm and with GEM VRAM helpers on
ast.


I'm not deep enough into all the details for a full review, but feel 
free to add an Acked-by: Christian König  to 
the series.


Thanks,
Christian.



v2:
* refactor GEM VRAM code before fixing it (Javier)
* print more error information in #1 (Javier)
* commit-message fixes (Javier)

[1] 
https://nam11.safelinks.protection.outlook.com/?url=https%3A%2F%2Flore.kernel.org%2Fdri-devel%2F20210730183511.20080-1-tzimmermann%40suse.de%2FT%2F%23md0172b10bb588d8f20f4f456e304f08d2a4505f7data=05%7C01%7Cchristian.koenig%40amd.com%7Cc512a92c206f4af0691108da37f915a1%7C3dd8961fe4884e608e11a82d994e183d%7C0%7C0%7C637883840259373791%7CUnknown%7CTWFpbGZsb3d8eyJWIjoiMC4wLjAwMDAiLCJQIjoiV2luMzIiLCJBTiI6Ik1haWwiLCJXVCI6Mn0%3D%7C3000%7C%7C%7Csdata=6fXRY%2BWlo1I47tcuRHiamAZ9JGM%2FHaYTfyGnxNrqUts%3Dreserved=0
[2] 
https://nam11.safelinks.protection.outlook.com/?url=https%3A%2F%2Flore.kernel.org%2Fdri-devel%2F877dc0d9-c6c6-022c-20d8-14b33e863934%40suse.de%2Fdata=05%7C01%7Cchristian.koenig%40amd.com%7Cc512a92c206f4af0691108da37f915a1%7C3dd8961fe4884e608e11a82d994e183d%7C0%7C0%7C637883840259373791%7CUnknown%7CTWFpbGZsb3d8eyJWIjoiMC4wLjAwMDAiLCJQIjoiV2luMzIiLCJBTiI6Ik1haWwiLCJXVCI6Mn0%3D%7C3000%7C%7C%7Csdata=6iFYw5EtQNaSiPrHIAawBw87lj6M66o7j0ADY0ipifs%3Dreserved=0

Thomas Zimmermann (5):
   drm/gem: Share code between drm_gem_fb_{begin,end}_cpu_access()
   drm/gem: Ignore color planes that are unused by framebuffer format
   drm/gem-vram: Share code between GEM VRAM's _{prepare,cleanup}_fb()
   drm/gem-vram: Ignore planes that are unused by framebuffer format
   drm/gem: Warn on trying to use a non-existing framebuffer plane

  drivers/gpu/drm/drm_gem_atomic_helper.c  |   6 +-
  drivers/gpu/drm/drm_gem_framebuffer_helper.c | 104 +--
  drivers/gpu/drm/drm_gem_vram_helper.c|  54 ++
  include/drm/drm_gem_framebuffer_helper.h |  10 +-
  4 files changed, 92 insertions(+), 82 deletions(-)

[PATCH v2 5/5] drm/gem: Warn on trying to use a non-existing framebuffer plane

Warn if callers of drm_gem_fb_get_obj() try to use a GEM buffer for
a non-existing or unset plane.

Signed-off-by: Thomas Zimmermann 
Reviewed-by: Javier Martinez Canillas 
Tested-by: Noralf Trønnes 
---
 drivers/gpu/drm/drm_gem_framebuffer_helper.c | 6 +-
 1 file changed, 5 insertions(+), 1 deletion(-)

diff --git a/drivers/gpu/drm/drm_gem_framebuffer_helper.c 
b/drivers/gpu/drm/drm_gem_framebuffer_helper.c
index 6d3c26efdeeb..61339a9cd010 100644
--- a/drivers/gpu/drm/drm_gem_framebuffer_helper.c
+++ b/drivers/gpu/drm/drm_gem_framebuffer_helper.c
@@ -53,7 +53,11 @@ MODULE_IMPORT_NS(DMA_BUF);
 struct drm_gem_object *drm_gem_fb_get_obj(struct drm_framebuffer *fb,
  unsigned int plane)
 {
-   if (plane >= ARRAY_SIZE(fb->obj))
+   struct drm_device *dev = fb->dev;
+
+   if (drm_WARN_ON_ONCE(dev, plane >= ARRAY_SIZE(fb->obj)))
+   return NULL;
+   else if (drm_WARN_ON_ONCE(dev, !fb->obj[plane]))
return NULL;
 
return fb->obj[plane];
-- 
2.36.1

[PATCH v2 1/5] drm/gem: Share code between drm_gem_fb_{begin, end}_cpu_access()

The error-recovery code in drm_gem_fb_begin() is of the same pattern
as drm_gem_fb_end(). Implement both of them using an internal helper.
No functional changes.

v2:
* print additional information in error message (Javier)
* fix commit description (Javier)

Signed-off-by: Thomas Zimmermann 
Reviewed-by: Javier Martinez Canillas 
Tested-by: Noralf Trønnes 
---
 drivers/gpu/drm/drm_gem_framebuffer_helper.c | 63 +---
 1 file changed, 27 insertions(+), 36 deletions(-)

diff --git a/drivers/gpu/drm/drm_gem_framebuffer_helper.c 
b/drivers/gpu/drm/drm_gem_framebuffer_helper.c
index f4619803acd0..931828784dfe 100644
--- a/drivers/gpu/drm/drm_gem_framebuffer_helper.c
+++ b/drivers/gpu/drm/drm_gem_framebuffer_helper.c
@@ -403,6 +403,28 @@ void drm_gem_fb_vunmap(struct drm_framebuffer *fb,
 }
 EXPORT_SYMBOL(drm_gem_fb_vunmap);
 
+static void __drm_gem_fb_end_cpu_access(struct drm_framebuffer *fb, enum 
dma_data_direction dir,
+   unsigned int num_planes)
+{
+   struct dma_buf_attachment *import_attach;
+   struct drm_gem_object *obj;
+   int ret;
+
+   while (num_planes) {
+   --num_planes;
+   obj = drm_gem_fb_get_obj(fb, num_planes);
+   if (!obj)
+   continue;
+   import_attach = obj->import_attach;
+   if (!import_attach)
+   continue;
+   ret = dma_buf_end_cpu_access(import_attach->dmabuf, dir);
+   if (ret)
+   drm_err(fb->dev, "dma_buf_end_cpu_access(%u, %d) 
failed: %d\n",
+   ret, num_planes, dir);
+   }
+}
+
 /**
  * drm_gem_fb_begin_cpu_access - prepares GEM buffer objects for CPU access
  * @fb: the framebuffer
@@ -422,7 +444,7 @@ int drm_gem_fb_begin_cpu_access(struct drm_framebuffer *fb, 
enum dma_data_direct
struct dma_buf_attachment *import_attach;
struct drm_gem_object *obj;
size_t i;
-   int ret, ret2;
+   int ret;
 
for (i = 0; i < ARRAY_SIZE(fb->obj); ++i) {
obj = drm_gem_fb_get_obj(fb, i);
@@ -433,28 +455,13 @@ int drm_gem_fb_begin_cpu_access(struct drm_framebuffer 
*fb, enum dma_data_direct
continue;
ret = dma_buf_begin_cpu_access(import_attach->dmabuf, dir);
if (ret)
-   goto err_dma_buf_end_cpu_access;
+   goto err___drm_gem_fb_end_cpu_access;
}
 
return 0;
 
-err_dma_buf_end_cpu_access:
-   while (i) {
-   --i;
-   obj = drm_gem_fb_get_obj(fb, i);
-   if (!obj)
-   continue;
-   import_attach = obj->import_attach;
-   if (!import_attach)
-   continue;
-   ret2 = dma_buf_end_cpu_access(import_attach->dmabuf, dir);
-   if (ret2) {
-   drm_err(fb->dev,
-   "dma_buf_end_cpu_access() failed during error 
handling: %d\n",
-   ret2);
-   }
-   }
-
+err___drm_gem_fb_end_cpu_access:
+   __drm_gem_fb_end_cpu_access(fb, dir, i);
return ret;
 }
 EXPORT_SYMBOL(drm_gem_fb_begin_cpu_access);
@@ -472,23 +479,7 @@ EXPORT_SYMBOL(drm_gem_fb_begin_cpu_access);
  */
 void drm_gem_fb_end_cpu_access(struct drm_framebuffer *fb, enum 
dma_data_direction dir)
 {
-   size_t i = ARRAY_SIZE(fb->obj);
-   struct dma_buf_attachment *import_attach;
-   struct drm_gem_object *obj;
-   int ret;
-
-   while (i) {
-   --i;
-   obj = drm_gem_fb_get_obj(fb, i);
-   if (!obj)
-   continue;
-   import_attach = obj->import_attach;
-   if (!import_attach)
-   continue;
-   ret = dma_buf_end_cpu_access(import_attach->dmabuf, dir);
-   if (ret)
-   drm_err(fb->dev, "dma_buf_end_cpu_access() failed: 
%d\n", ret);
-   }
+   __drm_gem_fb_end_cpu_access(fb, dir, ARRAY_SIZE(fb->obj));
 }
 EXPORT_SYMBOL(drm_gem_fb_end_cpu_access);
 
-- 
2.36.1

[PATCH v2 4/5] drm/gem-vram: Ignore planes that are unused by framebuffer format

Only handle color planes that exist in a framebuffer's color format.
Ignore non-existing planes.

Signed-off-by: Thomas Zimmermann 
Reviewed-by: Javier Martinez Canillas 
Tested-by: Noralf Trønnes 
---
 drivers/gpu/drm/drm_gem_vram_helper.c | 19 ---
 1 file changed, 12 insertions(+), 7 deletions(-)

diff --git a/drivers/gpu/drm/drm_gem_vram_helper.c 
b/drivers/gpu/drm/drm_gem_vram_helper.c
index 566d139f0fa0..d607043716d3 100644
--- a/drivers/gpu/drm/drm_gem_vram_helper.c
+++ b/drivers/gpu/drm/drm_gem_vram_helper.c
@@ -667,17 +667,22 @@ int
 drm_gem_vram_plane_helper_prepare_fb(struct drm_plane *plane,
 struct drm_plane_state *new_state)
 {
-   size_t i;
+   struct drm_framebuffer *fb = new_state->fb;
struct drm_gem_vram_object *gbo;
+   struct drm_gem_object *obj;
+   unsigned int i;
int ret;
 
-   if (!new_state->fb)
+   if (!fb)
return 0;
 
-   for (i = 0; i < ARRAY_SIZE(new_state->fb->obj); ++i) {
-   if (!new_state->fb->obj[i])
-   continue;
-   gbo = drm_gem_vram_of_gem(new_state->fb->obj[i]);
+   for (i = 0; i < fb->format->num_planes; ++i) {
+   obj = drm_gem_fb_get_obj(fb, i);
+   if (!obj) {
+   ret = -EINVAL;
+   goto err_drm_gem_vram_unpin;
+   }
+   gbo = drm_gem_vram_of_gem(obj);
ret = drm_gem_vram_pin(gbo, DRM_GEM_VRAM_PL_FLAG_VRAM);
if (ret)
goto err_drm_gem_vram_unpin;
@@ -714,7 +719,7 @@ drm_gem_vram_plane_helper_cleanup_fb(struct drm_plane 
*plane,
if (!fb)
return;
 
-   __drm_gem_vram_plane_helper_cleanup_fb(plane, old_state, 
ARRAY_SIZE(fb->obj));
+   __drm_gem_vram_plane_helper_cleanup_fb(plane, old_state, 
fb->format->num_planes);
 }
 EXPORT_SYMBOL(drm_gem_vram_plane_helper_cleanup_fb);
 
-- 
2.36.1

[PATCH v2 2/5] drm/gem: Ignore color planes that are unused by framebuffer format

Only handle color planes that exist in a framebuffer's color format.
Ignore non-existing planes.

So far, several helpers assumed that all 4 planes are available and
silently ignored non-existing planes. This lead to subtil bugs with
uninitialized data in instances of struct iosys_map. [1]

Signed-off-by: Thomas Zimmermann 
Reviewed-by: Javier Martinez Canillas 
Tested-by: Noralf Trønnes 
Link: 
https://lore.kernel.org/dri-devel/20210730183511.20080-1-tzimmerm...@suse.de/T/#md0172b10bb588d8f20f4f456e304f08d2a4505f7
 # 1
---
 drivers/gpu/drm/drm_gem_atomic_helper.c  |  6 ++--
 drivers/gpu/drm/drm_gem_framebuffer_helper.c | 37 +++-
 include/drm/drm_gem_framebuffer_helper.h | 10 ++
 3 files changed, 27 insertions(+), 26 deletions(-)

diff --git a/drivers/gpu/drm/drm_gem_atomic_helper.c 
b/drivers/gpu/drm/drm_gem_atomic_helper.c
index a5026f617739..f16d60217c6c 100644
--- a/drivers/gpu/drm/drm_gem_atomic_helper.c
+++ b/drivers/gpu/drm/drm_gem_atomic_helper.c
@@ -169,8 +169,10 @@ int drm_gem_plane_helper_prepare_fb(struct drm_plane 
*plane,
struct drm_gem_object *obj = drm_gem_fb_get_obj(state->fb, i);
struct dma_fence *new;
 
-   if (WARN_ON_ONCE(!obj))
-   continue;
+   if (!obj) {
+   ret = -EINVAL;
+   goto error;
+   }
 
ret = dma_resv_get_singleton(obj->resv, usage, );
if (ret)
diff --git a/drivers/gpu/drm/drm_gem_framebuffer_helper.c 
b/drivers/gpu/drm/drm_gem_framebuffer_helper.c
index 931828784dfe..6d3c26efdeeb 100644
--- a/drivers/gpu/drm/drm_gem_framebuffer_helper.c
+++ b/drivers/gpu/drm/drm_gem_framebuffer_helper.c
@@ -92,9 +92,9 @@ drm_gem_fb_init(struct drm_device *dev,
  */
 void drm_gem_fb_destroy(struct drm_framebuffer *fb)
 {
-   size_t i;
+   unsigned int i;
 
-   for (i = 0; i < ARRAY_SIZE(fb->obj); i++)
+   for (i = 0; i < fb->format->num_planes; i++)
drm_gem_object_put(fb->obj[i]);
 
drm_framebuffer_cleanup(fb);
@@ -329,24 +329,26 @@ EXPORT_SYMBOL_GPL(drm_gem_fb_create_with_dirty);
  * The argument returns the addresses of the data stored in each BO. This
  * is different from @map if the framebuffer's offsets field is non-zero.
  *
+ * Both, @map and @data, must each refer to arrays with at least
+ * fb->format->num_planes elements.
+ *
  * See drm_gem_fb_vunmap() for unmapping.
  *
  * Returns:
  * 0 on success, or a negative errno code otherwise.
  */
-int drm_gem_fb_vmap(struct drm_framebuffer *fb,
-   struct iosys_map map[static DRM_FORMAT_MAX_PLANES],
-   struct iosys_map data[DRM_FORMAT_MAX_PLANES])
+int drm_gem_fb_vmap(struct drm_framebuffer *fb, struct iosys_map *map,
+   struct iosys_map *data)
 {
struct drm_gem_object *obj;
unsigned int i;
int ret;
 
-   for (i = 0; i < DRM_FORMAT_MAX_PLANES; ++i) {
+   for (i = 0; i < fb->format->num_planes; ++i) {
obj = drm_gem_fb_get_obj(fb, i);
if (!obj) {
-   iosys_map_clear([i]);
-   continue;
+   ret = -EINVAL;
+   goto err_drm_gem_vunmap;
}
ret = drm_gem_vmap(obj, [i]);
if (ret)
@@ -354,7 +356,7 @@ int drm_gem_fb_vmap(struct drm_framebuffer *fb,
}
 
if (data) {
-   for (i = 0; i < DRM_FORMAT_MAX_PLANES; ++i) {
+   for (i = 0; i < fb->format->num_planes; ++i) {
memcpy([i], [i], sizeof(data[i]));
if (iosys_map_is_null([i]))
continue;
@@ -385,10 +387,9 @@ EXPORT_SYMBOL(drm_gem_fb_vmap);
  *
  * See drm_gem_fb_vmap() for more information.
  */
-void drm_gem_fb_vunmap(struct drm_framebuffer *fb,
-  struct iosys_map map[static DRM_FORMAT_MAX_PLANES])
+void drm_gem_fb_vunmap(struct drm_framebuffer *fb, struct iosys_map *map)
 {
-   unsigned int i = DRM_FORMAT_MAX_PLANES;
+   unsigned int i = fb->format->num_planes;
struct drm_gem_object *obj;
 
while (i) {
@@ -443,13 +444,15 @@ int drm_gem_fb_begin_cpu_access(struct drm_framebuffer 
*fb, enum dma_data_direct
 {
struct dma_buf_attachment *import_attach;
struct drm_gem_object *obj;
-   size_t i;
+   unsigned int i;
int ret;
 
-   for (i = 0; i < ARRAY_SIZE(fb->obj); ++i) {
+   for (i = 0; i < fb->format->num_planes; ++i) {
obj = drm_gem_fb_get_obj(fb, i);
-   if (!obj)
-   continue;
+   if (!obj) {
+   ret = -EINVAL;
+   goto err___drm_gem_fb_end_cpu_access;
+   }
import_attach = obj->import_attach;
if (!import_attach)
continue;
@@ -479,7 +482,7 @@

[PATCH v2 0/5] drm: Ignore non-existing color planes in helpers

Some DRM helpers assume that all potential color planes of a framebuffer
are available; even if the color format didn't specify them. Non-existing
planes are silently ignored. This behavior is inconsistent with other
helpers and apparently leads to subtle bugs with uninitialized GEM buffer
mappings. [1]

Change all affected code to look at the framebuffer format's number of
color planes and only process these planes. The update has been discussed
in [2] before.

Tested with GEM SHMEM helpers on simpledrm and with GEM VRAM helpers on
ast.

v2:
* refactor GEM VRAM code before fixing it (Javier)
* print more error information in #1 (Javier)
* commit-message fixes (Javier)

[1] 
https://lore.kernel.org/dri-devel/20210730183511.20080-1-tzimmerm...@suse.de/T/#md0172b10bb588d8f20f4f456e304f08d2a4505f7
[2] 
https://lore.kernel.org/dri-devel/877dc0d9-c6c6-022c-20d8-14b33e863...@suse.de/

Thomas Zimmermann (5):
  drm/gem: Share code between drm_gem_fb_{begin,end}_cpu_access()
  drm/gem: Ignore color planes that are unused by framebuffer format
  drm/gem-vram: Share code between GEM VRAM's _{prepare,cleanup}_fb()
  drm/gem-vram: Ignore planes that are unused by framebuffer format
  drm/gem: Warn on trying to use a non-existing framebuffer plane

 drivers/gpu/drm/drm_gem_atomic_helper.c  |   6 +-
 drivers/gpu/drm/drm_gem_framebuffer_helper.c | 104 +--
 drivers/gpu/drm/drm_gem_vram_helper.c|  54 ++
 include/drm/drm_gem_framebuffer_helper.h |  10 +-
 4 files changed, 92 insertions(+), 82 deletions(-)

-- 
2.36.1

[PATCH v2 3/5] drm/gem-vram: Share code between GEM VRAM's _{prepare, cleanup}_fb()

The error-recovery code in drm_gem_vram_plane_helper_prepare_fb() is of
the same pattern as drm_gem_vram_plane_helper_cleanup_fb(). Implement
both of them using an internal helper. No functional changes.

Signed-off-by: Thomas Zimmermann 
---
 drivers/gpu/drm/drm_gem_vram_helper.c | 37 +--
 1 file changed, 23 insertions(+), 14 deletions(-)

diff --git a/drivers/gpu/drm/drm_gem_vram_helper.c 
b/drivers/gpu/drm/drm_gem_vram_helper.c
index 123045b58fec..566d139f0fa0 100644
--- a/drivers/gpu/drm/drm_gem_vram_helper.c
+++ b/drivers/gpu/drm/drm_gem_vram_helper.c
@@ -9,6 +9,7 @@
 #include 
 #include 
 #include 
+#include 
 #include 
 #include 
 #include 
@@ -630,6 +631,24 @@ EXPORT_SYMBOL(drm_gem_vram_driver_dumb_create);
  * Helpers for struct drm_plane_helper_funcs
  */
 
+static void __drm_gem_vram_plane_helper_cleanup_fb(struct drm_plane *plane,
+  struct drm_plane_state 
*state,
+  unsigned int num_planes)
+{
+   struct drm_gem_object *obj;
+   struct drm_gem_vram_object *gbo;
+   struct drm_framebuffer *fb = state->fb;
+
+   while (num_planes) {
+   --num_planes;
+   obj = drm_gem_fb_get_obj(fb, num_planes);
+   if (!obj)
+   continue;
+   gbo = drm_gem_vram_of_gem(obj);
+   drm_gem_vram_unpin(gbo);
+   }
+}
+
 /**
  * drm_gem_vram_plane_helper_prepare_fb() - \
  * Implements  drm_plane_helper_funcs.prepare_fb
@@ -671,11 +690,7 @@ drm_gem_vram_plane_helper_prepare_fb(struct drm_plane 
*plane,
return 0;
 
 err_drm_gem_vram_unpin:
-   while (i) {
-   --i;
-   gbo = drm_gem_vram_of_gem(new_state->fb->obj[i]);
-   drm_gem_vram_unpin(gbo);
-   }
+   __drm_gem_vram_plane_helper_cleanup_fb(plane, new_state, i);
return ret;
 }
 EXPORT_SYMBOL(drm_gem_vram_plane_helper_prepare_fb);
@@ -694,18 +709,12 @@ void
 drm_gem_vram_plane_helper_cleanup_fb(struct drm_plane *plane,
 struct drm_plane_state *old_state)
 {
-   size_t i;
-   struct drm_gem_vram_object *gbo;
+   struct drm_framebuffer *fb = old_state->fb;
 
-   if (!old_state->fb)
+   if (!fb)
return;
 
-   for (i = 0; i < ARRAY_SIZE(old_state->fb->obj); ++i) {
-   if (!old_state->fb->obj[i])
-   continue;
-   gbo = drm_gem_vram_of_gem(old_state->fb->obj[i]);
-   drm_gem_vram_unpin(gbo);
-   }
+   __drm_gem_vram_plane_helper_cleanup_fb(plane, old_state, 
ARRAY_SIZE(fb->obj));
 }
 EXPORT_SYMBOL(drm_gem_vram_plane_helper_cleanup_fb);
 
-- 
2.36.1

nouveau lockdep deadlock report with 5.18-rc6

Hi All,

I just noticed the below lockdep possible deadlock report with a 5.18-rc6
kernel on a Dell Latitude E6430 laptop with the following nvidia GPU:

01:00.0 VGA compatible controller [0300]: NVIDIA Corporation GF108GLM [NVS 
5200M] [10de:0dfc] (rev a1)
01:00.1 Audio device [0403]: NVIDIA Corporation GF108 High Definition Audio 
Controller [10de:0bea] (rev a1)

This is with the laptop in Optimus mode, so with the Intel integrated
gfx from the i5-3320M CPU driving the LCD panel and with nothing connected
to the HDMI connector, which is always routed to the NVIDIA GPU on this
laptop.

The lockdep possible deadlock warning seems to happen when the NVIDIA GPU
is runtime suspended shortly after gdm has loaded:

[   24.859171] ==
[   24.859173] WARNING: possible circular locking dependency detected
[   24.859175] 5.18.0-rc6+ #34 Tainted: GE
[   24.859178] --
[   24.859179] kworker/1:1/46 is trying to acquire lock:
[   24.859181] 92b0c0ee0518 (>mutex){+.+.}-{3:3}, at: 
nouveau_vga_lastclose+0x910/0x1030 [nouveau]
[   24.859231] 
   but task is already holding lock:
[   24.859233] 92b0c4bf35a0 (reservation_ww_class_mutex){+.+.}-{3:3}, at: 
ttm_bo_wait+0x7d/0x140 [ttm]
[   24.859243] 
   which lock already depends on the new lock.

[   24.859244] 
   the existing dependency chain (in reverse order) is:
[   24.859246] 
   -> #1 (reservation_ww_class_mutex){+.+.}-{3:3}:
[   24.859249]__ww_mutex_lock.constprop.0+0xb3/0xfb0
[   24.859256]ww_mutex_lock+0x38/0xa0
[   24.859259]nouveau_bo_pin+0x30/0x380 [nouveau]
[   24.859297]nouveau_channel_del+0x1d7/0x3e0 [nouveau]
[   24.859328]nouveau_channel_new+0x48/0x730 [nouveau]
[   24.859358]nouveau_abi16_ioctl_channel_alloc+0x113/0x360 [nouveau]
[   24.859389]drm_ioctl_kernel+0xa1/0x150
[   24.859392]drm_ioctl+0x21c/0x410
[   24.859395]nouveau_drm_ioctl+0x56/0x1820 [nouveau]
[   24.859431]__x64_sys_ioctl+0x8d/0xc0
[   24.859436]do_syscall_64+0x5b/0x80
[   24.859440]entry_SYSCALL_64_after_hwframe+0x44/0xae
[   24.859443] 
   -> #0 (>mutex){+.+.}-{3:3}:
[   24.859446]__lock_acquire+0x12e2/0x1f90
[   24.859450]lock_acquire+0xad/0x290
[   24.859453]__mutex_lock+0x90/0x830
[   24.859456]nouveau_vga_lastclose+0x910/0x1030 [nouveau]
[   24.859493]ttm_bo_move_to_lru_tail+0x32c/0x980 [ttm]
[   24.859498]ttm_mem_evict_first+0x25c/0x4b0 [ttm]
[   24.859503]ttm_resource_manager_evict_all+0x93/0x1b0 [ttm]
[   24.859509]nouveau_debugfs_fini+0x161/0x260 [nouveau]
[   24.859545]nouveau_drm_ioctl+0xa4a/0x1820 [nouveau]
[   24.859582]pci_pm_runtime_suspend+0x5c/0x180
[   24.859585]__rpm_callback+0x48/0x1b0
[   24.859589]rpm_callback+0x5a/0x70
[   24.859591]rpm_suspend+0x10a/0x6f0
[   24.859594]pm_runtime_work+0xa0/0xb0
[   24.859596]process_one_work+0x254/0x560
[   24.859601]worker_thread+0x4f/0x390
[   24.859604]kthread+0xe6/0x110
[   24.859607]ret_from_fork+0x22/0x30
[   24.859611] 
   other info that might help us debug this:

[   24.859612]  Possible unsafe locking scenario:

[   24.859613]CPU0CPU1
[   24.859615]
[   24.859616]   lock(reservation_ww_class_mutex);
[   24.859618]lock(>mutex);
[   24.859620]lock(reservation_ww_class_mutex);
[   24.859622]   lock(>mutex);
[   24.859624] 
*** DEADLOCK ***

[   24.859625] 3 locks held by kworker/1:1/46:
[   24.859627]  #0: 92b0c0bb4338 ((wq_completion)pm){+.+.}-{0:0}, at: 
process_one_work+0x1d0/0x560
[   24.859634]  #1: a8ffc02dfe80 
((work_completion)(>power.work)){+.+.}-{0:0}, at: 
process_one_work+0x1d0/0x560
[   24.859641]  #2: 92b0c4bf35a0 (reservation_ww_class_mutex){+.+.}-{3:3}, 
at: ttm_bo_wait+0x7d/0x140 [ttm]
[   24.859649] 
   stack backtrace:
[   24.859651] CPU: 1 PID: 46 Comm: kworker/1:1 Tainted: GE 
5.18.0-rc6+ #34
[   24.859654] Hardware name: Dell Inc. Latitude E6430/0H3MT5, BIOS A21 
05/08/2017
[   24.859656] Workqueue: pm pm_runtime_work
[   24.859660] Call Trace:
[   24.859662]  
[   24.859665]  dump_stack_lvl+0x5b/0x74
[   24.859669]  check_noncircular+0xdf/0x100
[   24.859672]  ? register_lock_class+0x38/0x470
[   24.859678]  __lock_acquire+0x12e2/0x1f90
[   24.859683]  lock_acquire+0xad/0x290
[   24.859686]  ? nouveau_vga_lastclose+0x910/0x1030 [nouveau]
[   24.859724]  ? lock_is_held_type+0xa6/0x120
[   24.859730]  __mutex_lock+0x90/0x830
[   24.859733]  ? nouveau_vga_lastclose+0x910/0x1030 [nouveau]
[   24.859770]  ? nvif_vmm_map+0x114/0x130 [nouveau]
[   24.859791]  ? nouveau_vga_lastclose+0x910/0x1030

[PATCH v4] drm/doc: add rfc section for small BAR uapi

2022-05-17 Thread Matthew Auld

Add an entry for the new uapi needed for small BAR on DG2+.

v2:
  - Some spelling fixes and other small tweaks. (Akeem & Thomas)
  - Rework error capture interactions, including no longer needing
NEEDS_CPU_ACCESS for objects marked for capture. (Thomas)
  - Add probed_cpu_visible_size. (Lionel)
v3:
  - Drop the vma query for now.
  - Add unallocated_cpu_visible_size as part of the region query.
  - Improve the docs some more, including documenting the expected
behaviour on older kernels, since this came up in some offline
discussion.
v4:
  - Various improvements all over. (Tvrtko)

Signed-off-by: Matthew Auld 
Cc: Thomas Hellström 
Cc: Lionel Landwerlin 
Cc: Tvrtko Ursulin 
Cc: Jon Bloomfield 
Cc: Daniel Vetter 
Cc: Jon Bloomfield 
Cc: Jordan Justen 
Cc: Kenneth Graunke 
Cc: Akeem G Abodunrin 
Cc: mesa-...@lists.freedesktop.org
---
 Documentation/gpu/rfc/i915_small_bar.h   | 189 +++
 Documentation/gpu/rfc/i915_small_bar.rst |  47 ++
 Documentation/gpu/rfc/index.rst  |   4 +
 3 files changed, 240 insertions(+)
 create mode 100644 Documentation/gpu/rfc/i915_small_bar.h
 create mode 100644 Documentation/gpu/rfc/i915_small_bar.rst

diff --git a/Documentation/gpu/rfc/i915_small_bar.h 
b/Documentation/gpu/rfc/i915_small_bar.h
new file mode 100644
index ..c676640b23ef
--- /dev/null
+++ b/Documentation/gpu/rfc/i915_small_bar.h
@@ -0,0 +1,189 @@
+/**
+ * struct __drm_i915_memory_region_info - Describes one region as known to the
+ * driver.
+ *
+ * Note this is using both struct drm_i915_query_item and struct 
drm_i915_query.
+ * For this new query we are adding the new query id 
DRM_I915_QUERY_MEMORY_REGIONS
+ * at _i915_query_item.query_id.
+ */
+struct __drm_i915_memory_region_info {
+   /** @region: The class:instance pair encoding */
+   struct drm_i915_gem_memory_class_instance region;
+
+   /** @rsvd0: MBZ */
+   __u32 rsvd0;
+
+   /**
+* @probed_size: Memory probed by the driver (-1 = unknown)
+*
+* Note that it should not be possible to ever encounter a zero value
+* here, also note that no current region type will ever return -1 here.
+* Although for future region types, this might be a possibility. The
+* same applies to the other size fields.
+*/
+   __u64 probed_size;
+
+   /**
+* @unallocated_size: Estimate of memory remaining (-1 = unknown)
+*
+* Requires CAP_PERFMON or CAP_SYS_ADMIN to get reliable accounting.
+* Without this (or if this is an older kernel) the value here will
+* always equal the @probed_size. Note this is only currently tracked
+* for I915_MEMORY_CLASS_DEVICE regions (for other types the value here
+* will always equal the @probed_size).
+*/
+   __u64 unallocated_size;
+
+   union {
+   /** @rsvd1: MBZ */
+   __u64 rsvd1[8];
+   struct {
+   /**
+* @probed_cpu_visible_size: Memory probed by the driver
+* that is CPU accessible. (-1 = unknown).
+*
+* This will be always be <= @probed_size, and the
+* remainder (if there is any) will not be CPU
+* accessible.
+*
+* On systems without small BAR, the @probed_size will
+* always equal the @probed_cpu_visible_size, since all
+* of it will be CPU accessible.
+*
+* Note this is only tracked for
+* I915_MEMORY_CLASS_DEVICE regions (for other types the
+* value here will always equal the @probed_size).
+*
+* Note that if the value returned here is zero, then
+* this must be an old kernel which lacks the relevant
+* small-bar uAPI support (including
+* I915_GEM_CREATE_EXT_FLAG_NEEDS_CPU_ACCESS), but on
+* such systems we should never actually end up with a
+* small BAR configuration, assuming we are able to load
+* the kernel module. Hence it should be safe to treat
+* this the same as when @probed_cpu_visible_size ==
+* @probed_size.
+*/
+   __u64 probed_cpu_visible_size;
+
+   /**
+* @unallocated_cpu_visible_size: Estimate of CPU
+* visible memory remaining (-1 = unknown).
+*
+* Note this is only tracked for
+* I915_MEMORY_CLASS_DEVICE regions (for other types the
+* value here will always equal the
+

Re: [PATCH v20 8/8] soc: mediatek: remove DDP_DOMPONENT_DITHER from enum

2022-05-17 Thread Matthias Brugger





On 15/05/2022 00:45, Chun-Kuang Hu wrote:

Hi, Matthias:

Matthias Brugger  於 2022年5月13日 週五 下午3:42寫道：


Hi Chun-Kuang,

On 02/05/2022 00:54, Chun-Kuang Hu wrote:

Hi, Matthias:

Matthias Brugger  於 2022年4月22日 週五 下午8:42寫道：




On 19/04/2022 11:41, jason-jh.lin wrote:

After mmsys and drm change DITHER enum to DDP_COMPONENT_DITHER0,
mmsys header can remove the useless DDP_COMPONENT_DITHER enum.

Signed-off-by: jason-jh.lin 
Reviewed-by: AngeloGioacchino Del Regno 



Acked-by: Matthias Brugger 

Chun-Kuang, I think it would make sense to take that through your tree as it
depends on the previous patches.

I provide you a stable tag so that you can take it:
v5.18-next-vdso0-stable-tag


After I take this tag, I find one checkpatch warning:

WARNING: DT compatible string "mediatek,mt8195-mmsys" appears
un-documented -- check ./Documentation/devicetree/bindings/
#670: FILE: drivers/soc/mediatek/mtk-mmsys.c:390:
+   .compatible = "mediatek,mt8195-mmsys",

I think this tag lost one binding patch, it's better that this tag has
no this warning.



Sorry for the late reply I was sick.
The warning is, because the stable branch misses commit:
https://git.kernel.org/pub/scm/linux/kernel/git/matthias.bgg/linux.git/commit/?h=v5.18-next/soc=81c5a41d10b968ea89d5f44fe1e5c2fc70289209

So it's not a real issue and will go away once our branches land in upstream.
Is it OK for you to ignore the issue?


It's OK for me, but the patch would go through different maintainer's
tree and I'm not sure it's OK for all of them. So I would wait for the
necessary patch land in upstream.



Ok makes sense. Sorry for the bad coordination from my side on this.

Regards,
Matthias


Regards,
Chun-Kuang.



Regards,
Matthias


Regards,
Chun-Kuang.



Regards,
Matthias


---
include/linux/soc/mediatek/mtk-mmsys.h | 3 +--
1 file changed, 1 insertion(+), 2 deletions(-)

diff --git a/include/linux/soc/mediatek/mtk-mmsys.h 
b/include/linux/soc/mediatek/mtk-mmsys.h
index 59117d970daf..fb719fd1281c 100644
--- a/include/linux/soc/mediatek/mtk-mmsys.h
+++ b/include/linux/soc/mediatek/mtk-mmsys.h
@@ -16,8 +16,7 @@ enum mtk_ddp_comp_id {
DDP_COMPONENT_CCORR,
DDP_COMPONENT_COLOR0,
DDP_COMPONENT_COLOR1,
- DDP_COMPONENT_DITHER,
- DDP_COMPONENT_DITHER0 = DDP_COMPONENT_DITHER,
+ DDP_COMPONENT_DITHER0,
DDP_COMPONENT_DITHER1,
DDP_COMPONENT_DP_INTF0,
DDP_COMPONENT_DP_INTF1,

Re: Tackling the indefinite/user DMA fence problem

2022-05-17 Thread Christian König

Am 09.05.22 um 16:10 schrieb Daniel Vetter:

On Mon, May 09, 2022 at 08:56:41AM +0200, Christian König wrote:

Am 04.05.22 um 12:08 schrieb Daniel Vetter:

On Mon, May 02, 2022 at 06:37:07PM +0200, Christian König wrote:

Hello everyone,

it's a well known problem that the DMA-buf subsystem mixed
synchronization and memory management requirements into the same
dma_fence and dma_resv objects. Because of this dma_fence objects need
to guarantee that they complete within a finite amount of time or
otherwise the system can easily deadlock.

One of the few good things about this problem is that it is really good
understood by now.

Daniel and others came up with some documentation:
https://dri.freedesktop.org/docs/drm/driver-api/dma-buf.html?highlight=dma_buf#indefinite-dma-fences

And Jason did an excellent presentation about that problem on last years
LPC: https://lpc.events/event/11/contributions/1115/

Based on that we had been able to reject new implementations of
infinite/user DMA fences and mitigate the effect of the few existing
ones.

The still remaining down side is that we don't have a way of using user
fences as dependency in both the explicit (sync_file, drm_syncobj) as
well as the implicit (dma_resv) synchronization objects, resulting in
numerous problems and limitations for things like HMM, user queues
etc

This patch set here now tries to tackle this problem by untangling the
synchronization from the memory management. What it does *not* try to do
is to fix the existing kernel fences, because I think we now can all
agree on that this isn't really possible.

To archive this goal what I do in this patch set is to add some parallel
infrastructure to cleanly separate normal kernel dma_fence objects from
indefinite/user fences:

1. It introduce a DMA_FENCE_FLAG_USER define (after renaming some
existing driver defines). To note that a certain dma_fence is an user
fence and *must* be ignore by memory management and never used as
dependency for normal none user dma_fence objects.

2. The dma_fence_array and dma_fence_chain containers are modified so
that they are marked as user fences whenever any of their contained
fences are an user fence.

3. The dma_resv object gets a new DMA_RESV_USAGE_USER flag which must be
used with indefinite/user fences and separates those into it's own
synchronization domain.

4. The existing dma_buf_poll_add_cb() function is modified so that
indefinite/user fences are included in the polling.

5. The sync_file synchronization object is modified so that we
essentially have two fence streams instead of just one.

6. The drm_syncobj is modified in a similar way. User fences are just
ignored unless the driver explicitly states support to wait for them.

7. The DRM subsystem gains a new DRIVER_USER_FENCE flag which drivers
can use to indicate the need for user fences. If user fences are used
the atomic mode setting starts to support user fences as IN/OUT fences.

8. Lockdep is used at various critical locations to ensure that nobody
ever tries to mix user fences with non user fences.

The general approach is to just ignore user fences unless a driver
stated explicitely support for them.

On top of all of this I've hacked amdgpu so that we add the resulting CS
fence only as kernel dependency to the dma_resv object and an additional
wrapped up with a dma_fence_array and a stub user fence.

The result is that the newly added atomic modeset functions now
correctly wait for the user fence to complete before doing the flip. And
dependent CS don't pipeline any more, but rather block on the CPU before
submitting work.

After tons of debugging and testing everything now seems to not go up in
flames immediately and even lockdep is happy with the annotations.

I'm perfectly aware that this is probably by far the most controversial
patch set I've ever created and I really wish we wouldn't need it. But
we certainly have the requirement for this and I don't see much other
chance to get that working in an UAPI compatible way.

Thoughts/comments?

I think you need to type up the goal or exact problem statement you're
trying to solve first. What you typed up is a solution along the lines of
"try to stuff userspace memory fences into dma_fence and see how horrible
it all is", and that's certainly an interesting experiment, but what are
you trying to solve with it?

Well, good point. I explained to much how it works, but now why.

In general I would describe the goal as: Providing a standard kernel
infrastructure for user fences.

So on that goal the part I fully agree on is that drm_syncobj can (and
should imo) be able to contain userspace memory fences. The uapi semantics
and everything is already fully set up to support that, but maybe with
reduced performance: Non-aware userspace (or when you don't trust the
supplier of the umf) needs to block when looking up the fence, and the
dma_fence returned will always be signalled already. But that's just a
mild performance issue (and vk drivers

Re: [PATCH 3/4] drm/gem-vram: Ignore planes that are unused by framebuffer format


Hi

Am 16.05.22 um 15:34 schrieb Javier Martinez Canillas:

On 5/9/22 10:16, Thomas Zimmermann wrote:

Only handle color planes that exist in a framebuffer's color format.
Ignore non-existing planes.

Signed-off-by: Thomas Zimmermann 
---


Reviewed-by: Javier Martinez Canillas 


@@ -673,7 +679,10 @@ drm_gem_vram_plane_helper_prepare_fb(struct drm_plane 
*plane,
  err_drm_gem_vram_unpin:
while (i) {
--i;
-   gbo = drm_gem_vram_of_gem(new_state->fb->obj[i]);
+   obj = drm_gem_fb_get_obj(fb, i);
+   if (!obj)
+   continue;
+   gbo = drm_gem_vram_of_gem(obj);
drm_gem_vram_unpin(gbo);


The code in this error path to unpin the GEM vram objects...


}
return ret;
@@ -694,16 +703,19 @@ void
  drm_gem_vram_plane_helper_cleanup_fb(struct drm_plane *plane,
 struct drm_plane_state *old_state)
  {
-   size_t i;
+   struct drm_framebuffer *fb = old_state->fb;
struct drm_gem_vram_object *gbo;
+   struct drm_gem_object *obj;
+   unsigned int i;
  
-	if (!old_state->fb)

+   if (!fb)
return;
  
-	for (i = 0; i < ARRAY_SIZE(old_state->fb->obj); ++i) {

-   if (!old_state->fb->obj[i])
+   for (i = 0; i < fb->format->num_planes; ++i) {
+   obj = drm_gem_fb_get_obj(fb, i);
+   if (!obj)
continue;
-   gbo = drm_gem_vram_of_gem(old_state->fb->obj[i]);
+   gbo = drm_gem_vram_of_gem(obj);
drm_gem_vram_unpin(gbo);


... and this, seems to be basically the same.

So maybe as a follow-up you can do a similar cleanup like you did in patch
1/4 and use a helper function to handle both.


I was thinking the same when I wrote this patch and somehow decided 
against it.  But since you also mention it, I'm going to add this change 
to the patchset.


Best regards
Thomas





--
Thomas Zimmermann
Graphics Driver Developer
SUSE Software Solutions Germany GmbH
Maxfeldstr. 5, 90409 Nürnberg, Germany
(HRB 36809, AG Nürnberg)
Geschäftsführer: Ivo Totev


OpenPGP_signature
Description: OpenPGP digital signature

Re: [PATCH 1/4] drm/gem: Share code between drm_gem_fb_{begin, end}_cpu_access()


Hi Javier

Am 16.05.22 um 15:00 schrieb Javier Martinez Canillas:

Hello Thomas,

On 5/9/22 10:15, Thomas Zimmermann wrote:

The error-recovery code in drm_gem_fb_begin() is the same pattern
as drm_gem_fb_end(). Implement both this an internal helper. No


Maybe "both of these using using an" or something like that ?


Of course.




functional changes.

Signed-off-by: Thomas Zimmermann 
---
  drivers/gpu/drm/drm_gem_framebuffer_helper.c | 62 
  1 file changed, 26 insertions(+), 36 deletions(-)



Nice cleanup. For the patch:

Reviewed-by: Javier Martinez Canillas 

I just have a few minor comments below.


diff --git a/drivers/gpu/drm/drm_gem_framebuffer_helper.c 
b/drivers/gpu/drm/drm_gem_framebuffer_helper.c
index f4619803acd0..2fcacab9f812 100644
--- a/drivers/gpu/drm/drm_gem_framebuffer_helper.c
+++ b/drivers/gpu/drm/drm_gem_framebuffer_helper.c
@@ -403,6 +403,27 @@ void drm_gem_fb_vunmap(struct drm_framebuffer *fb,
  }
  EXPORT_SYMBOL(drm_gem_fb_vunmap);
  
+static void __drm_gem_fb_end_cpu_access(struct drm_framebuffer *fb, enum dma_data_direction dir,

+   unsigned int num_planes)
+{
+   struct dma_buf_attachment *import_attach;
+   struct drm_gem_object *obj;
+   int ret;
+
+   while (num_planes) {
+   --num_planes;
+   obj = drm_gem_fb_get_obj(fb, num_planes);
+   if (!obj)
+   continue;
+   import_attach = obj->import_attach;
+   if (!import_attach)
+   continue;
+   ret = dma_buf_end_cpu_access(import_attach->dmabuf, dir);
+   if (ret)
+   drm_err(fb->dev, "dma_buf_end_cpu_access() failed: 
%d\n", ret);


I wonder if would be useful to also print the plane index and access mode here.


Ok.




+   }
+}
+
  /**
   * drm_gem_fb_begin_cpu_access - prepares GEM buffer objects for CPU access
   * @fb: the framebuffer
@@ -422,7 +443,7 @@ int drm_gem_fb_begin_cpu_access(struct drm_framebuffer *fb, 
enum dma_data_direct
struct dma_buf_attachment *import_attach;
struct drm_gem_object *obj;
size_t i;
-   int ret, ret2;
+   int ret;
  
  	for (i = 0; i < ARRAY_SIZE(fb->obj); ++i) {

obj = drm_gem_fb_get_obj(fb, i);
@@ -433,28 +454,13 @@ int drm_gem_fb_begin_cpu_access(struct drm_framebuffer 
*fb, enum dma_data_direct
continue;
ret = dma_buf_begin_cpu_access(import_attach->dmabuf, dir);
if (ret)
-   goto err_dma_buf_end_cpu_access;
+   goto err___drm_gem_fb_end_cpu_access;


I wouldn't rename this. As I read it, the goto label doesn't denote what
function is called but rather what action is being taken in an error path.

Since finally what's being done is a dma_buf_end_cpu_access in the helper,
I think that just keeping err_dma_buf_end_cpu_access is enough. Also, the
additional underscores added make it harder to read IMO.


I usually name the goto labels after the function that comes next. It's 
not really pretty here, but being inconsistent would probably annoy me 
more than the underscores.


Best regards
Thomas





--
Thomas Zimmermann
Graphics Driver Developer
SUSE Software Solutions Germany GmbH
Maxfeldstr. 5, 90409 Nürnberg, Germany
(HRB 36809, AG Nürnberg)
Geschäftsführer: Ivo Totev


OpenPGP_signature
Description: OpenPGP digital signature

Re: [PATCH v4 2/5] drm: Add HPD state to drm_connector_oob_hotplug_event()

2022-05-17 Thread Heikki Krogerus

On Mon, May 02, 2022 at 09:53:13AM -0700, Bjorn Andersson wrote:
> In some implementations, such as the Qualcomm platforms, the display
> driver has no way to query the current HPD state and as such it's
> impossible to distinguish between disconnect and attention events.
> 
> Add a parameter to drm_connector_oob_hotplug_event() to pass the HPD
> state.
> 
> Also push the test for unchanged state in the displayport altmode driver
> into the i915 driver, to allow other drivers to act upon each update.
> 
> Signed-off-by: Bjorn Andersson 

Acked-by: Heikki Krogerus 

> ---
> 
> Changes since v3:
> - Transition to drm_connector_status instead of custom hpd_state 
> 
>  drivers/gpu/drm/drm_connector.c  |  6 --
>  drivers/gpu/drm/i915/display/intel_dp.c  | 17 ++---
>  drivers/gpu/drm/i915/i915_drv.h  |  3 +++
>  drivers/usb/typec/altmodes/displayport.c | 10 +++---
>  include/drm/drm_connector.h  |  6 --
>  5 files changed, 28 insertions(+), 14 deletions(-)
> 
> diff --git a/drivers/gpu/drm/drm_connector.c b/drivers/gpu/drm/drm_connector.c
> index 1c48d162c77e..e86c69f0640f 100644
> --- a/drivers/gpu/drm/drm_connector.c
> +++ b/drivers/gpu/drm/drm_connector.c
> @@ -2794,6 +2794,7 @@ struct drm_connector 
> *drm_connector_find_by_fwnode(struct fwnode_handle *fwnode)
>  /**
>   * drm_connector_oob_hotplug_event - Report out-of-band hotplug event to 
> connector
>   * @connector_fwnode: fwnode_handle to report the event on
> + * @status: hot plug detect logical state
>   *
>   * On some hardware a hotplug event notification may come from outside the 
> display
>   * driver / device. An example of this is some USB Type-C setups where the 
> hardware
> @@ -2803,7 +2804,8 @@ struct drm_connector 
> *drm_connector_find_by_fwnode(struct fwnode_handle *fwnode)
>   * This function can be used to report these out-of-band events after 
> obtaining
>   * a drm_connector reference through calling drm_connector_find_by_fwnode().
>   */
> -void drm_connector_oob_hotplug_event(struct fwnode_handle *connector_fwnode)
> +void drm_connector_oob_hotplug_event(struct fwnode_handle *connector_fwnode,
> +  enum drm_connector_status status)
>  {
>   struct drm_connector *connector;
>  
> @@ -2812,7 +2814,7 @@ void drm_connector_oob_hotplug_event(struct 
> fwnode_handle *connector_fwnode)
>   return;
>  
>   if (connector->funcs->oob_hotplug_event)
> - connector->funcs->oob_hotplug_event(connector);
> + connector->funcs->oob_hotplug_event(connector, status);
>  
>   drm_connector_put(connector);
>  }
> diff --git a/drivers/gpu/drm/i915/display/intel_dp.c 
> b/drivers/gpu/drm/i915/display/intel_dp.c
> index e4a79c11fd25..56cc023f7bbd 100644
> --- a/drivers/gpu/drm/i915/display/intel_dp.c
> +++ b/drivers/gpu/drm/i915/display/intel_dp.c
> @@ -4951,15 +4951,26 @@ static int intel_dp_connector_atomic_check(struct 
> drm_connector *conn,
>   return intel_modeset_synced_crtcs(state, conn);
>  }
>  
> -static void intel_dp_oob_hotplug_event(struct drm_connector *connector)
> +static void intel_dp_oob_hotplug_event(struct drm_connector *connector,
> +enum drm_connector_status hpd_state)
>  {
>   struct intel_encoder *encoder = 
> intel_attached_encoder(to_intel_connector(connector));
>   struct drm_i915_private *i915 = to_i915(connector->dev);
> + bool hpd_high = hpd_state == connector_status_connected;
> + unsigned int hpd_pin = encoder->hpd_pin;
> + bool need_work = false;
>  
>   spin_lock_irq(>irq_lock);
> - i915->hotplug.event_bits |= BIT(encoder->hpd_pin);
> + if (hpd_high != test_bit(hpd_pin, 
> >hotplug.oob_hotplug_last_state)) {
> + i915->hotplug.event_bits |= BIT(hpd_pin);
> +
> + __assign_bit(hpd_pin, >hotplug.oob_hotplug_last_state, 
> hpd_high);
> + need_work = true;
> + }
>   spin_unlock_irq(>irq_lock);
> - queue_delayed_work(system_wq, >hotplug.hotplug_work, 0);
> +
> + if (need_work)
> + queue_delayed_work(system_wq, >hotplug.hotplug_work, 0);
>  }
>  
>  static const struct drm_connector_funcs intel_dp_connector_funcs = {
> diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h
> index 24111bf42ce0..96c088bb5522 100644
> --- a/drivers/gpu/drm/i915/i915_drv.h
> +++ b/drivers/gpu/drm/i915/i915_drv.h
> @@ -135,6 +135,9 @@ struct i915_hotplug {
>   /* Whether or not to count short HPD IRQs in HPD storms */
>   u8 hpd_short_storm_enabled;
>  
> + /* Last state reported by oob_hotplug_event for each encoder */
> + unsigned long oob_hotplug_last_state;
> +
>   /*
>* if we get a HPD irq from DP and a HPD irq from non-DP
>* the non-DP HPD could block the workqueue on a mode config
> diff --git a/drivers/usb/typec/altmodes/displayport.c 
> b/drivers/usb/typec/altmodes/displayport.c
> index c1d8c23baa39..9360ca177c7d

Re: [PATCH v3] drm/doc: add rfc section for small BAR uapi

2022-05-17 Thread Lionel Landwerlin


On 17/05/2022 12:23, Tvrtko Ursulin wrote:


On 17/05/2022 09:55, Lionel Landwerlin wrote:

On 17/05/2022 11:29, Tvrtko Ursulin wrote:


On 16/05/2022 19:11, Matthew Auld wrote:

Add an entry for the new uapi needed for small BAR on DG2+.

v2:
   - Some spelling fixes and other small tweaks. (Akeem & Thomas)
   - Rework error capture interactions, including no longer needing
 NEEDS_CPU_ACCESS for objects marked for capture. (Thomas)
   - Add probed_cpu_visible_size. (Lionel)
v3:
   - Drop the vma query for now.
   - Add unallocated_cpu_visible_size as part of the region query.
   - Improve the docs some more, including documenting the expected
 behaviour on older kernels, since this came up in some offline
 discussion.

Signed-off-by: Matthew Auld 
Cc: Thomas Hellström 
Cc: Lionel Landwerlin 
Cc: Tvrtko Ursulin 
Cc: Jon Bloomfield 
Cc: Daniel Vetter 
Cc: Jon Bloomfield 
Cc: Jordan Justen 
Cc: Kenneth Graunke 
Cc: Akeem G Abodunrin 
Cc: mesa-...@lists.freedesktop.org
---
  Documentation/gpu/rfc/i915_small_bar.h   | 164 
+++

  Documentation/gpu/rfc/i915_small_bar.rst |  47 +++
  Documentation/gpu/rfc/index.rst  |   4 +
  3 files changed, 215 insertions(+)
  create mode 100644 Documentation/gpu/rfc/i915_small_bar.h
  create mode 100644 Documentation/gpu/rfc/i915_small_bar.rst

diff --git a/Documentation/gpu/rfc/i915_small_bar.h 
b/Documentation/gpu/rfc/i915_small_bar.h

new file mode 100644
index ..4079d287750b
--- /dev/null
+++ b/Documentation/gpu/rfc/i915_small_bar.h
@@ -0,0 +1,164 @@
+/**
+ * struct __drm_i915_memory_region_info - Describes one region as 
known to the

+ * driver.
+ *
+ * Note this is using both struct drm_i915_query_item and struct 
drm_i915_query.
+ * For this new query we are adding the new query id 
DRM_I915_QUERY_MEMORY_REGIONS

+ * at _i915_query_item.query_id.
+ */
+struct __drm_i915_memory_region_info {
+    /** @region: The class:instance pair encoding */
+    struct drm_i915_gem_memory_class_instance region;
+
+    /** @rsvd0: MBZ */
+    __u32 rsvd0;
+
+    /** @probed_size: Memory probed by the driver (-1 = unknown) */
+    __u64 probed_size;


Is -1 possible today or when it will be? For system memory it 
appears zeroes are returned today so that has to stay I think. Does 
it effectively mean userspace has to consider both 0 and -1 as 
unknown is the question.



I raised this on v2. As far as I can tell there are no situation 
where we would get -1.


Is it really probed_size=0 on smem?? It's not the case on the 
internal branch.


My bad, I misread the arguments to intel_memory_region_create while 
grepping:


struct intel_memory_region *i915_gem_shmem_setup(struct 
drm_i915_private *i915,

 u16 type, u16 instance)
{
return intel_memory_region_create(i915, 0,
  totalram_pages() << PAGE_SHIFT,
  PAGE_SIZE, 0, 0,
  type, instance,
  _region_ops);

I saw "0, 0" and wrongly assumed that would be the data, since it 
matched with my mental model and the comment against unallocated_size 
saying it's only tracked for device memory.


Although I'd say it is questionable for i915 to return this data. I 
wonder it use case is possible where it would even be wrong but don't 
know. I guess the cat is out of the bag now.



Not sure how questionable that is. There are a bunch of tools reporting 
the amount of memory available (free, top, htop, etc...).


It might not be totalram_pages() but probably something close to it.

Having a non 0 & non -1 value is useful.


-Lionel




If the situation is -1 for unknown and some valid size (not zero) I 
don't think there is a problem here.


Regards,

Tvrtko


Anv is not currently handling that case.


I would very much like to not deal with 0 for smem.

It really makes it easier for userspace rather than having to fish 
information from 2 different places and on top of dealing with 
multiple kernel versions.



-Lionel





+
+    /**
+ * @unallocated_size: Estimate of memory remaining (-1 = unknown)
+ *
+ * Note this is only currently tracked for 
I915_MEMORY_CLASS_DEVICE

+ * regions, and also requires CAP_PERFMON or CAP_SYS_ADMIN to get
+ * reliable accounting. Without this(or if this an older 
kernel) the


s/if this an/if this is an/

Also same question as above about -1.


+ * value here will always match the @probed_size.
+ */
+    __u64 unallocated_size;
+
+    union {
+    /** @rsvd1: MBZ */
+    __u64 rsvd1[8];
+    struct {
+    /**
+ * @probed_cpu_visible_size: Memory probed by the driver
+ * that is CPU accessible. (-1 = unknown).


Also question about -1. In this case this could be done since the 
field is yet to be added but I am curious if it ever can be -1.



+ *
+ * This will be always be <= @probed_size, and the
+ * remainder(if there is any) will not be

Re: [RFC v4] drm: Add initial ci/ subdirectory

2022-05-17 Thread Tomeu Vizoso


On 5/17/22 11:18 AM, Neil Armstrong wrote:

On 17/05/2022 10:16, Tomeu Vizoso wrote:

And use it to store expectations about what the DRM drivers are
supposed to pass in the IGT test suite.

Also include a configuration file that points to the out-of-tree CI
scripts.

By storing the test expectations along the code we can make sure both
stay in sync with each other, and so we can know when a code change
breaks those expectations.

This will allow all contributors to drm to reuse the infrastructure
already in gitlab.freedesktop.org to test the driver on several
generations of the hardware.

v2:
   - Fix names of result expectation files to match SoC
   - Don't execute tests that are going to skip on all boards

v3:
   - Remove tracking of dmesg output during test execution

v4:
   - Move up to drivers/gpu/drm
   - Add support for a bunch of other drivers
   - Explain how to incorporate fixes for CI from a
 ${TARGET_BRANCH}-external-fixes branch
   - Remove tests that pass from expected results file, to reduce the
 size of in-tree files
   - Add docs about how to deal with outages in automated testing labs
   - Specify the exact SHA of the CI scripts to be used

Signed-off-by: Tomeu Vizoso 
---
  Documentation/gpu/automated_testing.rst   |  84 +++
  drivers/gpu/drm/ci/amdgpu.testlist    | 130 +++
  drivers/gpu/drm/ci/amdgpu_stoney_results.txt  |  17 ++
  drivers/gpu/drm/ci/gitlab-ci.yml  |  13 ++
  drivers/gpu/drm/ci/i915.testlist  | 219 ++
  drivers/gpu/drm/ci/i915_amly_results.txt  |   9 +
  drivers/gpu/drm/ci/i915_apl_results.txt   |   9 +
  drivers/gpu/drm/ci/i915_cml_results.txt   |  10 +
  drivers/gpu/drm/ci/i915_glk_results.txt   |  13 ++
  drivers/gpu/drm/ci/i915_kbl_results.txt   |   9 +
  drivers/gpu/drm/ci/i915_tgl_results.txt   |  12 +
  drivers/gpu/drm/ci/i915_whl_results.txt   |   9 +
  drivers/gpu/drm/ci/mediatek.testlist  | 102 
  .../gpu/drm/ci/mediatek_mt8173_results.txt    |  11 +
  .../gpu/drm/ci/mediatek_mt8183_results.txt    |  11 +
  .../gpu/drm/ci/mediatek_mt8192_results.txt    |  11 +
  drivers/gpu/drm/ci/meson.testlist |  71 ++
  drivers/gpu/drm/ci/meson_g12b_results.txt |  11 +
  drivers/gpu/drm/ci/msm.testlist   | 144 
  drivers/gpu/drm/ci/msm_apq8016_results.txt    |  77 ++
  drivers/gpu/drm/ci/msm_apq8096_results.txt    |  76 ++
  drivers/gpu/drm/ci/msm_sc7180_results.txt |  18 ++
  drivers/gpu/drm/ci/msm_sdm845_results.txt |  33 +++
  drivers/gpu/drm/ci/rockchip.testlist  | 188 +++
  .../gpu/drm/ci/rockchip_rk3288_results.txt    |  11 +
  .../gpu/drm/ci/rockchip_rk3399_results.txt    |  14 ++
  drivers/gpu/drm/ci/virtio_gpu.testlist    |  92 
  drivers/gpu/drm/ci/virtio_gpu_results.txt |  28 +++
  28 files changed, 1432 insertions(+)
  create mode 100644 Documentation/gpu/automated_testing.rst
  create mode 100644 drivers/gpu/drm/ci/amdgpu.testlist
  create mode 100644 drivers/gpu/drm/ci/amdgpu_stoney_results.txt
  create mode 100644 drivers/gpu/drm/ci/gitlab-ci.yml
  create mode 100644 drivers/gpu/drm/ci/i915.testlist
  create mode 100644 drivers/gpu/drm/ci/i915_amly_results.txt
  create mode 100644 drivers/gpu/drm/ci/i915_apl_results.txt
  create mode 100644 drivers/gpu/drm/ci/i915_cml_results.txt
  create mode 100644 drivers/gpu/drm/ci/i915_glk_results.txt
  create mode 100644 drivers/gpu/drm/ci/i915_kbl_results.txt
  create mode 100644 drivers/gpu/drm/ci/i915_tgl_results.txt
  create mode 100644 drivers/gpu/drm/ci/i915_whl_results.txt
  create mode 100644 drivers/gpu/drm/ci/mediatek.testlist
  create mode 100644 drivers/gpu/drm/ci/mediatek_mt8173_results.txt
  create mode 100644 drivers/gpu/drm/ci/mediatek_mt8183_results.txt
  create mode 100644 drivers/gpu/drm/ci/mediatek_mt8192_results.txt
  create mode 100644 drivers/gpu/drm/ci/meson.testlist
  create mode 100644 drivers/gpu/drm/ci/meson_g12b_results.txt
  create mode 100644 drivers/gpu/drm/ci/msm.testlist
  create mode 100644 drivers/gpu/drm/ci/msm_apq8016_results.txt
  create mode 100644 drivers/gpu/drm/ci/msm_apq8096_results.txt
  create mode 100644 drivers/gpu/drm/ci/msm_sc7180_results.txt
  create mode 100644 drivers/gpu/drm/ci/msm_sdm845_results.txt
  create mode 100644 drivers/gpu/drm/ci/rockchip.testlist
  create mode 100644 drivers/gpu/drm/ci/rockchip_rk3288_results.txt
  create mode 100644 drivers/gpu/drm/ci/rockchip_rk3399_results.txt
  create mode 100644 drivers/gpu/drm/ci/virtio_gpu.testlist
  create mode 100644 drivers/gpu/drm/ci/virtio_gpu_results.txt

diff --git a/Documentation/gpu/automated_testing.rst 
b/Documentation/gpu/automated_testing.rst

new file mode 100644
index ..61003ecc7b6e
--- /dev/null
+++ b/Documentation/gpu/automated_testing.rst
@@ -0,0 +1,84 @@
+.. SPDX-License-Identifier: GPL-2.0+
+
+=
+Automated testing of the DRM subsystem

Re: [PATCH v3] drm/doc: add rfc section for small BAR uapi




On 17/05/2022 09:55, Lionel Landwerlin wrote:

On 17/05/2022 11:29, Tvrtko Ursulin wrote:


On 16/05/2022 19:11, Matthew Auld wrote:

Add an entry for the new uapi needed for small BAR on DG2+.

v2:
   - Some spelling fixes and other small tweaks. (Akeem & Thomas)
   - Rework error capture interactions, including no longer needing
 NEEDS_CPU_ACCESS for objects marked for capture. (Thomas)
   - Add probed_cpu_visible_size. (Lionel)
v3:
   - Drop the vma query for now.
   - Add unallocated_cpu_visible_size as part of the region query.
   - Improve the docs some more, including documenting the expected
 behaviour on older kernels, since this came up in some offline
 discussion.

Signed-off-by: Matthew Auld 
Cc: Thomas Hellström 
Cc: Lionel Landwerlin 
Cc: Tvrtko Ursulin 
Cc: Jon Bloomfield 
Cc: Daniel Vetter 
Cc: Jon Bloomfield 
Cc: Jordan Justen 
Cc: Kenneth Graunke 
Cc: Akeem G Abodunrin 
Cc: mesa-...@lists.freedesktop.org
---
  Documentation/gpu/rfc/i915_small_bar.h   | 164 +++
  Documentation/gpu/rfc/i915_small_bar.rst |  47 +++
  Documentation/gpu/rfc/index.rst  |   4 +
  3 files changed, 215 insertions(+)
  create mode 100644 Documentation/gpu/rfc/i915_small_bar.h
  create mode 100644 Documentation/gpu/rfc/i915_small_bar.rst

diff --git a/Documentation/gpu/rfc/i915_small_bar.h 
b/Documentation/gpu/rfc/i915_small_bar.h

new file mode 100644
index ..4079d287750b
--- /dev/null
+++ b/Documentation/gpu/rfc/i915_small_bar.h
@@ -0,0 +1,164 @@
+/**
+ * struct __drm_i915_memory_region_info - Describes one region as 
known to the

+ * driver.
+ *
+ * Note this is using both struct drm_i915_query_item and struct 
drm_i915_query.
+ * For this new query we are adding the new query id 
DRM_I915_QUERY_MEMORY_REGIONS

+ * at _i915_query_item.query_id.
+ */
+struct __drm_i915_memory_region_info {
+    /** @region: The class:instance pair encoding */
+    struct drm_i915_gem_memory_class_instance region;
+
+    /** @rsvd0: MBZ */
+    __u32 rsvd0;
+
+    /** @probed_size: Memory probed by the driver (-1 = unknown) */
+    __u64 probed_size;


Is -1 possible today or when it will be? For system memory it appears 
zeroes are returned today so that has to stay I think. Does it 
effectively mean userspace has to consider both 0 and -1 as unknown is 
the question.



I raised this on v2. As far as I can tell there are no situation where 
we would get -1.


Is it really probed_size=0 on smem?? It's not the case on the internal 
branch.


My bad, I misread the arguments to intel_memory_region_create while grepping:

struct intel_memory_region *i915_gem_shmem_setup(struct drm_i915_private *i915,
 u16 type, u16 instance)
{
return intel_memory_region_create(i915, 0,
  totalram_pages() << PAGE_SHIFT,
  PAGE_SIZE, 0, 0,
  type, instance,
  _region_ops);

I saw "0, 0" and wrongly assumed that would be the data, since it matched with 
my mental model and the comment against unallocated_size saying it's only tracked for 
device memory.

Although I'd say it is questionable for i915 to return this data. I wonder it 
use case is possible where it would even be wrong but don't know. I guess the 
cat is out of the bag now.

If the situation is -1 for unknown and some valid size (not zero) I don't think 
there is a problem here.

Regards,

Tvrtko


Anv is not currently handling that case.


I would very much like to not deal with 0 for smem.

It really makes it easier for userspace rather than having to fish 
information from 2 different places and on top of dealing with multiple 
kernel versions.



-Lionel





+
+    /**
+ * @unallocated_size: Estimate of memory remaining (-1 = unknown)
+ *
+ * Note this is only currently tracked for I915_MEMORY_CLASS_DEVICE
+ * regions, and also requires CAP_PERFMON or CAP_SYS_ADMIN to get
+ * reliable accounting. Without this(or if this an older kernel) 
the


s/if this an/if this is an/

Also same question as above about -1.


+ * value here will always match the @probed_size.
+ */
+    __u64 unallocated_size;
+
+    union {
+    /** @rsvd1: MBZ */
+    __u64 rsvd1[8];
+    struct {
+    /**
+ * @probed_cpu_visible_size: Memory probed by the driver
+ * that is CPU accessible. (-1 = unknown).


Also question about -1. In this case this could be done since the 
field is yet to be added but I am curious if it ever can be -1.



+ *
+ * This will be always be <= @probed_size, and the
+ * remainder(if there is any) will not be CPU
+ * accessible.
+ *
+ * On systems without small BAR, the @probed_size will
+ * always equal the @probed_cpu_visible_size, since all
+ * of it

Re: [RFC v4] drm: Add initial ci/ subdirectory

2022-05-17 Thread Neil Armstrong


On 17/05/2022 10:16, Tomeu Vizoso wrote:

And use it to store expectations about what the DRM drivers are
supposed to pass in the IGT test suite.

Also include a configuration file that points to the out-of-tree CI
scripts.

By storing the test expectations along the code we can make sure both
stay in sync with each other, and so we can know when a code change
breaks those expectations.

This will allow all contributors to drm to reuse the infrastructure
already in gitlab.freedesktop.org to test the driver on several
generations of the hardware.

v2:
   - Fix names of result expectation files to match SoC
   - Don't execute tests that are going to skip on all boards

v3:
   - Remove tracking of dmesg output during test execution

v4:
   - Move up to drivers/gpu/drm
   - Add support for a bunch of other drivers
   - Explain how to incorporate fixes for CI from a
 ${TARGET_BRANCH}-external-fixes branch
   - Remove tests that pass from expected results file, to reduce the
 size of in-tree files
   - Add docs about how to deal with outages in automated testing labs
   - Specify the exact SHA of the CI scripts to be used

Signed-off-by: Tomeu Vizoso 
---
  Documentation/gpu/automated_testing.rst   |  84 +++
  drivers/gpu/drm/ci/amdgpu.testlist| 130 +++
  drivers/gpu/drm/ci/amdgpu_stoney_results.txt  |  17 ++
  drivers/gpu/drm/ci/gitlab-ci.yml  |  13 ++
  drivers/gpu/drm/ci/i915.testlist  | 219 ++
  drivers/gpu/drm/ci/i915_amly_results.txt  |   9 +
  drivers/gpu/drm/ci/i915_apl_results.txt   |   9 +
  drivers/gpu/drm/ci/i915_cml_results.txt   |  10 +
  drivers/gpu/drm/ci/i915_glk_results.txt   |  13 ++
  drivers/gpu/drm/ci/i915_kbl_results.txt   |   9 +
  drivers/gpu/drm/ci/i915_tgl_results.txt   |  12 +
  drivers/gpu/drm/ci/i915_whl_results.txt   |   9 +
  drivers/gpu/drm/ci/mediatek.testlist  | 102 
  .../gpu/drm/ci/mediatek_mt8173_results.txt|  11 +
  .../gpu/drm/ci/mediatek_mt8183_results.txt|  11 +
  .../gpu/drm/ci/mediatek_mt8192_results.txt|  11 +
  drivers/gpu/drm/ci/meson.testlist |  71 ++
  drivers/gpu/drm/ci/meson_g12b_results.txt |  11 +
  drivers/gpu/drm/ci/msm.testlist   | 144 
  drivers/gpu/drm/ci/msm_apq8016_results.txt|  77 ++
  drivers/gpu/drm/ci/msm_apq8096_results.txt|  76 ++
  drivers/gpu/drm/ci/msm_sc7180_results.txt |  18 ++
  drivers/gpu/drm/ci/msm_sdm845_results.txt |  33 +++
  drivers/gpu/drm/ci/rockchip.testlist  | 188 +++
  .../gpu/drm/ci/rockchip_rk3288_results.txt|  11 +
  .../gpu/drm/ci/rockchip_rk3399_results.txt|  14 ++
  drivers/gpu/drm/ci/virtio_gpu.testlist|  92 
  drivers/gpu/drm/ci/virtio_gpu_results.txt |  28 +++
  28 files changed, 1432 insertions(+)
  create mode 100644 Documentation/gpu/automated_testing.rst
  create mode 100644 drivers/gpu/drm/ci/amdgpu.testlist
  create mode 100644 drivers/gpu/drm/ci/amdgpu_stoney_results.txt
  create mode 100644 drivers/gpu/drm/ci/gitlab-ci.yml
  create mode 100644 drivers/gpu/drm/ci/i915.testlist
  create mode 100644 drivers/gpu/drm/ci/i915_amly_results.txt
  create mode 100644 drivers/gpu/drm/ci/i915_apl_results.txt
  create mode 100644 drivers/gpu/drm/ci/i915_cml_results.txt
  create mode 100644 drivers/gpu/drm/ci/i915_glk_results.txt
  create mode 100644 drivers/gpu/drm/ci/i915_kbl_results.txt
  create mode 100644 drivers/gpu/drm/ci/i915_tgl_results.txt
  create mode 100644 drivers/gpu/drm/ci/i915_whl_results.txt
  create mode 100644 drivers/gpu/drm/ci/mediatek.testlist
  create mode 100644 drivers/gpu/drm/ci/mediatek_mt8173_results.txt
  create mode 100644 drivers/gpu/drm/ci/mediatek_mt8183_results.txt
  create mode 100644 drivers/gpu/drm/ci/mediatek_mt8192_results.txt
  create mode 100644 drivers/gpu/drm/ci/meson.testlist
  create mode 100644 drivers/gpu/drm/ci/meson_g12b_results.txt
  create mode 100644 drivers/gpu/drm/ci/msm.testlist
  create mode 100644 drivers/gpu/drm/ci/msm_apq8016_results.txt
  create mode 100644 drivers/gpu/drm/ci/msm_apq8096_results.txt
  create mode 100644 drivers/gpu/drm/ci/msm_sc7180_results.txt
  create mode 100644 drivers/gpu/drm/ci/msm_sdm845_results.txt
  create mode 100644 drivers/gpu/drm/ci/rockchip.testlist
  create mode 100644 drivers/gpu/drm/ci/rockchip_rk3288_results.txt
  create mode 100644 drivers/gpu/drm/ci/rockchip_rk3399_results.txt
  create mode 100644 drivers/gpu/drm/ci/virtio_gpu.testlist
  create mode 100644 drivers/gpu/drm/ci/virtio_gpu_results.txt

diff --git a/Documentation/gpu/automated_testing.rst 
b/Documentation/gpu/automated_testing.rst
new file mode 100644
index ..61003ecc7b6e
--- /dev/null
+++ b/Documentation/gpu/automated_testing.rst
@@ -0,0 +1,84 @@
+.. SPDX-License-Identifier: GPL-2.0+
+
+=
+Automated testing of the DRM subsystem
+=
+

Re: [PATCH v3] drm/doc: add rfc section for small BAR uapi

2022-05-17 Thread Matthew Auld


On 17/05/2022 09:29, Tvrtko Ursulin wrote:


On 16/05/2022 19:11, Matthew Auld wrote:

Add an entry for the new uapi needed for small BAR on DG2+.

v2:
   - Some spelling fixes and other small tweaks. (Akeem & Thomas)
   - Rework error capture interactions, including no longer needing
 NEEDS_CPU_ACCESS for objects marked for capture. (Thomas)
   - Add probed_cpu_visible_size. (Lionel)
v3:
   - Drop the vma query for now.
   - Add unallocated_cpu_visible_size as part of the region query.
   - Improve the docs some more, including documenting the expected
 behaviour on older kernels, since this came up in some offline
 discussion.

Signed-off-by: Matthew Auld 
Cc: Thomas Hellström 
Cc: Lionel Landwerlin 
Cc: Tvrtko Ursulin 
Cc: Jon Bloomfield 
Cc: Daniel Vetter 
Cc: Jon Bloomfield 
Cc: Jordan Justen 
Cc: Kenneth Graunke 
Cc: Akeem G Abodunrin 
Cc: mesa-...@lists.freedesktop.org
---
  Documentation/gpu/rfc/i915_small_bar.h   | 164 +++
  Documentation/gpu/rfc/i915_small_bar.rst |  47 +++
  Documentation/gpu/rfc/index.rst  |   4 +
  3 files changed, 215 insertions(+)
  create mode 100644 Documentation/gpu/rfc/i915_small_bar.h
  create mode 100644 Documentation/gpu/rfc/i915_small_bar.rst

diff --git a/Documentation/gpu/rfc/i915_small_bar.h 
b/Documentation/gpu/rfc/i915_small_bar.h

new file mode 100644
index ..4079d287750b
--- /dev/null
+++ b/Documentation/gpu/rfc/i915_small_bar.h
@@ -0,0 +1,164 @@
+/**
+ * struct __drm_i915_memory_region_info - Describes one region as 
known to the

+ * driver.
+ *
+ * Note this is using both struct drm_i915_query_item and struct 
drm_i915_query.
+ * For this new query we are adding the new query id 
DRM_I915_QUERY_MEMORY_REGIONS

+ * at _i915_query_item.query_id.
+ */
+struct __drm_i915_memory_region_info {
+    /** @region: The class:instance pair encoding */
+    struct drm_i915_gem_memory_class_instance region;
+
+    /** @rsvd0: MBZ */
+    __u32 rsvd0;
+
+    /** @probed_size: Memory probed by the driver (-1 = unknown) */
+    __u64 probed_size;


Is -1 possible today or when it will be? For system memory it appears 
zeroes are returned today so that has to stay I think. Does it 
effectively mean userspace has to consider both 0 and -1 as unknown is 
the question.


It looks like it just returns the totalram_pages(). So at the moment 
nothing ever currently returns -1 or 0. Maybe that was a mistake for 
I915_MEMORY_SYSTEM.





+
+    /**
+ * @unallocated_size: Estimate of memory remaining (-1 = unknown)
+ *
+ * Note this is only currently tracked for I915_MEMORY_CLASS_DEVICE
+ * regions, and also requires CAP_PERFMON or CAP_SYS_ADMIN to get
+ * reliable accounting. Without this(or if this an older kernel) the


s/if this an/if this is an/

Also same question as above about -1.


This should be the same as above, since this will give the same value as 
probed_size, or give the real avail value for lmem.





+ * value here will always match the @probed_size.
+ */
+    __u64 unallocated_size;
+
+    union {
+    /** @rsvd1: MBZ */
+    __u64 rsvd1[8];
+    struct {
+    /**
+ * @probed_cpu_visible_size: Memory probed by the driver
+ * that is CPU accessible. (-1 = unknown).


Also question about -1. In this case this could be done since the field 
is yet to be added but I am curious if it ever can be -1.


I was just going to make this the same as probed_size for system memory. 
But we could use -1 here instead. What do you think? Same for 
unallocated below.





+ *
+ * This will be always be <= @probed_size, and the
+ * remainder(if there is any) will not be CPU
+ * accessible.
+ *
+ * On systems without small BAR, the @probed_size will
+ * always equal the @probed_cpu_visible_size, since all
+ * of it will be CPU accessible.
+ *
+ * Note that if the value returned here is zero, then
+ * this must be an old kernel which lacks the relevant
+ * small-bar uAPI support(including


I have noticed you prefer no space before parentheses throughout the 
text so I guess it's just my preference to have it. Very nitpicky even 
if I am right so up to you.


Ok, will change :)




+ * I915_GEM_CREATE_EXT_FLAG_NEEDS_CPU_ACCESS), but on
+ * such systems we should never actually end up with a
+ * small BAR configuration, assuming we are able to load
+ * the kernel module. Hence it should be safe to treat
+ * this the same as when @probed_cpu_visible_size ==
+ * @probed_size.
+ */
+    __u64 probed_cpu_visible_size;
+
+    /**
+ * @unallocated_cpu_visible_size: Estimate of CPU
+ * visible memory remaining (-1 = unknown).
+ *
+ * Note this is only currently tracked for
+

Re: [PATCH v2 1/3] dt-bindings: display: rockchip: make reg-names mandatory for VOP2

2022-05-17 Thread Heiko Stübner

Am Dienstag, 17. Mai 2022, 11:02:06 CEST schrieb Krzysztof Kozlowski:
> On 14/05/2022 00:26, Heiko Stuebner wrote:
> > Hi Rob, Krzysztof,
> > 
> > Am Mittwoch, 11. Mai 2022, 10:21:07 CEST schrieb Sascha Hauer:
> >> The VOP2 driver relies on reg-names properties, but these are not
> >> documented. Add the missing documentation, make reg-names mandatory
> >> and increase minItems to 2 as always both register spaces are needed.
> >>
> >> Signed-off-by: Sascha Hauer 
> > 
> > does this look ok now?
> 
> Neither Rob nor me was Cced on original email, so I don't have it in my
> mailbox... I would expect that scripts/get_maintainers.pl is a known tool.

Rob wrote in the past that he's working off of the devicetree-patchwork,
and has reviewed the binding yesterday [0].

But you're right, including all the maintainers get_maintainers.pl suggests
would be better in the future.


[0] https://lore.kernel.org/r/20220516151226.ga2653888-r...@kernel.org

Re: [PATCH v1] drm/scheduler: Don't kill jobs in interrupt context

2022-05-17 Thread Dmitry Osipenko

On 5/17/22 10:40, Erico Nunes wrote:
> On Wed, Apr 13, 2022 at 12:05 PM Steven Price  wrote:
>>
>> On 11/04/2022 23:15, Dmitry Osipenko wrote:
>>> Interrupt context can't sleep. Drivers like Panfrost and MSM are taking
>>> mutex when job is released, and thus, that code can sleep. This results
>>> into "BUG: scheduling while atomic" if locks are contented while job is
>>> freed. There is no good reason for releasing scheduler's jobs in IRQ
>>> context, hence use normal context to fix the trouble.
>>>
>>> Cc: sta...@vger.kernel.org
>>> Fixes: 542cff7893a3 ("drm/sched: Avoid lockdep spalt on killing a 
>>> processes")
>>> Signed-off-by: Dmitry Osipenko 
>>
>> Reviewed-by: Steven Price 
> 
> Is there something blocking this patch?
> Mesa CI is still hitting the issue and I have been waiting for it to
> be applied/backported to update CI with it.
> Thanks

If this patch won't be picked up anytime soon, then I'll include it into
my "memory shrinker" patchset together with the rest of the fixes, so it
won't get lost.

-- 
Best regards,
Dmitry

Re: [PATCH v2 1/3] dt-bindings: display: rockchip: make reg-names mandatory for VOP2

On 14/05/2022 00:26, Heiko Stuebner wrote:
> Hi Rob, Krzysztof,
> 
> Am Mittwoch, 11. Mai 2022, 10:21:07 CEST schrieb Sascha Hauer:
>> The VOP2 driver relies on reg-names properties, but these are not
>> documented. Add the missing documentation, make reg-names mandatory
>> and increase minItems to 2 as always both register spaces are needed.
>>
>> Signed-off-by: Sascha Hauer 
> 
> does this look ok now?

Neither Rob nor me was Cced on original email, so I don't have it in my
mailbox... I would expect that scripts/get_maintainers.pl is a known tool.


Best regards,
Krzysztof

Re: [PATCH v3] drm/doc: add rfc section for small BAR uapi

2022-05-17 Thread Lionel Landwerlin


On 17/05/2022 11:29, Tvrtko Ursulin wrote:


On 16/05/2022 19:11, Matthew Auld wrote:

Add an entry for the new uapi needed for small BAR on DG2+.

v2:
   - Some spelling fixes and other small tweaks. (Akeem & Thomas)
   - Rework error capture interactions, including no longer needing
 NEEDS_CPU_ACCESS for objects marked for capture. (Thomas)
   - Add probed_cpu_visible_size. (Lionel)
v3:
   - Drop the vma query for now.
   - Add unallocated_cpu_visible_size as part of the region query.
   - Improve the docs some more, including documenting the expected
 behaviour on older kernels, since this came up in some offline
 discussion.

Signed-off-by: Matthew Auld 
Cc: Thomas Hellström 
Cc: Lionel Landwerlin 
Cc: Tvrtko Ursulin 
Cc: Jon Bloomfield 
Cc: Daniel Vetter 
Cc: Jon Bloomfield 
Cc: Jordan Justen 
Cc: Kenneth Graunke 
Cc: Akeem G Abodunrin 
Cc: mesa-...@lists.freedesktop.org
---
  Documentation/gpu/rfc/i915_small_bar.h   | 164 +++
  Documentation/gpu/rfc/i915_small_bar.rst |  47 +++
  Documentation/gpu/rfc/index.rst  |   4 +
  3 files changed, 215 insertions(+)
  create mode 100644 Documentation/gpu/rfc/i915_small_bar.h
  create mode 100644 Documentation/gpu/rfc/i915_small_bar.rst

diff --git a/Documentation/gpu/rfc/i915_small_bar.h 
b/Documentation/gpu/rfc/i915_small_bar.h

new file mode 100644
index ..4079d287750b
--- /dev/null
+++ b/Documentation/gpu/rfc/i915_small_bar.h
@@ -0,0 +1,164 @@
+/**
+ * struct __drm_i915_memory_region_info - Describes one region as 
known to the

+ * driver.
+ *
+ * Note this is using both struct drm_i915_query_item and struct 
drm_i915_query.
+ * For this new query we are adding the new query id 
DRM_I915_QUERY_MEMORY_REGIONS

+ * at _i915_query_item.query_id.
+ */
+struct __drm_i915_memory_region_info {
+    /** @region: The class:instance pair encoding */
+    struct drm_i915_gem_memory_class_instance region;
+
+    /** @rsvd0: MBZ */
+    __u32 rsvd0;
+
+    /** @probed_size: Memory probed by the driver (-1 = unknown) */
+    __u64 probed_size;


Is -1 possible today or when it will be? For system memory it appears 
zeroes are returned today so that has to stay I think. Does it 
effectively mean userspace has to consider both 0 and -1 as unknown is 
the question.



I raised this on v2. As far as I can tell there are no situation where 
we would get -1.


Is it really probed_size=0 on smem?? It's not the case on the internal 
branch.


Anv is not currently handling that case.


I would very much like to not deal with 0 for smem.

It really makes it easier for userspace rather than having to fish 
information from 2 different places and on top of dealing with multiple 
kernel versions.



-Lionel





+
+    /**
+ * @unallocated_size: Estimate of memory remaining (-1 = unknown)
+ *
+ * Note this is only currently tracked for I915_MEMORY_CLASS_DEVICE
+ * regions, and also requires CAP_PERFMON or CAP_SYS_ADMIN to get
+ * reliable accounting. Without this(or if this an older kernel) 
the


s/if this an/if this is an/

Also same question as above about -1.


+ * value here will always match the @probed_size.
+ */
+    __u64 unallocated_size;
+
+    union {
+    /** @rsvd1: MBZ */
+    __u64 rsvd1[8];
+    struct {
+    /**
+ * @probed_cpu_visible_size: Memory probed by the driver
+ * that is CPU accessible. (-1 = unknown).


Also question about -1. In this case this could be done since the 
field is yet to be added but I am curious if it ever can be -1.



+ *
+ * This will be always be <= @probed_size, and the
+ * remainder(if there is any) will not be CPU
+ * accessible.
+ *
+ * On systems without small BAR, the @probed_size will
+ * always equal the @probed_cpu_visible_size, since all
+ * of it will be CPU accessible.
+ *
+ * Note that if the value returned here is zero, then
+ * this must be an old kernel which lacks the relevant
+ * small-bar uAPI support(including


I have noticed you prefer no space before parentheses throughout the 
text so I guess it's just my preference to have it. Very nitpicky even 
if I am right so up to you.



+ * I915_GEM_CREATE_EXT_FLAG_NEEDS_CPU_ACCESS), but on
+ * such systems we should never actually end up with a
+ * small BAR configuration, assuming we are able to load
+ * the kernel module. Hence it should be safe to treat
+ * this the same as when @probed_cpu_visible_size ==
+ * @probed_size.
+ */
+    __u64 probed_cpu_visible_size;
+
+    /**
+ * @unallocated_cpu_visible_size: Estimate of CPU
+ * visible memory remaining (-1 = unknown).
+ *
+ * Note this is only currently tracked for
+ *

Re: [PATCH v1 02/13] dt-bindings: Add headers for Host1x and VIC on Tegra234


On 5/17/22 11:43, Krzysztof Kozlowski wrote:

On 17/05/2022 10:41, Mikko Perttunen wrote:

On 5/17/22 11:02, Krzysztof Kozlowski wrote:

On 16/05/2022 12:02, cyn...@kapsi.fi wrote:

From: Mikko Perttunen 

Add clock, memory controller, powergate and reset dt-binding headers
for Host1x and VIC on Tegra234.

Signed-off-by: Mikko Perttunen 


All your patches are send from wrong email address and the SoB chain is
not correct. Please add respective SoB.

This applies to entire patchset.


Best regards,
Krzysztof


I can do

Signed-off-by: Mikko Perttunen 
Signed-off-by: Mikko Perttunen 

if that is better.


That's not needed, if your name was in the sender field. The trouble is
that your name was not there and I have no clue who is cyn...@kapsi.fi.


Best regards,
Krzysztof


I see, that makes sense. Looks like when I did the above From field 
change I must have messed up my configuration somehow since it looks 
like older patches I've sent do have the full name.


Thanks, I'll try to fix it for next version.
Mikko

Re: [PATCH v1 01/13] dt-bindings: Add bindings for Tegra234 Host1x and VIC


On 5/16/22 19:33, Rob Herring wrote:

On Mon, May 16, 2022 at 01:02:01PM +0300, cyn...@kapsi.fi wrote:

From: Mikko Perttunen 

Update VIC and Host1x bindings for changes in Tegra234.

Namely,
- New compatible strings
- Sharded syncpoint interrupts
- Optional reset.

Signed-off-by: Mikko Perttunen 
---
  .../display/tegra/nvidia,tegra124-vic.yaml|   1 +
  .../display/tegra/nvidia,tegra20-host1x.yaml  | 108 +++---
  2 files changed, 95 insertions(+), 14 deletions(-)

diff --git 
a/Documentation/devicetree/bindings/display/tegra/nvidia,tegra124-vic.yaml 
b/Documentation/devicetree/bindings/display/tegra/nvidia,tegra124-vic.yaml
index 37bb5ddc1963..7200095ef19e 100644
--- a/Documentation/devicetree/bindings/display/tegra/nvidia,tegra124-vic.yaml
+++ b/Documentation/devicetree/bindings/display/tegra/nvidia,tegra124-vic.yaml
@@ -21,6 +21,7 @@ properties:
- nvidia,tegra210-vic
- nvidia,tegra186-vic
- nvidia,tegra194-vic
+  - nvidia,tegra234-vic
  
- items:

- const: nvidia,tegra132-vic
diff --git 
a/Documentation/devicetree/bindings/display/tegra/nvidia,tegra20-host1x.yaml 
b/Documentation/devicetree/bindings/display/tegra/nvidia,tegra20-host1x.yaml
index 0adeb03b9e3a..83c58b7dae98 100644
--- a/Documentation/devicetree/bindings/display/tegra/nvidia,tegra20-host1x.yaml
+++ b/Documentation/devicetree/bindings/display/tegra/nvidia,tegra20-host1x.yaml
@@ -24,6 +24,7 @@ properties:
- nvidia,tegra210-host1x
- nvidia,tegra186-host1x
- nvidia,tegra194-host1x
+  - nvidia,tegra234-host1x
  
- items:

- const: nvidia,tegra132-host1x
@@ -31,23 +32,19 @@ properties:
  
reg:

  minItems: 1
-maxItems: 2
+maxItems: 3
  
reg-names:

  minItems: 1
-maxItems: 2
+maxItems: 3
  
interrupts:

-items:
-  - description: host1x syncpoint interrupt
-  - description: host1x general interrupt
  minItems: 1
+maxItems: 9
  
interrupt-names:

-items:
-  - const: syncpt
-  - const: host1x
  minItems: 1
+maxItems: 9
  
'#address-cells':

  description: The number of cells used to represent physical base addresses
@@ -110,13 +107,32 @@ required:
- reg
- clocks
- clock-names
-  - resets
-  - reset-names


Shouldn't these still be required on some platforms?


Yes, I'll add them back in the tegra20..tegra210 conditional.



  
  additionalProperties:

type: object
  
  allOf:

+  - if:
+  properties:
+compatible:
+  contains:
+enum:
+  - nvidia,tegra20-host1x
+  - nvidia,tegra30-host1x
+  - nvidia,tegra114-host1x
+  - nvidia,tegra124-host1x
+  - nvidia,tegra210-host1x
+then:
+  properties:
+interrupts:
+  items:
+- description: host1x syncpoint interrupt
+- description: host1x general interrupt
+
+interrupt-names:
+  items:
+- const: syncpt
+- const: host1x
- if:
properties:
  compatible:
@@ -133,10 +149,10 @@ allOf:
  
  reg:

items:
-- description: physical base address and length of the register
-region assigned to the VM
  - description: physical base address and length of the register
  region used by the hypervisor
+- description: physical base address and length of the register
+region assigned to the VM


You can't just change the order at least without a good explanation why
in the commit message. It's an ABI.


Yeah, this doesn't change ABI, it's just a documentation bugfix, but 
indeed I should have mentioned it in the commit message. In 'reg-names' 
the order is given as 'hypervisor, vm' and the descriptions here were 
the wrong way around.




  
  resets:

maxItems: 1
@@ -144,6 +160,70 @@ allOf:
  reset-names:
maxItems: 1
  
+interrupts:

+  items:
+- description: host1x syncpoint interrupt
+- description: host1x general interrupt
+
+interrupt-names:
+  items:
+- const: syncpt
+- const: host1x
+
+iommu-map:
+  description: Specification of stream IDs available for memory 
context device
+use. Should be a mapping of IDs 0..n to IOMMU entries 
corresponding to
+usable stream IDs.
+
+  required:
+- reg-names
+  - if:
+  properties:
+compatible:
+  contains:
+enum:
+  - nvidia,tegra234-host1x
+then:
+  properties:
+reg-names:
+  items:
+- const: common
+- const: hypervisor
+- const: vm
+
+reg:
+  items:
+- description: physical base address and length of the register
+region used

Re: [PATCH v1 02/13] dt-bindings: Add headers for Host1x and VIC on Tegra234

On 17/05/2022 10:41, Mikko Perttunen wrote:
> On 5/17/22 11:02, Krzysztof Kozlowski wrote:
>> On 16/05/2022 12:02, cyn...@kapsi.fi wrote:
>>> From: Mikko Perttunen 
>>>
>>> Add clock, memory controller, powergate and reset dt-binding headers
>>> for Host1x and VIC on Tegra234.
>>>
>>> Signed-off-by: Mikko Perttunen 
>>
>> All your patches are send from wrong email address and the SoB chain is
>> not correct. Please add respective SoB.
>>
>> This applies to entire patchset.
>>
>>
>> Best regards,
>> Krzysztof
> 
> I can do
> 
>Signed-off-by: Mikko Perttunen 
>Signed-off-by: Mikko Perttunen 
> 
> if that is better.

That's not needed, if your name was in the sender field. The trouble is
that your name was not there and I have no clue who is cyn...@kapsi.fi.


Best regards,
Krzysztof

Re: [PATCH v1 02/13] dt-bindings: Add headers for Host1x and VIC on Tegra234


On 5/17/22 11:02, Krzysztof Kozlowski wrote:

On 16/05/2022 12:02, cyn...@kapsi.fi wrote:

From: Mikko Perttunen 

Add clock, memory controller, powergate and reset dt-binding headers
for Host1x and VIC on Tegra234.

Signed-off-by: Mikko Perttunen 


All your patches are send from wrong email address and the SoB chain is
not correct. Please add respective SoB.

This applies to entire patchset.


Best regards,
Krzysztof


I can do

  Signed-off-by: Mikko Perttunen 
  Signed-off-by: Mikko Perttunen 

if that is better.

I used to just send with From: mperttu...@nvidia.com but unsurprisingly 
many mail servers have started rejecting that.


Mikko

Re: [PATCH v1 03/13] arm64: tegra: Add Host1x and VIC on Tegra234


On 5/17/22 11:01, Krzysztof Kozlowski wrote:

On 16/05/2022 12:02, cyn...@kapsi.fi wrote:

From: Mikko Perttunen 

Add device tree nodes for Host1x and VIC on Tegra234.

Signed-off-by: Mikko Perttunen 
---
  arch/arm64/boot/dts/nvidia/tegra234.dtsi | 46 
  1 file changed, 46 insertions(+)

diff --git a/arch/arm64/boot/dts/nvidia/tegra234.dtsi 
b/arch/arm64/boot/dts/nvidia/tegra234.dtsi
index cb3af539e477..cae68e59580c 100644
--- a/arch/arm64/boot/dts/nvidia/tegra234.dtsi
+++ b/arch/arm64/boot/dts/nvidia/tegra234.dtsi
@@ -454,6 +454,52 @@ misc@10 {
status = "okay";
};
  
+		host1x@13e0 {


Generic node names, if that possible. Since the bindings do not exist in
the next, I actually cannot figure out what's host1x...


Host1x is a hardware block that provides programmable DMA channels, HW 
synchronization primitives, and virtualization support for IP blocks 
connected to its "host1x bus". So far I haven't found a one or two word 
way to describe it despite efforts. In any case, considering all the 
existing documentation and device trees that use this name, I'd prefer 
not changing it (especially as I don't know what else it could be called).





+   compatible = "nvidia,tegra234-host1x";
+   reg = <0x13e0 0x1>,
+ <0x13e1 0x1>,
+ <0x13e4 0x1>;
+   reg-names = "common", "hypervisor", "vm";
+   interrupts = ,
+,
+,
+,
+,
+,
+,
+,
+;
+   interrupt-names = "syncpt0", "syncpt1", "syncpt2", "syncpt3", 
"syncpt4",
+ "syncpt5", "syncpt6", "syncpt7", 
"host1x";
+   clocks = < TEGRA234_CLK_HOST1X>;
+   clock-names = "host1x";
+
+   #address-cells = <1>;
+   #size-cells = <1>;
+
+   ranges = <0x1500 0x1500 0x0100>;
+   interconnects = < TEGRA234_MEMORY_CLIENT_HOST1XDMAR 
>;
+   interconnect-names = "dma-mem";
+   iommus = <_niso1 TEGRA234_SID_HOST1X>;
+
+   vic@1534 {


The same... vic is usually a vectored interrupt controller, so this
should be interrupt-controller. Unless it is something entirely else, so
then you need to come with a generic name.


VIC here is video image compositor (with various other 2d operations). I 
suppose I can invent some generic name. Any thoughts, Thierry?


Mikko




Best regards,
Krzysztof

Re: [PATCH v3] drm/doc: add rfc section for small BAR uapi




On 16/05/2022 19:11, Matthew Auld wrote:

Add an entry for the new uapi needed for small BAR on DG2+.

v2:
   - Some spelling fixes and other small tweaks. (Akeem & Thomas)
   - Rework error capture interactions, including no longer needing
 NEEDS_CPU_ACCESS for objects marked for capture. (Thomas)
   - Add probed_cpu_visible_size. (Lionel)
v3:
   - Drop the vma query for now.
   - Add unallocated_cpu_visible_size as part of the region query.
   - Improve the docs some more, including documenting the expected
 behaviour on older kernels, since this came up in some offline
 discussion.

Signed-off-by: Matthew Auld 
Cc: Thomas Hellström 
Cc: Lionel Landwerlin 
Cc: Tvrtko Ursulin 
Cc: Jon Bloomfield 
Cc: Daniel Vetter 
Cc: Jon Bloomfield 
Cc: Jordan Justen 
Cc: Kenneth Graunke 
Cc: Akeem G Abodunrin 
Cc: mesa-...@lists.freedesktop.org
---
  Documentation/gpu/rfc/i915_small_bar.h   | 164 +++
  Documentation/gpu/rfc/i915_small_bar.rst |  47 +++
  Documentation/gpu/rfc/index.rst  |   4 +
  3 files changed, 215 insertions(+)
  create mode 100644 Documentation/gpu/rfc/i915_small_bar.h
  create mode 100644 Documentation/gpu/rfc/i915_small_bar.rst

diff --git a/Documentation/gpu/rfc/i915_small_bar.h 
b/Documentation/gpu/rfc/i915_small_bar.h
new file mode 100644
index ..4079d287750b
--- /dev/null
+++ b/Documentation/gpu/rfc/i915_small_bar.h
@@ -0,0 +1,164 @@
+/**
+ * struct __drm_i915_memory_region_info - Describes one region as known to the
+ * driver.
+ *
+ * Note this is using both struct drm_i915_query_item and struct 
drm_i915_query.
+ * For this new query we are adding the new query id 
DRM_I915_QUERY_MEMORY_REGIONS
+ * at _i915_query_item.query_id.
+ */
+struct __drm_i915_memory_region_info {
+   /** @region: The class:instance pair encoding */
+   struct drm_i915_gem_memory_class_instance region;
+
+   /** @rsvd0: MBZ */
+   __u32 rsvd0;
+
+   /** @probed_size: Memory probed by the driver (-1 = unknown) */
+   __u64 probed_size;


Is -1 possible today or when it will be? For system memory it appears 
zeroes are returned today so that has to stay I think. Does it 
effectively mean userspace has to consider both 0 and -1 as unknown is 
the question.



+
+   /**
+* @unallocated_size: Estimate of memory remaining (-1 = unknown)
+*
+* Note this is only currently tracked for I915_MEMORY_CLASS_DEVICE
+* regions, and also requires CAP_PERFMON or CAP_SYS_ADMIN to get
+* reliable accounting. Without this(or if this an older kernel) the


s/if this an/if this is an/

Also same question as above about -1.


+* value here will always match the @probed_size.
+*/
+   __u64 unallocated_size;
+
+   union {
+   /** @rsvd1: MBZ */
+   __u64 rsvd1[8];
+   struct {
+   /**
+* @probed_cpu_visible_size: Memory probed by the driver
+* that is CPU accessible. (-1 = unknown).


Also question about -1. In this case this could be done since the field 
is yet to be added but I am curious if it ever can be -1.



+*
+* This will be always be <= @probed_size, and the
+* remainder(if there is any) will not be CPU
+* accessible.
+*
+* On systems without small BAR, the @probed_size will
+* always equal the @probed_cpu_visible_size, since all
+* of it will be CPU accessible.
+*
+* Note that if the value returned here is zero, then
+* this must be an old kernel which lacks the relevant
+* small-bar uAPI support(including


I have noticed you prefer no space before parentheses throughout the 
text so I guess it's just my preference to have it. Very nitpicky even 
if I am right so up to you.



+* I915_GEM_CREATE_EXT_FLAG_NEEDS_CPU_ACCESS), but on
+* such systems we should never actually end up with a
+* small BAR configuration, assuming we are able to load
+* the kernel module. Hence it should be safe to treat
+* this the same as when @probed_cpu_visible_size ==
+* @probed_size.
+*/
+   __u64 probed_cpu_visible_size;
+
+   /**
+* @unallocated_cpu_visible_size: Estimate of CPU
+* visible memory remaining (-1 = unknown).
+*
+* Note this is only currently tracked for
+* I915_MEMORY_CLASS_DEVICE regions, and also requires
+* CAP_PERFMON or CAP_SYS_ADMIN to get reliable

Re: [PATCH v6] drm/msm/dp: Always clear mask bits to disable interrupts at dp_ctrl_reset_irq_ctrl()

2022-05-17 Thread Stephen Boyd

Quoting Kuogee Hsieh (2022-05-12 12:43:18)
> diff --git a/drivers/gpu/drm/msm/dp/dp_ctrl.c 
> b/drivers/gpu/drm/msm/dp/dp_ctrl.c
> index af7a80c..f3e333e 100644
> --- a/drivers/gpu/drm/msm/dp/dp_ctrl.c
> +++ b/drivers/gpu/drm/msm/dp/dp_ctrl.c
> @@ -1389,8 +1389,13 @@ void dp_ctrl_reset_irq_ctrl(struct dp_ctrl *dp_ctrl, 
> bool enable)
>
> dp_catalog_ctrl_reset(ctrl->catalog);
>
> -   if (enable)
> -   dp_catalog_ctrl_enable_irq(ctrl->catalog, enable);
> +   /*
> +* all dp controller programmable registers will not
> +* be reset to default value after DP_SW_RESET
> +* therefore interrupt mask bits have to be updated
> +* to enable/disable interrupts
> +*/
> +   dp_catalog_ctrl_enable_irq(ctrl->catalog, enable);

I think Dmitry requested that this part be split off to a different
patch. It's fixing the call to dp_ctrl_reset_irq_ctrl() for the disable
case.

>  }
>
>  void dp_ctrl_phy_init(struct dp_ctrl *dp_ctrl)
> diff --git a/drivers/gpu/drm/msm/dp/dp_display.c 
> b/drivers/gpu/drm/msm/dp/dp_display.c
> index c388323..ab691aa 100644
> --- a/drivers/gpu/drm/msm/dp/dp_display.c
> +++ b/drivers/gpu/drm/msm/dp/dp_display.c
> @@ -98,6 +98,8 @@ struct dp_display_private {
> struct dp_ctrl*ctrl;
> struct dp_debug   *debug;
>
> +   bool suspended;
> +

Can we not have this flag? I also don't understand how this patch waits
for the event queue to drain. There are now multiple places we check to
see if we've suspended, but is it even possible to be in those places
during suspend? What is wrong with moving to an irq thread? Is it
because we want to wait for a modeset (EV_USER_NOTIFICATION)?

> struct dp_usbpd_cb usbpd_cb;
> struct dp_display_mode dp_mode;
> struct msm_dp dp_display;

[RFC v4] drm: Add initial ci/ subdirectory

2022-05-17 Thread Tomeu Vizoso

And use it to store expectations about what the DRM drivers are
supposed to pass in the IGT test suite.

Also include a configuration file that points to the out-of-tree CI
scripts.

By storing the test expectations along the code we can make sure both
stay in sync with each other, and so we can know when a code change
breaks those expectations.

This will allow all contributors to drm to reuse the infrastructure
already in gitlab.freedesktop.org to test the driver on several
generations of the hardware.

v2:
  - Fix names of result expectation files to match SoC
  - Don't execute tests that are going to skip on all boards

v3:
  - Remove tracking of dmesg output during test execution

v4:
  - Move up to drivers/gpu/drm
  - Add support for a bunch of other drivers
  - Explain how to incorporate fixes for CI from a
${TARGET_BRANCH}-external-fixes branch
  - Remove tests that pass from expected results file, to reduce the
size of in-tree files
  - Add docs about how to deal with outages in automated testing labs
  - Specify the exact SHA of the CI scripts to be used

Signed-off-by: Tomeu Vizoso 
---
 Documentation/gpu/automated_testing.rst   |  84 +++
 drivers/gpu/drm/ci/amdgpu.testlist| 130 +++
 drivers/gpu/drm/ci/amdgpu_stoney_results.txt  |  17 ++
 drivers/gpu/drm/ci/gitlab-ci.yml  |  13 ++
 drivers/gpu/drm/ci/i915.testlist  | 219 ++
 drivers/gpu/drm/ci/i915_amly_results.txt  |   9 +
 drivers/gpu/drm/ci/i915_apl_results.txt   |   9 +
 drivers/gpu/drm/ci/i915_cml_results.txt   |  10 +
 drivers/gpu/drm/ci/i915_glk_results.txt   |  13 ++
 drivers/gpu/drm/ci/i915_kbl_results.txt   |   9 +
 drivers/gpu/drm/ci/i915_tgl_results.txt   |  12 +
 drivers/gpu/drm/ci/i915_whl_results.txt   |   9 +
 drivers/gpu/drm/ci/mediatek.testlist  | 102 
 .../gpu/drm/ci/mediatek_mt8173_results.txt|  11 +
 .../gpu/drm/ci/mediatek_mt8183_results.txt|  11 +
 .../gpu/drm/ci/mediatek_mt8192_results.txt|  11 +
 drivers/gpu/drm/ci/meson.testlist |  71 ++
 drivers/gpu/drm/ci/meson_g12b_results.txt |  11 +
 drivers/gpu/drm/ci/msm.testlist   | 144 
 drivers/gpu/drm/ci/msm_apq8016_results.txt|  77 ++
 drivers/gpu/drm/ci/msm_apq8096_results.txt|  76 ++
 drivers/gpu/drm/ci/msm_sc7180_results.txt |  18 ++
 drivers/gpu/drm/ci/msm_sdm845_results.txt |  33 +++
 drivers/gpu/drm/ci/rockchip.testlist  | 188 +++
 .../gpu/drm/ci/rockchip_rk3288_results.txt|  11 +
 .../gpu/drm/ci/rockchip_rk3399_results.txt|  14 ++
 drivers/gpu/drm/ci/virtio_gpu.testlist|  92 
 drivers/gpu/drm/ci/virtio_gpu_results.txt |  28 +++
 28 files changed, 1432 insertions(+)
 create mode 100644 Documentation/gpu/automated_testing.rst
 create mode 100644 drivers/gpu/drm/ci/amdgpu.testlist
 create mode 100644 drivers/gpu/drm/ci/amdgpu_stoney_results.txt
 create mode 100644 drivers/gpu/drm/ci/gitlab-ci.yml
 create mode 100644 drivers/gpu/drm/ci/i915.testlist
 create mode 100644 drivers/gpu/drm/ci/i915_amly_results.txt
 create mode 100644 drivers/gpu/drm/ci/i915_apl_results.txt
 create mode 100644 drivers/gpu/drm/ci/i915_cml_results.txt
 create mode 100644 drivers/gpu/drm/ci/i915_glk_results.txt
 create mode 100644 drivers/gpu/drm/ci/i915_kbl_results.txt
 create mode 100644 drivers/gpu/drm/ci/i915_tgl_results.txt
 create mode 100644 drivers/gpu/drm/ci/i915_whl_results.txt
 create mode 100644 drivers/gpu/drm/ci/mediatek.testlist
 create mode 100644 drivers/gpu/drm/ci/mediatek_mt8173_results.txt
 create mode 100644 drivers/gpu/drm/ci/mediatek_mt8183_results.txt
 create mode 100644 drivers/gpu/drm/ci/mediatek_mt8192_results.txt
 create mode 100644 drivers/gpu/drm/ci/meson.testlist
 create mode 100644 drivers/gpu/drm/ci/meson_g12b_results.txt
 create mode 100644 drivers/gpu/drm/ci/msm.testlist
 create mode 100644 drivers/gpu/drm/ci/msm_apq8016_results.txt
 create mode 100644 drivers/gpu/drm/ci/msm_apq8096_results.txt
 create mode 100644 drivers/gpu/drm/ci/msm_sc7180_results.txt
 create mode 100644 drivers/gpu/drm/ci/msm_sdm845_results.txt
 create mode 100644 drivers/gpu/drm/ci/rockchip.testlist
 create mode 100644 drivers/gpu/drm/ci/rockchip_rk3288_results.txt
 create mode 100644 drivers/gpu/drm/ci/rockchip_rk3399_results.txt
 create mode 100644 drivers/gpu/drm/ci/virtio_gpu.testlist
 create mode 100644 drivers/gpu/drm/ci/virtio_gpu_results.txt

diff --git a/Documentation/gpu/automated_testing.rst 
b/Documentation/gpu/automated_testing.rst
new file mode 100644
index ..61003ecc7b6e
--- /dev/null
+++ b/Documentation/gpu/automated_testing.rst
@@ -0,0 +1,84 @@
+.. SPDX-License-Identifier: GPL-2.0+
+
+=
+Automated testing of the DRM subsystem
+=
+
+
+Introduction
+
+
+Making sure that changes to the core or drivers don't introduce regressions

Re: [PATCH v1 02/13] dt-bindings: Add headers for Host1x and VIC on Tegra234

On 16/05/2022 12:02, cyn...@kapsi.fi wrote:
> From: Mikko Perttunen 
> 
> Add clock, memory controller, powergate and reset dt-binding headers
> for Host1x and VIC on Tegra234.
> 
> Signed-off-by: Mikko Perttunen 

All your patches are send from wrong email address and the SoB chain is
not correct. Please add respective SoB.

This applies to entire patchset.

Best regards,
Krzysztof

Re: [PATCH v1 03/13] arm64: tegra: Add Host1x and VIC on Tegra234

On 16/05/2022 12:02, cyn...@kapsi.fi wrote:
> From: Mikko Perttunen 
> 
> Add device tree nodes for Host1x and VIC on Tegra234.
> 
> Signed-off-by: Mikko Perttunen 
> ---
>  arch/arm64/boot/dts/nvidia/tegra234.dtsi | 46 
>  1 file changed, 46 insertions(+)
> 
> diff --git a/arch/arm64/boot/dts/nvidia/tegra234.dtsi 
> b/arch/arm64/boot/dts/nvidia/tegra234.dtsi
> index cb3af539e477..cae68e59580c 100644
> --- a/arch/arm64/boot/dts/nvidia/tegra234.dtsi
> +++ b/arch/arm64/boot/dts/nvidia/tegra234.dtsi
> @@ -454,6 +454,52 @@ misc@10 {
>   status = "okay";
>   };
>  
> + host1x@13e0 {

Generic node names, if that possible. Since the bindings do not exist in
the next, I actually cannot figure out what's host1x...

> + compatible = "nvidia,tegra234-host1x";
> + reg = <0x13e0 0x1>,
> +   <0x13e1 0x1>,
> +   <0x13e4 0x1>;
> + reg-names = "common", "hypervisor", "vm";
> + interrupts = ,
> +  ,
> +  ,
> +  ,
> +  ,
> +  ,
> +  ,
> +  ,
> +  ;
> + interrupt-names = "syncpt0", "syncpt1", "syncpt2", 
> "syncpt3", "syncpt4",
> +   "syncpt5", "syncpt6", "syncpt7", 
> "host1x";
> + clocks = < TEGRA234_CLK_HOST1X>;
> + clock-names = "host1x";
> +
> + #address-cells = <1>;
> + #size-cells = <1>;
> +
> + ranges = <0x1500 0x1500 0x0100>;
> + interconnects = < TEGRA234_MEMORY_CLIENT_HOST1XDMAR 
> >;
> + interconnect-names = "dma-mem";
> + iommus = <_niso1 TEGRA234_SID_HOST1X>;
> +
> + vic@1534 {

The same... vic is usually a vectored interrupt controller, so this
should be interrupt-controller. Unless it is something entirely else, so
then you need to come with a generic name.


Best regards,
Krzysztof

Re: [PATCH] drm: Create support for Write-Only property blob

2022-05-17 Thread Jani Nikula

On Tue, 10 May 2022, Mark Yacoub  wrote:
> [Why]
> User space might need to inject data into the kernel without allowing it
> to be read again by any user space.
> An example of where this is particularly useful is secret keys fetched
> by user space and injected into the kernel to enable content protection.

I think we're going to need more than an example in the commit
message. See Documentation/gpu/drm-uapi.rst.

BR,
Jani.


>
> [How]
> Create a DRM_MODE_CREATE_BLOB_WRITE_ONLY flag used by user space to
> create a blob and mark the blob as write only.
> On reading back the blob, data will be not be copied if it's a write
> only blob
>
> Signed-off-by: Mark Yacoub 
>
> ---
>  drivers/gpu/drm/drm_property.c | 3 ++-
>  include/drm/drm_property.h | 2 ++
>  include/uapi/drm/drm_mode.h| 6 ++
>  3 files changed, 10 insertions(+), 1 deletion(-)
>
> diff --git a/drivers/gpu/drm/drm_property.c b/drivers/gpu/drm/drm_property.c
> index dfec479830e4..afedf7109d00 100644
> --- a/drivers/gpu/drm/drm_property.c
> +++ b/drivers/gpu/drm/drm_property.c
> @@ -765,7 +765,7 @@ int drm_mode_getblob_ioctl(struct drm_device *dev,
>   if (!blob)
>   return -ENOENT;
>  
> - if (out_resp->length == blob->length) {
> + if (out_resp->length == blob->length && !blob->is_write_only) {
>   if (copy_to_user(u64_to_user_ptr(out_resp->data),
>blob->data,
>blob->length)) {
> @@ -800,6 +800,7 @@ int drm_mode_createblob_ioctl(struct drm_device *dev,
>   ret = -EFAULT;
>   goto out_blob;
>   }
> + blob->is_write_only = out_resp->flags & DRM_MODE_CREATE_BLOB_WRITE_ONLY;
>  
>   /* Dropping the lock between create_blob and our access here is safe
>* as only the same file_priv can remove the blob; at this point, it is
> diff --git a/include/drm/drm_property.h b/include/drm/drm_property.h
> index 65bc9710a470..700782f021b9 100644
> --- a/include/drm/drm_property.h
> +++ b/include/drm/drm_property.h
> @@ -205,6 +205,7 @@ struct drm_property {
>   *   _mode_config.property_blob_list.
>   * @head_file: entry on the per-file blob list in _file.blobs list.
>   * @length: size of the blob in bytes, invariant over the lifetime of the 
> object
> + * @is_write_only: user space can't read the blob data.
>   * @data: actual data, embedded at the end of this structure
>   *
>   * Blobs are used to store bigger values than what fits directly into the 64
> @@ -219,6 +220,7 @@ struct drm_property_blob {
>   struct list_head head_global;
>   struct list_head head_file;
>   size_t length;
> + bool is_write_only;
>   void *data;
>  };
>  
> diff --git a/include/uapi/drm/drm_mode.h b/include/uapi/drm/drm_mode.h
> index 0a0d56a6158e..de192d3813e9 100644
> --- a/include/uapi/drm/drm_mode.h
> +++ b/include/uapi/drm/drm_mode.h
> @@ -1107,6 +1107,9 @@ struct drm_format_modifier {
>   __u64 modifier;
>  };
>  
> +#define DRM_MODE_CREATE_BLOB_WRITE_ONLY  
>   \
> + (1 << 0) /* data of the blob can't be read by user space */
> +
>  /**
>   * struct drm_mode_create_blob - Create New blob property
>   *
> @@ -1120,6 +1123,9 @@ struct drm_mode_create_blob {
>   __u32 length;
>   /** @blob_id: Return: new property ID. */
>   __u32 blob_id;
> + /** Flags for special handling. */
> + __u32 flags;
> + __u32 pad;
>  };
>  
>  /**

-- 
Jani Nikula, Intel Open Source Graphics Center

Re: [PATCH] drm: Add a debug message when getting a prop is missing

2022-05-17 Thread Jani Nikula

On Tue, 10 May 2022, Mark Yacoub  wrote:
> [Why]
> If a connector property is attached but
> drm_atomic_connector_get_property doesn't handle a case for it,
> modeteset will crash with a segfault without.
>
> [How]
> Add a debug message indicating that a connector property is not handled
> when user space is trying to read it.
>
> TEST=modetest
>
> Signed-off-by: Mark Yacoub 
> ---
>  drivers/gpu/drm/drm_atomic_uapi.c | 6 ++
>  1 file changed, 6 insertions(+)
>
> diff --git a/drivers/gpu/drm/drm_atomic_uapi.c 
> b/drivers/gpu/drm/drm_atomic_uapi.c
> index acb1ee93d206..36b0f664dd80 100644
> --- a/drivers/gpu/drm/drm_atomic_uapi.c
> +++ b/drivers/gpu/drm/drm_atomic_uapi.c
> @@ -884,6 +884,12 @@ drm_atomic_connector_get_property(struct drm_connector 
> *connector,
>   return connector->funcs->atomic_get_property(connector,
>   state, property, val);
>   } else {
> + // LOG that the kernel is missing handling this property as a 
> case here.

The comment is unnecessary, and we also don't use // comments.

> + drm_dbg_atomic(
> + dev,
> + "[CONNECTOR:%d:%s] Get Property [PROP:%d:%s] is not 
> handled\n",
> + connector->base.id, connector->name, property->base.id,
> + property->name);

Why not use the same message as in drm_atomic_connector_set_property()?

Blank line here.

BR,
Jani.

>   return -EINVAL;
>   }

-- 
Jani Nikula, Intel Open Source Graphics Center

Re: [PATCH v1] drm/scheduler: Don't kill jobs in interrupt context

2022-05-17 Thread Erico Nunes

On Wed, Apr 13, 2022 at 12:05 PM Steven Price  wrote:
>
> On 11/04/2022 23:15, Dmitry Osipenko wrote:
> > Interrupt context can't sleep. Drivers like Panfrost and MSM are taking
> > mutex when job is released, and thus, that code can sleep. This results
> > into "BUG: scheduling while atomic" if locks are contented while job is
> > freed. There is no good reason for releasing scheduler's jobs in IRQ
> > context, hence use normal context to fix the trouble.
> >
> > Cc: sta...@vger.kernel.org
> > Fixes: 542cff7893a3 ("drm/sched: Avoid lockdep spalt on killing a 
> > processes")
> > Signed-off-by: Dmitry Osipenko 
>
> Reviewed-by: Steven Price 

Is there something blocking this patch?
Mesa CI is still hitting the issue and I have been waiting for it to
be applied/backported to update CI with it.
Thanks

Erico

Re: linux-next: Tree for May 16 (drm/i915/gt/intel_gt_sysfs_pm.c)




Hi,

On 16/05/2022 22:22, Randy Dunlap wrote:



On 5/16/22 03:57, Stephen Rothwell wrote:

Hi all,

Changes since 20220513:



on i386:

   CC  drivers/gpu/drm/i915/gt/intel_gt_sysfs_pm.o
../drivers/gpu/drm/i915/gt/intel_gt_sysfs_pm.c: In function ‘act_freq_mhz_show’:
../drivers/gpu/drm/i915/gt/intel_gt_sysfs_pm.c:276:20: error: implicit 
declaration of function ‘sysfs_gt_attribute_r_max_func’ 
[-Werror=implicit-function-declaration]
   u32 actual_freq = sysfs_gt_attribute_r_max_func(dev, attr,
 ^
../drivers/gpu/drm/i915/gt/intel_gt_sysfs_pm.c: In function 
‘boost_freq_mhz_store’:
../drivers/gpu/drm/i915/gt/intel_gt_sysfs_pm.c:327:9: error: implicit 
declaration of function ‘sysfs_gt_attribute_w_func’ 
[-Werror=implicit-function-declaration]
   return sysfs_gt_attribute_w_func(dev, attr,
  ^
../drivers/gpu/drm/i915/gt/intel_gt_sysfs_pm.c: In function ‘min_freq_mhz_show’:
../drivers/gpu/drm/i915/gt/intel_gt_sysfs_pm.c:416:17: error: implicit 
declaration of function ‘sysfs_gt_attribute_r_min_func’ 
[-Werror=implicit-function-declaration]
   u32 min_freq = sysfs_gt_attribute_r_min_func(dev, attr,
  ^
cc1: some warnings being treated as errors


Full randconfig file is attached.


There is a fix for this in 09708b6d82ef ("drm/i915/gt: Fix build error 
without CONFIG_PM") queued up, waiting for the next pull request, which 
the plan was to send out next week or so. Is that okay?


Regards,

Tvrtko

Re: [PATCH 1/3] dt-bindings: display: simple: add support for Samsung LTL101AL01