Re: [PATCH v3 12/14] leds: mt6370: Add Mediatek MT6370 current sink type LED Indicator support
Hi Linus, Thank you for the comment. Linus Walleij 於 2022年6月24日 週五 下午2:25寫道： > > On Fri, Jun 24, 2022 at 8:23 AM Linus Walleij > wrote: > > Thanks for your patch! > > > > On Thu, Jun 23, 2022 at 1:58 PM ChiaEn Wu wrote: > > > > > From: ChiYuan Huang > > > > > > Add Mediatek MT6370 current sink type LED Indicator driver. > > > > > > Signed-off-by: ChiYuan Huang > > (...) > > > drivers/leds/Kconfig | 11 + > > > drivers/leds/Makefile | 1 + > > > drivers/leds/leds-mt6370.c | 989 > > > + > > > > There is a drivers/leds/flash subdirectory these days, put the driver > > in that directory instead. > > Sorry I'm commenting on the wrong patch. > > I meant this one. Move that into drivers/leds/flash > drivers/leds/flash/leds-mt6370-flash.c | 657 In next version, I'll use "leds: flash: .." instead of "leds: flashlight: .." in subject. May I confirm that the driver has already in the drivers/leds/flash, so I don’t have to move it in next version? Sincerely, Alice Chen
Re: [PATCH] drm/ast: Fix black screen when getting out of suspend
Hi
Am 22.06.22 um 14:48 schrieb Jocelyn Falempe:
With an AST2600, the screen is garbage when going out of suspend.
This is because color settings are lost, and not restored on resume.
Force the color settings on DPMS_ON, to make sure the settings are correct.
I didn't write this code, it comes from the out-of-tree aspeed driver v1.13
https://www.aspeedtech.com/support_driver/
Signed-off-by: Jocelyn Falempe
Tested-by: Venkat Tadikonda
---
drivers/gpu/drm/ast/ast_mode.c | 13 +
1 file changed, 13 insertions(+)
diff --git a/drivers/gpu/drm/ast/ast_mode.c b/drivers/gpu/drm/ast/ast_mode.c
index 3eb9afecd9d4..cdddcb5c4439 100644
--- a/drivers/gpu/drm/ast/ast_mode.c
+++ b/drivers/gpu/drm/ast/ast_mode.c
@@ -990,6 +990,9 @@ static void ast_crtc_dpms(struct drm_crtc *crtc, int mode)
{
struct ast_private *ast = to_ast_private(crtc->dev);
u8 ch = AST_DPMS_VSYNC_OFF | AST_DPMS_HSYNC_OFF;
+ struct ast_crtc_state *ast_state;
+ const struct drm_format_info *format;
+ struct ast_vbios_mode_info *vbios_mode_info;
/* TODO: Maybe control display signal generation with
* Sync Enable (bit CR17.7).
@@ -1007,6 +1010,16 @@ static void ast_crtc_dpms(struct drm_crtc *crtc, int
mode)
ast_dp_set_on_off(crtc->dev, 1);
}
+ ast_state = to_ast_crtc_state(crtc->state);
+ format = ast_state->format;
+
+ if (format){
+ vbios_mode_info = &ast_state->vbios_mode_info;
+
+ ast_set_color_reg(ast, format);
+ ast_set_vbios_color_reg(ast, format, vbios_mode_info);
+ }
+
I've seen suspend issues on other AST devices besides the 2600. This
seems to be an improvement on AST2300 at least. Therefore
Tested-by: Thomas Zimmermann
The DPMS code need to be integrated into atomic_enable at some point.
(It's for a later patchset.) It's a relict of the old non-atomic
modesetting that never got done correctly.
Best regards
Thomas
ast_crtc_load_lut(ast, crtc);
break;
case DRM_MODE_DPMS_STANDBY:
--
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] drm/rockchip: vop: Don't crash for invalid duplicate_state()
Am Freitag, 24. Juni 2022, 01:44:52 CEST schrieb Doug Anderson: > Hi, > > On Fri, Jun 17, 2022 at 5:27 PM Brian Norris wrote: > > > > It's possible for users to try to duplicate the CRTC state even when the > > state doesn't exist. drm_atomic_helper_crtc_duplicate_state() (and other > > users of __drm_atomic_helper_crtc_duplicate_state()) already guard this > > with a WARN_ON() instead of crashing, so let's do that here too. > > > > Signed-off-by: Brian Norris > > --- > > > > drivers/gpu/drm/rockchip/rockchip_drm_vop.c | 3 +++ > > 1 file changed, 3 insertions(+) > > I'm not an expert in this area, but it makes sense to me to match > drm_atomic_helper_crtc_duplicate_state() in this way. Thus: > > Reviewed-by: Douglas Anderson > > I would tend to assume that this would be landed in drm-misc by Heiko > if he's good with it. After several weeks of silence, however, I'll > commit it myself. I do tend to batch up drm-misc patches, as that is always a different workflow but I'll pick that up :-) The interesting question would be, do we want some fixes tag for it? Heiko
[RFC] Per file OOM-badness / RSS once more
Hello everyone, To summarize the issue I'm trying to address here: Processes can allocate resources through a file descriptor without being held responsible for it. I'm not explaining all the details again. See here for a more deeply description of the problem: https://lwn.net/ml/linux-kernel/2022053117.174649-1-christian.koe...@amd.com/ With this iteration I'm trying to address a bunch of the comments Michal Hocko (thanks a lot for that) gave as well as giving some new ideas. Changes made so far: 1. Renamed the callback into file_rss(). This is at least a start to better describe what this is all about. I've been going back and forth over the naming here, if you have any better idea please speak up. 2. Cleanups, e.g. now providing a helper function in the fs layer to sum up all the pages allocated by the files in a file descriptor table. 3. Using the actual number of allocated pages for the shmem implementation instead of just the size. I also tried to ignore shmem files which are part of tmpfs, cause that has a separate accounting/limitation approach. 4. The OOM killer now prints the memory of the killed process including the per file pages which makes the whole things much more comprehensible. 5. I've added the per file pages to the different reports in RSS in procfs. This has the interesting effect that tools like top suddenly give a much more accurate overview of the memory use as well. This of course increases the overhead of gathering those information quite a bit and I'm not sure how feasible that is for up-streaming. On the other hand this once more clearly shows that we need to do something about this issue. Another rather interesting observation is that multiple subsystems (shmem, tmpfs, ttm) came up with the same workaround of limiting the memory which can be allocated through them to 50% of the whole system memory. Unfortunately that isn't the same 50% and it doesn't apply everywhere, so you can still easily crash the box. Ideas and/or comments are really welcome. Thanks, Christian.
[PATCH 01/14] fs: add per file RSS
From: Andrey Grodzovsky
Some files allocate large amounts of memory on behalf of userspace without
any on disk backing store. This memory isn't necessarily mapped into the
address space, but should still accounts towards the RSS of a process just
like mapped shared pages do.
That information can then be used by the OOM killer to make better
decisions which process to reap.
For easy debugging this also adds printing of the per file RSS to fdinfo.
Signed-off-by: Andrey Grodzovsky
Signed-off-by: Christian König
---
fs/file.c | 23 +++
fs/proc/fd.c| 3 +++
include/linux/fdtable.h | 1 +
include/linux/fs.h | 1 +
4 files changed, 28 insertions(+)
diff --git a/fs/file.c b/fs/file.c
index 3bcc1ecc314a..b58730a513be 100644
--- a/fs/file.c
+++ b/fs/file.c
@@ -1307,3 +1307,26 @@ int iterate_fd(struct files_struct *files, unsigned n,
return res;
}
EXPORT_SYMBOL(iterate_fd);
+
+static int sumup_file_rss(const void *sum, struct file *file, unsigned n)
+{
+ if (!file->f_op->file_rss)
+ return 0;
+
+ *((unsigned long *)sum) += file->f_op->file_rss(file);
+ return 0;
+}
+
+/**
+ * files_rss- how much resources are bound by opened files
+ * @files: opened files
+ *
+ * Returns sum of all resources bound by files not accounted in file systems.
+ */
+unsigned long files_rss(struct files_struct *files)
+{
+ unsigned long sum = 0;
+
+ iterate_fd(files, 0, sumup_file_rss, &sum);
+ return sum;
+}
diff --git a/fs/proc/fd.c b/fs/proc/fd.c
index 913bef0d2a36..9943bfca74f7 100644
--- a/fs/proc/fd.c
+++ b/fs/proc/fd.c
@@ -59,6 +59,9 @@ static int seq_show(struct seq_file *m, void *v)
real_mount(file->f_path.mnt)->mnt_id,
file_inode(file)->i_ino);
+ if (file->f_op->file_rss)
+ seq_printf(m, "rss:\t%lu\n", file->f_op->file_rss(file));
+
/* show_fd_locks() never deferences files so a stale value is safe */
show_fd_locks(m, file, files);
if (seq_has_overflowed(m))
diff --git a/include/linux/fdtable.h b/include/linux/fdtable.h
index e066816f3519..101770266f38 100644
--- a/include/linux/fdtable.h
+++ b/include/linux/fdtable.h
@@ -122,6 +122,7 @@ void do_close_on_exec(struct files_struct *);
int iterate_fd(struct files_struct *, unsigned,
int (*)(const void *, struct file *, unsigned),
const void *);
+unsigned long files_rss(struct files_struct *files);
extern int close_fd(unsigned int fd);
extern int __close_range(unsigned int fd, unsigned int max_fd, unsigned int
flags);
diff --git a/include/linux/fs.h b/include/linux/fs.h
index 9ad5e3520fae..edacbdce5e4c 100644
--- a/include/linux/fs.h
+++ b/include/linux/fs.h
@@ -2003,6 +2003,7 @@ struct file_operations {
loff_t len, unsigned int remap_flags);
int (*fadvise)(struct file *, loff_t, loff_t, int);
int (*uring_cmd)(struct io_uring_cmd *ioucmd, unsigned int issue_flags);
+ long (*file_rss)(struct file *);
} __randomize_layout;
struct inode_operations {
--
2.25.1
[PATCH 02/14] oom: take per file RSS into account
From: Andrey Grodzovsky
Try to make better decisions which process to kill based on
per file RSS.
Signed-off-by: Andrey Grodzovsky
---
mm/oom_kill.c | 13 -
1 file changed, 8 insertions(+), 5 deletions(-)
diff --git a/mm/oom_kill.c b/mm/oom_kill.c
index 3c6cf9e3cd66..76a5ea73eb6a 100644
--- a/mm/oom_kill.c
+++ b/mm/oom_kill.c
@@ -18,6 +18,7 @@
* kernel subsystems and hints as to where to find out what things do.
*/
+#include
#include
#include
#include
@@ -228,7 +229,8 @@ long oom_badness(struct task_struct *p, unsigned long
totalpages)
* task's rss, pagetable and swap space use.
*/
points = get_mm_rss(p->mm) + get_mm_counter(p->mm, MM_SWAPENTS) +
- mm_pgtables_bytes(p->mm) / PAGE_SIZE;
+ files_rss(p->files) + mm_pgtables_bytes(p->mm) / PAGE_SIZE;
+
task_unlock(p);
/* Normalize to oom_score_adj units */
@@ -401,8 +403,8 @@ static int dump_task(struct task_struct *p, void *arg)
pr_info("[%7d] %5d %5d %8lu %8lu %8ld %8lu %5hd %s\n",
task->pid, from_kuid(&init_user_ns, task_uid(task)),
- task->tgid, task->mm->total_vm, get_mm_rss(task->mm),
- mm_pgtables_bytes(task->mm),
+ task->tgid, task->mm->total_vm, get_mm_rss(task->mm) +
+ files_rss(task->files), mm_pgtables_bytes(task->mm),
get_mm_counter(task->mm, MM_SWAPENTS),
task->signal->oom_score_adj, task->comm);
task_unlock(task);
@@ -594,7 +596,8 @@ static bool oom_reap_task_mm(struct task_struct *tsk,
struct mm_struct *mm)
pr_info("oom_reaper: reaped process %d (%s), now anon-rss:%lukB,
file-rss:%lukB, shmem-rss:%lukB\n",
task_pid_nr(tsk), tsk->comm,
K(get_mm_counter(mm, MM_ANONPAGES)),
- K(get_mm_counter(mm, MM_FILEPAGES)),
+ K(get_mm_counter(mm, MM_FILEPAGES) +
+ files_rss(tsk->files)),
K(get_mm_counter(mm, MM_SHMEMPAGES)));
out_finish:
trace_finish_task_reaping(tsk->pid);
@@ -950,7 +953,7 @@ static void __oom_kill_process(struct task_struct *victim,
const char *message)
pr_err("%s: Killed process %d (%s) total-vm:%lukB, anon-rss:%lukB,
file-rss:%lukB, shmem-rss:%lukB, UID:%u pgtables:%lukB oom_score_adj:%hd\n",
message, task_pid_nr(victim), victim->comm, K(mm->total_vm),
K(get_mm_counter(mm, MM_ANONPAGES)),
- K(get_mm_counter(mm, MM_FILEPAGES)),
+ K(get_mm_counter(mm, MM_FILEPAGES) + files_rss(victim->files)),
K(get_mm_counter(mm, MM_SHMEMPAGES)),
from_kuid(&init_user_ns, task_uid(victim)),
mm_pgtables_bytes(mm) >> 10, victim->signal->oom_score_adj);
--
2.25.1
[PATCH 03/14] proc: expose per file RSS
Add the per file RSS to the memory management accounting.
This allows to see the per file RSS in tools like top as well.
Signed-off-by: Christian König
---
fs/proc/array.c | 7 +--
fs/proc/internal.h | 3 ++-
fs/proc/task_mmu.c | 6 --
fs/proc/task_nommu.c | 3 ++-
4 files changed, 13 insertions(+), 6 deletions(-)
diff --git a/fs/proc/array.c b/fs/proc/array.c
index eb815759842c..a3aabe4ac7c8 100644
--- a/fs/proc/array.c
+++ b/fs/proc/array.c
@@ -437,7 +437,7 @@ int proc_pid_status(struct seq_file *m, struct
pid_namespace *ns,
task_state(m, ns, pid, task);
if (mm) {
- task_mem(m, mm);
+ task_mem(m, mm, task->files);
task_core_dumping(m, task);
task_thp_status(m, mm);
mmput(mm);
@@ -589,7 +589,8 @@ static int do_task_stat(struct seq_file *m, struct
pid_namespace *ns,
seq_put_decimal_ull(m, " ", 0);
seq_put_decimal_ull(m, " ", start_time);
seq_put_decimal_ull(m, " ", vsize);
- seq_put_decimal_ull(m, " ", mm ? get_mm_rss(mm) : 0);
+ seq_put_decimal_ull(m, " ", (mm ? get_mm_rss(mm) : 0) +
+ files_rss(task->files));
seq_put_decimal_ull(m, " ", rsslim);
seq_put_decimal_ull(m, " ", mm ? (permitted ? mm->start_code : 1) : 0);
seq_put_decimal_ull(m, " ", mm ? (permitted ? mm->end_code : 1) : 0);
@@ -673,6 +674,8 @@ int proc_pid_statm(struct seq_file *m, struct pid_namespace
*ns,
size = task_statm(mm, &shared, &text, &data, &resident);
mmput(mm);
+ shared += files_rss(task->files);
+
/*
* For quick read, open code by putting numbers directly
* expected format is
diff --git a/fs/proc/internal.h b/fs/proc/internal.h
index 06a80f78433d..1b123893f7ae 100644
--- a/fs/proc/internal.h
+++ b/fs/proc/internal.h
@@ -305,7 +305,8 @@ extern unsigned long task_vsize(struct mm_struct *);
extern unsigned long task_statm(struct mm_struct *,
unsigned long *, unsigned long *,
unsigned long *, unsigned long *);
-extern void task_mem(struct seq_file *, struct mm_struct *);
+extern void task_mem(struct seq_file *, struct mm_struct *,
+struct files_struct *);
extern const struct dentry_operations proc_net_dentry_ops;
static inline void pde_force_lookup(struct proc_dir_entry *pde)
diff --git a/fs/proc/task_mmu.c b/fs/proc/task_mmu.c
index 2d04e3470d4c..a4adc6fc13d3 100644
--- a/fs/proc/task_mmu.c
+++ b/fs/proc/task_mmu.c
@@ -20,6 +20,7 @@
#include
#include
#include
+#include
#include
#include
@@ -28,13 +29,14 @@
#define SEQ_PUT_DEC(str, val) \
seq_put_decimal_ull_width(m, str, (val) << (PAGE_SHIFT-10), 8)
-void task_mem(struct seq_file *m, struct mm_struct *mm)
+void task_mem(struct seq_file *m, struct mm_struct *mm,
+ struct files_struct *files)
{
unsigned long text, lib, swap, anon, file, shmem;
unsigned long hiwater_vm, total_vm, hiwater_rss, total_rss;
anon = get_mm_counter(mm, MM_ANONPAGES);
- file = get_mm_counter(mm, MM_FILEPAGES);
+ file = get_mm_counter(mm, MM_FILEPAGES) + files_rss(files);
shmem = get_mm_counter(mm, MM_SHMEMPAGES);
/*
diff --git a/fs/proc/task_nommu.c b/fs/proc/task_nommu.c
index a6d21fc0033c..5b6b9c5ed9ec 100644
--- a/fs/proc/task_nommu.c
+++ b/fs/proc/task_nommu.c
@@ -18,7 +18,8 @@
* each process that owns it. Non-shared memory is counted
* accurately.
*/
-void task_mem(struct seq_file *m, struct mm_struct *mm)
+void task_mem(struct seq_file *m, struct mm_struct *mm,
+ struct files_struct *files)
{
struct vm_area_struct *vma;
struct vm_region *region;
--
2.25.1
[PATCH 06/14] drm/gem: adjust per file RSS on handling buffers
From: Andrey Grodzovsky
Large amounts of VRAM are usually not CPU accessible, so they are not mapped
into the processes address space. But since the device drivers usually support
swapping buffers from VRAM to system memory we can still run into an out of
memory situation when userspace starts to allocate to much.
This patch gives the OOM killer another hint which process is
holding references to memory resources.
A GEM helper is provided and automatically used for all drivers using the
DEFINE_DRM_GEM_FOPS() and DEFINE_DRM_GEM_CMA_FOPS() macros.
Signed-off-by: Andrey Grodzovsky
---
drivers/gpu/drm/drm_file.c | 24
drivers/gpu/drm/drm_gem.c| 5 +
include/drm/drm_file.h | 9 +
include/drm/drm_gem.h| 1 +
include/drm/drm_gem_cma_helper.h | 1 +
5 files changed, 40 insertions(+)
diff --git a/drivers/gpu/drm/drm_file.c b/drivers/gpu/drm/drm_file.c
index ed25168619fc..b60795c5067c 100644
--- a/drivers/gpu/drm/drm_file.c
+++ b/drivers/gpu/drm/drm_file.c
@@ -1049,3 +1049,27 @@ unsigned long drm_get_unmapped_area(struct file *file,
#endif /* CONFIG_TRANSPARENT_HUGEPAGE */
EXPORT_SYMBOL_GPL(drm_get_unmapped_area);
#endif /* CONFIG_MMU */
+
+
+/**
+ * drm_file_rss() - get number of pages held by struct drm_file
+ * @f: struct drm_file to get the number of pages for
+ *
+ * Return how many pages are allocated for this client.
+ */
+long drm_file_rss(struct file *f)
+{
+
+ struct drm_file *file_priv = f->private_data;
+
+ if (!file_priv)
+ return 0;
+
+ /*
+* Since DRM file descriptors are often DUP()ed divide by the file count
+* reference so that each descriptor gets an equal share.
+*/
+ return DIV_ROUND_UP(atomic_long_read(&file_priv->f_rss),
+ file_count(f));
+}
+EXPORT_SYMBOL(drm_file_rss);
diff --git a/drivers/gpu/drm/drm_gem.c b/drivers/gpu/drm/drm_gem.c
index eb0c2d041f13..69b3e93db816 100644
--- a/drivers/gpu/drm/drm_gem.c
+++ b/drivers/gpu/drm/drm_gem.c
@@ -256,6 +256,7 @@ drm_gem_object_release_handle(int id, void *ptr, void *data)
drm_gem_remove_prime_handles(obj, file_priv);
drm_vma_node_revoke(&obj->vma_node, file_priv);
+ atomic_long_sub(obj->size >> PAGE_SHIFT, &file_priv->f_rss);
drm_gem_object_handle_put_unlocked(obj);
return 0;
@@ -291,6 +292,8 @@ drm_gem_handle_delete(struct drm_file *filp, u32 handle)
idr_remove(&filp->object_idr, handle);
spin_unlock(&filp->table_lock);
+ atomic_long_sub(obj->size >> PAGE_SHIFT, &filp->f_rss);
+
return 0;
}
EXPORT_SYMBOL(drm_gem_handle_delete);
@@ -399,6 +402,8 @@ drm_gem_handle_create_tail(struct drm_file *file_priv,
}
*handlep = handle;
+
+ atomic_long_add(obj->size >> PAGE_SHIFT, &file_priv->f_rss);
return 0;
err_revoke:
diff --git a/include/drm/drm_file.h b/include/drm/drm_file.h
index e0a73a1e2df7..7c6ca13d8549 100644
--- a/include/drm/drm_file.h
+++ b/include/drm/drm_file.h
@@ -366,6 +366,13 @@ struct drm_file {
#if IS_ENABLED(CONFIG_DRM_LEGACY)
unsigned long lock_count; /* DRI1 legacy lock count */
#endif
+
+ /**
+* @f_rss:
+*
+* How many pages are allocated through this driver connection.
+*/
+ atomic_long_t f_rss;
};
/**
@@ -430,4 +437,6 @@ unsigned long drm_get_unmapped_area(struct file *file,
#endif /* CONFIG_MMU */
+long drm_file_rss(struct file *f);
+
#endif /* _DRM_FILE_H_ */
diff --git a/include/drm/drm_gem.h b/include/drm/drm_gem.h
index 9d7c61a122dc..b64cad26e9e9 100644
--- a/include/drm/drm_gem.h
+++ b/include/drm/drm_gem.h
@@ -338,6 +338,7 @@ struct drm_gem_object {
.read = drm_read,\
.llseek = noop_llseek,\
.mmap = drm_gem_mmap,\
+ .file_rss = drm_file_rss,\
}
void drm_gem_object_release(struct drm_gem_object *obj);
diff --git a/include/drm/drm_gem_cma_helper.h b/include/drm/drm_gem_cma_helper.h
index fbda4ce5d5fb..8c56cbc8d10f 100644
--- a/include/drm/drm_gem_cma_helper.h
+++ b/include/drm/drm_gem_cma_helper.h
@@ -273,6 +273,7 @@ unsigned long drm_gem_cma_get_unmapped_area(struct file
*filp,
.read = drm_read,\
.llseek = noop_llseek,\
.mmap = drm_gem_mmap,\
+ .file_rss = drm_file_rss,\
DRM_GEM_CMA_UNMAPPED_AREA_FOPS \
}
--
2.25.1
[PATCH 04/14] mm: shmem: provide RSS for shmem files
This gives the OOM killer an additional hint which processes are
referencing shmem files with potentially no other accounting for them.
Signed-off-by: Christian König
---
mm/shmem.c | 16
1 file changed, 16 insertions(+)
diff --git a/mm/shmem.c b/mm/shmem.c
index a6f565308133..b068ac5ba4bf 100644
--- a/mm/shmem.c
+++ b/mm/shmem.c
@@ -2209,6 +2209,21 @@ unsigned long shmem_get_unmapped_area(struct file *file,
return inflated_addr;
}
+static long shmem_file_rss(struct file *file)
+{
+ struct inode *inode = file_inode(file);
+ unsigned long nrpages;
+
+ /* Only account shmem files which aren't part of any fs */
+ if (atomic_read(&inode->i_count) > 1)
+ return 0;
+
+ xa_lock(&file->f_mapping->i_pages);
+ nrpages = file->f_mapping->nrpages;
+ xa_unlock(&file->f_mapping->i_pages);
+ return nrpages;
+}
+
#ifdef CONFIG_NUMA
static int shmem_set_policy(struct vm_area_struct *vma, struct mempolicy *mpol)
{
@@ -3811,6 +3826,7 @@ EXPORT_SYMBOL(shmem_aops);
static const struct file_operations shmem_file_operations = {
.mmap = shmem_mmap,
.get_unmapped_area = shmem_get_unmapped_area,
+ .file_rss = shmem_file_rss,
#ifdef CONFIG_TMPFS
.llseek = shmem_file_llseek,
.read_iter = shmem_file_read_iter,
--
2.25.1
[PATCH 07/14] drm/gma500: use drm_file_rss
This allows the OOM killer to make a better decision which process to reap.
Signed-off-by: Christian König
---
drivers/gpu/drm/gma500/psb_drv.c | 1 +
1 file changed, 1 insertion(+)
diff --git a/drivers/gpu/drm/gma500/psb_drv.c b/drivers/gpu/drm/gma500/psb_drv.c
index 1d8744f3e702..92c005aa6e9e 100644
--- a/drivers/gpu/drm/gma500/psb_drv.c
+++ b/drivers/gpu/drm/gma500/psb_drv.c
@@ -513,6 +513,7 @@ static const struct file_operations psb_gem_fops = {
.mmap = drm_gem_mmap,
.poll = drm_poll,
.read = drm_read,
+ .file_rss = drm_file_rss,
};
static const struct drm_driver driver = {
--
2.25.1
[PATCH 05/14] dma-buf: provide file RSS for DMA-buf files
Just return the size of the DMA-buf in pages since pages allocated or
mapped through DMA-bufs are usually not accounted elsewhere.
Signed-off-by: Christian König
---
drivers/dma-buf/dma-buf.c | 6 ++
1 file changed, 6 insertions(+)
diff --git a/drivers/dma-buf/dma-buf.c b/drivers/dma-buf/dma-buf.c
index 630133284e2b..16162ec3538c 100644
--- a/drivers/dma-buf/dma-buf.c
+++ b/drivers/dma-buf/dma-buf.c
@@ -494,6 +494,11 @@ static void dma_buf_show_fdinfo(struct seq_file *m, struct
file *file)
spin_unlock(&dmabuf->name_lock);
}
+static long dma_buf_file_rss(struct file *file)
+{
+ return i_size_read(file_inode(file)) >> PAGE_SHIFT;
+}
+
static const struct file_operations dma_buf_fops = {
.release= dma_buf_file_release,
.mmap = dma_buf_mmap_internal,
@@ -502,6 +507,7 @@ static const struct file_operations dma_buf_fops = {
.unlocked_ioctl = dma_buf_ioctl,
.compat_ioctl = compat_ptr_ioctl,
.show_fdinfo= dma_buf_show_fdinfo,
+ .file_rss = dma_buf_file_rss,
};
/*
--
2.25.1
[PATCH 08/14] drm/amdgpu: use drm_file_rss
From: Andrey Grodzovsky
This allows the OOM killer to make a better decision which process to reap.
Signed-off-by: Andrey Grodzovsky
---
drivers/gpu/drm/amd/amdgpu/amdgpu_drv.c | 3 ++-
1 file changed, 2 insertions(+), 1 deletion(-)
diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_drv.c
b/drivers/gpu/drm/amd/amdgpu/amdgpu_drv.c
index 8890300766a5..4508791fe80c 100644
--- a/drivers/gpu/drm/amd/amdgpu/amdgpu_drv.c
+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_drv.c
@@ -2635,8 +2635,9 @@ static const struct file_operations
amdgpu_driver_kms_fops = {
.compat_ioctl = amdgpu_kms_compat_ioctl,
#endif
#ifdef CONFIG_PROC_FS
- .show_fdinfo = amdgpu_show_fdinfo
+ .show_fdinfo = amdgpu_show_fdinfo,
#endif
+ .file_rss = drm_file_rss,
};
int amdgpu_file_to_fpriv(struct file *filp, struct amdgpu_fpriv **fpriv)
--
2.25.1
[PATCH 09/14] drm/radeon: use drm_oom_badness
This allows the OOM killer to make a better decision which process to reap.
Signed-off-by: Christian König
---
drivers/gpu/drm/radeon/radeon_drv.c | 1 +
1 file changed, 1 insertion(+)
diff --git a/drivers/gpu/drm/radeon/radeon_drv.c
b/drivers/gpu/drm/radeon/radeon_drv.c
index 956c72b5aa33..11d310cdd2e8 100644
--- a/drivers/gpu/drm/radeon/radeon_drv.c
+++ b/drivers/gpu/drm/radeon/radeon_drv.c
@@ -550,6 +550,7 @@ static const struct file_operations radeon_driver_kms_fops
= {
#ifdef CONFIG_COMPAT
.compat_ioctl = radeon_kms_compat_ioctl,
#endif
+ .file_rss = drm_file_rss,
};
static const struct drm_ioctl_desc radeon_ioctls_kms[] = {
--
2.25.1
[PATCH 10/14] drm/i915: use drm_file_rss
This allows the OOM killer to make a better decision which process to reap.
Signed-off-by: Christian König
---
drivers/gpu/drm/i915/i915_driver.c | 1 +
1 file changed, 1 insertion(+)
diff --git a/drivers/gpu/drm/i915/i915_driver.c
b/drivers/gpu/drm/i915/i915_driver.c
index 90b0ce5051af..fc269055a07c 100644
--- a/drivers/gpu/drm/i915/i915_driver.c
+++ b/drivers/gpu/drm/i915/i915_driver.c
@@ -1741,6 +1741,7 @@ static const struct file_operations i915_driver_fops = {
#ifdef CONFIG_PROC_FS
.show_fdinfo = i915_drm_client_fdinfo,
#endif
+ .file_rss = drm_file_rss,
};
static int
--
2.25.1
[PATCH 11/14] drm/nouveau: use drm_file_rss
This allows the OOM killer to make a better decision which process to reap.
Signed-off-by: Christian König
---
drivers/gpu/drm/nouveau/nouveau_drm.c | 1 +
1 file changed, 1 insertion(+)
diff --git a/drivers/gpu/drm/nouveau/nouveau_drm.c
b/drivers/gpu/drm/nouveau/nouveau_drm.c
index 561309d447e0..cc0ac7b059fe 100644
--- a/drivers/gpu/drm/nouveau/nouveau_drm.c
+++ b/drivers/gpu/drm/nouveau/nouveau_drm.c
@@ -1218,6 +1218,7 @@ nouveau_driver_fops = {
.compat_ioctl = nouveau_compat_ioctl,
#endif
.llseek = noop_llseek,
+ .file_rss = drm_file_rss,
};
static struct drm_driver
--
2.25.1
[PATCH 12/14] drm/omap: use drm_file_rss
This allows the OOM killer to make a better decision which process to reap.
Signed-off-by: Christian König
---
drivers/gpu/drm/omapdrm/omap_drv.c | 1 +
1 file changed, 1 insertion(+)
diff --git a/drivers/gpu/drm/omapdrm/omap_drv.c
b/drivers/gpu/drm/omapdrm/omap_drv.c
index eaf67b9e5f12..dff637de00a3 100644
--- a/drivers/gpu/drm/omapdrm/omap_drv.c
+++ b/drivers/gpu/drm/omapdrm/omap_drv.c
@@ -684,6 +684,7 @@ static const struct file_operations omapdriver_fops = {
.poll = drm_poll,
.read = drm_read,
.llseek = noop_llseek,
+ .file_rss = drm_file_rss,
};
static const struct drm_driver omap_drm_driver = {
--
2.25.1
[PATCH 13/14] drm/vmwgfx: use drm_file_rss
This allows the OOM killer to make a better decision which process to reap.
Signed-off-by: Christian König
---
drivers/gpu/drm/vmwgfx/vmwgfx_drv.c | 1 +
1 file changed, 1 insertion(+)
diff --git a/drivers/gpu/drm/vmwgfx/vmwgfx_drv.c
b/drivers/gpu/drm/vmwgfx/vmwgfx_drv.c
index 01a5b47e95f9..99bf405d31b9 100644
--- a/drivers/gpu/drm/vmwgfx/vmwgfx_drv.c
+++ b/drivers/gpu/drm/vmwgfx/vmwgfx_drv.c
@@ -1577,6 +1577,7 @@ static const struct file_operations vmwgfx_driver_fops = {
#endif
.llseek = noop_llseek,
.get_unmapped_area = vmw_get_unmapped_area,
+ .file_rss = drm_file_rss,
};
static const struct drm_driver driver = {
--
2.25.1
[PATCH 14/14] drm/tegra: use drm_file_rss
This allows the OOM killer to make a better decision which process to reap.
Signed-off-by: Christian König
---
drivers/gpu/drm/tegra/drm.c | 1 +
1 file changed, 1 insertion(+)
diff --git a/drivers/gpu/drm/tegra/drm.c b/drivers/gpu/drm/tegra/drm.c
index 4cdc8faf798f..cc0c2fc57250 100644
--- a/drivers/gpu/drm/tegra/drm.c
+++ b/drivers/gpu/drm/tegra/drm.c
@@ -804,6 +804,7 @@ static const struct file_operations tegra_drm_fops = {
.read = drm_read,
.compat_ioctl = drm_compat_ioctl,
.llseek = noop_llseek,
+ .file_rss = drm_file_rss,
};
static int tegra_drm_context_cleanup(int id, void *p, void *data)
--
2.25.1
Re: [PATCH 0/3] drm: Test for primary plane in new drm_atomic_helper_check_crtc_state()
On 17/06/2022 12:32, Thomas Zimmermann wrote: Provide drm_atomic_helper_check_crtc_state() for validating a CRTC state against common constraints. As many CRTC need a primary plane to work correctly, add this as the first test. The simple-KMS helpers already contain related code. Convert it to the new helper. Also add this test to ast. The simple-kms changes were tested with simpledrm. The ast changes were tested in AST2300 hardware. Thomas Zimmermann (3): drm/atomic-helper: Add helper drm_atomic_helper_check_crtc_state() drm/simple-kms: Use drm_atomic_helper_check_crtc_state() drm/ast: Enable primary plane with CRTC drivers/gpu/drm/ast/ast_mode.c | 13 -- drivers/gpu/drm/drm_atomic_helper.c | 55 + drivers/gpu/drm/drm_simple_kms_helper.c | 14 +++ include/drm/drm_atomic_helper.h | 2 + 4 files changed, 72 insertions(+), 12 deletions(-) base-commit: 822a8442835012ce405080cb40f6317ef1e854ac the whole serie looks good to me, Acked-by Jocelyn Falempe
Re: DMA-buf and uncached system memory
Am Freitag, dem 24.06.2022 um 08:54 +0200 schrieb Christian König: > Am 23.06.22 um 17:26 schrieb Lucas Stach: > > Am Donnerstag, dem 23.06.2022 um 14:52 +0200 schrieb Christian König: > > > Am 23.06.22 um 14:14 schrieb Lucas Stach: > > > > Am Donnerstag, dem 23.06.2022 um 13:54 +0200 schrieb Christian König: > > > > > Am 23.06.22 um 13:29 schrieb Lucas Stach: > > > > > [SNIP] > > > > > I mean I even had somebody from ARM which told me that this is not > > > > > going > > > > > to work with our GPUs on a specific SoC. That there are ARM internal > > > > > use > > > > > cases which just seem to work because all the devices are non-coherent > > > > > is completely new to me. > > > > > > > > > Yes, trying to hook up a peripheral that assumes cache snooping in some > > > > design details to a non coherent SoC may end up exploding in various > > > > ways. On the other hand you can work around most of those assumptions > > > > by marking the memory as uncached to the CPU, which may tank > > > > performance, but will work from a correctness PoV. > > > Yeah, and exactly that's what I meant with "DMA-buf is not the framework > > > for this". > > > > > > See we do support using uncached/not snooped memory in DMA-buf, but only > > > for the exporter side. > > > > > > For example the AMD and Intel GPUs have a per buffer flag for this. > > > > > > The importer on the other hand needs to be able to handle whatever the > > > exporter provides. > > > > > I fail to construct a case where you want the Vulkan/GL "no domain > > transition" coherent semantic without the allocator knowing about this. > > If you need this and the system is non-snooping, surely the allocator > > will choose uncached memory. > > No it won't. The allocator in the exporter is independent of the importer. > > That is an important and intentional design decision, cause otherwise > you wouldn't have exporters/importers in the first place and rather a > centralized allocation pool like what dma-heap implements. > > See the purpose of DMA-buf is to expose the buffers in the way the > exporter wants to expose them. So when the exporting driver wants to > allocate normal cached system memory then that is perfectly fine and > completely fits into this design. > I'm specifically talking about the case where a snooping exporter would allocate the GL coherent buffer and a non-snooping importer would need to access that buffer with the same "no domain transition needed" semantic. That is the thing which we can not make work at all and need to fail the attach. If both the exporter and importer are non-snooping you would probably get uncached memory, as long as the exporter knows how the buffer will be used. Is there a real use-case where the exporter doesn't know that the buffer will be used as GL/Vulkan coherent and we can't do fallback on the importer side? > Otherwise we would need to adjust all exporters to the importers, which > is potentially not even possible. > > > I agree that you absolutely need to fail the usage when someone imports > > a CPU cached buffer and then tries to use it as GL coherent on a non- > > snooping system. That simply will not work. > > Exactly that, yes. That's what the attach callback is good for. > > See we already have tons of cases where buffers can't be shared because > they wasn't initially allocated in a way the importer can deal with > them. But that's perfectly ok and intentional. > > For example just take a configuration where a dedicated GPU clones the > display with an integrated GPU. The dedicated GPU needs the image in > local memory for scanout which is usually not accessible to the > integrated GPU. > > So either attaching the DMA-buf or creating the KMS framebuffer config > will fail and we are running into the fallback path which involves an > extra copy. And that is perfectly fine and intentional since this > configuration is not supported by the hardware. > > > > > > [SNIP] > > And here is where our line of thought diverges: the DMA API allows > > snooping and non-snooping devices to work together just fine, as it has > > explicit domain transitions, which are no-ops if both devices are > > snooping, but will do the necessary cache maintenance when one of them > > is non-snooping but the memory is CPU cached. > > > > I don't see why DMA-buf should be any different here. Yes, you can not > > support the "no domain transition" sharing when the memory is CPU > > cached and one of the devices in non-snooping, but you can support 99% > > of real use-cases like the non-snooped scanout or the UVC video import. > > Well I didn't say we couldn't do it that way. What I'm saying that it > was intentionally decided against it. > > We could re-iterate that decision, but this would mean that all existing > exporters would now need to provide additional functionality. > The way I see it we would only need this for exporters that potentially export CPU cached memory, but need to interop wi
Re: [Intel-gfx] [PATCH v5 3/3] drm/doc/rfc: VM_BIND uapi definition
On 24/06/2022 06:32, Niranjana Vishwanathapura wrote:
VM_BIND and related uapi definitions
v2: Reduce the scope to simple Mesa use case.
v3: Expand VM_UNBIND documentation and add
I915_GEM_VM_BIND/UNBIND_FENCE_VALID
and I915_GEM_VM_BIND_TLB_FLUSH flags.
v4: Remove I915_GEM_VM_BIND_TLB_FLUSH flag and add additional
documentation for vm_bind/unbind.
v5: Remove TLB flush requirement on VM_UNBIND.
Add version support to stage implementation.
Mostly LGTM with one final question.
Would an extension to execbuf3 saying "async wait on any ongoing
bind/unbind activity on this vm"? Would such an easy "fire and forget"
mechanism be useful to userspace? Or are separate "queues" the minimal
useful thing?
Regards,
Tvrtko
Signed-off-by: Niranjana Vishwanathapura
---
Documentation/gpu/rfc/i915_vm_bind.h | 256 +++
1 file changed, 256 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 ..8af6c035ccf4
--- /dev/null
+++ b/Documentation/gpu/rfc/i915_vm_bind.h
@@ -0,0 +1,256 @@
+/* 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.
+ * bit[0]: If set, VM_BIND is supported, otherwise not.
+ * bits[8-15]: VM_BIND implementation version.
+ * Version 0 requires in VM_UNBIND call, UMDs to specify the exact mapping
+ * created previously with the VM_BIND call. i.e., i915 will not support
+ * splitting/merging of the mappings created with VM_BIND call (See
+ * struct drm_i915_gem_vm_bind and struct drm_i915_gem_vm_unbind).
+ */
+#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.
+ *
+ * The older execbuf2 ioctl will not support VM_BIND mode of operation.
+ * For VM_BIND mode, we have new execbuf3 ioctl which will not accept any
+ * execlist (See struct drm_i915_gem_execbuffer3 for more details).
+ *
+ */
+#define I915_VM_CREATE_FLAGS_USE_VM_BIND (1 << 0)
+
+/* VM_BIND related ioctls */
+#define DRM_I915_GEM_VM_BIND 0x3d
+#define DRM_I915_GEM_VM_UNBIND 0x3e
+#define DRM_I915_GEM_EXECBUFFER3 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_EXECBUFFER3 DRM_IOWR(DRM_COMMAND_BASE +
DRM_I915_GEM_EXECBUFFER3, struct drm_i915_gem_execbuffer3)
+
+/**
+ * struct drm_i915_gem_vm_bind_fence - Bind/unbind completion notification.
+ *
+ * A timeline out fence for vm_bind/unbind completion notification.
+ */
+struct drm_i915_gem_vm_bind_fence {
+ /** @handle: User's handle for a drm_syncobj to signal. */
+ __u32 handle;
+
+ /** @rsvd: Reserved, MBZ */
+ __u32 rsvd;
+
+ /**
+* @value: A point in the timeline.
+* Value 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 value;
+};
+
+/**
+ * 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).
+ *
+ * The @start, @offset and @length should be 4K page aligned. However the DG2
+ * and XEHPSDV has 64K page size for device local-memory and has compact page
+ * table. On those platforms, for binding device local-memory objects, the
+ * @start should be 2M aligned, @offset and @length should be 64K aligned.
+ * Also, on those platforms, error -ENOSPC will be returned if user tries to
+ * bind a device local-memory object and a system memory object in a single 2M
+ * section of VA range.
+ *
+ * Error code -EINVAL will be returned if @start, @offset and @length are not
+ * properly aligned. Error code of -ENOSPC will be returned if the VA range
+ * specified can't be reserved.
+ *
+ * The bind operation can get completed asynchronously and out of submission
+ * order. When I915_GEM_VM_BIND_FENCE_VALID flag is set, the @fence will be
+ * signaled upon completion of bind operation.
+ */
+struct drm_i915_gem_vm_bind {
+ /** @vm_id: VM (address space) id to bind */
+ __u32
Re: [PATCH] drm/exynos: replace drm_detect_hdmi_monitor() with drm_display_info.is_hdmi
22. 6. 16. 16:22에 hongao 이(가) 쓴 글: > Once EDID is parsed, the monitor HDMI support information is available > through drm_display_info.is_hdmi. > > This driver calls drm_detect_hdmi_monitor() to receive the same > information, which is less efficient. > > Avoid calling drm_detect_hdmi_monitor() and use drm_display_info.is_hdmi > instead. > Applied. Thanks, Inki Dae > Signed-off-by: hongao > --- > drivers/gpu/drm/exynos/exynos_hdmi.c | 2 +- > 1 file changed, 1 insertion(+), 1 deletion(-) > > diff --git a/drivers/gpu/drm/exynos/exynos_hdmi.c > b/drivers/gpu/drm/exynos/exynos_hdmi.c > index 7655142a4651..17e9f5efbcfc 100644 > --- a/drivers/gpu/drm/exynos/exynos_hdmi.c > +++ b/drivers/gpu/drm/exynos/exynos_hdmi.c > @@ -893,7 +893,7 @@ static int hdmi_get_modes(struct drm_connector *connector) > if (!edid) > return -ENODEV; > > - hdata->dvi_mode = !drm_detect_hdmi_monitor(edid); > + hdata->dvi_mode = !connector->display_info.is_hdmi; > DRM_DEV_DEBUG_KMS(hdata->dev, "%s : width[%d] x height[%d]\n", > (hdata->dvi_mode ? "dvi monitor" : "hdmi monitor"), > edid->width_cm, edid->height_cm);
Re: [PATCH v11 20/24] arm64: dts: rockchip: enable vop2 and hdmi tx on rock-3a
> Wiadomość napisana przez Piotr Oniszczuk w dniu > 14.05.2022, o godz. 15:58: > > > >> Wiadomość napisana przez Peter Geis w dniu 09.05.2022, >> o godz. 18:00: >> >> If you want to confirm the hardware is configured correctly you can >> remove the cec pin from the hdmi node and set up a cec-gpio node. >> https://elixir.bootlin.com/linux/v5.18-rc5/source/Documentation/devicetree/bindings/media/cec-gpio.txt > > Peter, Sascha > > I configured cec-gpio and can confirm: with gpio cec works on my rock3-a > board v1.31. > > So summarising my tests: > >rock3-a v1.1 rock3-a v1.31 rock3-b > > radxa debian: ok ok > ok > > other ppl mainline 5.18: ok n/tn/t > > me with mainline 5.18: n/tnok ok > > me with mainline 5.18 gpio-cec: n/t okn/t > > Non-working combination is: rock3-a v1.31 hw on mainline 5.18. > For me it looks like there is bug in case when rk3568 using cec on > hdmitxm1_cec line. > (The same binaries working ok on my rock3-b where cec is on hdmitxm0_cec > line. It also works on Peter's rock3a v1.1 - which also uses hdmitxm0_cec > line). > > It looks like upper cec driver can talk to lower driver (dw-hdmi?) but > nothing is received from lower driver, as my app says: > CECAdapter: CEC device can't poll TV: TV does not respond to CEC polls > > btw: I verified with oscilloscope connected to hdmitxm1_cec line: starting > cec-compliance -v -T shows expected series of 0V pulses on hdmitxm1_cec > line > > Sascha, Peter I returned to trying to find why hdmi-cec is not working on rock3-a v1.31 hw. I'm on vop2 v11 on 5.18 mainline. Current findings: (1) the same sw. stack/binaries works on rock-3b (where cec uses hdmitx_m0 instead of hdmitx_m1 I/O line); (2) gpio-cec however works no problem on rock3-a; (3) monitoring cec messages with v4-utils 'cec-follower -s' shows exact the same events in non-working rock3-a and working rock3-b (tested by issue configure cec by 'cec-ctl -d /dev/cec0 --phys-addr=1.0.0.0 --playback' command) --> i'm interpreting this as confirmation that low level phy layer receives ok cec data from connected device on non-working rock3-a; (4) debug sysfs for cec shows "has CEC follower (in passthrough mode)" for working rock-3b and there is NO "has CEC follower" debug message in failing rock3-a. For me It looks like low-level hdmi-cec works ok on failing rock3-a - but upper layers (in hdmi vop2?) are not registering (or failing to register) cec-follower filehandle. This happens just when hdmitx I/O in DT is changed from hdmitx_m0 to hdmutx_m1. A bit strange - but all my tests consistently confirming this observation I'm too weak in kernel cec internals - so maybe you have any pointers how to further debug/investigate this issue? BTW: i'm not alone with cec issue on rock3a v1.31 - already 2 other users contacted me with the same issue...
Re: [Intel-gfx] [PATCH v3 3/3] drm/doc/rfc: VM_BIND uapi definition
On 23/06/2022 22:05, Zeng, Oak wrote: -Original Message- From: Intel-gfx On Behalf Of Tvrtko Ursulin Sent: June 23, 2022 7:06 AM To: Landwerlin, Lionel G ; Vishwanathapura, Niranjana Cc: Zanoni, Paulo R ; intel-...@lists.freedesktop.org; dri-devel@lists.freedesktop.org; Hellstrom, Thomas ; Wilson, Chris P ; Vetter, Daniel ; christian.koe...@amd.com; Auld, Matthew Subject: Re: [Intel-gfx] [PATCH v3 3/3] drm/doc/rfc: VM_BIND uapi definition On 23/06/2022 09:57, Lionel Landwerlin wrote: On 23/06/2022 11:27, Tvrtko Ursulin wrote: After a vm_unbind, UMD can re-bind to same VA range against an active VM. Though I am not sue with Mesa usecase if that new mapping is required for running GPU job or it will be for the next submission. But ensuring the tlb flush upon unbind, KMD can ensure correctness. Isn't that their problem? If they re-bind for submitting _new_ work then they get the flush as part of batch buffer pre-amble. In the non sparse case, if a VA range is unbound, it is invalid to use that range for anything until it has been rebound by something else. We'll take the fence provided by vm_bind and put it as a wait fence on the next execbuffer. It might be safer in case of memory over fetching? TLB flush will have to happen at some point right? What's the alternative to do it in unbind? Currently TLB flush happens from the ring before every BB_START and also when i915 returns the backing store pages to the system. Can you explain more why tlb flush when i915 retire the backing storage? I never figured that out when I looked at the codes. As I understand it, tlb caches the gpu page tables which map a va to a pa. So it is straight forward to me that we perform a tlb flush when we change the page table (either at vm bind time or unbind time. Better at unbind time for performance reason). I don't know what performs better - someone can measure the two approaches? Certainly on platforms where we only have global TLB flushing the cost is quite high so my thinking was to allow i915 to control when it will be done and not guarantee it in the uapi if it isn't needed for security reasons. But it is rather tricky to me to flush tlb when we retire a backing storage. I don't see how backing storage can be connected to page table. Let's say user unbind va1 from pa1, then bind va1 to pa2. Then retire pa1. Submit shader code using va1. If we don't tlb flush after unbind va1, the new shader code which is supposed to use pa2 will still use pa1 due to the stale entries in tlb, right? The point is, tlb cached is tagged with virtual address, not physical address. so after we unbind va1 from pa1, regardless we retire pa1 or not, va1 can be bound to another pa2. When you say "retire pa1" I will assume you meant release backing storage for pa1. At this point i915 currently does do the TLB flush and that ensures no PTE can point to pa1. This approach deals with security of the system as a whole. Client may still cause rendering corruption or a GPU hang for itself but that should be completely isolated. (This is the part where you say "regardless if we retire pa1 or not" I think.) But I think those are advanced use cases where userspace wants to manipulate PTEs while something is running on the GPU in parallel. AFAIK limited to compute "infinite batch" so my thinking is to avoid adding a performance penalty to the common case. Especially on platforms which only have global flush. But.. to circle back on the measuring angle. Until someone invests time and effort to benchmark the two approaches (flush on unbind vs flush on backing store release) we don't really know. All I know is the perf hit with the current solution was significant, AFAIR up to teen digits on some games. And considering the flushes were driven only by the shrinker activity, my thinking was they would be less frequent than the unbinds, therefore have the potential for a smaller perf hit. Regards, Tvrtko
Re: [PATCH 5/6] drm/i915/gt: Serialize GRDOM access between multiple engine resets
On 23/06/2022 12:17, Andi Shyti wrote:
Hi Mauro,
On Wed, Jun 15, 2022 at 04:27:39PM +0100, Mauro Carvalho Chehab wrote:
From: Chris Wilson
Don't allow two engines to be reset in parallel, as they would both
try to select a reset bit (and send requests to common registers)
and wait on that register, at the same time. Serialize control of
the reset requests/acks using the uncore->lock, which will also ensure
that no other GT state changes at the same time as the actual reset.
Fixes: 7938d61591d3 ("drm/i915: Flush TLBs before releasing backing store")
Reported-by: Mika Kuoppala
Signed-off-by: Chris Wilson
Cc: Mika Kuoppala
Cc: Andi Shyti
Cc: sta...@vger.kernel.org
Acked-by: Thomas Hellström
Signed-off-by: Mauro Carvalho Chehab
Reviewed-by: Andi Shyti
Notice I had a bunch of questions and asks in this series so please do
not merge until those are addressed.
In this particular patch (and some others) for instance Fixes: tag, at
least against that sha, shouldn't be there.
Regards,
Tvrtko
Re: [PATCH v2 1/4] drm/etnaviv: add simple moving average (SMA)
Hi Christian,
Am Dienstag, dem 21.06.2022 um 09:20 +0200 schrieb Christian Gmeiner:
> This adds a SMA algorithm inspired by Exponentially weighted moving
> average (EWMA) algorithm found in the kernel.
>
Still not sure about this one. I _feel_ that a simple moving average
over a period of one second does not do a good job of reflecting the
real GPU load for a bursty workload, where EWMA might be better suited.
But then I also don't have a real informed opinion to offer on this.
Regards,
Lucas
> Signed-off-by: Christian Gmeiner
> ---
> drivers/gpu/drm/etnaviv/etnaviv_sma.h | 53 +++
> 1 file changed, 53 insertions(+)
> create mode 100644 drivers/gpu/drm/etnaviv/etnaviv_sma.h
>
> diff --git a/drivers/gpu/drm/etnaviv/etnaviv_sma.h
> b/drivers/gpu/drm/etnaviv/etnaviv_sma.h
> new file mode 100644
> index ..81564d5cbdc3
> --- /dev/null
> +++ b/drivers/gpu/drm/etnaviv/etnaviv_sma.h
> @@ -0,0 +1,53 @@
> +/* SPDX-License-Identifier: GPL-2.0 */
> +/*
> + * Copyright (C) 2020 Etnaviv Project
> + */
> +
> +#ifndef __ETNAVIV_SMA_H__
> +#define __ETNAVIV_SMA_H__
> +
> +#include
> +#include
> +
> +/*
> + * Simple moving average (SMA)
> + *
> + * This implements a fixed-size SMA algorithm.
> + *
> + * The first argument to the macro is the name that will be used
> + * for the struct and helper functions.
> + *
> + * The second argument, the samples, expresses how many samples are
> + * used for the SMA algorithm.
> + */
> +
> +#define DECLARE_SMA(name, _samples) \
> +struct sma_##name { \
> +unsigned long pos; \
> +unsigned long sum; \
> +unsigned long samples[_samples]; \
> +}; \
> +static inline void sma_##name##_init(struct sma_##name *s) \
> +{ \
> +BUILD_BUG_ON(!__builtin_constant_p(_samples)); \
> +memset(s, 0, sizeof(struct sma_##name)); \
> +} \
> +static inline unsigned long sma_##name##_read(struct sma_##name *s) \
> +{ \
> +BUILD_BUG_ON(!__builtin_constant_p(_samples)); \
> +return s->sum / _samples; \
> +} \
> +static inline void sma_##name##_add(struct sma_##name *s, unsigned long
> val) \
> +{ \
> +unsigned long pos = READ_ONCE(s->pos); \
> +unsigned long sum = READ_ONCE(s->sum); \
> +unsigned long sample = READ_ONCE(s->samples[pos]); \
> + \
> +BUILD_BUG_ON(!__builtin_constant_p(_samples)); \
> + \
> + WRITE_ONCE(s->sum, sum - sample + val); \
> + WRITE_ONCE(s->samples[pos], val); \
> + WRITE_ONCE(s->pos, pos + 1 == _samples ? 0 : pos + 1); \
> +}
> +
> +#endif /* __ETNAVIV_SMA_H__ */
Re: [PATCH v3 05/14] dt-bindings: backlight: Add Mediatek MT6370 backlight
Hi Joe, Joe Simmons-Talbott 於 2022年6月23日 週四 晚上9:17寫道： > > On Thu, Jun 23, 2022 at 07:56:22PM +0800, ChiaEn Wu wrote: > > From: ChiYuan Huang > > > > Add mt6370 backlight binding documentation. > > > > Signed-off-by: ChiYuan Huang > > --- > > > > v3 > > - Rename "mediatek,bled-pwm-hys-input-threshold-steps" to > > "mediatek,bled-pwm-hys-input-th-steps" > > - Refine "bled-pwm-hys-input-th-steps", "bled-ovp-microvolt", > > "bled-ocp-microamp" enum values > > --- > > .../leds/backlight/mediatek,mt6370-backlight.yaml | 92 > > ++ > > 1 file changed, 92 insertions(+) > > create mode 100644 > > Documentation/devicetree/bindings/leds/backlight/mediatek,mt6370-backlight.yaml > > > > diff --git > > a/Documentation/devicetree/bindings/leds/backlight/mediatek,mt6370-backlight.yaml > > > > b/Documentation/devicetree/bindings/leds/backlight/mediatek,mt6370-backlight.yaml > > new file mode 100644 > > index 000..26563ae > > --- /dev/null > > +++ > > b/Documentation/devicetree/bindings/leds/backlight/mediatek,mt6370-backlight.yaml > > @@ -0,0 +1,92 @@ > > +# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause) > > +%YAML 1.2 > > +--- > > +$id: > > http://devicetree.org/schemas/leds/backlight/mediatek,mt6370-backlight.yaml# > > +$schema: http://devicetree.org/meta-schemas/core.yaml# > > + > > +title: Mediatek MT6370 Backlight > > + > > +maintainers: > > + - ChiaEn Wu > > + > > +description: | > > + This module is part of the MT6370 MFD device. > > + The MT6370 Backlight WLED driver supports up to a 29V output voltage for > > + 4 channels of 8 series WLEDs. Each channel supports up to 30mA of current > > + capability with 2048 current steps (11 bits) in exponential or linear > > + mapping curves. > > + > > +allOf: > > + - $ref: common.yaml# > > + > > +properties: > > + compatible: > > +const: mediatek,mt6370-backlight > > + > > + default-brightness: > > +minimum: 0 > > +maximum: 2048 > > + > > + max-brightness: > > +minimum: 0 > > +maximum: 2048 > > + > > + enable-gpios: > > +description: External backlight 'enable' pin > > +maxItems: 1 > > + > > + mediatek,bled-pwm-enable: > > +description: | > > + Enable external PWM input for backlight dimming > > +type: boolean > > + > > + mediatek,bled-pwm-hys-enable: > > +description: | > > + Enable the backlight input-hysteresis for PWM mode > > +type: boolean > > + > > + mediatek,bled-pwm-hys-input-th-steps: > > +$ref: /schemas/types.yaml#/definitions/uint8 > > +enum: [1, 4, 16, 64] > > +description: | > > + The selection of the upper and lower bounds threshold of backlight > > + PWM resolution. If we choose selection 64, the variation of PWM > > + resolution needs over than 64 steps. > > more than? > > Thanks, > Joe > Thanks for your helpful comments! I will revise this in the next patch. Thanks! > > + > > + mediatek,bled-ovp-shutdown: > > +description: | > > + Enable the backlight shutdown when OVP level triggered > > +type: boolean > > + > > + mediatek,bled-ovp-microvolt: > > +enum: [1700, 2100, 2500, 2900] > > +description: | > > + Backlight OVP level selection. > > + > > + mediatek,bled-ocp-shutdown: > > +description: | > > + Enable the backlight shutdown when OCP level triggerred. > > +type: boolean > > + > > + mediatek,bled-ocp-microamp: > > +enum: [90, 120, 150, 180] > > +description: | > > + Backlight OC level selection. > > + > > + mediatek,bled-channel-use: > > +$ref: /schemas/types.yaml#/definitions/uint8 > > +description: | > > + Backlight LED channel to be used. > > + Each bit mapping to: > > +- 0: CH4 > > +- 1: CH3 > > +- 2: CH2 > > +- 3: CH1 > > +minimum: 1 > > +maximum: 15 > > + > > +required: > > + - compatible > > + - mediatek,bled-channel-use > > + > > +additionalProperties: false > > -- > > 2.7.4 > > Best regards, ChiaEn Wu
Re: [PATCH v2 2/4] drm/etnaviv: add loadavg accounting
Am Dienstag, dem 21.06.2022 um 09:20 +0200 schrieb Christian Gmeiner:
> The GPU has an idle state register where each bit represents the idle
> state of a sub-GPU component like FE or TX. Sample this register
> every 10ms and calculate a simple moving average over the sub-GPU
> component idle states with a total observation time frame of 1s.
>
> This provides us with a percentage based load of each sub-GPU
> component.
>
> Signed-off-by: Christian Gmeiner
> ---
> drivers/gpu/drm/etnaviv/etnaviv_drv.c | 14 ++
> drivers/gpu/drm/etnaviv/etnaviv_gpu.c | 64 ++-
> drivers/gpu/drm/etnaviv/etnaviv_gpu.h | 37
> 3 files changed, 114 insertions(+), 1 deletion(-)
>
> diff --git a/drivers/gpu/drm/etnaviv/etnaviv_drv.c
> b/drivers/gpu/drm/etnaviv/etnaviv_drv.c
> index 1d2b4fb4bcf8..d5c6115e56bd 100644
> --- a/drivers/gpu/drm/etnaviv/etnaviv_drv.c
> +++ b/drivers/gpu/drm/etnaviv/etnaviv_drv.c
> @@ -46,6 +46,19 @@ static void load_gpu(struct drm_device *dev)
> }
> }
>
> +static void unload_gpu(struct drm_device *dev)
> +{
> + struct etnaviv_drm_private *priv = dev->dev_private;
> + unsigned int i;
> +
> + for (i = 0; i < ETNA_MAX_PIPES; i++) {
> + struct etnaviv_gpu *g = priv->gpu[i];
> +
> + if (g)
> + etnaviv_gpu_shutdown(g);
> + }
> +}
> +
> static int etnaviv_open(struct drm_device *dev, struct drm_file *file)
> {
> struct etnaviv_drm_private *priv = dev->dev_private;
> @@ -557,6 +570,7 @@ static void etnaviv_unbind(struct device *dev)
> struct drm_device *drm = dev_get_drvdata(dev);
> struct etnaviv_drm_private *priv = drm->dev_private;
>
> + unload_gpu(drm);
> drm_dev_unregister(drm);
>
> component_unbind_all(dev, drm);
> diff --git a/drivers/gpu/drm/etnaviv/etnaviv_gpu.c
> b/drivers/gpu/drm/etnaviv/etnaviv_gpu.c
> index 37018bc55810..202002ae75ee 100644
> --- a/drivers/gpu/drm/etnaviv/etnaviv_gpu.c
> +++ b/drivers/gpu/drm/etnaviv/etnaviv_gpu.c
> @@ -27,6 +27,8 @@
> #include "state_hi.xml.h"
> #include "cmdstream.xml.h"
>
> +static const ktime_t loadavg_polling_frequency = 10 * NSEC_PER_MSEC;
> +
Feeling like a nitpicker, but the thing defined here isn't a frequency,
but a time delta/interval.
> static const struct platform_device_id gpu_ids[] = {
> { .name = "etnaviv-gpu,2d" },
> { },
> @@ -745,6 +747,32 @@ static void etnaviv_gpu_hw_init(struct etnaviv_gpu *gpu)
> gpu_write(gpu, VIVS_HI_INTR_ENBL, ~0U);
> }
>
> +static enum hrtimer_restart etnaviv_loadavg_function(struct hrtimer *t)
> +{
> + struct etnaviv_gpu *gpu = container_of(t, struct etnaviv_gpu,
> loadavg_timer);
> + const u32 idle = gpu_read(gpu, VIVS_HI_IDLE_STATE);
> + int i;
> +
> + gpu->loadavg_last_sample_time = ktime_get();
> +
> + for (i = 0; i < ARRAY_SIZE(etna_idle_module_names); i++)
> + if ((idle & etna_idle_module_names[i].bit))
> + sma_loadavg_add(&gpu->loadavg_value[i], 0);
> + else
> + sma_loadavg_add(&gpu->loadavg_value[i], 100);
> +
> + spin_lock(&gpu->loadavg_spinlock);
> +
> + for (i = 0; i < ARRAY_SIZE(etna_idle_module_names); i++)
> + gpu->loadavg_percentage[i] =
> sma_loadavg_read(&gpu->loadavg_value[i]);
> +
> + spin_unlock(&gpu->loadavg_spinlock);
After pondering this for a bit, I don't think we need this spinlock.
The percentage is a single value per engine, so they are already single
write atomic. The worst thing that can happen without this spinlock is
that on read of the loadavg some engines already have the value of
sample period n+1 integrated, while another set is still at n, which I
don't think we care much about, as those load values are already quite
coarse.
> +
> + hrtimer_forward_now(t, loadavg_polling_frequency);
> +
> + return HRTIMER_RESTART;
> +}
> +
> int etnaviv_gpu_init(struct etnaviv_gpu *gpu)
> {
> struct etnaviv_drm_private *priv = gpu->drm->dev_private;
> @@ -839,6 +867,11 @@ int etnaviv_gpu_init(struct etnaviv_gpu *gpu)
> for (i = 0; i < ARRAY_SIZE(gpu->event); i++)
> complete(&gpu->event_free);
>
> + /* Setup loadavg timer */
> + hrtimer_init(&gpu->loadavg_timer, CLOCK_MONOTONIC,
> HRTIMER_MODE_ABS_SOFT);
> + gpu->loadavg_timer.function = etnaviv_loadavg_function;
> + hrtimer_start(&gpu->loadavg_timer, loadavg_polling_frequency,
> HRTIMER_MODE_ABS_SOFT);
> +
> /* Now program the hardware */
> mutex_lock(&gpu->lock);
> etnaviv_gpu_hw_init(gpu);
> @@ -859,6 +892,11 @@ int etnaviv_gpu_init(struct etnaviv_gpu *gpu)
> return ret;
> }
>
> +void etnaviv_gpu_shutdown(struct etnaviv_gpu *gpu)
> +{
> + hrtimer_cancel(&gpu->loadavg_timer);
> +}
> +
> #ifdef CONFIG_DEBUG_FS
> struct dma_debug {
> u32 address[2];
> @@ -1585,6 +1623,8 @@ int etnaviv_gpu_wait_idle(struct etnaviv_gpu *gpu,
> unsigned int timeout_ms)
> static int et
Re: [PATCH v2 4/4] drm/etnaviv: export loadavg via perfmon
Am Dienstag, dem 21.06.2022 um 09:20 +0200 schrieb Christian Gmeiner:
> Make it possible to access the sub-GPU component load value from
> user space with the perfmon infrastructure.
>
You need to explain a bit more how you intend to use those.
Contrary to all other perfmon values, where we go to great lengths to
only count the load put onto the GPU by the specific process requesting
the perfmon, the loadavg values also include the load caused by other
submits. Due to this difference in behavior I fear that those new
counters might be confusing to use. But maybe you have a use-case in
mind that I don't see right now.
Regards,
Lucas
> Signed-off-by: Christian Gmeiner
> ---
> drivers/gpu/drm/etnaviv/etnaviv_perfmon.c | 79 +++
> 1 file changed, 79 insertions(+)
>
> diff --git a/drivers/gpu/drm/etnaviv/etnaviv_perfmon.c
> b/drivers/gpu/drm/etnaviv/etnaviv_perfmon.c
> index bafdfe49c1d8..d65d9af3a74a 100644
> --- a/drivers/gpu/drm/etnaviv/etnaviv_perfmon.c
> +++ b/drivers/gpu/drm/etnaviv/etnaviv_perfmon.c
> @@ -120,6 +120,19 @@ static u32 hi_total_idle_cycle_read(struct etnaviv_gpu
> *gpu,
> return gpu_read(gpu, reg);
> }
>
> +static u32 load_read(struct etnaviv_gpu *gpu,
> + const struct etnaviv_pm_domain *domain,
> + const struct etnaviv_pm_signal *signal)
> +{
> + u32 load;
> +
> + spin_lock_bh(&gpu->loadavg_spinlock);
> + load = gpu->loadavg_percentage[signal->data];
> + spin_unlock_bh(&gpu->loadavg_spinlock);
> +
> + return load;
> +}
> +
> static const struct etnaviv_pm_domain doms_3d[] = {
> {
> .name = "HI",
> @@ -419,6 +432,72 @@ static const struct etnaviv_pm_domain doms_3d[] = {
> &perf_reg_read
> }
> }
> + },
> + {
> + .name = "LOAD",
> + .nr_signals = 12,
> + .signal = (const struct etnaviv_pm_signal[]) {
> + {
> + "FE",
> + 0,
> + &load_read
> + },
> + {
> + "DE",
> + 1,
> + &load_read
> + },
> + {
> + "PE",
> + 2,
> + &load_read
> + },
> + {
> + "SH",
> + 3,
> + &load_read
> + },
> + {
> + "PA",
> + 4,
> + &load_read
> + },
> + {
> + "SE",
> + 5,
> + &load_read
> + },
> + {
> + "RA",
> + 6,
> + &load_read
> + },
> + {
> + "TX",
> + 7,
> + &load_read
> + },
> + {
> + "VG",
> + 8,
> + &load_read
> + },
> + {
> + "IM",
> + 9,
> + &load_read
> + },
> + {
> + "FP",
> + 10,
> + &load_read
> + },
> + {
> + "TS",
> + 11,
> + &load_read
> + }
> + }
> }
> };
>
Re: [PATCH v3 14/14] video: backlight: mt6370: Add Mediatek MT6370 support
Hi Daniel,
Thanks for your comments!
Daniel Thompson 於 2022年6月23日 週四 晚上9:43寫道：
>
> On Thu, Jun 23, 2022 at 07:56:31PM +0800, ChiaEn Wu wrote:
> > From: ChiaEn Wu
> >
> > Add Mediatek MT6370 Backlight support.
> >
> > Signed-off-by: ChiaEn Wu
>
> > diff --git a/drivers/video/backlight/Kconfig
> > b/drivers/video/backlight/Kconfig
> > index a003e02..7cd823d 100644
> >
> > +static int mt6370_init_backlight_properties(struct mt6370_priv *priv,
> > + struct backlight_properties
> > *props)
> > +{
> > + struct device *dev = priv->dev;
> > + u8 prop_val;
> > + u32 brightness, ovp_uV, ocp_uA;
> > + unsigned int mask, val;
> > + int ret;
> > +
> > + /* Vendor optional properties */
> > + val = 0;
> > + if (device_property_read_bool(dev, "mediatek,bled-pwm-enable"))
> > + val |= MT6370_BL_PWM_EN_MASK;
> > +
> > + if (device_property_read_bool(dev, "mediatek,bled-pwm-hys-enable"))
> > + val |= MT6370_BL_PWM_HYS_EN_MASK;
> > +
> > + ret = device_property_read_u8(dev,
> > + "mediatek,bled-pwm-hys-input-th-steps",
> > + &prop_val);
> > + if (!ret) {
> > + prop_val = clamp_val(prop_val,
> > + MT6370_BL_PWM_HYS_TH_MIN_STEP,
> > + MT6370_BL_PWM_HYS_TH_MAX_STEP);
> > + /*
> > + * prop_val = 1 --> 1 steps --> 0x00
> > + * prop_val = 2 ~ 4 --> 4 steps --> 0x01
> > + * prop_val = 5 ~ 16 --> 16 steps --> 0x10
> > + * prop_val = 17 ~ 64 --> 64 steps --> 0x11
>
> ^
> These numbers are binary, not hex, right? If so, the comments
> should be 0b00 to 0b03 .
Ohh! Yes! These numbers are binary!
I so apologize for making this mistake...
I will revise the comments in the next patch!
Thank you so much!
>
>
> > + */
> > + prop_val = (ilog2(roundup_pow_of_two(prop_val)) + 1) >> 1;
> > + val |= prop_val << (ffs(MT6370_BL_PWM_HYS_SEL_MASK) - 1);
> > + }
> > +
> > + ret = regmap_update_bits(priv->regmap, MT6370_REG_BL_PWM,
> > + val, val);
> > + if (ret)
> > + return ret;
>
> Overall, I like this approach! Easy to read and understand.
>
>
> >
> > +static int mt6370_bl_probe(struct platform_device *pdev)
> > +{
> > + struct mt6370_priv *priv;
> > + struct backlight_properties props = {
> > + .type = BACKLIGHT_RAW,
> > + .scale = BACKLIGHT_SCALE_LINEAR,
>
> Sorry, I missed this before but the KConfig comment says that the
> backlight can support both linear and exponential curves.
>
> Is there a good reason to default to linear?
Well...
The customers who used this PMIC have very few or even no use exponential curve,
so I set the default to linear.
If you think this is inappropriate, I will add a DT property to
control this feature in the next patch!
By the way,
I found some mistakes in my probe() function... I didn't use "return"
when I use dev_err_probe()...
I will refine it in the next patch!
>
>
> Daniel.
> >
Best regards,
ChiaEn Wu
Re: [PATCH v3 07/14] mfd: mt6370: Add Mediatek MT6370 support
Hi Andy,
Thanks for your helpful comments! We have some questions below.
Andy Shevchenko 於 2022年6月24日 週五 凌晨2:01寫道：
>
> On Thu, Jun 23, 2022 at 1:59 PM ChiaEn Wu wrote:
> >
> > From: ChiYuan Huang
> >
> > Add Mediatek MT6370 MFD support.
>
> ...
>
> > +config MFD_MT6370
> > + tristate "Mediatek MT6370 SubPMIC"
> > + select MFD_CORE
> > + select REGMAP_I2C
> > + select REGMAP_IRQ
> > + depends on I2C
> > + help
> > + Say Y here to enable MT6370 SubPMIC functional support.
> > + It consists of a single cell battery charger with ADC monitoring,
> > RGB
> > + LEDs, dual channel flashlight, WLED backlight driver, display bias
> > + voltage supply, one general purpose LDO, and the USB Type-C & PD
> > + controller complies with the latest USB Type-C and PD standards.
>
> What will be the module name in case it's chosen to be built as a module?
OK, we will add related text in the next patch! Thanks!
>
> ...
>
> > obj-$(CONFIG_INTEL_SOC_PMIC_BXTWC) += intel_soc_pmic_bxtwc.o
> > obj-$(CONFIG_INTEL_SOC_PMIC_CHTWC) += intel_soc_pmic_chtwc.o
> > obj-$(CONFIG_INTEL_SOC_PMIC_CHTDC_TI) += intel_soc_pmic_chtdc_ti.o
> > obj-$(CONFIG_MFD_MT6360) += mt6360-core.o
> > +obj-$(CONFIG_MFD_MT6370) += mt6370.o
> > mt6397-objs:= mt6397-core.o mt6397-irq.o mt6358-irq.o
> > obj-$(CONFIG_MFD_MT6397) += mt6397.o
> > obj-$(CONFIG_INTEL_SOC_PMIC_MRFLD) += intel_soc_pmic_mrfld.o
>
> This whole bunch of drivers is in the wrong place in Makefile.
>
> https://lore.kernel.org/all/20220616182524.7956-2-andriy.shevche...@linux.intel.com/
>
hmm... So shall we need to cherry-pick your this patch first,
then modify the Makefile before the next submission??
> ...
>
> > +#define MT6370_REG_MAXADDR 0x1FF
>
> Wondering if (BIT(10) - 1) gives a better hint on how hardware limits
> this (so it will be clear it's 10-bit address).
well... This "0x1FF" is just a virtual mapping value to map the max
address of the PMU bank(0x1XX).
So, I feel its means is different from using (BIT(10) - 1) here.
>
> ...
>
> > +static int mt6370_check_vendor_info(struct mt6370_info *info)
> > +{
> > + unsigned int devinfo;
> > + int ret;
> > +
> > + ret = regmap_read(info->regmap, MT6370_REG_DEV_INFO, &devinfo);
> > + if (ret)
> > + return ret;
> > +
> > + switch (FIELD_GET(MT6370_VENID_MASK, devinfo)) {
> > + case MT6370_VENID_RT5081:
> > + case MT6370_VENID_RT5081A:
> > + case MT6370_VENID_MT6370:
> > + case MT6370_VENID_MT6371:
> > + case MT6370_VENID_MT6372P:
> > + case MT6370_VENID_MT6372CP:
>
> return 0;
>
> > + break;
> > + default:
> > + dev_err(info->dev, "Unknown Vendor ID 0x%02x\n", devinfo);
> > + return -ENODEV;
> > + }
> > +
> > + return 0;
>
> ...and drop these two lines?
OK! We will refine it in the next patch!
>
> > +}
>
> ...
>
> > + bank_idx = *(u8 *)reg_buf;
> > + bank_addr = *(u8 *)(reg_buf + 1);
>
> Why not
>
> const u8 *u8_buf = reg_buf;
>
> bank_idx = u8_buf[0];
> bank_addr = u8_buf[1];
>
> ?
We will refine it in the next patch! Thanks!
>
> ...
>
> > + if (ret < 0)
> > + return ret;
> > + else if (ret != val_size)
>
> Redundant 'else'.
I'm not quite sure what you mean, so I made the following changes first.
if (ret < 0)
return ret;
if (ret != val_size)
return -EIO;
I don't know if it meets your expectations??
>
> > + return -EIO;
>
> ...
>
> > + bank_idx = *(u8 *)data;
> > + bank_addr = *(u8 *)(data + 1);
>
> As per above.
>
> --
> With Best Regards,
> Andy Shevchenko
Best regards,
ChiaEn Wu
Re: [PATCH v3 02/14] dt-bindings: power: supply: Add Mediatek MT6370 Charger
On 23/06/2022 13:56, ChiaEn Wu wrote: > From: ChiaEn Wu > > Add Mediatek MT6370 Charger binding documentation. > > Signed-off-by: ChiaEn Wu > --- > > v3 > - Add items and remove maxItems of io-channels > - Add io-channel-names and describe each item > - Add "unevaluatedProperties: false" in "usb-otg-vbus-regulator" > - Rename "enable-gpio" to "enable-gpios" in "usb-otg-vbus-regulator" > --- > .../power/supply/mediatek,mt6370-charger.yaml | 87 > ++ > 1 file changed, 87 insertions(+) > create mode 100644 > Documentation/devicetree/bindings/power/supply/mediatek,mt6370-charger.yaml > > diff --git > a/Documentation/devicetree/bindings/power/supply/mediatek,mt6370-charger.yaml > b/Documentation/devicetree/bindings/power/supply/mediatek,mt6370-charger.yaml > new file mode 100644 > index 000..f138db6 > --- /dev/null > +++ > b/Documentation/devicetree/bindings/power/supply/mediatek,mt6370-charger.yaml > @@ -0,0 +1,87 @@ > +# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause) > +%YAML 1.2 > +--- > +$id: http://devicetree.org/schemas/power/supply/mediatek,mt6370-charger.yaml# > +$schema: http://devicetree.org/meta-schemas/core.yaml# > + > +title: Mediatek MT6370 Battery Charger > + > +maintainers: > + - ChiaEn Wu > + > +description: | > + This module is part of the MT6370 MFD device. > + Provides Battery Charger, Boost for OTG devices and BC1.2 detection. > + > +properties: > + compatible: > +const: mediatek,mt6370-charger > + > + interrupts: > +description: | > + Specify what irqs are needed to be handled by MT6370 Charger driver. > IRQ > + "MT6370_IRQ_CHG_MIVR", "MT6370_IRQ_ATTACH" and > "MT6370_IRQ_OVPCTRL_UVP_D" > + are required. > +items: > + - description: BC1.2 done irq > + - description: usb plug in irq > + - description: mivr irq > + > + interrupt-names: > +items: > + - const: attach_i > + - const: uvp_d_evt > + - const: mivr > + > + io-channels: > +description: | > + Use ADC channel to read VBUS, IBUS, IBAT, etc., info. > +minItems: 1 > +items: > + - description: | > + VBUS voltage with lower accuracy (+-75mV) but higher measure > + range (1~22V) > + - description: | > + VBUS voltage with higher accuracy (+-30mV) but lower measure > + range (1~9.76V) > + - description: the main system input voltage > + - description: battery voltage > + - description: battery temperature-sense input voltage > + - description: IBUS current (required) > + - description: battery current > + - description: | > + regulated output voltage to supply for the PWM low-side gate driver > + and the bootstrap capacitor > + - description: IC junction temperature > + > + io-channel-names: It does not match io-channels, you need minItems here as well. > +items: > + - const: vbusdiv5 > + - const: vbusdiv2 > + - const: vsys > + - const: vbat > + - const: ts_bat > + - const: ibus > + - const: ibat > + - const: chg_vddp > + - const: temp_jc > + Best regards, Krzysztof
Re: [PATCH v3 09/14] regulator: mt6370: Add mt6370 DisplayBias and VibLDO support
Hi Andy,
Thanks for your helpful comments!
Andy Shevchenko 於 2022年6月24日 週五 凌晨2:19寫道：
>
> On Thu, Jun 23, 2022 at 2:00 PM ChiaEn Wu wrote:
> >
> > From: ChiYuan Huang
> >
> > Add mt6370 DisplayBias and VibLDO support.
>
> ...
>
> > +#include
> > +#include
> > +#include
> > +#include
> > +#include
>
> > +#include
>
> Any users of this? (See below)
>
> > +#include
> > +#include
> > +#include
> > +#include
>
> ...
>
> > +#define MT6370_LDO_MINUV 160
> > +#define MT6370_LDO_STPUV 20
> > +#define MT6370_LDO_N_VOLT 13
> > +#define MT6370_DBVBOOST_MINUV 400
> > +#define MT6370_DBVBOOST_STPUV 5
> > +#define MT6370_DBVBOOST_N_VOLT 45
> > +#define MT6370_DBVOUT_MINUV400
> > +#define MT6370_DBVOUT_STPUV5
> > +#define MT6370_DBVOUT_N_VOLT 41
>
> If UV is a unit suffix, make it _UV.
>
> ...
>
> > + .of_match = of_match_ptr("dsvbst"),
>
> Would it even be called / used if CONFIG_OF=n?
We got a notification from Mark telling us that this patch has been
applied to git.
(
https://lore.kernel.org/linux-arm-kernel/165599931844.321775.8085559092337130067.b4...@kernel.org/
)
So, should we need to make any other changes in the next submission?
>
> ...
>
> > + .regulators_node = of_match_ptr("regulators"),
>
> Ditto.
>
> ...
>
> > + for (i = 0; i < ARRAY_SIZE(mt6370_irqs); i++) {
> > + irq = platform_get_irq_byname(pdev, mt6370_irqs[i].name);
> > +
> > + rdev = priv->rdev[mt6370_irqs[i].rid];
> > +
> > + ret = devm_request_threaded_irq(priv->dev, irq, NULL,
> > + mt6370_irqs[i].handler, 0,
> > + mt6370_irqs[i].name, rdev);
> > + if (ret) {
>
> > + dev_err(priv->dev,
> > + "Failed to register (%d) interrupt\n", i);
> > + return ret;
>
> return dev_err_probe(...); ?
>
> > + }
> > + }
>
> ...
>
> > + for (i = 0; i < MT6370_MAX_IDX; i++) {
> > + rdev = devm_regulator_register(priv->dev,
> > + mt6370_regulator_descs + i,
> > + &cfg);
> > + if (IS_ERR(rdev)) {
>
> > + dev_err(priv->dev,
> > + "Failed to register (%d) regulator\n", i);
> > + return PTR_ERR(rdev);
>
> return dev_err_probe(...); ?
>
> > + }
> > +
> > + priv->rdev[i] = rdev;
> > + }
>
> ...
>
> > + if (!priv->regmap) {
> > + dev_err(&pdev->dev, "Failed to init regmap\n");
> > + return -ENODEV;
> > + }
>
> return dev_err_probe(...);
>
> --
> With Best Regards,
> Andy Shevchenko
Best regards,
ChiaEn Wu
Re: [PATCH v3 03/14] dt-bindings: leds: mt6370: Add Mediatek mt6370 current sink type LED indicator
On 23/06/2022 13:56, ChiaEn Wu wrote: > From: ChiYuan Huang > > Add Mediatek mt6370 current sink type LED indicator binding documentation. > > Signed-off-by: ChiYuan Huang > --- > > v3 > - Use leds-class-multicolor.yaml instead of common.yaml. > - Split multi-led and led node. > - Add subdevice "led" in "multi-led". > --- > .../bindings/leds/mediatek,mt6370-indicator.yaml | 77 > ++ > 1 file changed, 77 insertions(+) > create mode 100644 > Documentation/devicetree/bindings/leds/mediatek,mt6370-indicator.yaml > > diff --git > a/Documentation/devicetree/bindings/leds/mediatek,mt6370-indicator.yaml > b/Documentation/devicetree/bindings/leds/mediatek,mt6370-indicator.yaml > new file mode 100644 > index 000..45030f3 > --- /dev/null > +++ b/Documentation/devicetree/bindings/leds/mediatek,mt6370-indicator.yaml > @@ -0,0 +1,77 @@ > +# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause) > +%YAML 1.2 > +--- > +$id: http://devicetree.org/schemas/leds/mediatek,mt6370-indicator.yaml# > +$schema: http://devicetree.org/meta-schemas/core.yaml# > + > +title: LED driver for MT6370 PMIC from MediaTek Integrated. > + > +maintainers: > + - Alice Chen > + > +description: | > + This module is part of the MT6370 MFD device. > + Add MT6370 LED driver include 4-channel RGB LED support > Register/PWM/Breath Mode > + > +allOf: > + - $ref: leds-class-multicolor.yaml# > + > +properties: > + compatible: > +const: mediatek,mt6370-indicator > + > + "#address-cells": > +const: 1 > + > + "#size-cells": > +const: 0 > + > +patternProperties: > + "^multi-led@[0-3]$": > +type: object Here as well unevaluatedProperties:false (on the type level) > + > +properties: > + reg: > +enum: [0, 1, 2, 3] > + > + "#address-cells": > +const: 1 > + > + "#size-cells": > +const: 0 > + > +patternProperties: > + "^led@[0-2]$": > +type: object > +$ref: common.yaml# > +unevaluatedProperties: false > + > +required: > + - reg > + - color > + > +required: > + - reg > + - color > + - "#address-cells" > + - "#size-cells" > + > + "^led@[0-3]$": > +type: object > +$ref: common.yaml# > +unevaluatedProperties: false > + > +properties: > + reg: > +enum: [0, 1, 2, 3] > + > +required: > + - reg > + - color > + > +required: > + - compatible > + - "#address-cells" > + - "#size-cells" > + > +additionalProperties: false Best regards, Krzysztof
Re: [PATCH v3 03/14] dt-bindings: leds: mt6370: Add Mediatek mt6370 current sink type LED indicator
On 24/06/2022 12:35, Krzysztof Kozlowski wrote: > On 23/06/2022 13:56, ChiaEn Wu wrote: >> From: ChiYuan Huang >> >> Add Mediatek mt6370 current sink type LED indicator binding documentation. >> >> Signed-off-by: ChiYuan Huang >> --- >> >> v3 >> - Use leds-class-multicolor.yaml instead of common.yaml. >> - Split multi-led and led node. >> - Add subdevice "led" in "multi-led". >> --- >> .../bindings/leds/mediatek,mt6370-indicator.yaml | 77 >> ++ >> 1 file changed, 77 insertions(+) >> create mode 100644 >> Documentation/devicetree/bindings/leds/mediatek,mt6370-indicator.yaml >> >> diff --git >> a/Documentation/devicetree/bindings/leds/mediatek,mt6370-indicator.yaml >> b/Documentation/devicetree/bindings/leds/mediatek,mt6370-indicator.yaml >> new file mode 100644 >> index 000..45030f3 >> --- /dev/null >> +++ b/Documentation/devicetree/bindings/leds/mediatek,mt6370-indicator.yaml >> @@ -0,0 +1,77 @@ >> +# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause) >> +%YAML 1.2 >> +--- >> +$id: http://devicetree.org/schemas/leds/mediatek,mt6370-indicator.yaml# >> +$schema: http://devicetree.org/meta-schemas/core.yaml# >> + >> +title: LED driver for MT6370 PMIC from MediaTek Integrated. >> + >> +maintainers: >> + - Alice Chen >> + >> +description: | >> + This module is part of the MT6370 MFD device. >> + Add MT6370 LED driver include 4-channel RGB LED support >> Register/PWM/Breath Mode >> + >> +allOf: >> + - $ref: leds-class-multicolor.yaml# >> + >> +properties: >> + compatible: >> +const: mediatek,mt6370-indicator >> + >> + "#address-cells": >> +const: 1 >> + >> + "#size-cells": >> +const: 0 >> + >> +patternProperties: >> + "^multi-led@[0-3]$": >> +type: object > > Here as well unevaluatedProperties:false (on the type level) Ah, no, it does not work currently. Your code looks good. Reviewed-by: Krzysztof Kozlowski Best regards, Krzysztof
Re: [PATCH v3 04/14] dt-bindings: leds: Add Mediatek MT6370 flashlight
On 23/06/2022 13:56, ChiaEn Wu wrote: > From: Alice Chen > > Add Mediatek MT6370 flashlight binding documentation. > > Signed-off-by: Alice Chen > --- Reviewed-by: Krzysztof Kozlowski Best regards, Krzysztof
Re: [PATCH v3 06/14] dt-bindings: mfd: Add Mediatek MT6370
On 23/06/2022 13:56, ChiaEn Wu wrote: > From: ChiYuan Huang > > Add Mediatek MT6370 binding documentation. > > Signed-off-by: ChiYuan Huang > --- > > v3 > - Use " in entire patchset > - Refine ADC description > - Rename "enable-gpio" to "enable-gpios" in "regualtor" > - Change "/schemas/" to "../" in every reference of all MT6370 modules > --- > .../devicetree/bindings/mfd/mediatek,mt6370.yaml | 280 > + > include/dt-bindings/iio/adc/mediatek,mt6370_adc.h | 18 ++ > 2 files changed, 298 insertions(+) > create mode 100644 Documentation/devicetree/bindings/mfd/mediatek,mt6370.yaml > create mode 100644 include/dt-bindings/iio/adc/mediatek,mt6370_adc.h > > diff --git a/Documentation/devicetree/bindings/mfd/mediatek,mt6370.yaml > b/Documentation/devicetree/bindings/mfd/mediatek,mt6370.yaml > new file mode 100644 > index 000..fa9da13 > --- /dev/null > +++ b/Documentation/devicetree/bindings/mfd/mediatek,mt6370.yaml > @@ -0,0 +1,280 @@ > +# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause) > +%YAML 1.2 > +--- > +$id: http://devicetree.org/schemas/mfd/mediatek,mt6370.yaml# > +$schema: http://devicetree.org/meta-schemas/core.yaml# > + > +title: Mediatek MT6370 SubPMIC > + > +maintainers: > + - ChiYuan Huang > + > +description: | > + MT6370 is a highly-integrated smart power management IC, which includes a > + single cell Li-Ion/Li-Polymer switching battery charger, a USB Type-C & > + Power Delivery (PD) controller, dual flash LED current sources, a RGB LED > + driver, a backlight WLED driver, a display bias driver and a general LDO > for > + portable devices. > + > +properties: > + compatible: > +const: mediatek,mt6370 > + > + reg: > +maxItems: 1 > + > + wakeup-source: true > + > + interrupts: > +maxItems: 1 > + > + interrupt-controller: true > + > + "#interrupt-cells": > +const: 1 > + > + adc: > +type: object > +description: | > + Provides 9 channels for system monitoring, including VBUSDIV5 (lower > + accuracy, higher measure range), VBUSDIV2 (higher accuracy, lower > + measure range), VBAT, VSYS, CHG_VDDP, TS_BAT, IBUS, IBAT, and TEMP_JC. > + > +properties: > + compatible: > +const: mediatek,mt6370-adc > + > + "#io-channel-cells": > +const: 1 > + > +required: > + - compatible > + - "#io-channel-cells" > + > + backlight: > +type: object > +$ref: ../leds/backlight/mediatek,mt6370-backlight.yaml# This was correct before: /schemas/leds/ Same in other places. Best regards, Krzysztof
Re: [PATCH v3 08/14] usb: typec: tcpci_mt6370: Add Mediatek MT6370 tcpci driver
On Thu, Jun 23, 2022 at 07:56:25PM +0800, ChiaEn Wu wrote: > --- /dev/null > +++ b/drivers/usb/typec/tcpm/tcpci_mt6370.c > @@ -0,0 +1,212 @@ > +// SPDX-License-Identifier: GPL-2.0+ Are you sure you mean "+" here? I have to ask, sorry. And no copyright line? Your company is ok with that, nice! :) thanks, greg k-h
Re: [PATCH] usb: gadget: Fix unsigned comparison with less than zero
On Thu, Jun 23, 2022 at 04:43:47PM +0800, Jiapeng Chong wrote: > This was found by coccicheck: > > ./drivers/usb/gadget/udc/aspeed_udc.c:496:8-13: WARNING: Unsigned expression > compared with zero: chunk >= 0. What does this mean? Where is the error? Please explain the reason for changes, not just the output of a random tool that you ran on the code. > Signed-off-by: Jiapeng Chong What commit does this fix? thanks, greg k-h
[PATCH v2] drm/bridge: imx: i.MX8 bridge drivers should depend on ARCH_MXC
The various Freescale i.MX8 display bridges are only present on
Freescale i.MX8 SoCs. Hence add a dependency on ARCH_MXC, to prevent
asking the user about these drivers when configuring a kernel without
i.MX SoC support.
Fixes: e60c4354840b2fe8 ("drm/bridge: imx: Add LDB support for i.MX8qm")
Fixes: 3818715f62b42b5c ("drm/bridge: imx: Add LDB support for i.MX8qxp")
Fixes: 96988a526c97cfbe ("drm/bridge: imx: Add i.MX8qxp pixel link to DPI
support")
Fixes: 1ec17c26bc06289d ("drm/bridge: imx: Add i.MX8qm/qxp display pixel link
support")
Fixes: 93e163a9e0392aca ("drm/bridge: imx: Add i.MX8qm/qxp pixel combiner
support")
Signed-off-by: Geert Uytterhoeven
Reviewed-by: Liu Ying
---
v2:
- s/i.MX8MP/i.MX8/,
- Add Reviewed-by.
---
drivers/gpu/drm/bridge/imx/Kconfig | 4
1 file changed, 4 insertions(+)
diff --git a/drivers/gpu/drm/bridge/imx/Kconfig
b/drivers/gpu/drm/bridge/imx/Kconfig
index 212a7b0e64fd8b5a..608f47f41bcd1c81 100644
--- a/drivers/gpu/drm/bridge/imx/Kconfig
+++ b/drivers/gpu/drm/bridge/imx/Kconfig
@@ -1,3 +1,5 @@
+if ARCH_MXC || COMPILE_TEST
+
config DRM_IMX8QM_LDB
tristate "Freescale i.MX8QM LVDS display bridge"
depends on OF
@@ -41,3 +43,5 @@ config DRM_IMX8QXP_PIXEL_LINK_TO_DPI
help
Choose this to enable pixel link to display pixel interface(PXL2DPI)
found in Freescale i.MX8qxp processor.
+
+endif # ARCH_MXC || COMPILE_TEST
--
2.25.1
Re: [PATCH v11 20/24] arm64: dts: rockchip: enable vop2 and hdmi tx on rock-3a
On Fri, Jun 24, 2022 at 4:30 AM Piotr Oniszczuk wrote: > > > > > Wiadomość napisana przez Piotr Oniszczuk w dniu > > 14.05.2022, o godz. 15:58: > > > > > > > >> Wiadomość napisana przez Peter Geis w dniu > >> 09.05.2022, o godz. 18:00: > >> > >> If you want to confirm the hardware is configured correctly you can > >> remove the cec pin from the hdmi node and set up a cec-gpio node. > >> https://elixir.bootlin.com/linux/v5.18-rc5/source/Documentation/devicetree/bindings/media/cec-gpio.txt > > > > Peter, Sascha > > > > I configured cec-gpio and can confirm: with gpio cec works on my rock3-a > > board v1.31. > > > > So summarising my tests: > > > >rock3-a v1.1 rock3-a v1.31 > > rock3-b > > > > radxa debian: ok ok > > ok > > > > other ppl mainline 5.18: ok n/tn/t > > > > me with mainline 5.18: n/tnok ok > > > > me with mainline 5.18 gpio-cec: n/t okn/t > > > > Non-working combination is: rock3-a v1.31 hw on mainline 5.18. > > For me it looks like there is bug in case when rk3568 using cec on > > hdmitxm1_cec line. > > (The same binaries working ok on my rock3-b where cec is on hdmitxm0_cec > > line. It also works on Peter's rock3a v1.1 - which also uses hdmitxm0_cec > > line). > > > > It looks like upper cec driver can talk to lower driver (dw-hdmi?) but > > nothing is received from lower driver, as my app says: > > CECAdapter: CEC device can't poll TV: TV does not respond to CEC polls > > > > btw: I verified with oscilloscope connected to hdmitxm1_cec line: starting > > cec-compliance -v -T shows expected series of 0V pulses on hdmitxm1_cec > > line > > > > > > Sascha, Peter > > I returned to trying to find why hdmi-cec is not working on rock3-a v1.31 hw. > > I'm on vop2 v11 on 5.18 mainline. > > Current findings: > > (1) the same sw. stack/binaries works on rock-3b (where cec uses hdmitx_m0 > instead of hdmitx_m1 I/O line); > > (2) gpio-cec however works no problem on rock3-a; > > (3) monitoring cec messages with v4-utils 'cec-follower -s' shows exact the > same events in non-working rock3-a and working rock3-b > (tested by issue configure cec by 'cec-ctl -d /dev/cec0 --phys-addr=1.0.0.0 > --playback' command) --phys-addr isn't a valid command for this controller. The device designation is only required if you have more than one cec device. > > --> i'm interpreting this as confirmation that low level phy layer receives > ok cec data from connected device on non-working rock3-a; > > (4) debug sysfs for cec shows "has CEC follower (in passthrough mode)" for > working rock-3b and there is NO "has CEC follower" debug message in failing > rock3-a. This makes me suspect you are in fact not using the same software stack, or are not running the same commands. Running `cec-follower -v -m -T` on a rk3566 device with working cec using 5.19-rc1, I see no mention of cec-follower in the debugfs cec0 status entry. > > For me It looks like low-level hdmi-cec works ok on failing rock3-a - but > upper layers (in hdmi vop2?) are not registering (or failing to register) > cec-follower filehandle. This happens just when hdmitx I/O in DT is changed > from hdmitx_m0 to hdmutx_m1. A bit strange - but all my tests consistently > confirming this observation There is nothing wrong with vop2 as it is not involved at all in this. The rockchip hdmi driver (which is not specific to vop2) does nothing more than call the cec registration method in the dw hdmi bridge driver, which then calls the kernel cec registration functions. Changing pinmux changes nothing in how this functions. > > I'm too weak in kernel cec internals - so maybe you have any pointers how to > further debug/investigate this issue? As we discussed in the pine64 room, this is still very likely a hardware issue with this board. A configuration issue with your u-boot or tf-a is also a possibility, but is less likely. You showed with your logic analyzer with nothing plugged in and cec not muxed to m1, data was present on m1. I requested you investigate the following, to which you haven't replied: Have you tried forcing m0 to be assigned to a device other than hdmi-cec? Have you checked if m1 is shorted to another pin? In regards to your data frames for 4.19 vs 5.18, image-view-on is not a valid command if the topology doesn't detect a device on the bus. I recommend running the same test, except run `cec-ctl -S --playback` and post the results for both. > > > > BTW: i'm not alone with cec issue on rock3a v1.31 - already 2 other users > contacted me with the same issue... > >
Re: [PATCH v5 2/2] drm: lcdif: Add support for i.MX8MP LCDIF variant
Am Montag, dem 13.06.2022 um 23:31 +0200 schrieb Marek Vasut:
> Add support for i.MX8MP LCDIF variant. This is called LCDIFv3 and is
> completely different from the LCDIFv3 found in i.MX23 in that it has
> a completely scrambled register layout compared to all previous LCDIF
> variants. The new LCDIFv3 also supports 36bit address space.
>
> Add a separate driver which is really a fork of MXSFB driver with the
> i.MX8MP LCDIF variant handling filled in.
>
> Tested-by: Alexander Stein
> Tested-by: Martyn Welch
> Signed-off-by: Marek Vasut
> Cc: Alexander Stein
> Cc: Laurent Pinchart
> Cc: Lucas Stach
> Cc: Peng Fan
> Cc: Robby Cai
> Cc: Sam Ravnborg
> Cc: Stefan Agner
> ---
> V2: - Drop the pitch check from lcdif_fb_create()
> - Drop connector caching
> - Wait for shadow load bit to be cleared in IRQ handler
> - Make all clock mandatory and grab them all by name
> - Wait for EN to be cleared in lcdif_disable_controller
> - Rename to imx-lcdif
> - Move shadow load to atomic_flush
> V3: - Invert DE polarity to match MX8MPRM datasheet
> - Enable CSC in RGB to YUV mode for MEDIA_BUS_FMT_UYVY8_1X16
> V4: - Drop lcdif_overlay_plane_formats, it is unused
> V5: - Add TB from Martyn
> - Drop lcdif_fb_create in favor of drm_gem_fb_create
> - Pull in mxsfb/ directory from drm top level Makefile
> - Drop busy polling of CTRLDESCL0_5_SHADOW_LOAD_EN in irq handler
> - Use devm_request_irq
> - Drop useless dev.of_node validity check in probe
> - Drop lcdif_*_axi_clk() prototypes
> - Invert HS/VS polarity
> - Drop polling from lcdif_reset_block()
> - Add TB from Alexander
> ---
> drivers/gpu/drm/Makefile | 2 +-
> drivers/gpu/drm/mxsfb/Kconfig | 16 +
> drivers/gpu/drm/mxsfb/Makefile | 2 +
> drivers/gpu/drm/mxsfb/lcdif_drv.c | 342
> drivers/gpu/drm/mxsfb/lcdif_drv.h | 44 +++
> drivers/gpu/drm/mxsfb/lcdif_kms.c | 482 +
> drivers/gpu/drm/mxsfb/lcdif_regs.h | 257 +++
> 7 files changed, 1144 insertions(+), 1 deletion(-)
> create mode 100644 drivers/gpu/drm/mxsfb/lcdif_drv.c
> create mode 100644 drivers/gpu/drm/mxsfb/lcdif_drv.h
> create mode 100644 drivers/gpu/drm/mxsfb/lcdif_kms.c
> create mode 100644 drivers/gpu/drm/mxsfb/lcdif_regs.h
>
> diff --git a/drivers/gpu/drm/Makefile b/drivers/gpu/drm/Makefile
> index 13ef240b3d2b2..75b5ac7c2663c 100644
> --- a/drivers/gpu/drm/Makefile
> +++ b/drivers/gpu/drm/Makefile
> @@ -130,7 +130,7 @@ obj-y += bridge/
> obj-$(CONFIG_DRM_FSL_DCU) += fsl-dcu/
> obj-$(CONFIG_DRM_ETNAVIV) += etnaviv/
> obj-y+= hisilicon/
> -obj-$(CONFIG_DRM_MXSFB) += mxsfb/
> +obj-y+= mxsfb/
> obj-y+= tiny/
> obj-$(CONFIG_DRM_PL111) += pl111/
> obj-$(CONFIG_DRM_TVE200) += tve200/
> diff --git a/drivers/gpu/drm/mxsfb/Kconfig b/drivers/gpu/drm/mxsfb/Kconfig
> index 987170e16ebd6..873551b4552f5 100644
> --- a/drivers/gpu/drm/mxsfb/Kconfig
> +++ b/drivers/gpu/drm/mxsfb/Kconfig
> @@ -19,3 +19,19 @@ config DRM_MXSFB
> i.MX28, i.MX6SX, i.MX7 and i.MX8M).
>
> If M is selected the module will be called mxsfb.
> +
> +config DRM_IMX_LCDIF
> + tristate "i.MX LCDIFv3 LCD controller"
> + depends on DRM && OF
> + depends on COMMON_CLK
> + select DRM_MXS
> + select DRM_KMS_HELPER
> + select DRM_GEM_CMA_HELPER
> + select DRM_PANEL
> + select DRM_PANEL_BRIDGE
> + help
> + Choose this option if you have an LCDIFv3 LCD controller.
> + Those devices are found in various i.MX SoC (i.MX8MP,
> + i.MXRT).
> +
> + If M is selected the module will be called imx-lcdif.
> diff --git a/drivers/gpu/drm/mxsfb/Makefile b/drivers/gpu/drm/mxsfb/Makefile
> index 26d153896d720..3fa44059b9d85 100644
> --- a/drivers/gpu/drm/mxsfb/Makefile
> +++ b/drivers/gpu/drm/mxsfb/Makefile
> @@ -1,3 +1,5 @@
> # SPDX-License-Identifier: GPL-2.0-only
> mxsfb-y := mxsfb_drv.o mxsfb_kms.o
> obj-$(CONFIG_DRM_MXSFB) += mxsfb.o
> +imx-lcdif-y := lcdif_drv.o lcdif_kms.o
> +obj-$(CONFIG_DRM_IMX_LCDIF) += imx-lcdif.o
> diff --git a/drivers/gpu/drm/mxsfb/lcdif_drv.c
> b/drivers/gpu/drm/mxsfb/lcdif_drv.c
> new file mode 100644
> index 0..021d5511d4a6c
> --- /dev/null
> +++ b/drivers/gpu/drm/mxsfb/lcdif_drv.c
> @@ -0,0 +1,342 @@
> +// SPDX-License-Identifier: GPL-2.0-or-later
> +/*
> + * Copyright (C) 2022 Marek Vasut
> + *
> + * This code is based on drivers/gpu/drm/mxsfb/mxsfb*
> + */
> +
> +#include
> +#include
> +#include
> +#include
> +#include
> +#include
> +#include
> +#include
> +
> +#include
> +#include
> +#include
> +#include
> +#include
> +#include
> +#include
> +#include
> +#include
> +#include
> +#include
> +#include
> +#include
> +
> +#include "lcdif_drv.h"
> +#include "lcdif_regs.h"
> +
> +static const struct drm_mode_config_funcs lcdif_mode_config_funcs = {
> + .fb_cr
Re: [RFT][PATCH v1 5/6] vfio/ccw: Add kmap_local_page() for memcpy
On Tue, Jun 21, 2022 at 02:21:22PM -0700, Nicolin Chen wrote: > On Sun, Jun 19, 2022 at 11:32:07PM -0700, Christoph Hellwig wrote: > > On Sun, Jun 19, 2022 at 11:57:26PM -0300, Jason Gunthorpe wrote: > > > The remark about io memory is because on s390 memcpy() will crash even > > > on ioremapped memory, you have to use the memcpy_to/fromio() which > > > uses the special s390 io access instructions. > > > > Yes. The same is true for various other architectures, inluding arm64 > > under the right circumstances. > > > > > This helps because we now block io memory from ever getting into these > > > call paths. I'm pretty sure this is a serious security bug, but would > > > let the IBM folks remark as I don't know it all that well.. > > > > Prevent as in crash when trying to convert it to a page? > > > > > As for the kmap, I thought it was standard practice even if it is a > > > non-highmem? Aren't people trying to use this for other security > > > stuff these days? > > > > Ira has been lookin into the protection keys, although they don't > > apply to s390. Either way I don't object to using kmap, but the > > commit log doesn't make much sense to me. > > How about the updated commit log below? Thanks. > > The pinned PFN list returned from vfio_pin_pages() is converted using > page_to_pfn(), so direct access via memcpy() will crash on S390 if the > PFN is an IO PFN, as we have to use the memcpy_to/fromio(), which uses > the special s390 IO access instructions. > > As a standard practice for security purpose, add kmap_local_page() to > block any IO memory from ever getting into this call path. The kmap_local_page is not about the IO memory, the switch to struct page is what is protecting against IO memory. Use kmap_local_page() is just the correct way to convert a struct page into a CPU address to use with memcpy and it is a NOP on S390 because it doesn't use highmem/etc. Jason
Re: [PATCH] drm: logicvc: Fix uninitialized variable in probe
Hi,
On Fri, Jun 24, 2022 at 04:35:25PM +0200, Paul Kocialkowski wrote:
> On Tue 14 Jun 22, 15:08, Dan Carpenter wrote:
> > The "regmap" is supposed to be initialized to NULL but it's used
> > without being initialized.
> >
> > Fixes: efeeaefe9be5 ("drm: Add support for the LogiCVC display controller")
> > Signed-off-by: Dan Carpenter
>
> Nice catch, thanks a lot!
>
> Acked-by: Paul Kocialkowski
Since you have the commit rights to drm-misc, you should apply it
Maxime
Re: [bug report] drm: Add support for the LogiCVC display controller
Hello Dan,
On Tue 14 Jun 22, 15:07, Dan Carpenter wrote:
> Hello Paul Kocialkowski,
>
> The patch efeeaefe9be5: "drm: Add support for the LogiCVC display
> controller" from May 20, 2022, leads to the following Smatch static
> checker warning:
>
> drivers/gpu/drm/logicvc/logicvc_layer.c:320
> logicvc_layer_buffer_find_setup()
> warn: impossible condition '(hoffset > (1))) << (16)) - 1)) =>
> (0-u16max > u16max)'
>
> drivers/gpu/drm/logicvc/logicvc_layer.c
> 258 int logicvc_layer_buffer_find_setup(struct logicvc_drm *logicvc,
> 259 struct logicvc_layer *layer,
> 260 struct drm_plane_state *state,
> 261 struct logicvc_layer_buffer_setup
> *setup)
> 262 {
> 263 struct drm_device *drm_dev = &logicvc->drm_dev;
> 264 struct drm_framebuffer *fb = state->fb;
> 265 /* All the supported formats have a single data plane. */
> 266 u32 layer_bytespp = fb->format->cpp[0];
> 267 u32 layer_stride = layer_bytespp * logicvc->config.row_stride;
> 268 u32 base_offset = layer->config.base_offset * layer_stride;
> 269 u32 buffer_offset = layer->config.buffer_offset *
> layer_stride;
> 270 u8 buffer_sel = 0;
> 271 u16 voffset = 0;
> 272 u16 hoffset = 0;
> 273 phys_addr_t fb_addr;
> 274 u32 fb_offset;
> 275 u32 gap;
> 276
> 277 if (!logicvc->reserved_mem_base) {
> 278 drm_err(drm_dev, "No reserved memory base was
> registered!\n");
> 279 return -ENOMEM;
> 280 }
> 281
> 282 fb_addr = drm_fb_cma_get_gem_addr(fb, state, 0);
> 283 if (fb_addr < logicvc->reserved_mem_base) {
> 284 drm_err(drm_dev,
> 285 "Framebuffer memory below reserved memory
> base!\n");
> 286 return -EINVAL;
> 287 }
> 288
> 289 fb_offset = (u32) (fb_addr - logicvc->reserved_mem_base);
> 290
> 291 if (fb_offset < base_offset) {
> 292 drm_err(drm_dev,
> 293 "Framebuffer offset below layer base
> offset!\n");
> 294 return -EINVAL;
> 295 }
> 296
> 297 gap = fb_offset - base_offset;
> 298
> 299 /* Use the possible video buffers selection. */
> 300 if (gap && buffer_offset) {
> 301 buffer_sel = gap / buffer_offset;
> 302 if (buffer_sel > LOGICVC_BUFFER_SEL_MAX)
> 303 buffer_sel = LOGICVC_BUFFER_SEL_MAX;
> 304
> 305 gap -= buffer_sel * buffer_offset;
> 306 }
> 307
> 308 /* Use the vertical offset. */
> 309 if (gap && layer_stride &&
> logicvc->config.layers_configurable) {
> 310 voffset = gap / layer_stride;
> 311 if (voffset > LOGICVC_LAYER_VOFFSET_MAX)
> 312 voffset = LOGICVC_LAYER_VOFFSET_MAX;
> 313
> 314 gap -= voffset * layer_stride;
> 315 }
> 316
> 317 /* Use the horizontal offset. */
> 318 if (gap && layer_bytespp &&
> logicvc->config.layers_configurable) {
> 319 hoffset = gap / layer_bytespp;
>
> Can "gap / layer_bytespp" ever be more than USHRT_MAX? Because if so
> that won't fit into "hoffset"
Well there is nothing that really restricts the size of the gap, so yes this
could happen. At this stage the gap should have been reduced already but we
never really know.
Would it make sense to add a check that gap / layer_bytespp <= USHRT_MAX
in that if statement?
Thanks for the catch.
Paul
> --> 320 if (hoffset > LOGICVC_DIMENSIONS_MAX)
> 321 hoffset = LOGICVC_DIMENSIONS_MAX;
> 322
> 323 gap -= hoffset * layer_bytespp;
> 324 }
> 325
> 326 if (gap) {
> 327 drm_err(drm_dev,
> 328 "Unable to find layer %d buffer setup for
> 0x%x byte gap\n",
> 329 layer->index, fb_offset - base_offset);
> 330 return -EINVAL;
> 331 }
> 332
> 333 drm_dbg_kms(drm_dev, "Found layer %d buffer setup for 0x%x
> byte gap:\n",
> 334 layer->index, fb_offset - base_offset);
> 335
> 336 drm_dbg_kms(drm_dev, "- buffer_sel = 0x%x chunks of 0x%x
> bytes\n",
> 337 buffer_sel, buffer_offset);
> 338 drm_dbg_kms(drm_dev, "- voffset = 0x%x chunks of 0x%x
> bytes\n", voffset,
> 339 layer_stride);
> 340 drm_dbg_kms(drm_dev, "- hoffset = 0x%x chunks o
Re: [Intel-gfx] [PATCH v5 3/3] drm/doc/rfc: VM_BIND uapi definition
On Fri, Jun 24, 2022 at 09:11:35AM +0100, Tvrtko Ursulin wrote:
On 24/06/2022 06:32, Niranjana Vishwanathapura wrote:
VM_BIND and related uapi definitions
v2: Reduce the scope to simple Mesa use case.
v3: Expand VM_UNBIND documentation and add
I915_GEM_VM_BIND/UNBIND_FENCE_VALID
and I915_GEM_VM_BIND_TLB_FLUSH flags.
v4: Remove I915_GEM_VM_BIND_TLB_FLUSH flag and add additional
documentation for vm_bind/unbind.
v5: Remove TLB flush requirement on VM_UNBIND.
Add version support to stage implementation.
Mostly LGTM with one final question.
Would an extension to execbuf3 saying "async wait on any ongoing
bind/unbind activity on this vm"? Would such an easy "fire and forget"
mechanism be useful to userspace? Or are separate "queues" the minimal
useful thing?
UMDs can easily do this with timeline fence array which execbuf3 supports.
I think adding any new mechanism for the same is not required here.
Niranjana
Regards,
Tvrtko
Signed-off-by: Niranjana Vishwanathapura
---
Documentation/gpu/rfc/i915_vm_bind.h | 256 +++
1 file changed, 256 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 ..8af6c035ccf4
--- /dev/null
+++ b/Documentation/gpu/rfc/i915_vm_bind.h
@@ -0,0 +1,256 @@
+/* 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.
+ * bit[0]: If set, VM_BIND is supported, otherwise not.
+ * bits[8-15]: VM_BIND implementation version.
+ * Version 0 requires in VM_UNBIND call, UMDs to specify the exact mapping
+ * created previously with the VM_BIND call. i.e., i915 will not support
+ * splitting/merging of the mappings created with VM_BIND call (See
+ * struct drm_i915_gem_vm_bind and struct drm_i915_gem_vm_unbind).
+ */
+#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.
+ *
+ * The older execbuf2 ioctl will not support VM_BIND mode of operation.
+ * For VM_BIND mode, we have new execbuf3 ioctl which will not accept any
+ * execlist (See struct drm_i915_gem_execbuffer3 for more details).
+ *
+ */
+#define I915_VM_CREATE_FLAGS_USE_VM_BIND (1 << 0)
+
+/* VM_BIND related ioctls */
+#define DRM_I915_GEM_VM_BIND 0x3d
+#define DRM_I915_GEM_VM_UNBIND 0x3e
+#define DRM_I915_GEM_EXECBUFFER3 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_EXECBUFFER3 DRM_IOWR(DRM_COMMAND_BASE +
DRM_I915_GEM_EXECBUFFER3, struct drm_i915_gem_execbuffer3)
+
+/**
+ * struct drm_i915_gem_vm_bind_fence - Bind/unbind completion notification.
+ *
+ * A timeline out fence for vm_bind/unbind completion notification.
+ */
+struct drm_i915_gem_vm_bind_fence {
+ /** @handle: User's handle for a drm_syncobj to signal. */
+ __u32 handle;
+
+ /** @rsvd: Reserved, MBZ */
+ __u32 rsvd;
+
+ /**
+* @value: A point in the timeline.
+* Value 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 value;
+};
+
+/**
+ * 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).
+ *
+ * The @start, @offset and @length should be 4K page aligned. However the DG2
+ * and XEHPSDV has 64K page size for device local-memory and has compact page
+ * table. On those platforms, for binding device local-memory objects, the
+ * @start should be 2M aligned, @offset and @length should be 64K aligned.
+ * Also, on those platforms, error -ENOSPC will be returned if user tries to
+ * bind a device local-memory object and a system memory object in a single 2M
+ * section of VA range.
+ *
+ * Error code -EINVAL will be returned if @start, @offset and @length are not
+ * properly aligned. Error code of -ENOSPC will be returned if the VA range
+ * specified can't be reserved.
+ *
+ * The bind operation can get completed asynchronously and out of submission
+ * order. Wh
Re: [PATCH] drm/msm/dp: no dp_hpd_unplug_handle() required for eDP
On 6/23/2022 5:09 PM, Stephen Boyd wrote:
Quoting Kuogee Hsieh (2022-06-23 16:34:16)
eDP implementation does not reuried to support hpd signal. Therefore
s/reuried/require/
it only has either ST_DISPLAY_OFF or ST_CONNECTED state during normal
operation. This patch remove unnecessary dp_hpd_unplug_handle() for
eDP but still keep dp_hpd_plug_handle() to support eDP to either
booting up or resume from ST_DISCONNECTED state.
I take it that making this change also fixes a glitch seen on the eDP
panel when a second modeset happens? Can you add that detail to the
commit text? The way it reads makes it sound like this is purely a
cleanup patch, but then there's a Fixes tag so it must be a bug fix or
worthy optimization, neither of which is described.
no, this patch will not fix edp (primary display) corruption issue.
This patch is pure clean up edp.
I will submit fixes edp corruption issue at other patch.
Fixes: 391c96ff0555 ("drm/msm/dp: Support only IRQ_HPD and REPLUG interrupts for
eDP")
Signed-off-by: Kuogee Hsieh
---
drivers/gpu/drm/msm/dp/dp_display.c | 5 +
1 file changed, 1 insertion(+), 4 deletions(-)
diff --git a/drivers/gpu/drm/msm/dp/dp_display.c
b/drivers/gpu/drm/msm/dp/dp_display.c
index da5c03a..ef9794e 100644
--- a/drivers/gpu/drm/msm/dp/dp_display.c
+++ b/drivers/gpu/drm/msm/dp/dp_display.c
@@ -1666,7 +1666,7 @@ void dp_bridge_enable(struct drm_bridge *drm_bridge)
return;
}
- if (dp->is_edp)
+ if (dp->is_edp && dp_display->hpd_state == ST_DISCONNECTED)
dp_hpd_plug_handle(dp_display, 0);
mutex_lock(&dp_display->event_mutex);
@@ -1737,9 +1737,6 @@ void dp_bridge_post_disable(struct drm_bridge *drm_bridge)
dp_display = container_of(dp, struct dp_display_private, dp_display);
- if (dp->is_edp)
- dp_hpd_unplug_handle(dp_display, 0);
dp_hpd_unplug_handle() has a !edp check, and from what I can tell after
this patch that condition will always trigger? But then I wonder why we
aren't masking the irqs for hpd when the eDP display is disabled.
Shouldn't we at least be doing that so that we don't get spurious hpd
irqs when the eDP display is off or on the path to suspend where I
suspect the power may be removed from the panel?
Re: [PATCH] drm: logicvc: Fix uninitialized variable in probe
On Fri, Jun 24, 2022 at 04:46:36PM +0200, Paul Kocialkowski wrote:
> Hi,
>
> On Fri 24 Jun 22, 16:37, Maxime Ripard wrote:
> > Hi,
> >
> > On Fri, Jun 24, 2022 at 04:35:25PM +0200, Paul Kocialkowski wrote:
> > > On Tue 14 Jun 22, 15:08, Dan Carpenter wrote:
> > > > The "regmap" is supposed to be initialized to NULL but it's used
> > > > without being initialized.
> > > >
> > > > Fixes: efeeaefe9be5 ("drm: Add support for the LogiCVC display
> > > > controller")
> > > > Signed-off-by: Dan Carpenter
> > >
> > > Nice catch, thanks a lot!
> > >
> > > Acked-by: Paul Kocialkowski
> >
> > Since you have the commit rights to drm-misc, you should apply it
>
> Absolutely, I'm on my way to doing that.
Giving your Acked-by is confusing then, because it's kind of implied if
you apply it.
> Do I need to reply to the emails with a message indicating that I
> merged them or is using the tool sufficient?
Sending an email is better, dim doesn't do that
Maxime
Re: [PATCH v5 2/2] drm: lcdif: Add support for i.MX8MP LCDIF variant
> > +
> > +static int lcdif_rpm_resume(struct device *dev)
> > +{
> > + struct drm_device *drm = dev_get_drvdata(dev);
> > + struct lcdif_drm_private *lcdif = drm->dev_private;
> > +
> > + /* These clock supply the Control Bus, APB, APBH Ctrl Registers */
> > + clk_prepare_enable(lcdif->clk_axi);
> > + /* These clock supply the System Bus, AXI, Write Path, LFIFO */
> > + clk_prepare_enable(lcdif->clk_disp_axi);
> > + /* These clock supply the DISPLAY CLOCK Domain */
> > + clk_prepare_enable(lcdif->clk);
> > +
> > + return 0;
> > +}
> > +
>
> To avoid unused function warnings, suspend and resume should be under
> #ifdef CONFIG_PM_SLEEP
If SET_RUNTIME_PM_OPS is used then it is good practice to annotate the
functions __maybe_unused and no #ifdef - then they see build coverage also
without PM_SLEEP but they are discarded so do not consume memory if not used.
Sam
Re: [PATCH v3 1/4] dt-bindings: display: bridge: Convert cdns, dsi.txt to yaml
On 20/06/2022 22:54, Rahul T R wrote:
> Convert cdns,dsi.txt binding to yaml format
>
> Signed-off-by: Rahul T R
> ---
> .../bindings/display/bridge/cdns,dsi.txt | 112 --
> .../bindings/display/bridge/cdns,dsi.yaml | 193 ++
> 2 files changed, 193 insertions(+), 112 deletions(-)
> delete mode 100644
> Documentation/devicetree/bindings/display/bridge/cdns,dsi.txt
> create mode 100644
> Documentation/devicetree/bindings/display/bridge/cdns,dsi.yaml
>
> diff --git a/Documentation/devicetree/bindings/display/bridge/cdns,dsi.txt
> b/Documentation/devicetree/bindings/display/bridge/cdns,dsi.txt
> deleted file mode 100644
> index 525a4bfd8634..
> --- a/Documentation/devicetree/bindings/display/bridge/cdns,dsi.txt
> +++ /dev/null
> @@ -1,112 +0,0 @@
> -Cadence DSI bridge
> -==
> -
> -The Cadence DSI bridge is a DPI to DSI bridge supporting up to 4 DSI lanes.
> -
> -Required properties:
> -- compatible: should be set to "cdns,dsi".
> -- reg: physical base address and length of the controller's registers.
> -- interrupts: interrupt line connected to the DSI bridge.
> -- clocks: DSI bridge clocks.
> -- clock-names: must contain "dsi_p_clk" and "dsi_sys_clk".
> -- phys: phandle link to the MIPI D-PHY controller.
> -- phy-names: must contain "dphy".
> -- #address-cells: must be set to 1.
> -- #size-cells: must be set to 0.
> -
> -Optional properties:
> -- resets: DSI reset lines.
> -- reset-names: can contain "dsi_p_rst".
> -
> -Required subnodes:
> -- ports: Ports as described in Documentation/devicetree/bindings/graph.txt.
> - 2 ports are available:
> - * port 0: this port is only needed if some of your DSI devices are
> - controlled through an external bus like I2C or SPI. Can have at
> - most 4 endpoints. The endpoint number is directly encoding the
> - DSI virtual channel used by this device.
> - * port 1: represents the DPI input.
> - Other ports will be added later to support the new kind of inputs.
> -
> -- one subnode per DSI device connected on the DSI bus. Each DSI device should
> - contain a reg property encoding its virtual channel.
> -
> -Example:
> - dsi0: dsi@fd0c {
> - compatible = "cdns,dsi";
> - reg = <0x0 0xfd0c 0x0 0x1000>;
> - clocks = <&pclk>, <&sysclk>;
> - clock-names = "dsi_p_clk", "dsi_sys_clk";
> - interrupts = <1>;
> - phys = <&dphy0>;
> - phy-names = "dphy";
> - #address-cells = <1>;
> - #size-cells = <0>;
> -
> - ports {
> - #address-cells = <1>;
> - #size-cells = <0>;
> -
> - port@1 {
> - reg = <1>;
> - dsi0_dpi_input: endpoint {
> - remote-endpoint = <&xxx_dpi_output>;
> - };
> - };
> - };
> -
> - panel: dsi-dev@0 {
> - compatible = "";
> - reg = <0>;
> - };
> - };
> -
> -or
> -
> - dsi0: dsi@fd0c {
> - compatible = "cdns,dsi";
> - reg = <0x0 0xfd0c 0x0 0x1000>;
> - clocks = <&pclk>, <&sysclk>;
> - clock-names = "dsi_p_clk", "dsi_sys_clk";
> - interrupts = <1>;
> - phys = <&dphy1>;
> - phy-names = "dphy";
> - #address-cells = <1>;
> - #size-cells = <0>;
> -
> - ports {
> - #address-cells = <1>;
> - #size-cells = <0>;
> -
> - port@0 {
> - reg = <0>;
> - #address-cells = <1>;
> - #size-cells = <0>;
> -
> - dsi0_output: endpoint@0 {
> - reg = <0>;
> - remote-endpoint = <&dsi_panel_input>;
> - };
> - };
> -
> - port@1 {
> - reg = <1>;
> - dsi0_dpi_input: endpoint {
> - remote-endpoint = <&xxx_dpi_output>;
> - };
> - };
> - };
> - };
> -
> - i2c@xxx {
> - panel: panel@59 {
> - compatible = "";
> - reg = <0x59>;
> -
> - port {
> - dsi_panel_input: endpoint {
> - remote-endpoint = <&dsi0_output>;
> - };
> - };
> - };
> - };
> diff --git a/Documentation/devicetree/bindings/display/bridge/cdns,dsi.yaml
> b/Documentation/devicetree/bindings/display/bridge/cdns,dsi.yaml
> new file mode 100644
> index
Re: [PATCH v5 01/13] mm: add zone device coherent type memory support
On 6/23/2022 1:21 PM, David Hildenbrand wrote:
On 23.06.22 20:20, Sierra Guiza, Alejandro (Alex) wrote:
On 6/23/2022 2:57 AM, David Hildenbrand wrote:
On 23.06.22 01:16, Sierra Guiza, Alejandro (Alex) wrote:
On 6/21/2022 11:16 AM, David Hildenbrand wrote:
On 21.06.22 18:08, Sierra Guiza, Alejandro (Alex) wrote:
On 6/21/2022 7:25 AM, David Hildenbrand wrote:
On 21.06.22 13:55, Alistair Popple wrote:
David Hildenbrand writes:
On 21.06.22 13:25, Felix Kuehling wrote:
Am 6/17/22 um 23:19 schrieb David Hildenbrand:
On 17.06.22 21:27, Sierra Guiza, Alejandro (Alex) wrote:
On 6/17/2022 12:33 PM, David Hildenbrand wrote:
On 17.06.22 19:20, Sierra Guiza, Alejandro (Alex) wrote:
On 6/17/2022 4:40 AM, David Hildenbrand wrote:
On 31.05.22 22:00, Alex Sierra wrote:
Device memory that is cache coherent from device and CPU point of view.
This is used on platforms that have an advanced system bus (like CAPI
or CXL). Any page of a process can be migrated to such memory. However,
no one should be allowed to pin such memory so that it can always be
evicted.
Signed-off-by: Alex Sierra
Acked-by: Felix Kuehling
Reviewed-by: Alistair Popple
[hch: rebased ontop of the refcount changes,
removed is_dev_private_or_coherent_page]
Signed-off-by: Christoph Hellwig
---
include/linux/memremap.h | 19 +++
mm/memcontrol.c | 7 ---
mm/memory-failure.c | 8 ++--
mm/memremap.c| 10 ++
mm/migrate_device.c | 16 +++-
mm/rmap.c| 5 +++--
6 files changed, 49 insertions(+), 16 deletions(-)
diff --git a/include/linux/memremap.h b/include/linux/memremap.h
index 8af304f6b504..9f752ebed613 100644
--- a/include/linux/memremap.h
+++ b/include/linux/memremap.h
@@ -41,6 +41,13 @@ struct vmem_altmap {
* A more complete discussion of unaddressable memory may be found in
* include/linux/hmm.h and Documentation/vm/hmm.rst.
*
+ * MEMORY_DEVICE_COHERENT:
+ * Device memory that is cache coherent from device and CPU point of view. This
+ * is used on platforms that have an advanced system bus (like CAPI or CXL). A
+ * driver can hotplug the device memory using ZONE_DEVICE and with that memory
+ * type. Any page of a process can be migrated to such memory. However no one
Any page might not be right, I'm pretty sure. ... just thinking about special
pages
like vdso, shared zeropage, ... pinned pages ...
Well, you cannot migrate long term pages, that's what I meant :)
+ * should be allowed to pin such memory so that it can always be evicted.
+ *
* MEMORY_DEVICE_FS_DAX:
* Host memory that has similar access semantics as System RAM i.e. DMA
* coherent and supports page pinning. In support of coordinating page
@@ -61,6 +68,7 @@ struct vmem_altmap {
enum memory_type {
/* 0 is reserved to catch uninitialized type fields */
MEMORY_DEVICE_PRIVATE = 1,
+ MEMORY_DEVICE_COHERENT,
MEMORY_DEVICE_FS_DAX,
MEMORY_DEVICE_GENERIC,
MEMORY_DEVICE_PCI_P2PDMA,
@@ -143,6 +151,17 @@ static inline bool folio_is_device_private(const struct
folio *folio)
In general, this LGTM, and it should be correct with PageAnonExclusive I think.
However, where exactly is pinning forbidden?
Long-term pinning is forbidden since it would interfere with the device
memory manager owning the
device-coherent pages (e.g. evictions in TTM). However, normal pinning
is allowed on this device type.
I don't see updates to folio_is_pinnable() in this patch.
Device coherent type pages should return true here, as they are pinnable
pages.
That function is only called for long-term pinnings in try_grab_folio().
So wouldn't try_grab_folio() simply pin these pages? What am I missing?
As far as I understand this return NULL for long term pin pages.
Otherwise they get refcount incremented.
I don't follow.
You're saying
a) folio_is_pinnable() returns true for device coherent pages
and that
b) device coherent pages don't get long-term pinned
Yet, the code says
struct folio *try_grab_folio(struct page *page, int refs, unsigned int flags)
{
if (flags & FOLL_GET)
return try_get_folio(page, refs);
else if (flags & FOLL_PIN) {
struct folio *folio;
/*
* Can't do FOLL_LONGTERM + FOLL_PIN gup fast path if not in a
* right zone, so fail and let the caller fall back to the slow
* path.
*/
if (unlikely((flags & FOLL_LONGTERM) &&
!is_pinnable_page(page)))
return NULL;
...
return folio;
}
}
What prevents these pages from getting long-term pinned as stated in this patch?
Long-term pinning is handled by __gup_longterm_locked, which migrates
pages returned by
Re: [PATCH] MAINTAINERS: rectify entry for NVIDIA TEGRA DRM and VIDEO DRIVER
On Thu, Jun 23, 2022 at 11:54:52AM +0200, Lukas Bulwahn wrote:
> Commit fd27de58b0ad ("dt-bindings: display: tegra: Convert to json-schema")
> converts nvidia,tegra20-host1x.txt to yaml, but missed to adjust its
> references in MAINTAINERS.
>
> Hence, ./scripts/get_maintainer.pl --self-test=patterns complains about a
> broken reference.
>
> Repair these file references in NVIDIA TEGRA DRM and VIDEO DRIVER.
>
> Signed-off-by: Lukas Bulwahn
> ---
> Thierry, please pick this minor non-urgent clean-up on top of the commit
> above.
>
> MAINTAINERS | 4 ++--
> 1 file changed, 2 insertions(+), 2 deletions(-)
Applied, thanks.
Thierry
signature.asc
Description: PGP signature
[PATCH 5/6] dma-buf: remove useless FMODE_LSEEK flag
This is already on by default. Suggested-by: Al Viro Cc: Sumit Semwal Cc: Christian König Cc: dri-devel@lists.freedesktop.org Signed-off-by: Jason A. Donenfeld --- drivers/dma-buf/dma-buf.c | 1 - 1 file changed, 1 deletion(-) diff --git a/drivers/dma-buf/dma-buf.c b/drivers/dma-buf/dma-buf.c index 32f55640890c..3f08e0b960ec 100644 --- a/drivers/dma-buf/dma-buf.c +++ b/drivers/dma-buf/dma-buf.c @@ -549,7 +549,6 @@ struct dma_buf *dma_buf_export(const struct dma_buf_export_info *exp_info) goto err_dmabuf; } - file->f_mode |= FMODE_LSEEK; dmabuf->file = file; mutex_init(&dmabuf->lock); -- 2.35.1
[PATCH v1 0/3] fix primary corruption issue
fix primary corruption : 1) move struc of msm_display_info to msm_drv.h 2) decoupling dp->id out of dp controller_id at sc_dp_cfg table 3) place edp at head of drm bridge chain to fix screen corruption Kuogee Hsieh (3): drm/msm/dp: move struc of msm_display_info to msm_drv.h drm/msm/dp: decoupling dp->id out of dp controller_id at sc_dp_cfg table drm/msm/dp: place edp at head of drm bridge chain to fix screen corruption drivers/gpu/drm/msm/disp/dpu1/dpu_encoder.h | 20 --- drivers/gpu/drm/msm/disp/dpu1/dpu_kms.c | 16 +-- drivers/gpu/drm/msm/dp/dp_display.c | 30 ++--- drivers/gpu/drm/msm/msm_drv.h | 21 4 files changed, 54 insertions(+), 33 deletions(-) -- The Qualcomm Innovation Center, Inc. is a member of the Code Aurora Forum, a Linux Foundation Collaborative Project
[PATCH v1 3/3] drm/msm/dp: place edp at head of drm bridge chain to fix screen corruption
The msm_dp_modeset_init() is used to attach DP driver to drm bridge chain.
msm_dp_modeset_init() is executed in the order of index (dp->id) of DP
descriptor table.
Currently, DP is placed at first entry (dp->id = 0) of descriptor table
and eDP is placed at secondary entry (dp->id = 1 ) of descriptor table.
This means DP will be placed at head of bridge chain and eDP will be
placed right after DP at bridge chain.
Drm screen update is happen sequentially in the order from head to tail
of bridge chain. Therefore external DP display will have screen updated
happen before primary eDP display if external DP display presented.
This is wrong screen update order and cause one frame time screen
corruption happen at primary display during external DP plugged in.
This patch place eDP at first entry (dp->id = 0) of descriptor table and
place DP at secondary entry (dp->id = 1) to have primary eDP locate at
head of bridge chain. This correct screen update order and eliminated
the one frame time screen corruption happen d at primary display.
Signed-off-by: Kuogee Hsieh
---
drivers/gpu/drm/msm/dp/dp_display.c | 4 ++--
1 file changed, 2 insertions(+), 2 deletions(-)
diff --git a/drivers/gpu/drm/msm/dp/dp_display.c
b/drivers/gpu/drm/msm/dp/dp_display.c
index a87a9d8..2755ff3 100644
--- a/drivers/gpu/drm/msm/dp/dp_display.c
+++ b/drivers/gpu/drm/msm/dp/dp_display.c
@@ -143,10 +143,10 @@ static const struct msm_dp_config sc7180_dp_cfg = {
static const struct msm_dp_config sc7280_dp_cfg = {
.descs = (const struct msm_dp_desc[]) {
- { .io_start = 0x0ae9, .connector_type =
DRM_MODE_CONNECTOR_DisplayPort,
- .controller_id = MSM_DP_CONTROLLER_0, .wide_bus_en = true },
{ .io_start = 0x0aea, .connector_type =
DRM_MODE_CONNECTOR_eDP,
.controller_id = MSM_DP_CONTROLLER_1, .wide_bus_en = true },
+ { .io_start = 0x0ae9, .connector_type =
DRM_MODE_CONNECTOR_DisplayPort,
+ .controller_id = MSM_DP_CONTROLLER_0, .wide_bus_en = true },
},
.num_descs = 2,
};
--
The Qualcomm Innovation Center, Inc. is a member of the Code Aurora Forum,
a Linux Foundation Collaborative Project
[PATCH v1 1/3] drm/msm/dp: move struc of msm_display_info to msm_drv.h
With current implementation, communication between interface driver and
upper mdss encoder layer are implemented through function calls. This
increase code complexity. Since struct msm_display_info contains msm
generic display information, it can be expended to contains more useful
information, such as widebus and dcs, in future to serve as communication
channel purpose between interface driver and upper mdss encoder layer so
that existing function calls can be eliminated.
This patch more struct msm_display_info to msm_drv.h to be visible by
whole msm scope.
Signed-off-by: Kuogee Hsieh
---
drivers/gpu/drm/msm/disp/dpu1/dpu_encoder.h | 20
drivers/gpu/drm/msm/msm_drv.h | 19 +++
2 files changed, 19 insertions(+), 20 deletions(-)
diff --git a/drivers/gpu/drm/msm/disp/dpu1/dpu_encoder.h
b/drivers/gpu/drm/msm/disp/dpu1/dpu_encoder.h
index 781d41c..6b604c5 100644
--- a/drivers/gpu/drm/msm/disp/dpu1/dpu_encoder.h
+++ b/drivers/gpu/drm/msm/disp/dpu1/dpu_encoder.h
@@ -19,26 +19,6 @@
#define IDLE_TIMEOUT (66 - 16/2)
/**
- * struct msm_display_info - defines display properties
- * @intf_type: DRM_MODE_ENCODER_ type
- * @capabilities: Bitmask of display flags
- * @num_of_h_tiles: Number of horizontal tiles in case of split interface
- * @h_tile_instance:Controller instance used per tile. Number of elements
is
- * based on num_of_h_tiles
- * @is_te_using_watchdog_timer: Boolean to indicate watchdog TE is
- * used instead of panel TE in cmd mode panels
- * @dsc: DSC configuration data for DSC-enabled displays
- */
-struct msm_display_info {
- int intf_type;
- uint32_t capabilities;
- uint32_t num_of_h_tiles;
- uint32_t h_tile_instance[MAX_H_TILES_PER_DISPLAY];
- bool is_te_using_watchdog_timer;
- struct msm_display_dsc_config *dsc;
-};
-
-/**
* dpu_encoder_assign_crtc - Link the encoder to the crtc it's assigned to
* @encoder: encoder pointer
* @crtc: crtc pointer
diff --git a/drivers/gpu/drm/msm/msm_drv.h b/drivers/gpu/drm/msm/msm_drv.h
index fdbaad5..f9c263b 100644
--- a/drivers/gpu/drm/msm/msm_drv.h
+++ b/drivers/gpu/drm/msm/msm_drv.h
@@ -106,11 +106,30 @@ struct msm_drm_thread {
struct kthread_worker *worker;
};
+<<< HEAD
/* DSC config */
struct msm_display_dsc_config {
struct drm_dsc_config *drm;
};
+/**
+ * struct msm_display_info - defines display properties
+ * @intf_type: DRM_MODE_ENCODER_ type
+ * @capabilities: Bitmask of display flags
+ * @num_of_h_tiles: Number of horizontal tiles in case of split interface
+ * @h_tile_instance:Controller instance used per tile. Number of elements
is
+ * based on num_of_h_tiles
+ * @is_te_using_watchdog_timer: Boolean to indicate watchdog TE is
+ * used instead of panel TE in cmd mode panels
+ */
+struct msm_display_info {
+ int intf_type;
+ uint32_t capabilities;
+ uint32_t num_of_h_tiles;
+ uint32_t h_tile_instance[MAX_H_TILES_PER_DISPLAY];
+ bool is_te_using_watchdog_timer;
+};
+
struct msm_drm_private {
struct drm_device *dev;
--
The Qualcomm Innovation Center, Inc. is a member of the Code Aurora Forum,
a Linux Foundation Collaborative Project
[PATCH v1 2/3] drm/msm/dp: decoupling dp->id out of dp controller_id at scxxxx_dp_cfg table
Current the index (dp->id) of DP descriptor table (sc_dp_cfg[]) are tightly
coupled with DP controller_id. This means DP use controller id 0 must be placed
at first entry of DP descriptor table (sc_dp_cfg[]). Otherwise the internal
INTF will mismatch controller_id. This will cause controller kickoff wrong
interface timing engine and cause dpu_encoder_phys_vid_wait_for_commit_done
vblank timeout error.
This patch add controller_id field into struct msm_dp_desc to break the tightly
coupled relationship between index (dp->id) of DP descriptor table with DP
controller_id.
Signed-off-by: Kuogee Hsieh
---
drivers/gpu/drm/msm/disp/dpu1/dpu_kms.c | 16 ++--
drivers/gpu/drm/msm/dp/dp_display.c | 30 +++---
drivers/gpu/drm/msm/msm_drv.h | 2 ++
3 files changed, 35 insertions(+), 13 deletions(-)
diff --git a/drivers/gpu/drm/msm/disp/dpu1/dpu_kms.c
b/drivers/gpu/drm/msm/disp/dpu1/dpu_kms.c
index 2b9d931..8feeb89 100644
--- a/drivers/gpu/drm/msm/disp/dpu1/dpu_kms.c
+++ b/drivers/gpu/drm/msm/disp/dpu1/dpu_kms.c
@@ -615,7 +615,7 @@ static int _dpu_kms_initialize_displayport(struct
drm_device *dev,
struct dpu_kms *dpu_kms)
{
struct drm_encoder *encoder = NULL;
- struct msm_display_info info;
+ struct msm_display_info *info;
int rc;
int i;
@@ -637,11 +637,15 @@ static int _dpu_kms_initialize_displayport(struct
drm_device *dev,
return rc;
}
- info.num_of_h_tiles = 1;
- info.h_tile_instance[0] = i;
- info.capabilities = MSM_DISPLAY_CAP_VID_MODE;
- info.intf_type = encoder->encoder_type;
- rc = dpu_encoder_setup(dev, encoder, &info);
+ info = &priv->info[i];
+ info->intf_type = encoder->encoder_type;
+ /*
+* info->capabilities, info->num_of_h_tiles and
+* info->h_tile_instance are populated at
+* dp_display_bind()
+*/
+
+ rc = dpu_encoder_setup(dev, encoder, info);
if (rc) {
DPU_ERROR("failed to setup DPU encoder %d: rc:%d\n",
encoder->base.id, rc);
diff --git a/drivers/gpu/drm/msm/dp/dp_display.c
b/drivers/gpu/drm/msm/dp/dp_display.c
index da5c03a..a87a9d8 100644
--- a/drivers/gpu/drm/msm/dp/dp_display.c
+++ b/drivers/gpu/drm/msm/dp/dp_display.c
@@ -77,6 +77,7 @@ struct dp_display_private {
int irq;
unsigned int id;
+ unsigned int controller_id;
/* state variables */
bool core_initialized;
@@ -123,6 +124,7 @@ struct dp_display_private {
struct msm_dp_desc {
phys_addr_t io_start;
unsigned int connector_type;
+ unsigned int controller_id;
bool wide_bus_en;
};
@@ -133,31 +135,38 @@ struct msm_dp_config {
static const struct msm_dp_config sc7180_dp_cfg = {
.descs = (const struct msm_dp_desc[]) {
- [MSM_DP_CONTROLLER_0] = { .io_start = 0x0ae9,
.connector_type = DRM_MODE_CONNECTOR_DisplayPort },
+ { .io_start = 0x0ae9, .connector_type =
DRM_MODE_CONNECTOR_DisplayPort,
+ .controller_id = MSM_DP_CONTROLLER_0 },
},
.num_descs = 1,
};
static const struct msm_dp_config sc7280_dp_cfg = {
.descs = (const struct msm_dp_desc[]) {
- [MSM_DP_CONTROLLER_0] = { .io_start = 0x0ae9,
.connector_type = DRM_MODE_CONNECTOR_DisplayPort, .wide_bus_en = true },
- [MSM_DP_CONTROLLER_1] = { .io_start = 0x0aea,
.connector_type = DRM_MODE_CONNECTOR_eDP, .wide_bus_en = true },
+ { .io_start = 0x0ae9, .connector_type =
DRM_MODE_CONNECTOR_DisplayPort,
+ .controller_id = MSM_DP_CONTROLLER_0, .wide_bus_en = true },
+ { .io_start = 0x0aea, .connector_type =
DRM_MODE_CONNECTOR_eDP,
+ .controller_id = MSM_DP_CONTROLLER_1, .wide_bus_en = true },
},
.num_descs = 2,
};
static const struct msm_dp_config sc8180x_dp_cfg = {
.descs = (const struct msm_dp_desc[]) {
- [MSM_DP_CONTROLLER_0] = { .io_start = 0x0ae9,
.connector_type = DRM_MODE_CONNECTOR_DisplayPort },
- [MSM_DP_CONTROLLER_1] = { .io_start = 0x0ae98000,
.connector_type = DRM_MODE_CONNECTOR_DisplayPort },
- [MSM_DP_CONTROLLER_2] = { .io_start = 0x0ae9a000,
.connector_type = DRM_MODE_CONNECTOR_eDP },
+ {.io_start = 0x0ae9a000, .connector_type =
DRM_MODE_CONNECTOR_eDP,
+ .controller_id = MSM_DP_CONTROLLER_2 },
+ { .io_start = 0x0ae9, .connector_type =
DRM_MODE_CONNECTOR_DisplayPort,
+ .controller_id = MSM_DP_CONTROLLER_0 },
+ { .io_start = 0x0ae98000, .connector_type =
DRM_MODE_CONNECTOR_DisplayPort,
+ .controller_id = MSM_DP_CONTROLL
Re: [PATCH] drm/ingenic: Use resource_size function on resource object
Hi Jiapeng, On Fri, Jun 24, 2022 at 09:31:59AM +0800, Jiapeng Chong wrote: > This was found by coccicheck: > > ./drivers/gpu/drm/ingenic/ingenic-drm-drv.c:1149:35-38: WARNING: Suspicious > code. resource_size is maybe missing with res. > Nice one, now I have to go back and fix my code as well. > Signed-off-by: Jiapeng Chong Reviewed-by: Sam Ravnborg
Re: [RESEND v5 1/2] dt-bindings: display: simple: Add DataImage FG1001L0DSSWMG01 compatible string
Hi Philip, On Thu, Jun 23, 2022 at 01:22:56PM +0200, Philip Oberfichtner wrote: > Add DataImage FG1001L0DSSWMG01 10.1" 1280x800 TFT LCD panel compatible > string. > > Signed-off-by: Philip Oberfichtner > Acked-by: Krzysztof Kozlowski Both patches applied to drm-misc (drm-misc-next) Sam
Re: [PATCH v6 02/10] dt-bindings: display: tegra: Convert to json-schema
On Tue, 21 Jun 2022 18:10:14 +0300, Mikko Perttunen wrote:
> From: Thierry Reding
>
> Convert the Tegra host1x controller bindings from the free-form text
> format to json-schema.
>
> This also adds the missing display-hub DT bindings that were not
> previously documented.
>
> Reviewed-by: Rob Herring
> Signed-off-by: Thierry Reding
> ---
> .../display/tegra/nvidia,tegra114-mipi.txt| 41 --
> .../display/tegra/nvidia,tegra114-mipi.yaml | 74 ++
> .../display/tegra/nvidia,tegra124-dpaux.yaml | 149
> .../display/tegra/nvidia,tegra124-sor.yaml| 206 ++
> .../display/tegra/nvidia,tegra124-vic.yaml| 71 ++
> .../display/tegra/nvidia,tegra186-dc.yaml | 85 +++
> .../tegra/nvidia,tegra186-display.yaml| 310
> .../tegra/nvidia,tegra186-dsi-padctl.yaml | 45 ++
> .../display/tegra/nvidia,tegra20-dc.yaml | 181 +
> .../display/tegra/nvidia,tegra20-dsi.yaml | 159 +
> .../display/tegra/nvidia,tegra20-epp.yaml | 70 ++
> .../display/tegra/nvidia,tegra20-gr2d.yaml| 73 ++
> .../display/tegra/nvidia,tegra20-gr3d.yaml| 214 ++
> .../display/tegra/nvidia,tegra20-hdmi.yaml| 126
> .../display/tegra/nvidia,tegra20-host1x.txt | 675 --
> .../display/tegra/nvidia,tegra20-host1x.yaml | 347 +
> .../display/tegra/nvidia,tegra20-isp.yaml | 67 ++
> .../display/tegra/nvidia,tegra20-mpe.yaml | 73 ++
> .../display/tegra/nvidia,tegra20-tvo.yaml | 58 ++
> .../display/tegra/nvidia,tegra20-vi.yaml | 163 +
> .../display/tegra/nvidia,tegra210-csi.yaml| 52 ++
> .../pinctrl/nvidia,tegra124-dpaux-padctl.txt | 59 --
> 22 files changed, 2523 insertions(+), 775 deletions(-)
> delete mode 100644
> Documentation/devicetree/bindings/display/tegra/nvidia,tegra114-mipi.txt
> create mode 100644
> Documentation/devicetree/bindings/display/tegra/nvidia,tegra114-mipi.yaml
> create mode 100644
> Documentation/devicetree/bindings/display/tegra/nvidia,tegra124-dpaux.yaml
> create mode 100644
> Documentation/devicetree/bindings/display/tegra/nvidia,tegra124-sor.yaml
> create mode 100644
> Documentation/devicetree/bindings/display/tegra/nvidia,tegra124-vic.yaml
> create mode 100644
> Documentation/devicetree/bindings/display/tegra/nvidia,tegra186-dc.yaml
> create mode 100644
> Documentation/devicetree/bindings/display/tegra/nvidia,tegra186-display.yaml
> create mode 100644
> Documentation/devicetree/bindings/display/tegra/nvidia,tegra186-dsi-padctl.yaml
> create mode 100644
> Documentation/devicetree/bindings/display/tegra/nvidia,tegra20-dc.yaml
> create mode 100644
> Documentation/devicetree/bindings/display/tegra/nvidia,tegra20-dsi.yaml
> create mode 100644
> Documentation/devicetree/bindings/display/tegra/nvidia,tegra20-epp.yaml
> create mode 100644
> Documentation/devicetree/bindings/display/tegra/nvidia,tegra20-gr2d.yaml
> create mode 100644
> Documentation/devicetree/bindings/display/tegra/nvidia,tegra20-gr3d.yaml
> create mode 100644
> Documentation/devicetree/bindings/display/tegra/nvidia,tegra20-hdmi.yaml
> delete mode 100644
> Documentation/devicetree/bindings/display/tegra/nvidia,tegra20-host1x.txt
> create mode 100644
> Documentation/devicetree/bindings/display/tegra/nvidia,tegra20-host1x.yaml
> create mode 100644
> Documentation/devicetree/bindings/display/tegra/nvidia,tegra20-isp.yaml
> create mode 100644
> Documentation/devicetree/bindings/display/tegra/nvidia,tegra20-mpe.yaml
> create mode 100644
> Documentation/devicetree/bindings/display/tegra/nvidia,tegra20-tvo.yaml
> create mode 100644
> Documentation/devicetree/bindings/display/tegra/nvidia,tegra20-vi.yaml
> create mode 100644
> Documentation/devicetree/bindings/display/tegra/nvidia,tegra210-csi.yaml
> delete mode 100644
> Documentation/devicetree/bindings/pinctrl/nvidia,tegra124-dpaux-padctl.txt
>
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:
/builds/robherring/linux-dt-review/Documentation/devicetree/bindings/display/tegra/nvidia,tegra124-sor.yaml:
allOf:1:if:not:properties: {'contains': {'const': 'nvidia,panel'}} should not
be valid under {'$ref': '#/definitions/sub-schemas'}
hint: A json-schema keyword was found instead of a DT property name.
from schema $id: http://devicetree.org/meta-schemas/keywords.yaml#
/builds/robherring/linux-dt-review/Documentation/devicetree/bindings/display/tegra/nvidia,tegra124-sor.yaml:
ignoring, error in schema: allOf: 1: if: not: properties
/builds/robherring/linux-dt-review/Documentation/devicetree/bindings/display/tegra/nvidia,tegra20-gr3d.example.dtb:
gr3d@5418: resets: [[4294967295, 24]] is too short
From schema:
/builds/robherring/linux-dt-review/Documentation/devicetree/bindings/display/tegra/nvidia,tegra20-gr3d.yaml
/builds/robherring/linux-dt-review/Documentation/device
[PATCH v6 1/3] drm/doc/rfc: VM_BIND feature design document
VM_BIND design document with description of intended use cases. v2: Reduce the scope to simple Mesa use case. v3: Expand documentation on dma-resv usage, TLB flushing and execbuf3. v4: Remove vm_bind tlb flush request support. v5: Update TLB flushing documentation. Signed-off-by: Niranjana Vishwanathapura --- Documentation/gpu/rfc/i915_vm_bind.rst | 246 + Documentation/gpu/rfc/index.rst| 4 + 2 files changed, 250 insertions(+) create mode 100644 Documentation/gpu/rfc/i915_vm_bind.rst diff --git a/Documentation/gpu/rfc/i915_vm_bind.rst b/Documentation/gpu/rfc/i915_vm_bind.rst new file mode 100644 index ..534adf0c6c7a --- /dev/null +++ b/Documentation/gpu/rfc/i915_vm_bind.rst @@ -0,0 +1,246 @@ +== +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 (execbuf calls) +issued by the UMD, without user having to provide a list of all required +mappings during each submission (as required by older execbuf mode). + +The VM_BIND/UNBIND calls allow UMDs to request a timeline fence for signaling +the completion of bind/unbind operation. + +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. + +The bind/unbind operation can get completed asynchronously and out of +submission order. The out fence when specified will be signaled upon +completion of bind/unbind operation. + +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. +* Support for userptr gem objects (no special uapi is required for this). + +TLB flush consideration + +The i915 driver flushes the TLB for each submission and when an object's +pages are released. The VM_BIND/UNBIND operation will not do any additional +TLB flush. Any VM_BIND mapping added will be in the working set for subsequent +submissions on that VM and will not be in the working set for currently running +batches (which would require additional TLB flushes, which is not supported). + +Execbuf ioctl in VM_BIND mode +--- +A VM in VM_BIND mode will not support older execbuf mode of binding. +The execbuf ioctl handling in VM_BIND mode differs significantly from the +older execbuf2 ioctl (See struct drm_i915_gem_execbuffer2). +Hence, a new execbuf3 ioctl has been added to support VM_BIND mode. (See +struct drm_i915_gem_execbuffer3). The execbuf3 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/) + +The new execbuf3 ioctl only works in VM_BIND mode and the VM_BIND mode only +works with execbuf3 ioctl for submission. All BOs mapped on that VM (through +VM_BIND call) at the time of execbuf3 call are deemed required for that +submission. + +The execbuf3 ioctl directly specifies the batch addresses instead of as +object handles as in execbuf2 ioctl. The execbuf3 ioctl will also not +support many of the older features like in/out/submit fences, fence array, +default gem context and many more (See struct drm_i915_gem_execbuffer3). + +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 supporting execbuf2 ioctl, like relocations, VA +evictions, vma lookup table, implicit sync, vma active reference tracking etc., +are not applicable for execbuf3 ioctl. Hence, all execbuf3 specific handling +should be in a separate file and only functionalities common to these ioctls +can be the shared code where possible. + +VM_PRIVATE objects +--- +By default, BOs can be mapped on multiple VMs and can also be dma-buf +exported. Hence these BOs are referred to as Shared BOs. +During each execbuf submission, the request fence must be added to the +dma-resv fence list of all shared BOs mapped on the VM. + +VM_BIND feature introduces an optimization where user can create BO which
[PATCH v6 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
Reviewed-by: Matthew Auld
---
include/uapi/drm/i915_drm.h | 205
1 file changed, 160 insertions(+), 45 deletions(-)
diff --git a/include/uapi/drm/i915_drm.h b/include/uapi/drm/i915_drm.h
index de49b68b4fc8..4afe95d8b98b 100644
--- a/include/uapi/drm/i915_drm.h
+++ b/include/uapi/drm/i915_drm.h
@@ -751,14 +751,27 @@ typedef struct drm_i915_irq_wait {
/* Must be kept compact -- no holes and well documented */
-typedef struct drm_i915_getparam {
+/**
+ * struct drm_i915_getparam - Driver parameter query structure.
+ */
+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.
*/
int __user *value;
-} drm_i915_getparam_t;
+};
+
+/**
+ * typedef drm_i915_getparam_t - Driver parameter query structure.
+ * See struct drm_i915_getparam.
+ */
+typedef struct drm_i915_getparam drm_i915_getparam_t;
/* Ioctl to set kernel params:
*/
@@ -1239,76 +1252,119 @@ struct drm_i915_gem_exec_object2 {
__u64 rsvd2;
};
+/**
+ * struct drm_i915_gem_exec_fence - An input or output fence for the execbuf
+ * 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 execbuf ioctl.
+ *
* 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 elements 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 DRM_I915_GEM_EXECBUFFER2
+ * ioctl.
+ */
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: Length in bytes of the batch buffer, starting from the
+* @batch_start_offset. If 0, length is assumed to be the batch buffer
+* object size.
+*/
[PATCH v6 3/3] drm/doc/rfc: VM_BIND uapi definition
VM_BIND and related uapi definitions
v2: Reduce the scope to simple Mesa use case.
v3: Expand VM_UNBIND documentation and add
I915_GEM_VM_BIND/UNBIND_FENCE_VALID
and I915_GEM_VM_BIND_TLB_FLUSH flags.
v4: Remove I915_GEM_VM_BIND_TLB_FLUSH flag and add additional
documentation for vm_bind/unbind.
v5: Remove TLB flush requirement on VM_UNBIND.
Add version support to stage implementation.
v6: Define and use drm_i915_gem_timeline_fence structure for
all timeline fences.
Signed-off-by: Niranjana Vishwanathapura
---
Documentation/gpu/rfc/i915_vm_bind.h | 286 +++
1 file changed, 286 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 ..c784dc0c48b3
--- /dev/null
+++ b/Documentation/gpu/rfc/i915_vm_bind.h
@@ -0,0 +1,286 @@
+/* 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.
+ * bit[0]: If set, VM_BIND is supported, otherwise not.
+ * bits[8-15]: VM_BIND implementation version.
+ * Version 0 requires in VM_UNBIND call, UMDs to specify the exact mapping
+ * created previously with the VM_BIND call. i.e., i915 will not support
+ * splitting/merging of the mappings created with VM_BIND call (See
+ * struct drm_i915_gem_vm_bind and struct drm_i915_gem_vm_unbind).
+ */
+#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.
+ *
+ * The older execbuf2 ioctl will not support VM_BIND mode of operation.
+ * For VM_BIND mode, we have new execbuf3 ioctl which will not accept any
+ * execlist (See struct drm_i915_gem_execbuffer3 for more details).
+ *
+ */
+#define I915_VM_CREATE_FLAGS_USE_VM_BIND (1 << 0)
+
+/* VM_BIND related ioctls */
+#define DRM_I915_GEM_VM_BIND 0x3d
+#define DRM_I915_GEM_VM_UNBIND 0x3e
+#define DRM_I915_GEM_EXECBUFFER3 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_EXECBUFFER3 DRM_IOWR(DRM_COMMAND_BASE +
DRM_I915_GEM_EXECBUFFER3, struct drm_i915_gem_execbuffer3)
+
+/**
+ * struct drm_i915_gem_timeline_fence - An input or output timeline fence.
+ *
+ * The operation will wait for input fence to signal.
+ *
+ * The returned output fence will be signaled after the completion of the
+ * operation.
+ */
+struct drm_i915_gem_timeline_fence {
+ /** @handle: User's handle for a drm_syncobj to wait on or signal. */
+ __u32 handle;
+
+ /**
+* @flags: Supported flags are:
+*
+* I915_TIMELINE_FENCE_WAIT:
+* Wait for the input fence before the operation.
+*
+* I915_TIMELINE_FENCE_SIGNAL:
+* Return operation completion fence as output.
+*/
+ __u32 flags;
+#define I915_TIMELINE_FENCE_WAIT(1<<0)
+#define I915_TIMELINE_FENCE_SIGNAL (1<<1)
+#define __I915_TIMELINE_FENCE_UNKNOWN_FLAGS (-(I915_TIMELINE_FENCE_SIGNAL <<
1))
+
+ /**
+* @value: A point in the timeline.
+* Value 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 value;
+};
+
+/**
+ * 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).
+ *
+ * The @start, @offset and @length should be 4K page aligned. However the DG2
+ * and XEHPSDV has 64K page size for device local-memory and has compact page
+ * table. On those platforms, for binding device local-memory objects, the
+ * @start should be 2M aligned, @offset and @length should be 64K aligned.
+ * Also, on those platforms, error -ENOSPC will be returned if user tries to
+ * bind a device local-memory object and a system memory object in a single 2M
+ * section of VA range.
+ *
+ * Error code -EINVAL will be returned if @start, @offset and @length are not
+ * properly aligned. Error code of -ENOSPC will be returned if the VA range
+ * specified can't be reserved.
+ *
+ * The bind operation can get completed asynchro
[PATCH v6 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: Reduce the scope to simple Mesa use case. Remove all compute related uapi, vm_bind/unbind queue support and only support a timeline out fence instead of an in/out timeline fence array. v3: Expand documentation on dma-resv usage, TLB flushing, execbuf3 and VM_UNBIND. Add FENCE_VALID and TLB_FLUSH flags. v4: Remove I915_GEM_VM_BIND_TLB_FLUSH flag and add additional uapi documentation for vm_bind/unbind. v5: Update TLB flushing documentation. Add version support to stage implementation. v6: Define and use drm_i915_gem_timeline_fence structure for execbuf3 and vm_bind/unbind timeline fences. 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/gpu/rfc/i915_vm_bind.h | 286 + Documentation/gpu/rfc/i915_vm_bind.rst | 246 + Documentation/gpu/rfc/index.rst| 4 + include/uapi/drm/i915_drm.h| 205 ++ 4 files changed, 696 insertions(+), 45 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
Re: [PATCH] drm/rockchip: vop: Don't crash for invalid duplicate_state()
On Fri, Jun 24, 2022 at 12:23 AM Heiko Stuebner wrote: > The interesting question would be, do we want some fixes tag for it? I'm not aware of any currently-upstream code that will hit this [1]. I've hit it in out-of-tree code (or, code that I submitted to dri-devel, but wasn't accepted as-is), and this is the "belt and braces" part -- the primary fix is that we should avoid calling things like drm_atomic_get_crtc_state() at inappropriate times. So, is the "extra safety" check really something that should go to -stable? (Because let's be honest, everything with a Fixes tag goes there.) Maybe? Anyway, if you want to "blame" anything, this commit actually dropped the safety check: 4e257d9eee23 drm/rockchip: get rid of rockchip_drm_crtc_mode_config Brian [1] But I'm not omniscient. So maybe it's good to have anyway.
[PATCH v6 1/2] dt-bindings: lcdif: Add compatible for i.MX8MP
Add compatible string for i.MX8MP LCDIF variant. This is called LCDIFv3 and is completely different from the LCDIFv3 found in i.MX23 in that it has a completely scrambled register layout compared to all previous LCDIF variants. The new LCDIFv3 also supports 36bit address space. However, except for the complete bit reshuffling, this is still LCDIF and it still works like one. Acked-by: Rob Herring Signed-off-by: Marek Vasut Cc: Alexander Stein Cc: Laurent Pinchart Cc: Lucas Stach Cc: Peng Fan Cc: Rob Herring Cc: Robby Cai Cc: Sam Ravnborg Cc: Stefan Agner Cc: devicet...@vger.kernel.org --- V2: No change V3: No change V4: No change V5: Add AB from Rob V6: No change --- Documentation/devicetree/bindings/display/fsl,lcdif.yaml | 1 + 1 file changed, 1 insertion(+) diff --git a/Documentation/devicetree/bindings/display/fsl,lcdif.yaml b/Documentation/devicetree/bindings/display/fsl,lcdif.yaml index 900a56cae80e6..876015a44a1e6 100644 --- a/Documentation/devicetree/bindings/display/fsl,lcdif.yaml +++ b/Documentation/devicetree/bindings/display/fsl,lcdif.yaml @@ -20,6 +20,7 @@ properties: - fsl,imx23-lcdif - fsl,imx28-lcdif - fsl,imx6sx-lcdif + - fsl,imx8mp-lcdif - items: - enum: - fsl,imx6sl-lcdif -- 2.35.1
[PATCH v6 2/2] drm: lcdif: Add support for i.MX8MP LCDIF variant
Add support for i.MX8MP LCDIF variant. This is called LCDIFv3 and is
completely different from the LCDIFv3 found in i.MX23 in that it has
a completely scrambled register layout compared to all previous LCDIF
variants. The new LCDIFv3 also supports 36bit address space.
Add a separate driver which is really a fork of MXSFB driver with the
i.MX8MP LCDIF variant handling filled in.
Tested-by: Alexander Stein
Tested-by: Martyn Welch
Signed-off-by: Marek Vasut
Cc: Alexander Stein
Cc: Laurent Pinchart
Cc: Lucas Stach
Cc: Peng Fan
Cc: Robby Cai
Cc: Sam Ravnborg
Cc: Stefan Agner
---
V2: - Drop the pitch check from lcdif_fb_create()
- Drop connector caching
- Wait for shadow load bit to be cleared in IRQ handler
- Make all clock mandatory and grab them all by name
- Wait for EN to be cleared in lcdif_disable_controller
- Rename to imx-lcdif
- Move shadow load to atomic_flush
V3: - Invert DE polarity to match MX8MPRM datasheet
- Enable CSC in RGB to YUV mode for MEDIA_BUS_FMT_UYVY8_1X16
V4: - Drop lcdif_overlay_plane_formats, it is unused
V5: - Add TB from Martyn
- Drop lcdif_fb_create in favor of drm_gem_fb_create
- Pull in mxsfb/ directory from drm top level Makefile
- Drop busy polling of CTRLDESCL0_5_SHADOW_LOAD_EN in irq handler
- Use devm_request_irq
- Drop useless dev.of_node validity check in probe
- Drop lcdif_*_axi_clk() prototypes
- Invert HS/VS polarity
- Drop polling from lcdif_reset_block()
- Add TB from Alexander
V6: - Use SET_RUNTIME_PM_OPS() to set RPM ops and mark ops __maybe_unused
- Include drm/drm_framebuffer.h now required in linux-next
---
drivers/gpu/drm/Makefile | 2 +-
drivers/gpu/drm/mxsfb/Kconfig | 16 +
drivers/gpu/drm/mxsfb/Makefile | 2 +
drivers/gpu/drm/mxsfb/lcdif_drv.c | 341
drivers/gpu/drm/mxsfb/lcdif_drv.h | 44 +++
drivers/gpu/drm/mxsfb/lcdif_kms.c | 483 +
drivers/gpu/drm/mxsfb/lcdif_regs.h | 257 +++
7 files changed, 1144 insertions(+), 1 deletion(-)
create mode 100644 drivers/gpu/drm/mxsfb/lcdif_drv.c
create mode 100644 drivers/gpu/drm/mxsfb/lcdif_drv.h
create mode 100644 drivers/gpu/drm/mxsfb/lcdif_kms.c
create mode 100644 drivers/gpu/drm/mxsfb/lcdif_regs.h
diff --git a/drivers/gpu/drm/Makefile b/drivers/gpu/drm/Makefile
index 13ef240b3d2b2..75b5ac7c2663c 100644
--- a/drivers/gpu/drm/Makefile
+++ b/drivers/gpu/drm/Makefile
@@ -130,7 +130,7 @@ obj-y += bridge/
obj-$(CONFIG_DRM_FSL_DCU) += fsl-dcu/
obj-$(CONFIG_DRM_ETNAVIV) += etnaviv/
obj-y += hisilicon/
-obj-$(CONFIG_DRM_MXSFB)+= mxsfb/
+obj-y += mxsfb/
obj-y += tiny/
obj-$(CONFIG_DRM_PL111) += pl111/
obj-$(CONFIG_DRM_TVE200) += tve200/
diff --git a/drivers/gpu/drm/mxsfb/Kconfig b/drivers/gpu/drm/mxsfb/Kconfig
index 987170e16ebd6..873551b4552f5 100644
--- a/drivers/gpu/drm/mxsfb/Kconfig
+++ b/drivers/gpu/drm/mxsfb/Kconfig
@@ -19,3 +19,19 @@ config DRM_MXSFB
i.MX28, i.MX6SX, i.MX7 and i.MX8M).
If M is selected the module will be called mxsfb.
+
+config DRM_IMX_LCDIF
+ tristate "i.MX LCDIFv3 LCD controller"
+ depends on DRM && OF
+ depends on COMMON_CLK
+ select DRM_MXS
+ select DRM_KMS_HELPER
+ select DRM_GEM_CMA_HELPER
+ select DRM_PANEL
+ select DRM_PANEL_BRIDGE
+ help
+ Choose this option if you have an LCDIFv3 LCD controller.
+ Those devices are found in various i.MX SoC (i.MX8MP,
+ i.MXRT).
+
+ If M is selected the module will be called imx-lcdif.
diff --git a/drivers/gpu/drm/mxsfb/Makefile b/drivers/gpu/drm/mxsfb/Makefile
index 26d153896d720..3fa44059b9d85 100644
--- a/drivers/gpu/drm/mxsfb/Makefile
+++ b/drivers/gpu/drm/mxsfb/Makefile
@@ -1,3 +1,5 @@
# SPDX-License-Identifier: GPL-2.0-only
mxsfb-y := mxsfb_drv.o mxsfb_kms.o
obj-$(CONFIG_DRM_MXSFB)+= mxsfb.o
+imx-lcdif-y := lcdif_drv.o lcdif_kms.o
+obj-$(CONFIG_DRM_IMX_LCDIF) += imx-lcdif.o
diff --git a/drivers/gpu/drm/mxsfb/lcdif_drv.c
b/drivers/gpu/drm/mxsfb/lcdif_drv.c
new file mode 100644
index 0..76e14ffe84f69
--- /dev/null
+++ b/drivers/gpu/drm/mxsfb/lcdif_drv.c
@@ -0,0 +1,341 @@
+// SPDX-License-Identifier: GPL-2.0-or-later
+/*
+ * Copyright (C) 2022 Marek Vasut
+ *
+ * This code is based on drivers/gpu/drm/mxsfb/mxsfb*
+ */
+
+#include
+#include
+#include
+#include
+#include
+#include
+#include
+#include
+
+#include
+#include
+#include
+#include
+#include
+#include
+#include
+#include
+#include
+#include
+#include
+#include
+#include
+
+#include "lcdif_drv.h"
+#include "lcdif_regs.h"
+
+static const struct drm_mode_config_funcs lcdif_mode_config_funcs = {
+ .fb_create = drm_gem_fb_create,
+ .atomic_check = drm_atomic_helper_check,
+ .atomic_commit = drm_atomic_helper_commit,
+};
+
+static const struct drm_
[PATCH v2 0/4] Rework amdgpu HW fence refocunt and update scheduler parent fence refcount.
Yiqing raised a problem of negative fence refcount for resubmitted jobs in amdgpu and suggested a workaround in [1]. I took a look myself and discovered some deeper problems both in amdgpu and scheduler code. Yiqing helped with testing the new code and also drew a detailed refcount and flow tracing diagram for parent (HW) fence life cycle and refcount under various cases for the proposed patchset at [2]. v2: Update race preventionby fixing by swithing from amdgpu_irq_get/put to enable/desable_irq (Christian) Drop refcount fix for amdgpu_job->external_hw_fence as it was causing underflow in direct submissions TODO - Follow up cleanup to totally get rid of amdgpu_job->external_hw_fence [1] - https://lore.kernel.org/all/731b7ff1-3cc9-e314-df2a-7c51b76d4...@amd.com/t/#r00c728fcc069b1276642c325bfa9d82bf8fa21a3 [2] - https://drive.google.com/file/d/1yEoeW6OQC9WnwmzFW6NBLhFP_jD0xcHm/view?usp=sharing Andrey Grodzovsky (4): drm/amdgpu: Add put fence in amdgpu_fence_driver_clear_job_fences drm/amdgpu: Prevent race between late signaled fences and GPU reset. drm/sched: Partial revert of 'drm/sched: Keep s_fence->parent pointer' drm/amdgpu: Follow up change to previous drm scheduler change. drivers/gpu/drm/amd/amdgpu/amdgpu_amdkfd.c | 2 ++ drivers/gpu/drm/amd/amdgpu/amdgpu_device.c | 31 +- drivers/gpu/drm/amd/amdgpu/amdgpu_fence.c | 29 ++-- drivers/gpu/drm/amd/amdgpu/amdgpu_job.c| 4 --- drivers/gpu/drm/amd/amdgpu/amdgpu_ring.h | 1 + drivers/gpu/drm/scheduler/sched_main.c | 13 ++--- 6 files changed, 65 insertions(+), 15 deletions(-) -- 2.25.1
[PATCH v2 1/4] drm/amdgpu: Add put fence in amdgpu_fence_driver_clear_job_fences
This function should drop the fence refcount when it extracts the
fence from the fence array, just as it's done in amdgpu_fence_process.
Signed-off-by: Andrey Grodzovsky
Reviewed-by: Christian König
---
drivers/gpu/drm/amd/amdgpu/amdgpu_fence.c | 4 +++-
1 file changed, 3 insertions(+), 1 deletion(-)
diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_fence.c
b/drivers/gpu/drm/amd/amdgpu/amdgpu_fence.c
index 957437a5558c..a9ae3beaa1d3 100644
--- a/drivers/gpu/drm/amd/amdgpu/amdgpu_fence.c
+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_fence.c
@@ -595,8 +595,10 @@ void amdgpu_fence_driver_clear_job_fences(struct
amdgpu_ring *ring)
for (i = 0; i <= ring->fence_drv.num_fences_mask; i++) {
ptr = &ring->fence_drv.fences[i];
old = rcu_dereference_protected(*ptr, 1);
- if (old && old->ops == &amdgpu_job_fence_ops)
+ if (old && old->ops == &amdgpu_job_fence_ops) {
RCU_INIT_POINTER(*ptr, NULL);
+ dma_fence_put(old);
+ }
}
}
--
2.25.1
[PATCH v2 3/4] drm/sched: Partial revert of 'drm/sched: Keep s_fence->parent pointer'
Problem:
This patch caused negative refcount as described in [1] because
for that case parent fence did not signal by the time of drm_sched_stop and
hence
kept in pending list the assumption was they will not signal and
so fence was put to account for the s_fence->parent refcount but for
amdgpu which has embedded HW fence (always same parent fence)
drm_sched_fence_release_scheduled was always called and would
still drop the count for parent fence once more. For jobs that
never signaled this imbalance was masked by refcount bug in
amdgpu_fence_driver_clear_job_fences that would not drop
refcount on the fences that were removed from fence drive
fences array (against prevois insertion into the array in
get in amdgpu_fence_emit).
Fix:
Revert this patch and by setting s_job->s_fence->parent to NULL
as before prevent the extra refcount drop in amdgpu when
drm_sched_fence_release_scheduled is called on job release.
Also - align behaviour in drm_sched_resubmit_jobs_ext with that of
drm_sched_main when submitting jobs - take a refcount for the
new parent fence pointer and drop refcount for original kref_init
for new HW fence creation (or fake new HW fence in amdgpu - see next patch).
[1] -
https://lore.kernel.org/all/731b7ff1-3cc9-e314-df2a-7c51b76d4...@amd.com/t/#r00c728fcc069b1276642c325bfa9d82bf8fa21a3
Signed-off-by: Andrey Grodzovsky
Tested-by: Yiqing Yao
---
drivers/gpu/drm/scheduler/sched_main.c | 13 ++---
1 file changed, 10 insertions(+), 3 deletions(-)
diff --git a/drivers/gpu/drm/scheduler/sched_main.c
b/drivers/gpu/drm/scheduler/sched_main.c
index b81fceb0b8a2..c5437ee03e3f 100644
--- a/drivers/gpu/drm/scheduler/sched_main.c
+++ b/drivers/gpu/drm/scheduler/sched_main.c
@@ -419,6 +419,8 @@ void drm_sched_stop(struct drm_gpu_scheduler *sched, struct
drm_sched_job *bad)
if (s_job->s_fence->parent &&
dma_fence_remove_callback(s_job->s_fence->parent,
&s_job->cb)) {
+ dma_fence_put(s_job->s_fence->parent);
+ s_job->s_fence->parent = NULL;
atomic_dec(&sched->hw_rq_count);
} else {
/*
@@ -548,7 +550,6 @@ void drm_sched_resubmit_jobs_ext(struct drm_gpu_scheduler
*sched, int max)
if (found_guilty && s_job->s_fence->scheduled.context ==
guilty_context)
dma_fence_set_error(&s_fence->finished, -ECANCELED);
- dma_fence_put(s_job->s_fence->parent);
fence = sched->ops->run_job(s_job);
i++;
@@ -558,7 +559,11 @@ void drm_sched_resubmit_jobs_ext(struct drm_gpu_scheduler
*sched, int max)
s_job->s_fence->parent = NULL;
} else {
- s_job->s_fence->parent = fence;
+
+ s_job->s_fence->parent = dma_fence_get(fence);
+
+ /* Drop for orignal kref_init */
+ dma_fence_put(fence);
}
}
}
@@ -952,6 +957,9 @@ static int drm_sched_main(void *param)
if (!IS_ERR_OR_NULL(fence)) {
s_fence->parent = dma_fence_get(fence);
+ /* Drop for original kref_init of the fence */
+ dma_fence_put(fence);
+
r = dma_fence_add_callback(fence, &sched_job->cb,
drm_sched_job_done_cb);
if (r == -ENOENT)
@@ -959,7 +967,6 @@ static int drm_sched_main(void *param)
else if (r)
DRM_DEV_ERROR(sched->dev, "fence add callback
failed (%d)\n",
r);
- dma_fence_put(fence);
} else {
if (IS_ERR(fence))
dma_fence_set_error(&s_fence->finished,
PTR_ERR(fence));
--
2.25.1
[PATCH v2 2/4] drm/amdgpu: Prevent race between late signaled fences and GPU reset.
Problem:
After we start handling timed out jobs we assume there fences won't be
signaled but we cannot be sure and sometimes they fire late. We need
to prevent concurrent accesses to fence array from
amdgpu_fence_driver_clear_job_fences during GPU reset and amdgpu_fence_process
from a late EOP interrupt.
Fix:
Before accessing fence array in GPU disable EOP interrupt and flush
all pending interrupt handlers for amdgpu device's interrupt line.
v2: Switch from irq_get/put to full enable/disable_irq for amdgpu
Signed-off-by: Andrey Grodzovsky
---
drivers/gpu/drm/amd/amdgpu/amdgpu_device.c | 4
drivers/gpu/drm/amd/amdgpu/amdgpu_fence.c | 18 ++
drivers/gpu/drm/amd/amdgpu/amdgpu_ring.h | 1 +
3 files changed, 23 insertions(+)
diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_device.c
b/drivers/gpu/drm/amd/amdgpu/amdgpu_device.c
index eacecc672a4d..03519d58e630 100644
--- a/drivers/gpu/drm/amd/amdgpu/amdgpu_device.c
+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_device.c
@@ -4605,6 +4605,8 @@ int amdgpu_device_pre_asic_reset(struct amdgpu_device
*adev,
amdgpu_virt_fini_data_exchange(adev);
}
+ amdgpu_fence_driver_isr_toggle(adev, true);
+
/* block all schedulers and reset given job's ring */
for (i = 0; i < AMDGPU_MAX_RINGS; ++i) {
struct amdgpu_ring *ring = adev->rings[i];
@@ -4620,6 +4622,8 @@ int amdgpu_device_pre_asic_reset(struct amdgpu_device
*adev,
amdgpu_fence_driver_force_completion(ring);
}
+ amdgpu_fence_driver_isr_toggle(adev, false);
+
if (job && job->vm)
drm_sched_increase_karma(&job->base);
diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_fence.c
b/drivers/gpu/drm/amd/amdgpu/amdgpu_fence.c
index a9ae3beaa1d3..c1d04ea3c67f 100644
--- a/drivers/gpu/drm/amd/amdgpu/amdgpu_fence.c
+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_fence.c
@@ -532,6 +532,24 @@ void amdgpu_fence_driver_hw_fini(struct amdgpu_device
*adev)
}
}
+/* Will either stop and flush handlers for amdgpu interrupt or reanble it */
+void amdgpu_fence_driver_isr_toggle(struct amdgpu_device *adev, bool stop)
+{
+ int i;
+
+ for (i = 0; i < AMDGPU_MAX_RINGS; i++) {
+ struct amdgpu_ring *ring = adev->rings[i];
+
+ if (!ring || !ring->fence_drv.initialized ||
!ring->fence_drv.irq_src)
+ continue;
+
+ if (stop)
+ disable_irq(adev->irq.irq);
+ else
+ enable_irq(adev->irq.irq);
+ }
+}
+
void amdgpu_fence_driver_sw_fini(struct amdgpu_device *adev)
{
unsigned int i, j;
diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_ring.h
b/drivers/gpu/drm/amd/amdgpu/amdgpu_ring.h
index 7d89a52091c0..82c178a9033a 100644
--- a/drivers/gpu/drm/amd/amdgpu/amdgpu_ring.h
+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_ring.h
@@ -143,6 +143,7 @@ signed long amdgpu_fence_wait_polling(struct amdgpu_ring
*ring,
uint32_t wait_seq,
signed long timeout);
unsigned amdgpu_fence_count_emitted(struct amdgpu_ring *ring);
+void amdgpu_fence_driver_isr_toggle(struct amdgpu_device *adev, bool stop);
/*
* Rings.
--
2.25.1
[PATCH v2 4/4] drm/amdgpu: Follow up change to previous drm scheduler change.
Align refcount behaviour for amdgpu_job embedded HW fence with
classic pointer style HW fences by increasing refcount each
time emit is called so amdgpu code doesn't need to make workarounds
using amdgpu_job.job_run_counter to keep the HW fence refcount balanced.
Also since in the previous patch we resumed setting s_fence->parent to NULL
in drm_sched_stop switch to directly checking if job->hw_fence is
signaled to short circuit reset if already signed.
Signed-off-by: Andrey Grodzovsky
Tested-by: Yiqing Yao
---
drivers/gpu/drm/amd/amdgpu/amdgpu_amdkfd.c | 2 ++
drivers/gpu/drm/amd/amdgpu/amdgpu_device.c | 27 +-
drivers/gpu/drm/amd/amdgpu/amdgpu_fence.c | 7 +-
drivers/gpu/drm/amd/amdgpu/amdgpu_job.c| 4
4 files changed, 29 insertions(+), 11 deletions(-)
diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_amdkfd.c
b/drivers/gpu/drm/amd/amdgpu/amdgpu_amdkfd.c
index 44da025502ac..567597469a8a 100644
--- a/drivers/gpu/drm/amd/amdgpu/amdgpu_amdkfd.c
+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_amdkfd.c
@@ -684,6 +684,8 @@ int amdgpu_amdkfd_submit_ib(struct amdgpu_device *adev,
goto err_ib_sched;
}
+ /* Drop the initial kref_init count (see drm_sched_main as example) */
+ dma_fence_put(f);
ret = dma_fence_wait(f, false);
err_ib_sched:
diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_device.c
b/drivers/gpu/drm/amd/amdgpu/amdgpu_device.c
index 03519d58e630..a2c268d48edd 100644
--- a/drivers/gpu/drm/amd/amdgpu/amdgpu_device.c
+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_device.c
@@ -5009,16 +5009,32 @@ static void amdgpu_device_recheck_guilty_jobs(
/* clear job's guilty and depend the folowing step to decide
the real one */
drm_sched_reset_karma(s_job);
- /* for the real bad job, it will be resubmitted twice, adding a
dma_fence_get
-* to make sure fence is balanced */
- dma_fence_get(s_job->s_fence->parent);
drm_sched_resubmit_jobs_ext(&ring->sched, 1);
+ if (!s_job->s_fence->parent) {
+ DRM_WARN("Failed to get a HW fence for job!");
+ continue;
+ }
+
ret = dma_fence_wait_timeout(s_job->s_fence->parent, false,
ring->sched.timeout);
if (ret == 0) { /* timeout */
DRM_ERROR("Found the real bad job! ring:%s,
job_id:%llx\n",
ring->sched.name, s_job->id);
+
+ amdgpu_fence_driver_isr_toggle(adev, true);
+
+ /* Clear this failed job from fence array */
+ amdgpu_fence_driver_clear_job_fences(ring);
+
+ amdgpu_fence_driver_isr_toggle(adev, false);
+
+ /* Since the job won't signal and we go for
+* another resubmit drop this parent pointer
+*/
+ dma_fence_put(s_job->s_fence->parent);
+ s_job->s_fence->parent = NULL;
+
/* set guilty */
drm_sched_increase_karma(s_job);
retry:
@@ -5047,7 +5063,6 @@ static void amdgpu_device_recheck_guilty_jobs(
/* got the hw fence, signal finished fence */
atomic_dec(ring->sched.score);
- dma_fence_put(s_job->s_fence->parent);
dma_fence_get(&s_job->s_fence->finished);
dma_fence_signal(&s_job->s_fence->finished);
dma_fence_put(&s_job->s_fence->finished);
@@ -5220,8 +5235,8 @@ int amdgpu_device_gpu_recover(struct amdgpu_device *adev,
*
* job->base holds a reference to parent fence
*/
- if (job && job->base.s_fence->parent &&
- dma_fence_is_signaled(job->base.s_fence->parent)) {
+ if (job && (job->hw_fence.ops != NULL) &&
+ dma_fence_is_signaled(&job->hw_fence)) {
job_signaled = true;
dev_info(adev->dev, "Guilty job already signaled, skipping HW
reset");
goto skip_hw_reset;
diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_fence.c
b/drivers/gpu/drm/amd/amdgpu/amdgpu_fence.c
index c1d04ea3c67f..39597ab807d1 100644
--- a/drivers/gpu/drm/amd/amdgpu/amdgpu_fence.c
+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_fence.c
@@ -164,11 +164,16 @@ int amdgpu_fence_emit(struct amdgpu_ring *ring, struct
dma_fence **f, struct amd
if (job && job->job_run_counter) {
/* reinit seq for resubmitted jobs */
fence->seqno = seq;
+ /* TO be inline with external fence creation and other drivers
*/
+ dma_fence_get(fence);
} else {
- if (job)
+ if (job) {
dma_fence_init(fence, &amdgpu_job_fence_ops,
&ring->fence_drv.lock,
adev->fence_c
Re: [PATCH] drm/bridge: add it6505 driver read config from dt property
Hi allen.
On Thu, Jun 23, 2022 at 05:31:54PM +0800, allen wrote:
> From: allen chen
>
> add read max-lane and max-pixel-clock from dt property
>
> Signed-off-by: Allen-kh Cheng
>
Can you fix so your s-o-b mail and author mail matches?
As it is now an error is flagged as they do not match.
Sam
> Signed-off-by: Pin-yen Lin
> ---
> drivers/gpu/drm/bridge/ite-it6505.c | 35 ++---
> 1 file changed, 32 insertions(+), 3 deletions(-)
>
> diff --git a/drivers/gpu/drm/bridge/ite-it6505.c
> b/drivers/gpu/drm/bridge/ite-it6505.c
> index 4b673c4792d77..c9121d4635a52 100644
> --- a/drivers/gpu/drm/bridge/ite-it6505.c
> +++ b/drivers/gpu/drm/bridge/ite-it6505.c
> @@ -436,6 +436,8 @@ struct it6505 {
> bool powered;
> bool hpd_state;
> u32 afe_setting;
> + u32 max_dpi_pixel_clock;
> + u32 max_lane_count;
> enum hdcp_state hdcp_status;
> struct delayed_work hdcp_work;
> struct work_struct hdcp_wait_ksv_list;
> @@ -1466,7 +1468,8 @@ static void it6505_parse_link_capabilities(struct
> it6505 *it6505)
> it6505->lane_count = link->num_lanes;
> DRM_DEV_DEBUG_DRIVER(dev, "Sink support %d lanes training",
>it6505->lane_count);
> - it6505->lane_count = min_t(int, it6505->lane_count, MAX_LANE_COUNT);
> + it6505->lane_count = min_t(int, it6505->lane_count,
> +it6505->max_lane_count);
>
> it6505->branch_device = drm_dp_is_branch(it6505->dpcd);
> DRM_DEV_DEBUG_DRIVER(dev, "Sink %sbranch device",
> @@ -2895,7 +2898,7 @@ it6505_bridge_mode_valid(struct drm_bridge *bridge,
> if (mode->flags & DRM_MODE_FLAG_INTERLACE)
> return MODE_NO_INTERLACE;
>
> - if (mode->clock > DPI_PIXEL_CLK_MAX)
> + if (mode->clock > it6505->max_dpi_pixel_clock)
> return MODE_CLOCK_HIGH;
>
> it6505->video_info.clock = mode->clock;
> @@ -3057,6 +3060,8 @@ static void it6505_parse_dt(struct it6505 *it6505)
> {
> struct device *dev = &it6505->client->dev;
> u32 *afe_setting = &it6505->afe_setting;
> + u32 *max_lane_count = &it6505->max_lane_count;
> + u32 *max_dpi_pixel_clock = &it6505->max_dpi_pixel_clock;
>
> it6505->lane_swap_disabled =
> device_property_read_bool(dev, "no-laneswap");
> @@ -3072,7 +3077,31 @@ static void it6505_parse_dt(struct it6505 *it6505)
> } else {
> *afe_setting = 0;
> }
> - DRM_DEV_DEBUG_DRIVER(dev, "using afe_setting: %d", *afe_setting);
> +
> + if (device_property_read_u32(dev, "max-lane-count",
> + max_lane_count) == 0) {
> + if (*max_lane_count > 4 || *max_lane_count == 3) {
> + dev_err(dev, "max lane count error, use default");
> + *max_lane_count = MAX_LANE_COUNT;
> + }
> + } else {
> + *max_lane_count = MAX_LANE_COUNT;
> + }
> +
> + if (device_property_read_u32(dev, "max-dpi-pixel-clock",
> + max_dpi_pixel_clock) == 0) {
> + if (*max_dpi_pixel_clock > 297000) {
> + dev_err(dev, "max pixel clock error, use default");
> + *max_dpi_pixel_clock = DPI_PIXEL_CLK_MAX;
> + }
> + } else {
> + *max_dpi_pixel_clock = DPI_PIXEL_CLK_MAX;
> + }
> +
> + DRM_DEV_DEBUG_DRIVER(dev, "using afe_setting: %u, max_lane_count: %u",
> + it6505->afe_setting, it6505->max_lane_count);
> + DRM_DEV_DEBUG_DRIVER(dev, "using max_dpi_pixel_clock: %u kHz",
> + it6505->max_dpi_pixel_clock);
> }
>
> static ssize_t receive_timing_debugfs_show(struct file *file, char __user
> *buf,
> --
> 2.25.1
[PATCH] drm/bridge: tc358767: Do not cache dsi_lanes twice
The DSI lane count can be accessed via the dsi device pointer,
make use of that. No functional change.
Signed-off-by: Marek Vasut
Cc: Andrzej Hajda
Cc: Laurent Pinchart
Cc: Lucas Stach
Cc: Maxime Ripard
Cc: Robert Foss
Cc: Sam Ravnborg
---
drivers/gpu/drm/bridge/tc358767.c | 6 ++
1 file changed, 2 insertions(+), 4 deletions(-)
diff --git a/drivers/gpu/drm/bridge/tc358767.c
b/drivers/gpu/drm/bridge/tc358767.c
index e4dd4f05f94b3..44f32bf483c51 100644
--- a/drivers/gpu/drm/bridge/tc358767.c
+++ b/drivers/gpu/drm/bridge/tc358767.c
@@ -288,7 +288,6 @@ struct tc_data {
struct drm_connectorconnector;
struct mipi_dsi_device *dsi;
- u8 dsi_lanes;
/* link settings */
struct tc_edp_link link;
@@ -1261,7 +1260,7 @@ static int tc_dsi_rx_enable(struct tc_data *tc)
regmap_write(tc->regmap, PPI_TX_RX_TA, TTA_GET | TTA_SURE);
regmap_write(tc->regmap, PPI_LPTXTIMECNT, LPX_PERIOD);
- value = ((LANEENABLE_L0EN << tc->dsi_lanes) - LANEENABLE_L0EN) |
+ value = ((LANEENABLE_L0EN << tc->dsi->lanes) - LANEENABLE_L0EN) |
LANEENABLE_CLEN;
regmap_write(tc->regmap, PPI_LANEENABLE, value);
regmap_write(tc->regmap, DSI_LANEENABLE, value);
@@ -1909,8 +1908,7 @@ static int tc_mipi_dsi_host_attach(struct tc_data *tc)
tc->dsi = dsi;
- tc->dsi_lanes = dsi_lanes;
- dsi->lanes = tc->dsi_lanes;
+ dsi->lanes = dsi_lanes;
dsi->format = MIPI_DSI_FMT_RGB888;
dsi->mode_flags = MIPI_DSI_MODE_VIDEO | MIPI_DSI_MODE_VIDEO_SYNC_PULSE;
--
2.35.1
[PATCH] drm/msm/gem: Fix error return on fence id alloc fail
From: Rob Clark
This was a typo, we didn't actually want to return zero.
Fixes: a61acbbe9cf8 ("drm/msm: Track "seqno" fences by idr")
Signed-off-by: Rob Clark
---
drivers/gpu/drm/msm/msm_gem_submit.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/drivers/gpu/drm/msm/msm_gem_submit.c
b/drivers/gpu/drm/msm/msm_gem_submit.c
index 3c3a0cfade36..c9e4aeb14f4a 100644
--- a/drivers/gpu/drm/msm/msm_gem_submit.c
+++ b/drivers/gpu/drm/msm/msm_gem_submit.c
@@ -928,7 +928,7 @@ int msm_ioctl_gem_submit(struct drm_device *dev, void *data,
INT_MAX, GFP_KERNEL);
}
if (submit->fence_id < 0) {
- ret = submit->fence_id = 0;
+ ret = submit->fence_id;
submit->fence_id = 0;
}
--
2.36.1
Re: [git pull] drm fixes for 5.19-rc4
The pull request you sent on Fri, 24 Jun 2022 15:55:38 +1000: > git://anongit.freedesktop.org/drm/drm tags/drm-fixes-2022-06-24 has been merged into torvalds/linux.git: https://git.kernel.org/torvalds/c/38bc4ac431684498126f9baa3a530e5a132f0173 Thank you! -- Deet-doot-dot, I am a bot. https://korg.docs.kernel.org/prtracker.html
Re: [PATCH v11 20/24] arm64: dts: rockchip: enable vop2 and hdmi tx on rock-3a
> Wiadomość napisana przez Peter Geis w dniu 24.06.2022, > o godz. 14:40: > >> >> Sascha, Peter >> >> I returned to trying to find why hdmi-cec is not working on rock3-a v1.31 hw. >> >> I'm on vop2 v11 on 5.18 mainline. >> >> Current findings: >> >> (1) the same sw. stack/binaries works on rock-3b (where cec uses hdmitx_m0 >> instead of hdmitx_m1 I/O line); >> >> (2) gpio-cec however works no problem on rock3-a; >> >> (3) monitoring cec messages with v4-utils 'cec-follower -s' shows exact the >> same events in non-working rock3-a and working rock3-b >> (tested by issue configure cec by 'cec-ctl -d /dev/cec0 --phys-addr=1.0.0.0 >> --playback' command) > > --phys-addr isn't a valid command for this controller. The device > designation is only required if you have more than one cec device. > >> >> --> i'm interpreting this as confirmation that low level phy layer receives >> ok cec data from connected device on non-working rock3-a; >> >> (4) debug sysfs for cec shows "has CEC follower (in passthrough mode)" for >> working rock-3b and there is NO "has CEC follower" debug message in failing >> rock3-a. > > This makes me suspect you are in fact not using the same software > stack, or are not running the same commands. It was the same SD card - with only DT declaration changed in boot.scr Such SD card has cec perfectly working in rock3b > Running `cec-follower -v -m -T` on a rk3566 device with working cec > using 5.19-rc1, I see no mention of cec-follower in the debugfs cec0 > status entry. > >> >> For me It looks like low-level hdmi-cec works ok on failing rock3-a - but >> upper layers (in hdmi vop2?) are not registering (or failing to register) >> cec-follower filehandle. This happens just when hdmitx I/O in DT is changed >> from hdmitx_m0 to hdmutx_m1. A bit strange - but all my tests consistently >> confirming this observation > > There is nothing wrong with vop2 as it is not involved at all in this. > The rockchip hdmi driver (which is not specific to vop2) does nothing > more than call the cec registration method in the dw hdmi bridge > driver, which then calls the kernel cec registration functions. > Changing pinmux changes nothing in how this functions. > >> >> I'm too weak in kernel cec internals - so maybe you have any pointers how to >> further debug/investigate this issue? > > As we discussed in the pine64 room, this is still very likely a > hardware issue with this board. A configuration issue with your u-boot > or tf-a is also a possibility, but is less likely. > > You showed with your logic analyzer with nothing plugged in and cec > not muxed to m1, data was present on m1. Issue of presence of data on m1 with nothing plugged was mistake from my side: wrong board pin used to connect logic analyser GND. After correctly connecting GND - all is ok (no any unexpected data; pulses appears only after cec commands; pulses timings are ok so cec protocol analyser shows reasonable data; no cec timing errors reported by protocol analyser). > I requested you investigate > the following, to which you haven't replied: > Have you tried forcing m0 to be assigned to a device other than hdmi-cec? I'm a bit lost here: v1.31 hw uses m1 and m0 is unused. Is you idea to verify that m0 is not used by hdmi-cec - even when m1 is declared for hdmi-cec in DT? May you pls hint me with any example of DT snippet for Your m0 assignment idea? > Have you checked if m1 is shorted to another pin? Yes. Looks ok for me. (as radxa debian has working ok hdmi-cec i think hw is ok) > > In regards to your data frames for 4.19 vs 5.18, image-view-on is not > a valid command if the topology doesn't detect a device on the bus. > I recommend running the same test, except run `cec-ctl -S --playback` > and post the results for both. Pls find results for command `cec-ctl -S --playback`: 1. radxa ubuntu 4.19 bsp: logic analyser cec proto decoded + timings: https://pastebin.com/hHPmKv4i FYI logic analyser output (first 350msec): https://paste.pics/63cb4dc7f9b51d8825d377b45bf71ae4 2. mainline 5.18.6: logic analyser cec proto decoded + timings: https://pastebin.com/YYDUY4x1 FYI logic analyser output (first 350msec): https://paste.pics/0d894b8ceba164dc6d743f8044c7e01e
Re: [PATCH] drm/ingenic: Use resource_size function on resource object
Hi,
Le ven., juin 24 2022 at 09:31:59 +0800, Jiapeng Chong
a écrit :
This was found by coccicheck:
./drivers/gpu/drm/ingenic/ingenic-drm-drv.c:1149:35-38: WARNING:
Suspicious code. resource_size is maybe missing with res.
Signed-off-by: Jiapeng Chong
---
drivers/gpu/drm/ingenic/ingenic-drm-drv.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/drivers/gpu/drm/ingenic/ingenic-drm-drv.c
b/drivers/gpu/drm/ingenic/ingenic-drm-drv.c
index 2c559885347a..5514b163999f 100644
--- a/drivers/gpu/drm/ingenic/ingenic-drm-drv.c
+++ b/drivers/gpu/drm/ingenic/ingenic-drm-drv.c
@@ -1146,7 +1146,7 @@ static int ingenic_drm_bind(struct device *dev,
bool has_components)
}
regmap_config = ingenic_drm_regmap_config;
- regmap_config.max_register = res->end - res->start;
+ regmap_config.max_register = resource_size(res);
These two are not equivalent. resource_size() is (res->end - res->start
+ 1).
If the memory resource has a size of 0x10 bytes, then using
resource_size() will set .max_register == 0x10, which is invalid, as it
is already outside the memory resource.
Cheers,
-Paul
priv->map = devm_regmap_init_mmio(dev, base,
®map_config);
if (IS_ERR(priv->map)) {
--
2.20.1.7.g153144c
Re: [PATCH v2 04/68] drm/connector: Reorder headers
On Wed, Jun 22, 2022 at 04:31:05PM +0200, Maxime Ripard wrote: > Unlike most of the other files in DRM, and Linux in general, the headers in > drm_connector.c aren't sorted alphabetically. Let's fix that. > > Signed-off-by: Maxime Ripard Acked-by: Sam Ravnborg > --- > drivers/gpu/drm/drm_connector.c | 6 +++--- > 1 file changed, 3 insertions(+), 3 deletions(-) > > diff --git a/drivers/gpu/drm/drm_connector.c b/drivers/gpu/drm/drm_connector.c > index 1c48d162c77e..353d83ae09d3 100644 > --- a/drivers/gpu/drm/drm_connector.c > +++ b/drivers/gpu/drm/drm_connector.c > @@ -22,14 +22,14 @@ > > #include > #include > +#include > #include > #include > -#include > -#include > -#include > #include > +#include > #include > #include > +#include > > #include > > -- > 2.36.1
Re: [RFT][PATCH v1 5/6] vfio/ccw: Add kmap_local_page() for memcpy
On Fri, Jun 24, 2022 at 10:56:15AM -0300, Jason Gunthorpe wrote: > > How about the updated commit log below? Thanks. > > > > The pinned PFN list returned from vfio_pin_pages() is converted using > > page_to_pfn(), so direct access via memcpy() will crash on S390 if the > > PFN is an IO PFN, as we have to use the memcpy_to/fromio(), which uses > > the special s390 IO access instructions. > > > > As a standard practice for security purpose, add kmap_local_page() to > > block any IO memory from ever getting into this call path. > > The kmap_local_page is not about the IO memory, the switch to struct > page is what is protecting against IO memory. > > Use kmap_local_page() is just the correct way to convert a struct page > into a CPU address to use with memcpy and it is a NOP on S390 because > it doesn't use highmem/etc. I thought the whole purpose of switching to "struct page *" was to use kmap_local_page() for the memcpy call, and the combination of these two does the protection. Do you mind explaining how the switching part does the protection? Thanks!
Re: [PATCH v2 05/68] drm/connector: Mention the cleanup after drm_connector_init
On Wed, Jun 22, 2022 at 04:31:06PM +0200, Maxime Ripard wrote: > Unlike encoders and CRTCs, the drm_connector_init() and > drm_connector_init_with_ddc() don't mention how the cleanup is supposed to > be done. Let's add it. > > Signed-off-by: Maxime Ripard Looks sensible, Acked-by: Sam Ravnborg > --- > drivers/gpu/drm/drm_connector.c | 8 > 1 file changed, 8 insertions(+) > > diff --git a/drivers/gpu/drm/drm_connector.c b/drivers/gpu/drm/drm_connector.c > index 353d83ae09d3..f0c4665caf38 100644 > --- a/drivers/gpu/drm/drm_connector.c > +++ b/drivers/gpu/drm/drm_connector.c > @@ -222,6 +222,10 @@ void drm_connector_free_work_fn(struct work_struct *work) > * Initialises a preallocated connector. Connectors should be > * subclassed as part of driver connector objects. > * > + * At driver unload time the driver's &drm_connector_funcs.destroy hook > + * should call drm_connector_cleanup() and free the connector structure. > + * The connector structure should not be allocated with devm_kzalloc(). > + * > * Returns: > * Zero on success, error code on failure. > */ > @@ -345,6 +349,10 @@ EXPORT_SYMBOL(drm_connector_init); > * Initialises a preallocated connector. Connectors should be > * subclassed as part of driver connector objects. > * > + * At driver unload time the driver's &drm_connector_funcs.destroy hook > + * should call drm_connector_cleanup() and free the connector structure. > + * The connector structure should not be allocated with devm_kzalloc(). > + * > * Ensures that the ddc field of the connector is correctly set. > * > * Returns: > -- > 2.36.1
Re: [PATCH v2 06/68] drm/connector: Clarify when drm_connector_unregister is needed
On Wed, Jun 22, 2022 at 04:31:07PM +0200, Maxime Ripard wrote: > The current documentation for drm_connector_unregister() mentions that > it's needed for connectors that have been registered through > drm_dev_register(). > > However, this was a typo and was meant to be drm_connector_register(), > which only applies to connectors registered after drm_dev_register() has > been called. > > In addition, it was also mentioning that connectors are unregistered > automatically when drm_dev_unregister() is called. This part is a bit > misleading, since it might make it appear that > drm_connector_unregister() applies either to all connectors, or none of > them. > > After discussing it with Daniel, it appears that we always need to call > drm_connector_unregister() on connectors that have been registered with > drm_connector_register(), but only those. > > drm_connector_init() already mentions that it only needs > drm_connector_cleanup(), so let's clarify the drm_connector_register() > and drm_connector_unregister() documentation to point at each other, and > remove the misleading part about drm_dev_unregister(). > > Signed-off-by: Maxime Ripard Acked-by: Sam Ravnborg
Re: [PATCH v2 07/68] drm/connector: Introduce drmm_connector_init
On Wed, Jun 22, 2022 at 04:31:08PM +0200, Maxime Ripard wrote: > Unlike other DRM entities, there's no helper to create a DRM-managed > initialisation of a connector. > > Let's create an helper to initialise a connector that would be passed as an > argument, and handle the cleanup through a DRM-managed action. > > Acked-by: Thomas Zimmermann > Signed-off-by: Maxime Ripard Acked-by: Sam Ravnborg
Re: [PATCH v2 08/68] drm/connector: Introduce drmm_connector_init_with_ddc
On Wed, Jun 22, 2022 at 04:31:09PM +0200, Maxime Ripard wrote: > Let's create a DRM-managed variant of drm_connector_init_with_ddc that will > take care of an action of the connector cleanup. > > Signed-off-by: Maxime Ripard Acked-by: Sam Ravnborg
Re: [PATCH v2 09/68] drm/bridge: panel: Introduce drmm_panel_bridge_add
On Wed, Jun 22, 2022 at 04:31:10PM +0200, Maxime Ripard wrote: > Unlike what can be found for other entities, there's no DRM-managed > function to create a panel_bridge instance from a panel. > > Let's introduce one. > > Signed-off-by: Maxime Ripard Acked-by: Sam Ravnborg
Re: [RFT][PATCH v1 5/6] vfio/ccw: Add kmap_local_page() for memcpy
On Fri, Jun 24, 2022 at 12:22:36PM -0700, Nicolin Chen wrote: > On Fri, Jun 24, 2022 at 10:56:15AM -0300, Jason Gunthorpe wrote: > > > > How about the updated commit log below? Thanks. > > > > > > The pinned PFN list returned from vfio_pin_pages() is converted using > > > page_to_pfn(), so direct access via memcpy() will crash on S390 if the > > > PFN is an IO PFN, as we have to use the memcpy_to/fromio(), which uses > > > the special s390 IO access instructions. > > > > > > As a standard practice for security purpose, add kmap_local_page() to > > > block any IO memory from ever getting into this call path. > > > > The kmap_local_page is not about the IO memory, the switch to struct > > page is what is protecting against IO memory. > > > > Use kmap_local_page() is just the correct way to convert a struct page > > into a CPU address to use with memcpy and it is a NOP on S390 because > > it doesn't use highmem/etc. > > I thought the whole purpose of switching to "struct page *" was to use > kmap_local_page() for the memcpy call, and the combination of these two > does the protection. Do you mind explaining how the switching part does > the protection? A 'struct page' (ignoring ZONE_DEVICE) cannot represent IO memory inherently because a 'struct page' is always a CPU coherent thing. So, when VFIO returns only struct pages it also is promising that the memory is not IO. The kmap_local_page() arose because the code doing memcpy had to be updated to go from a struct page to a void * for use with memcpy and the kmap_local_page() is the correct API to use for that. The existing code which casts a pfn to a void * is improper. Jason
Re: [PATCH v2 10/68] drm/bridge: panel: Introduce drmm_of_get_bridge
On Wed, Jun 22, 2022 at 04:31:11PM +0200, Maxime Ripard wrote: > Unlike what can be found for other DRM entities, we don't have a > DRM-managed function equivalent to devm_drm_of_get_bridge(). > > Let's create it. > > Signed-off-by: Maxime Ripard > --- > drivers/gpu/drm/bridge/panel.c | 35 ++ > include/drm/drm_bridge.h | 2 ++ > 2 files changed, 37 insertions(+) > > diff --git a/drivers/gpu/drm/bridge/panel.c b/drivers/gpu/drm/bridge/panel.c > index 07d720aa38c6..0bf824ca1f25 100644 > --- a/drivers/gpu/drm/bridge/panel.c > +++ b/drivers/gpu/drm/bridge/panel.c > @@ -425,4 +425,39 @@ struct drm_bridge *devm_drm_of_get_bridge(struct device > *dev, > return bridge; > } > EXPORT_SYMBOL(devm_drm_of_get_bridge); > + > +/** > + * drmm_of_get_bridge - Return next bridge in the chain > + * @dev: device to tie the bridge lifetime to > + * @np: device tree node containing encoder output ports > + * @port: port in the device tree node > + * @endpoint: endpoint in the device tree node > + * > + * Given a DT node's port and endpoint number, finds the connected node > + * and returns the associated bridge if any, or creates and returns a > + * drm panel bridge instance if a panel is connected. > + * > + * Returns a pointer to the bridge if successful, or an error pointer > + * otherwise. Maybe extend this text to: Returns a drmm managed pointer to the bridge if successful, or an error pointer otherwise. To tell the reader that there is no need for any cleanup. With this or something similar added: Acked-by: Sam Ravnborg Sam
Re: [PATCH v6 1/3] drm/doc/rfc: VM_BIND feature design document
On Fri, Jun 24, 2022 at 10:49:34AM -0700, Niranjana Vishwanathapura wrote: > VM_BIND design document with description of intended use cases. > > v2: Reduce the scope to simple Mesa use case. > v3: Expand documentation on dma-resv usage, TLB flushing and > execbuf3. > v4: Remove vm_bind tlb flush request support. > v5: Update TLB flushing documentation. > > Signed-off-by: Niranjana Vishwanathapura > --- > Documentation/gpu/rfc/i915_vm_bind.rst | 246 + > Documentation/gpu/rfc/index.rst| 4 + > 2 files changed, 250 insertions(+) > create mode 100644 Documentation/gpu/rfc/i915_vm_bind.rst > > diff --git a/Documentation/gpu/rfc/i915_vm_bind.rst > b/Documentation/gpu/rfc/i915_vm_bind.rst > new file mode 100644 > index ..534adf0c6c7a > --- /dev/null > +++ b/Documentation/gpu/rfc/i915_vm_bind.rst > @@ -0,0 +1,246 @@ > +== > +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 (execbuf calls) > +issued by the UMD, without user having to provide a list of all required > +mappings during each submission (as required by older execbuf mode). > + > +The VM_BIND/UNBIND calls allow UMDs to request a timeline fence for signaling > +the completion of bind/unbind operation. > + > +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. > + > +The bind/unbind operation can get completed asynchronously and out of > +submission order. The out fence when specified will be signaled upon > +completion of bind/unbind operation. > + > +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. > +* Support for userptr gem objects (no special uapi is required for this). > + > +TLB flush consideration > + > +The i915 driver flushes the TLB for each submission and when an object's > +pages are released. The VM_BIND/UNBIND operation will not do any additional > +TLB flush. Any VM_BIND mapping added will be in the working set for > subsequent > +submissions on that VM and will not be in the working set for currently > running > +batches (which would require additional TLB flushes, which is not supported). > + > +Execbuf ioctl in VM_BIND mode > +--- > +A VM in VM_BIND mode will not support older execbuf mode of binding. > +The execbuf ioctl handling in VM_BIND mode differs significantly from the > +older execbuf2 ioctl (See struct drm_i915_gem_execbuffer2). > +Hence, a new execbuf3 ioctl has been added to support VM_BIND mode. (See > +struct drm_i915_gem_execbuffer3). The execbuf3 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/) > + > +The new execbuf3 ioctl only works in VM_BIND mode and the VM_BIND mode only > +works with execbuf3 ioctl for submission. All BOs mapped on that VM (through > +VM_BIND call) at the time of execbuf3 call are deemed required for that > +submission. > + > +The execbuf3 ioctl directly specifies the batch addresses instead of as > +object handles as in execbuf2 ioctl. The execbuf3 ioctl will also not > +support many of the older features like in/out/submit fences, fence array, > +default gem context and many more (See struct drm_i915_gem_execbuffer3). > + > +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 supporting execbuf2 ioctl, like relocations, VA > +evictions, vma lookup table, implicit sync, vma active reference tracking > etc., > +are not applicable for execbuf3 ioctl. Hence, all execbuf3 specific handling > +should be in a separate file and only functionalities common to these ioctls > +can be the shared code where possible. > + > +VM_PRIVATE objects > +--- > +By default, BOs can be mapped on multiple VMs a
Re: [PATCH] drm/pl111: drop unexpected word "the" in the comments
On Tue, Jun 21, 2022 at 09:31:07PM +0800, Jiang Jian wrote: > there is an unexpected word "the" in the comments that need to be dropped > > file: drivers/gpu/drm/pl111/pl111_display.c > line: 251 > * Note that the the ARM hardware's format reader takes 'r' from > changed to > * Note that the the ARM hardware's format reader takes 'r' from > > Signed-off-by: Jiang Jian Thanks, applied to drm-misc (drm-misc-next) Sam
Re: [PATCH] drm/panel: nt35510: Remove duplicate 'the' in two places.
On Tue, Jun 21, 2022 at 10:01:51PM +0800, Jiang Jian wrote: > file: ./drivers/gpu/drm/panel/panel-novatek-nt35510.c > line: 193,214,253 > * amplification for the the step-up circuit: > changed to > * amplification for the step-up circuit: > > Signed-off-by: Jiang Jian Thanks, applied to drm-misc (drm-misc-next) Sam
Re: [PATCH 1/1] drm/panel: panel-simple: Add dev_err_probe if backlight could not be found
On Tue, Jun 21, 2022 at 09:21:18AM +0200, Alexander Stein wrote: > If the backlight node is not enabled, this (silently) returns with > -EPROBE_DEFER. /sys/kernel/debug/devices_deferred also shows nothing > helpful: > $ cat /sys/kernel/debug/devices_deferred > display > > With this patch, there is a helpful hint: > $ cat /sys/kernel/debug/devices_deferred > display panel-simple: Could not find backlight > > Signed-off-by: Alexander Stein Thanks, applied to drm-misc (drm-misc-next) Sam
Re: [PATCH] drm/ssd130x: Use new regmap bulk write support to drop custom bus
Hi Javier,
On Sat, Jun 18, 2022 at 07:43:38PM +0200, Javier Martinez Canillas wrote:
> Data writes for the ssd130x 4-wire SPI protocol need special handling, due
> the Data/Command control (D/C) pin having to be toggled prior to the write.
>
> The regmap API only allowed drivers to provide .reg_{read,write} callbacks
> to do per register read/write, but didn't provide a way for drivers to do
> the same for bulk read/writes.
>
> For this reason, a custom regmap bus was used by the driver just to define
> a bulk write callback that implements the D/C pin toggling. But the regmap
> API has been extended to support defining bulk read/write handlers, so the
> custom regmap bus is not needed anymore and could just be dropped.
>
> Signed-off-by: Javier Martinez Canillas
Patch looks good, but obviously needs the dependencies sorted out.
Acked-by: Sam Ravnborg
Sam
Re: [PATCH v6 2/3] drm/i915: Update i915 uapi documentation
On Fri, Jun 24, 2022 at 10:49:35AM -0700, Niranjana Vishwanathapura wrote:
> Add some missing i915 upai documentation which the new
> i915 VM_BIND feature documentation will be refer to.
>
> Signed-off-by: Niranjana Vishwanathapura
> Reviewed-by: Matthew Auld
> ---
> include/uapi/drm/i915_drm.h | 205
> 1 file changed, 160 insertions(+), 45 deletions(-)
>
> diff --git a/include/uapi/drm/i915_drm.h b/include/uapi/drm/i915_drm.h
> index de49b68b4fc8..4afe95d8b98b 100644
> --- a/include/uapi/drm/i915_drm.h
> +++ b/include/uapi/drm/i915_drm.h
> @@ -751,14 +751,27 @@ typedef struct drm_i915_irq_wait {
>
> /* Must be kept compact -- no holes and well documented */
>
> -typedef struct drm_i915_getparam {
> +/**
> + * struct drm_i915_getparam - Driver parameter query structure.
> + */
> +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.
>*/
> int __user *value;
> -} drm_i915_getparam_t;
> +};
> +
> +/**
> + * typedef drm_i915_getparam_t - Driver parameter query structure.
> + * See struct drm_i915_getparam.
> + */
> +typedef struct drm_i915_getparam drm_i915_getparam_t;
>
> /* Ioctl to set kernel params:
> */
> @@ -1239,76 +1252,119 @@ struct drm_i915_gem_exec_object2 {
> __u64 rsvd2;
> };
>
> +/**
> + * struct drm_i915_gem_exec_fence - An input or output fence for the execbuf
> + * 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 execbuf ioctl.
> + *
> * 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 elements 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 DRM_I915_GEM_EXECBUFFER2
> + * ioctl.
> + */
> 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 batchbu
Re: [PATCH v2 3/3] drm/panel: sony-acx565akm: Use backlight helpers
On Thu, Jun 16, 2022 at 07:23:15PM +0200, Stephen Kitt wrote: > Instead of retrieving the backlight brightness in struct > backlight_properties manually, and then checking whether the backlight > should be on at all, use backlight_get_brightness() which does all > this and insulates this from future changes. > > Instead of manually checking the power state in struct > backlight_properties, use backlight_is_blank(). > > While we're at it, drop .fb_blank from the initialisation function; it > is deprecated, and this helps make progress towards enabling its > removal. This change makes no functional difference since > FB_BLANK_UNBLANK is the default value. > > Signed-off-by: Stephen Kitt > Cc: Thierry Reding > Cc: Sam Ravnborg > Cc: David Airlie > Cc: Daniel Vetter > Cc: dri-devel@lists.freedesktop.org Thanks, applied to drm-misc (drm-misc-next) Sam
Re: [PATCH v2 2/3] drm/panel: panel-dsi-cm: Use backlight helpers
On Thu, Jun 16, 2022 at 07:23:14PM +0200, Stephen Kitt wrote: > Instead of retrieving the backlight brightness in struct > backlight_properties manually, and then checking whether the backlight > should be on at all, use backlight_get_brightness() which does all > this and insulates this from future changes. > > Instead of setting the power state by manually updating fields in > struct backlight_properties, use backlight_enable() and > backlight_disable(). These also call backlight_update_status() so the > separate call is no longer needed. > > Signed-off-by: Stephen Kitt > Cc: Thierry Reding > Cc: Sam Ravnborg > Cc: David Airlie > Cc: Daniel Vetter > Cc: dri-devel@lists.freedesktop.org Thanks, applied to drm-misc (drm-misc-next) Sam
Re: [PATCH v2 1/3] drm/panel: Use backlight helper
On Thu, Jun 16, 2022 at 07:23:13PM +0200, Stephen Kitt wrote: > backlight_properties.fb_blank is deprecated. The states it represents > are handled by other properties; but instead of accessing those > properties directly, drivers should use the helpers provided by > backlight.h. > > Instead of retrieving the backlight brightness in struct > backlight_properties manually, and then checking whether the backlight > should be on at all, use backlight_get_brightness() which does all > this and insulates this from future changes. > > Signed-off-by: Stephen Kitt > Cc: Thierry Reding > Cc: Sam Ravnborg > Cc: David Airlie > Cc: Daniel Vetter > Cc: dri-devel@lists.freedesktop.org Thanks, applied to drm-misc (drm-misc-next) Sam
Re: [PATCH v2] drm: shmobile: Use backlight helper
On Thu, Jun 16, 2022 at 07:08:21PM +0200, Stephen Kitt wrote: > This started with work on the removal of backlight_properties' > deprecated fb_blank field, much of which can be taken care of by using > helper functions provided by backlight.h instead of directly accessing > fields in backlight_properties. This patch series doesn't involve > fb_blank, but it still seems useful to use helper functions where > appropriate. > > Instead of retrieving the backlight brightness in struct > backlight_properties manually, and then checking whether the backlight > should be on at all, use backlight_get_brightness() which does all > this and insulates this from future changes. > > Signed-off-by: Stephen Kitt > Cc: Laurent Pinchart > Cc: Kieran Bingham > Cc: David Airlie > Cc: Daniel Vetter > Cc: dri-devel@lists.freedesktop.org Thanks, added to drm-misc (drm-misc-next) Sam
Re: [PATCH v5 1/9] dt-bindings: usb: Add Type-C switch binding
Quoting Prashant Malani (2022-06-23 19:48:04) > On Thu, Jun 23, 2022 at 7:13 PM Stephen Boyd wrote: > > > > Quoting Prashant Malani (2022-06-23 17:35:38) > > > On Thu, Jun 23, 2022 at 4:14 PM Stephen Boyd wrote: > > > > > > > > I'm not aware of any documentation for the dos and don'ts here. Are > > > > there any examples in the bindings directory that split up a device into > > > > subnodes that isn't in bindings/mfd? > > > > > > usb-c-connector [3] and its users is an example. > > > > What are the subnodes? The graph ports? That is not what I meant. > > cros-ec-typec [4] uses subnodes of usb-c-connector. Chrome OS DTs > use the ports from the included usb-c-connector to switching hardware. Ok, got it. usb-c-connector nodes are children of the typec controller (in this case cros-ec-typec) because otherwise we would need to make a phandle link from the usb-c-connector node(s) under the root node / to the typec controller. The phandle link may need to be done in both directions, so it makes more sense to put the usb-c-connector nodes underneath the typec controller to express the direct relationship between the typec controller and the usb-c-connectors. Furthermore, the usb-c-connector is not integrated as part of the EC in the same package. There is a discrete part placed on the board that corresponds to the usb-c-connector and that is separate from the EC. The connectors are in essence only controllable through the EC because that's the typec controller. It's similar to how we place i2c devices as child nodes of the i2c controller. > > > I meant splitting up a device functionality, like type-c and display > > bridge, into subnodes. Composition of devices through DT bindings isn't > > how it's done. Instead, we dump all the different functionality into the > > same node. For example, look at the number of bindings that have both > > #clock-cells and #reset-cells, when those are distinct frameworks in the > > kernel and also different properties. We don't make subnodes to contain > > the different functionality of a device. > > > > And in this case I still don't see the point to making a subnode. > > I've already provided my best effort at explaining the rationale. > > > The > > API can simply setup a type-c switch based on a graph binding for the > > toplevel node, e.g. the drm-bridge, and the driver can tell the API > > which port+endpoint to use to search the graph for the usb-c-connector > > to associate with the switch. > > OK, drm-bridge uses that approach. This is another approach. I didn't fully > understand why we *have* to follow what drm-bridge is doing. > > > We don't need to connect the graph within > > the drm-bridge node to the graph within the typec-switch node to do > > that. That's an internal detail of the drm-bridge that we don't expose > > to DT, because the driver knows the detail. > > I still don't understand why we can't do that. These devices have actual > hardware blocks that represent the Type-C switch functionality. > We don't break up device functionality for an IC into different subnodes with different compatibles. Similarly, we don't describe internal connection details of device nodes. The device driver that binds to the compatible should know the details of the internal block diagram of the part. The DT binding should describe the external connections of the part and have properties that inform the driver about how the part was integrated into the system (e.g. mode-switch). The unwritten DT mantra is "less is more". We could definitely make many subnodes and add properties for everything inside an IC so that the DT describes the complete block diagram of the part, but at that point the driver is a shell of its former self. The driver will spend time parsing properties to learn details that are entirely unchanging for the lifetime of the device (e.g. that the device has typec switch capabilities); things that should be hard-coded in the driver. Of course, if the device is integrated into the system and doesn't need to perform typec switching, then we want a property to tell the driver that this device is integrated in a way that the typec switch is not needed/used. Basically the driver should key that functionality off of the presence of the 'mode-switch' or 'orientation-switch' property instead of off the presence of a typec-switch subnode. > > > > > > > > > > > How would I even know which two differential pairs correspond to port0 > > > > or port1 in this binding in the ITE case? > > > > > > Why do we need to know that? It doesn't affect this or the other > > > driver or hardware's > > > functioning in a perceivable way. > > > > If the device registers allow control of the DP lane to physical pin > > mapping, so that DP lane0 and DP lane1 can be swapped logically, then > > we'll want to know which DP lanes we need to swap by writing some lane > > remapping register in the device. Sometimes for routing purposes devices > > support this lane remapping feature so the P
[Bug 216119] 087451f372bf76d breaks hibernation on amdgpu Radeon R9 390
https://bugzilla.kernel.org/show_bug.cgi?id=216119 --- Comment #20 from Harald Judt (h.j...@gmx.at) --- One thing that I have noticed: Since these changes, the kernel seems to switch to text mode when hibernating. Before that I think it remained (frozen) on the X screen. Here are the results: - 1+4: screen black, display suspend led, keyboard comes online again but no ssh. can sysreq to reboot. - 2+4: screen black first, comes back after some time, restores screen with hibernation snapshotting progress visible (not those of resume), continues to resume (resuming progress visible), but then screen goes black again in dpms on, ssh available. was able to compile kernel and reboot via ssh. - 3+4: does not compile. unknown fb struct or so. because of that, i tried 2+3+4 since 2+4 compiled fine and worked fine mostly. - 2+3+4 works as good as 1+2+3+4. seems patch 1 is not necessary. -- You may reply to this email to add a comment. You are receiving this mail because: You are watching the assignee of the bug.
Re: [PATCH 2/2] drm/panel: simple: add AM-800600P5TMQW-TB8H
Hi Bastian,
On Fri, Jun 10, 2022 at 01:15:11PM +0200, Bastian Krause wrote:
> Add support for the Ampire AM-800600P5TMQW-TB8H 800x600 panel. Data
> sheet is currently not publicly available, unfortunately.
>
> Signed-off-by: Bastian Krause
Applied to drm-misc (drm-misc-next).
When applying I fixed up the compatible to match the binding.
You may need to fix your DT files if they used the old compatible.
The one from the binding had a dash like similar panels, so that is
the one I picked.
See below for the fix-up.
Sam
> ---
> drivers/gpu/drm/panel/panel-simple.c | 33
> 1 file changed, 33 insertions(+)
>
> diff --git a/drivers/gpu/drm/panel/panel-simple.c
> b/drivers/gpu/drm/panel/panel-simple.c
> index 4a2e580a2f7b7..3a61873dd887c 100644
> --- a/drivers/gpu/drm/panel/panel-simple.c
> +++ b/drivers/gpu/drm/panel/panel-simple.c
> @@ -778,6 +778,36 @@ static const struct panel_desc ampire_am800480r3tmqwa1h
> = {
> .bus_format = MEDIA_BUS_FMT_RGB666_1X18,
> };
>
> +static const struct display_timing ampire_am800600p5tmqw_tb8h_timing = {
> + .pixelclock = { 3450, 3960, 5040 },
> + .hactive = { 800, 800, 800 },
> + .hfront_porch = { 12, 112, 312 },
> + .hback_porch = { 87, 87, 48 },
> + .hsync_len = { 1, 1, 40 },
> + .vactive = { 600, 600, 600 },
> + .vfront_porch = { 1, 21, 61 },
> + .vback_porch = { 38, 38, 19 },
> + .vsync_len = { 1, 1, 20 },
> + .flags = DISPLAY_FLAGS_HSYNC_LOW | DISPLAY_FLAGS_VSYNC_LOW |
> + DISPLAY_FLAGS_DE_HIGH | DISPLAY_FLAGS_PIXDATA_POSEDGE |
> + DISPLAY_FLAGS_SYNC_POSEDGE,
> +};
> +
> +static const struct panel_desc ampire_am800600p5tmqwtb8h = {
> + .timings = &ire_am800600p5tmqw_tb8h_timing,
> + .num_timings = 1,
> + .bpc = 6,
> + .size = {
> + .width = 162,
> + .height = 122,
> + },
> + .bus_format = MEDIA_BUS_FMT_RGB666_1X18,
> + .bus_flags = DRM_BUS_FLAG_DE_HIGH |
> + DRM_BUS_FLAG_PIXDATA_SAMPLE_NEGEDGE |
> + DRM_BUS_FLAG_SYNC_SAMPLE_NEGEDGE,
> + .connector_type = DRM_MODE_CONNECTOR_DPI,
> +};
> +
> static const struct display_timing santek_st0700i5y_rbslw_f_timing = {
> .pixelclock = { 2640, 3330, 4680 },
> .hactive = { 800, 800, 800 },
> @@ -3754,6 +3784,9 @@ static const struct of_device_id platform_of_match[] = {
> }, {
> .compatible = "ampire,am800480r3tmqwa1h",
> .data = &ire_am800480r3tmqwa1h,
> + }, {
> + .compatible = "ampire,am800600p5tmqwtb8h",
was changed to
> + .compatible = "ampire,am800600p5tmqw-tb8h",
> + .data = &ire_am800600p5tmqwtb8h,
> }, {
> .compatible = "arm,rtsm-display",
> .data = &arm_rtsm,
> --
> 2.30.2
>
