[PATCH] drm/i915/selftests: Fix NULL vs IS_ERR checking for kernel_context
Since i915_gem_create_context() function return error pointers,
the kernel_context() function does not return null, It returns error
pointers too. Using IS_ERR() to check the return value to fix this.
Signed-off-by: Miaoqian Lin
---
drivers/gpu/drm/i915/gt/selftest_execlists.c | 41 ++--
1 file changed, 29 insertions(+), 12 deletions(-)
diff --git a/drivers/gpu/drm/i915/gt/selftest_execlists.c
b/drivers/gpu/drm/i915/gt/selftest_execlists.c
index b367ecfa42de..eacfe920afed 100644
--- a/drivers/gpu/drm/i915/gt/selftest_execlists.c
+++ b/drivers/gpu/drm/i915/gt/selftest_execlists.c
@@ -1540,13 +1540,16 @@ static int live_busywait_preempt(void *arg)
*/
ctx_hi = kernel_context(gt->i915, NULL);
- if (!ctx_hi)
- return -ENOMEM;
+ if (IS_ERR(ctx_hi))
+ return IS_ERR(ctx_hi);
+
ctx_hi->sched.priority = I915_CONTEXT_MAX_USER_PRIORITY;
ctx_lo = kernel_context(gt->i915, NULL);
- if (!ctx_lo)
+ if (IS_ERR(ctx_lo)) {
+ err = PTR_ERR(ctx_lo);
goto err_ctx_hi;
+ }
ctx_lo->sched.priority = I915_CONTEXT_MIN_USER_PRIORITY;
obj = i915_gem_object_create_internal(gt->i915, PAGE_SIZE);
@@ -1742,13 +1745,17 @@ static int live_preempt(void *arg)
goto err_spin_hi;
ctx_hi = kernel_context(gt->i915, NULL);
- if (!ctx_hi)
+ if (IS_ERR(ctx_hi)) {
+ err = PTR_ERR(ctx_hi);
goto err_spin_lo;
+ }
ctx_hi->sched.priority = I915_CONTEXT_MAX_USER_PRIORITY;
ctx_lo = kernel_context(gt->i915, NULL);
- if (!ctx_lo)
+ if (IS_ERR(ctx_lo)) {
+ err = PTR_ERR(ctx_lo);
goto err_ctx_hi;
+ }
ctx_lo->sched.priority = I915_CONTEXT_MIN_USER_PRIORITY;
for_each_engine(engine, gt, id) {
@@ -1834,12 +1841,16 @@ static int live_late_preempt(void *arg)
goto err_spin_hi;
ctx_hi = kernel_context(gt->i915, NULL);
- if (!ctx_hi)
+ if (IS_ERR(ctx_hi)) {
+ err = PTR_ERR(ctx_hi);
goto err_spin_lo;
+ }
ctx_lo = kernel_context(gt->i915, NULL);
- if (!ctx_lo)
+ if (IS_ERR(ctx_lo)) {
+ err = PTR_ERR(ctx_lo);
goto err_ctx_hi;
+ }
/* Make sure ctx_lo stays before ctx_hi until we trigger preemption. */
ctx_lo->sched.priority = 1;
@@ -1928,8 +1939,8 @@ struct preempt_client {
static int preempt_client_init(struct intel_gt *gt, struct preempt_client *c)
{
c->ctx = kernel_context(gt->i915, NULL);
- if (!c->ctx)
- return -ENOMEM;
+ if (IS_ERR(c->ctx))
+ return PTR_ERR(c->ctx);
if (igt_spinner_init(>spin, gt))
goto err_ctx;
@@ -3385,13 +3396,17 @@ static int live_preempt_timeout(void *arg)
return -ENOMEM;
ctx_hi = kernel_context(gt->i915, NULL);
- if (!ctx_hi)
+ if (IS_ERR(ctx_hi)) {
+ err = PTR_ERR(ctx_hi);
goto err_spin_lo;
+ }
ctx_hi->sched.priority = I915_CONTEXT_MAX_USER_PRIORITY;
ctx_lo = kernel_context(gt->i915, NULL);
- if (!ctx_lo)
+ if (IS_ERR(ctx_lo)) {
+ err = PTR_ERR(ctx_lo);
goto err_ctx_hi;
+ }
ctx_lo->sched.priority = I915_CONTEXT_MIN_USER_PRIORITY;
for_each_engine(engine, gt, id) {
@@ -3683,8 +3698,10 @@ static int live_preempt_smoke(void *arg)
for (n = 0; n < smoke.ncontext; n++) {
smoke.contexts[n] = kernel_context(smoke.gt->i915, NULL);
- if (!smoke.contexts[n])
+ if (IS_ERR(smoke.contexts[n])) {
+ err = PTR_ERR(smoke.contexts[n]);
goto err_ctx;
+ }
}
for (n = 0; n < ARRAY_SIZE(phase); n++) {
--
2.17.1
Re: [RFC 3/6] drm/amdgpu: Fix crash on modprobe
Am 21.12.21 um 17:03 schrieb Andrey Grodzovsky:
On 2021-12-21 2:02 a.m., Christian König wrote:
Am 20.12.21 um 20:22 schrieb Andrey Grodzovsky:
On 2021-12-20 2:17 a.m., Christian König wrote:
Am 17.12.21 um 23:27 schrieb Andrey Grodzovsky:
Restrict jobs resubmission to suspend case
only since schedulers not initialised yet on
probe.
Signed-off-by: Andrey Grodzovsky
---
drivers/gpu/drm/amd/amdgpu/amdgpu_fence.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_fence.c
b/drivers/gpu/drm/amd/amdgpu/amdgpu_fence.c
index 5527c68c51de..8ebd954e06c6 100644
--- a/drivers/gpu/drm/amd/amdgpu/amdgpu_fence.c
+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_fence.c
@@ -582,7 +582,7 @@ void amdgpu_fence_driver_hw_init(struct
amdgpu_device *adev)
if (!ring || !ring->fence_drv.initialized)
continue;
- if (!ring->no_scheduler) {
+ if (adev->in_suspend && !ring->no_scheduler) {
Uff, why is that suddenly necessary? Because of the changed order?
Christian.
Yes.
Mhm, that's quite bad design then.
If you look at the original patch for this
https://www.spinics.net/lists/amd-gfx/msg67560.html you will
see that that restarting scheduler here is only relevant for
suspend/resume case because there was
a race to fix. There is no point in this code on driver init because
nothing was submitted to scheduler yet
and so it seems to me ok to add condition that this code run only
in_suspend case.
Yeah, but having extra logic like this means that we have some design
issue in the IP block handling.
We need to clean that and some other odd approaches up at some point,
but probably not now.
Christian.
How about we keep the order as is and allow specifying the reset work
queue with drm_sched_start() ?
As i mentioned above, the fact we even have drm_sched_start there is
just part of a solution to resolve a race
during suspend/resume. It is not for device initialization and indeed,
other client drivers of gpu shcheduler never call
drm_sched_start on device init. We must guarantee that reset work
queue already initialized before any job submission to scheduler
and because of that IMHO the right place for this is drm_sched_init.
Andrey
Christian.
Andrey
drm_sched_resubmit_jobs(>sched);
drm_sched_start(>sched, true);
}
Re: linux-next: manual merge of the drm tree with the drm-misc-fixes tree
Hi Stephen,
Am 22.12.21 um 04:50 schrieb Stephen Rothwell:
Hi all,
Today's linux-next merge of the drm tree got a conflict in:
drivers/gpu/drm/nouveau/nouveau_fence.c
between commit:
67f74302f45d ("drm/nouveau: wait for the exclusive fence after the shared ones
v2")
from the drm-misc-fixes tree and commit:
40298cb45071 ("drm/nouveau: use the new iterator in nouveau_fence_sync")
from the drm tree.
I fixed it up (I just used the latter version) and can carry the fix as
necessary. This is now fixed as far as linux-next is concerned, but any
non trivial conflicts should be mentioned to your upstream maintainer
when your tree is submitted for merging. You may also want to consider
cooperating with the maintainer of the conflicting tree to minimise any
particularly complex conflicts.
That will cause a performance regression in linux-next for nouveau users.
The real fix is already queued up for merging into drm-next. So no
immediately action item here.
Just keep it in the back of your mind in case any nouveau users starts
to complain about the performance in linux-next.
Thanks,
Christian.
[PATCH] drm/virtio: Fix NULL vs IS_ERR checking in virtio_gpu_object_shmem_init
Since drm_prime_pages_to_sg() function return error pointers.
The drm_gem_shmem_get_sg_table() function returns error pointers too.
Using IS_ERR() to check the return value to fix this.
Fixes: f651c8b05542("drm/virtio: factor out the sg_table from
virtio_gpu_object")
Signed-off-by: Miaoqian Lin
---
drivers/gpu/drm/virtio/virtgpu_object.c | 4 ++--
1 file changed, 2 insertions(+), 2 deletions(-)
diff --git a/drivers/gpu/drm/virtio/virtgpu_object.c
b/drivers/gpu/drm/virtio/virtgpu_object.c
index f648b0e24447..8bb80289672c 100644
--- a/drivers/gpu/drm/virtio/virtgpu_object.c
+++ b/drivers/gpu/drm/virtio/virtgpu_object.c
@@ -168,9 +168,9 @@ static int virtio_gpu_object_shmem_init(struct
virtio_gpu_device *vgdev,
* since virtio_gpu doesn't support dma-buf import from other devices.
*/
shmem->pages = drm_gem_shmem_get_sg_table(>base.base);
- if (!shmem->pages) {
+ if (IS_ERR(shmem->pages)) {
drm_gem_shmem_unpin(>base.base);
- return -EINVAL;
+ return PTR_ERR(shmem->pages);
}
if (use_dma_api) {
--
2.17.1
Re: [PATCH] drm/etnaviv: consider completed fence seqno in hang check
Am Mi., 22. Dez. 2021 um 01:17 Uhr schrieb Lucas Stach : > > Some GPU heavy test programs manage to trigger the hangcheck quite often. > If there are no other GPU users in the system and the test program > exhibits a very regular structure in the commandstreams that are being > submitted, we can end up with two distinct submits managing to trigger > the hangcheck with the FE in a very similar address range. This leads > the hangcheck to believe that the GPU is stuck, while in reality the GPU > is already busy working on a different job. To avoid those spurious > GPU resets, also remember and consider the last completed fence seqno > in the hang check. > > Reported-by: Joerg Albert > Signed-off-by: Lucas Stach Reviewed-by: Christian Gmeiner -- greets -- Christian Gmeiner, MSc https://christian-gmeiner.info/privacypolicy
[PATCH V2] drm/i915: Replace kmap() with kmap_local_page()
From: Ira Weiny
kmap() is being deprecated and these usages are all local to the thread
so there is no reason kmap_local_page() can't be used.
Replace kmap() calls with kmap_local_page().
Signed-off-by: Ira Weiny
---
NOTE: I'm sending as a follow on to the V1 patch. Please let me know if you
prefer the entire series instead.
Changes for V2:
From Christoph Helwig
Prefer the use of memcpy_*_page() where appropriate.
---
drivers/gpu/drm/i915/gem/i915_gem_shmem.c | 6 ++
drivers/gpu/drm/i915/gem/selftests/i915_gem_mman.c | 8
drivers/gpu/drm/i915/gt/intel_ggtt_fencing.c | 4 ++--
drivers/gpu/drm/i915/gt/shmem_utils.c | 7 ++-
drivers/gpu/drm/i915/i915_gem.c| 8
drivers/gpu/drm/i915/i915_gpu_error.c | 4 ++--
6 files changed, 16 insertions(+), 21 deletions(-)
diff --git a/drivers/gpu/drm/i915/gem/i915_gem_shmem.c
b/drivers/gpu/drm/i915/gem/i915_gem_shmem.c
index d77da59fae04..842e0895 100644
--- a/drivers/gpu/drm/i915/gem/i915_gem_shmem.c
+++ b/drivers/gpu/drm/i915/gem/i915_gem_shmem.c
@@ -589,7 +589,7 @@ i915_gem_object_create_shmem_from_data(struct
drm_i915_private *dev_priv,
do {
unsigned int len = min_t(typeof(size), size, PAGE_SIZE);
struct page *page;
- void *pgdata, *vaddr;
+ void *pgdata;
err = pagecache_write_begin(file, file->f_mapping,
offset, len, 0,
@@ -597,9 +597,7 @@ i915_gem_object_create_shmem_from_data(struct
drm_i915_private *dev_priv,
if (err < 0)
goto fail;
- vaddr = kmap(page);
- memcpy(vaddr, data, len);
- kunmap(page);
+ memcpy_to_page(page, 0, data, len);
err = pagecache_write_end(file, file->f_mapping,
offset, len, len,
diff --git a/drivers/gpu/drm/i915/gem/selftests/i915_gem_mman.c
b/drivers/gpu/drm/i915/gem/selftests/i915_gem_mman.c
index 6d30cdfa80f3..e59e1725e29d 100644
--- a/drivers/gpu/drm/i915/gem/selftests/i915_gem_mman.c
+++ b/drivers/gpu/drm/i915/gem/selftests/i915_gem_mman.c
@@ -144,7 +144,7 @@ static int check_partial_mapping(struct drm_i915_gem_object
*obj,
intel_gt_flush_ggtt_writes(_i915(obj->base.dev)->gt);
p = i915_gem_object_get_page(obj, offset >> PAGE_SHIFT);
- cpu = kmap(p) + offset_in_page(offset);
+ cpu = kmap_local_page(p) + offset_in_page(offset);
drm_clflush_virt_range(cpu, sizeof(*cpu));
if (*cpu != (u32)page) {
pr_err("Partial view for %lu [%u] (offset=%llu, size=%u [%llu,
row size %u], fence=%d, tiling=%d, stride=%d) misalignment, expected write to
page (%llu + %u [0x%llx]) of 0x%x, found 0x%x\n",
@@ -162,7 +162,7 @@ static int check_partial_mapping(struct drm_i915_gem_object
*obj,
}
*cpu = 0;
drm_clflush_virt_range(cpu, sizeof(*cpu));
- kunmap(p);
+ kunmap_local(cpu);
out:
__i915_vma_put(vma);
@@ -237,7 +237,7 @@ static int check_partial_mappings(struct
drm_i915_gem_object *obj,
intel_gt_flush_ggtt_writes(_i915(obj->base.dev)->gt);
p = i915_gem_object_get_page(obj, offset >> PAGE_SHIFT);
- cpu = kmap(p) + offset_in_page(offset);
+ cpu = kmap_local_page(p) + offset_in_page(offset);
drm_clflush_virt_range(cpu, sizeof(*cpu));
if (*cpu != (u32)page) {
pr_err("Partial view for %lu [%u] (offset=%llu, size=%u
[%llu, row size %u], fence=%d, tiling=%d, stride=%d) misalignment, expected
write to page (%llu + %u [0x%llx]) of 0x%x, found 0x%x\n",
@@ -255,7 +255,7 @@ static int check_partial_mappings(struct
drm_i915_gem_object *obj,
}
*cpu = 0;
drm_clflush_virt_range(cpu, sizeof(*cpu));
- kunmap(p);
+ kunmap_local(cpu);
if (err)
return err;
diff --git a/drivers/gpu/drm/i915/gt/intel_ggtt_fencing.c
b/drivers/gpu/drm/i915/gt/intel_ggtt_fencing.c
index f8948de72036..743a414f86f3 100644
--- a/drivers/gpu/drm/i915/gt/intel_ggtt_fencing.c
+++ b/drivers/gpu/drm/i915/gt/intel_ggtt_fencing.c
@@ -743,7 +743,7 @@ static void swizzle_page(struct page *page)
char *vaddr;
int i;
- vaddr = kmap(page);
+ vaddr = kmap_local_page(page);
for (i = 0; i < PAGE_SIZE; i += 128) {
memcpy(temp, [i], 64);
@@ -751,7 +751,7 @@ static void swizzle_page(struct page *page)
memcpy([i + 64], temp, 64);
}
- kunmap(page);
+ kunmap_local(vaddr);
}
/**
diff --git a/drivers/gpu/drm/i915/gt/shmem_utils.c
b/drivers/gpu/drm/i915/gt/shmem_utils.c
index 0683b27a3890..d47f262d2f07 100644
--- a/drivers/gpu/drm/i915/gt/shmem_utils.c
+++
[RFC PATH 3/3] drm: replace allow_fb_modifiers with fb_modifiers_not_supported
Since almost drivers support fb modifiers, allow_fb_modifiers is
replaced with fb_modifiers_not_supported and removed.
Signed-off-by: Tomohito Esaki
---
drivers/gpu/drm/drm_framebuffer.c| 6 +++---
drivers/gpu/drm/drm_ioctl.c | 2 +-
drivers/gpu/drm/drm_plane.c | 9 -
drivers/gpu/drm/selftests/test-drm_framebuffer.c | 1 -
include/drm/drm_mode_config.h| 16
5 files changed, 4 insertions(+), 30 deletions(-)
diff --git a/drivers/gpu/drm/drm_framebuffer.c
b/drivers/gpu/drm/drm_framebuffer.c
index 07f5abc875e9..4562a8b86579 100644
--- a/drivers/gpu/drm/drm_framebuffer.c
+++ b/drivers/gpu/drm/drm_framebuffer.c
@@ -309,7 +309,7 @@ drm_internal_framebuffer_create(struct drm_device *dev,
}
if (r->flags & DRM_MODE_FB_MODIFIERS &&
- !dev->mode_config.allow_fb_modifiers) {
+ dev->mode_config.fb_modifiers_not_supported) {
DRM_DEBUG_KMS("driver does not support fb modifiers\n");
return ERR_PTR(-EINVAL);
}
@@ -594,7 +594,7 @@ int drm_mode_getfb2_ioctl(struct drm_device *dev,
r->pixel_format = fb->format->format;
r->flags = 0;
- if (dev->mode_config.allow_fb_modifiers)
+ if (!dev->mode_config.fb_modifiers_not_supported)
r->flags |= DRM_MODE_FB_MODIFIERS;
for (i = 0; i < ARRAY_SIZE(r->handles); i++) {
@@ -607,7 +607,7 @@ int drm_mode_getfb2_ioctl(struct drm_device *dev,
for (i = 0; i < fb->format->num_planes; i++) {
r->pitches[i] = fb->pitches[i];
r->offsets[i] = fb->offsets[i];
- if (dev->mode_config.allow_fb_modifiers)
+ if (!dev->mode_config.fb_modifiers_not_supported)
r->modifier[i] = fb->modifier;
}
diff --git a/drivers/gpu/drm/drm_ioctl.c b/drivers/gpu/drm/drm_ioctl.c
index 8b8744dcf691..51fcf1298023 100644
--- a/drivers/gpu/drm/drm_ioctl.c
+++ b/drivers/gpu/drm/drm_ioctl.c
@@ -297,7 +297,7 @@ static int drm_getcap(struct drm_device *dev, void *data,
struct drm_file *file_
req->value = 64;
break;
case DRM_CAP_ADDFB2_MODIFIERS:
- req->value = dev->mode_config.allow_fb_modifiers;
+ req->value = !dev->mode_config.fb_modifiers_not_supported;
break;
case DRM_CAP_CRTC_IN_VBLANK_EVENT:
req->value = 1;
diff --git a/drivers/gpu/drm/drm_plane.c b/drivers/gpu/drm/drm_plane.c
index 75308ee240c0..5b546d80d248 100644
--- a/drivers/gpu/drm/drm_plane.c
+++ b/drivers/gpu/drm/drm_plane.c
@@ -302,15 +302,6 @@ static int __drm_universal_plane_init(struct drm_device
*dev,
}
}
- /* autoset the cap and check for consistency across all planes */
- if (format_modifier_count) {
- drm_WARN_ON(dev, !config->allow_fb_modifiers &&
- !list_empty(>plane_list));
- config->allow_fb_modifiers = true;
- } else {
- drm_WARN_ON(dev, config->allow_fb_modifiers);
- }
-
plane->modifier_count = format_modifier_count;
plane->modifiers = kmalloc_array(format_modifier_count,
sizeof(format_modifiers[0]),
diff --git a/drivers/gpu/drm/selftests/test-drm_framebuffer.c
b/drivers/gpu/drm/selftests/test-drm_framebuffer.c
index 61b44d3a6a61..f6d66285c5fc 100644
--- a/drivers/gpu/drm/selftests/test-drm_framebuffer.c
+++ b/drivers/gpu/drm/selftests/test-drm_framebuffer.c
@@ -323,7 +323,6 @@ static struct drm_device mock_drm_device = {
.max_width = MAX_WIDTH,
.min_height = MIN_HEIGHT,
.max_height = MAX_HEIGHT,
- .allow_fb_modifiers = true,
.funcs = _config_funcs,
},
};
diff --git a/include/drm/drm_mode_config.h b/include/drm/drm_mode_config.h
index c56f298c55bd..6fd13d6510f1 100644
--- a/include/drm/drm_mode_config.h
+++ b/include/drm/drm_mode_config.h
@@ -904,22 +904,6 @@ struct drm_mode_config {
*/
bool async_page_flip;
- /**
-* @allow_fb_modifiers:
-*
-* Whether the driver supports fb modifiers in the ADDFB2.1 ioctl call.
-* Note that drivers should not set this directly, it is automatically
-* set in drm_universal_plane_init().
-*
-* IMPORTANT:
-*
-* If this is set the driver must fill out the full implicit modifier
-* information in their _mode_config_funcs.fb_create hook for legacy
-* userspace which does not set modifiers. Otherwise the GETFB2 ioctl is
-* broken for modifier aware userspace.
-*/
- bool allow_fb_modifiers;
-
/**
* @fb_modifiers_not_supported:
*
--
2.17.1
[RFC PATH 2/3] drm: set fb_modifiers_not_supported flag in legacy drivers
Set fb_modifiers_not_supported flag in legacy drivers whose planes
support non-linear layouts but does not support modifiers, and replace
allow_fb_modifiers with fb_modifiers_not_supported.
Signed-off-by: Tomohito Esaki
---
drivers/gpu/drm/amd/amdgpu/amdgpu_display.c | 6 +++---
drivers/gpu/drm/amd/amdgpu/dce_v10_0.c | 2 ++
drivers/gpu/drm/amd/amdgpu/dce_v11_0.c | 2 ++
drivers/gpu/drm/amd/amdgpu/dce_v6_0.c | 1 +
drivers/gpu/drm/amd/amdgpu/dce_v8_0.c | 2 ++
drivers/gpu/drm/nouveau/nouveau_display.c | 6 --
drivers/gpu/drm/radeon/radeon_display.c | 2 ++
7 files changed, 16 insertions(+), 5 deletions(-)
diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_display.c
b/drivers/gpu/drm/amd/amdgpu/amdgpu_display.c
index dc50c05f23fc..cbaea9c6cfda 100644
--- a/drivers/gpu/drm/amd/amdgpu/amdgpu_display.c
+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_display.c
@@ -958,7 +958,7 @@ static int amdgpu_display_verify_sizes(struct
amdgpu_framebuffer *rfb)
int ret;
unsigned int i, block_width, block_height, block_size_log2;
- if (!rfb->base.dev->mode_config.allow_fb_modifiers)
+ if (rfb->base.dev->mode_config.fb_modifiers_not_supported)
return 0;
for (i = 0; i < format_info->num_planes; ++i) {
@@ -1145,7 +1145,7 @@ int amdgpu_display_framebuffer_init(struct drm_device
*dev,
if (ret)
return ret;
- if (!dev->mode_config.allow_fb_modifiers) {
+ if (dev->mode_config.fb_modifiers_not_supported) {
drm_WARN_ONCE(dev, adev->family >= AMDGPU_FAMILY_AI,
"GFX9+ requires FB check based on format
modifier\n");
ret = check_tiling_flags_gfx6(rfb);
@@ -1153,7 +1153,7 @@ int amdgpu_display_framebuffer_init(struct drm_device
*dev,
return ret;
}
- if (dev->mode_config.allow_fb_modifiers &&
+ if (!dev->mode_config.fb_modifiers_not_supported &&
!(rfb->base.flags & DRM_MODE_FB_MODIFIERS)) {
ret = convert_tiling_flags_to_modifier(rfb);
if (ret) {
diff --git a/drivers/gpu/drm/amd/amdgpu/dce_v10_0.c
b/drivers/gpu/drm/amd/amdgpu/dce_v10_0.c
index d1570a462a51..fb61c0814115 100644
--- a/drivers/gpu/drm/amd/amdgpu/dce_v10_0.c
+++ b/drivers/gpu/drm/amd/amdgpu/dce_v10_0.c
@@ -2798,6 +2798,8 @@ static int dce_v10_0_sw_init(void *handle)
adev_to_drm(adev)->mode_config.preferred_depth = 24;
adev_to_drm(adev)->mode_config.prefer_shadow = 1;
+ adev_to_drm(adev)->mode_config.fb_modifiers_not_supported = true;
+
adev_to_drm(adev)->mode_config.fb_base = adev->gmc.aper_base;
r = amdgpu_display_modeset_create_props(adev);
diff --git a/drivers/gpu/drm/amd/amdgpu/dce_v11_0.c
b/drivers/gpu/drm/amd/amdgpu/dce_v11_0.c
index 18a7b3bd633b..17942a11366d 100644
--- a/drivers/gpu/drm/amd/amdgpu/dce_v11_0.c
+++ b/drivers/gpu/drm/amd/amdgpu/dce_v11_0.c
@@ -2916,6 +2916,8 @@ static int dce_v11_0_sw_init(void *handle)
adev_to_drm(adev)->mode_config.preferred_depth = 24;
adev_to_drm(adev)->mode_config.prefer_shadow = 1;
+ adev_to_drm(adev)->mode_config.fb_modifiers_not_supported = true;
+
adev_to_drm(adev)->mode_config.fb_base = adev->gmc.aper_base;
r = amdgpu_display_modeset_create_props(adev);
diff --git a/drivers/gpu/drm/amd/amdgpu/dce_v6_0.c
b/drivers/gpu/drm/amd/amdgpu/dce_v6_0.c
index c7803dc2b2d5..2ec99ec8e1a3 100644
--- a/drivers/gpu/drm/amd/amdgpu/dce_v6_0.c
+++ b/drivers/gpu/drm/amd/amdgpu/dce_v6_0.c
@@ -2674,6 +2674,7 @@ static int dce_v6_0_sw_init(void *handle)
adev_to_drm(adev)->mode_config.max_height = 16384;
adev_to_drm(adev)->mode_config.preferred_depth = 24;
adev_to_drm(adev)->mode_config.prefer_shadow = 1;
+ adev_to_drm(adev)->mode_config.fb_modifiers_not_supported = true;
adev_to_drm(adev)->mode_config.fb_base = adev->gmc.aper_base;
r = amdgpu_display_modeset_create_props(adev);
diff --git a/drivers/gpu/drm/amd/amdgpu/dce_v8_0.c
b/drivers/gpu/drm/amd/amdgpu/dce_v8_0.c
index b200b9e722d9..8369336cec90 100644
--- a/drivers/gpu/drm/amd/amdgpu/dce_v8_0.c
+++ b/drivers/gpu/drm/amd/amdgpu/dce_v8_0.c
@@ -2699,6 +2699,8 @@ static int dce_v8_0_sw_init(void *handle)
adev_to_drm(adev)->mode_config.preferred_depth = 24;
adev_to_drm(adev)->mode_config.prefer_shadow = 1;
+ adev_to_drm(adev)->mode_config.fb_modifiers_not_supported = true;
+
adev_to_drm(adev)->mode_config.fb_base = adev->gmc.aper_base;
r = amdgpu_display_modeset_create_props(adev);
diff --git a/drivers/gpu/drm/nouveau/nouveau_display.c
b/drivers/gpu/drm/nouveau/nouveau_display.c
index 929de41c281f..1ecad7fa3e8a 100644
--- a/drivers/gpu/drm/nouveau/nouveau_display.c
+++ b/drivers/gpu/drm/nouveau/nouveau_display.c
@@ -711,10 +711,12 @@ nouveau_display_create(struct drm_device *dev)
>disp);
if (ret == 0) {
[RFC PATH 1/3] drm: add support modifiers for drivers whose planes only support linear layout
The LINEAR modifier is advertised as default if a driver doesn't specify
modifiers. However, there are legacy drivers such as radeon that do not
support modifiers but infer the actual layout of the underlying buffer.
Therefore, a new flag not_support_fb_modifires is introduced for these
legacy drivers. Allow_fb_modifiers will be replaced with this new flag.
Signed-off-by: Tomohito Esaki
---
drivers/gpu/drm/drm_plane.c | 34 ++
include/drm/drm_mode_config.h | 10 ++
include/drm/drm_plane.h | 3 +++
3 files changed, 39 insertions(+), 8 deletions(-)
diff --git a/drivers/gpu/drm/drm_plane.c b/drivers/gpu/drm/drm_plane.c
index 82afb854141b..75308ee240c0 100644
--- a/drivers/gpu/drm/drm_plane.c
+++ b/drivers/gpu/drm/drm_plane.c
@@ -161,6 +161,16 @@ modifiers_ptr(struct drm_format_modifier_blob *blob)
return (struct drm_format_modifier *)(((char *)blob) +
blob->modifiers_offset);
}
+static bool check_format_modifier(struct drm_plane *plane, uint32_t format,
+ uint64_t modifier)
+{
+ if (plane->funcs->format_mod_supported)
+ return plane->funcs->format_mod_supported(plane, format,
+ modifier);
+
+ return modifier == DRM_FORMAT_MOD_LINEAR;
+}
+
static int create_in_format_blob(struct drm_device *dev, struct drm_plane
*plane)
{
const struct drm_mode_config *config = >mode_config;
@@ -203,16 +213,15 @@ static int create_in_format_blob(struct drm_device *dev,
struct drm_plane *plane
memcpy(formats_ptr(blob_data), plane->format_types, formats_size);
/* If we can't determine support, just bail */
- if (!plane->funcs->format_mod_supported)
+ if (config->fb_modifiers_not_supported)
goto done;
mod = modifiers_ptr(blob_data);
for (i = 0; i < plane->modifier_count; i++) {
for (j = 0; j < plane->format_count; j++) {
- if (plane->funcs->format_mod_supported(plane,
-
plane->format_types[j],
-
plane->modifiers[i])) {
-
+ if (check_format_modifier(plane,
+ plane->format_types[j],
+ plane->modifiers[i])) {
mod->formats |= 1ULL << j;
}
}
@@ -242,6 +251,10 @@ static int __drm_universal_plane_init(struct drm_device
*dev,
const char *name, va_list ap)
{
struct drm_mode_config *config = >mode_config;
+ const uint64_t default_modifiers[] = {
+ DRM_FORMAT_MOD_LINEAR,
+ DRM_FORMAT_MOD_INVALID
+ };
unsigned int format_modifier_count = 0;
int ret;
@@ -282,6 +295,11 @@ static int __drm_universal_plane_init(struct drm_device
*dev,
while (*temp_modifiers++ != DRM_FORMAT_MOD_INVALID)
format_modifier_count++;
+ } else {
+ if (!dev->mode_config.fb_modifiers_not_supported) {
+ format_modifiers = default_modifiers;
+ format_modifier_count = 1;
+ }
}
/* autoset the cap and check for consistency across all planes */
@@ -346,7 +364,7 @@ static int __drm_universal_plane_init(struct drm_device
*dev,
drm_object_attach_property(>base, config->prop_src_h, 0);
}
- if (config->allow_fb_modifiers)
+ if (format_modifier_count)
create_in_format_blob(dev, plane);
return 0;
@@ -373,8 +391,8 @@ static int __drm_universal_plane_init(struct drm_device
*dev,
* drm_universal_plane_init() to let the DRM managed resource infrastructure
* take care of cleanup and deallocation.
*
- * Drivers supporting modifiers must set @format_modifiers on all their planes,
- * even those that only support DRM_FORMAT_MOD_LINEAR.
+ * For drivers supporting modifiers, all planes will advertise
+ * DRM_FORMAT_MOD_LINEAR support, if @format_modifiers is not set.
*
* Returns:
* Zero on success, error code on failure.
diff --git a/include/drm/drm_mode_config.h b/include/drm/drm_mode_config.h
index 48b7de80daf5..c56f298c55bd 100644
--- a/include/drm/drm_mode_config.h
+++ b/include/drm/drm_mode_config.h
@@ -920,6 +920,16 @@ struct drm_mode_config {
*/
bool allow_fb_modifiers;
+ /**
+* @fb_modifiers_not_supported:
+*
+* This flag is for legacy drivers such as radeon that do not support
+* modifiers but infer the actual layout of the underlying buffer.
+* Generally, each drivers must support modifiers, this flag should not
+* be set.
+*/
+ bool fb_modifiers_not_supported;
+
/**
*
[RFC PATCH 0/3] Add support modifiers for drivers whose planes only support linear layout
Some drivers whose planes only support linear layout fb do not support format modifiers. These drivers should support modifiers, however the DRM core should handle this rather than open-coding in every driver. In this patch series, these drivers expose format modifiers based on the following suggestion[1]. On Thu, Nov 18, 2021 at 01:02:11PM +, Daniel Stone wrote: > I think the best way forward here is: > - add a new mode_config.cannot_support_modifiers flag, and enable > this in radeon (plus any other drivers in the same boat) > - change drm_universal_plane_init() to advertise the LINEAR modifier > when NULL is passed as the modifier list (including installing a > default .format_mod_supported hook) > - remove the mode_config.allow_fb_modifiers hook and always > advertise modifier support, unless > mode_config.cannot_support_modifiers is set [1] https://patchwork.kernel.org/project/linux-renesas-soc/patch/20190509054518.10781-1-e...@igel.co.jp/#24602575 Tomohito Esaki (3): drm: add support modifiers for drivers whose planes only support linear layout drm: set fb_modifiers_not_supported flag in legacy drivers drm: replace allow_fb_modifiers with fb_modifiers_not_supported drivers/gpu/drm/amd/amdgpu/amdgpu_display.c | 6 +-- drivers/gpu/drm/amd/amdgpu/dce_v10_0.c| 2 + drivers/gpu/drm/amd/amdgpu/dce_v11_0.c| 2 + drivers/gpu/drm/amd/amdgpu/dce_v6_0.c | 1 + drivers/gpu/drm/amd/amdgpu/dce_v8_0.c | 2 + drivers/gpu/drm/drm_framebuffer.c | 6 +-- drivers/gpu/drm/drm_ioctl.c | 2 +- drivers/gpu/drm/drm_plane.c | 41 +++ drivers/gpu/drm/nouveau/nouveau_display.c | 6 ++- drivers/gpu/drm/radeon/radeon_display.c | 2 + .../gpu/drm/selftests/test-drm_framebuffer.c | 1 - include/drm/drm_mode_config.h | 18 +++- include/drm/drm_plane.h | 3 ++ 13 files changed, 54 insertions(+), 38 deletions(-) -- 2.17.1
[GIT PULL] exynos-drm-next
Hi Dave and Daniel, Just four cleanups such as replacing the use of legacy interface, implementing generic gem mmap, fixing a build warning and dropping unnecessary code. Please let me know if there is any problem. Thanks, Inki Dae The following changes since commit 1c405ca11bf563de1725e5ecfb4a74ee289d2ee9: Merge tag 'mediatek-drm-next-5.17' of https://git.kernel.org/pub/scm/linux/kernel/git/chunkuang.hu/linux into drm-next (2021-12-17 16:16:16 +1000) are available in the Git repository at: git://git.kernel.org/pub/scm/linux/kernel/git/daeinki/drm-exynos tags/exynos-drm-next-for-v5.17 for you to fetch changes up to 760cceff996135fd4830f3d339446a04bd37ca0c: drm/exynos: drop the use of label from exynos_dsi_register_te_irq (2021-12-22 11:39:39 +0900) Four cleanups - Replacing lagacy gpio interface of dsi driver with gpiod one. - Implementing a generic GEM object mmap and use it instead of exynos specific one. - Dropping the use of label from dsi driver. Which also fixes a build warning. - Just trivial cleanup by dropping unnecessay code. Bernard Zhao (1): drm/exynos: remove useless type conversion Inki Dae (1): drm/exynos: drop the use of label from exynos_dsi_register_te_irq Maíra Canal (1): drm/exynos: Replace legacy gpio interface for gpiod interface Thomas Zimmermann (1): drm/exynos: Implement mmap as GEM object function drivers/gpu/drm/exynos/exynos_drm_drv.c | 13 ++-- drivers/gpu/drm/exynos/exynos_drm_dsi.c | 49 +++ drivers/gpu/drm/exynos/exynos_drm_fbdev.c | 20 ++--- drivers/gpu/drm/exynos/exynos_drm_fimc.c | 4 +-- drivers/gpu/drm/exynos/exynos_drm_gem.c | 43 ++- drivers/gpu/drm/exynos/exynos_drm_gem.h | 5 6 files changed, 32 insertions(+), 102 deletions(-)
linux-next: manual merge of the drm tree with the drm-misc-fixes tree
Hi all,
Today's linux-next merge of the drm tree got a conflict in:
drivers/gpu/drm/nouveau/nouveau_fence.c
between commit:
67f74302f45d ("drm/nouveau: wait for the exclusive fence after the shared
ones v2")
from the drm-misc-fixes tree and commit:
40298cb45071 ("drm/nouveau: use the new iterator in nouveau_fence_sync")
from the drm tree.
I fixed it up (I just used the latter version) and can carry the fix as
necessary. This is now fixed as far as linux-next is concerned, but any
non trivial conflicts should be mentioned to your upstream maintainer
when your tree is submitted for merging. You may also want to consider
cooperating with the maintainer of the conflicting tree to minimise any
particularly complex conflicts.
--
Cheers,
Stephen Rothwell
pgppj5QEnGI7X.pgp
Description: OpenPGP digital signature
Re: [PATCH 0/2] drm/tegra: Fix panel support on Venice 2 and Nyan
21.12.2021 21:01, Thierry Reding пишет: > On Tue, Dec 21, 2021 at 07:45:31PM +0300, Dmitry Osipenko wrote: >> 21.12.2021 19:17, Thierry Reding пишет: >>> On Tue, Dec 21, 2021 at 06:47:31PM +0300, Dmitry Osipenko wrote: 21.12.2021 13:58, Thierry Reding пишет: .. The panel->ddc isn't used by the new panel-edp driver unless panel is compatible with "edp-panel". Hence the generic_edp_panel_probe() should either fail or crash for a such "edp-panel" since panel->ddc isn't fully instantiated, AFAICS. >>> >>> I've tested this and it works fine on Venice 2. Since that was the >>> reference design for Nyan, I suspect that Nyan's will also work. >>> >>> It'd be great if Thomas or anyone else with access to a Nyan could >>> test this to verify that. >> >> There is no panel-edp driver in the v5.15. The EOL of v5.15 is Oct, >> 2023, hence we need to either use: > > All the (at least relevant) functionality that is in panel-edp was in > panel-simple before it was moved to panel-edp. I've backported this set > of patches to v5.15 and it works just fine there. Will we be able to add patch to bypass the panel's DT ddc-i2c-bus on Nyan to keep the older DTBs working? >>> >>> I don't see why we would want to do that. It's quite clear that the DTB >>> is buggy in this case and we have a more accurate way to describe what's >>> really there in hardware. In addition that more accurate representation >>> also gets rid of a bug. Obviously because the bug is caused by the >>> previous representation that was not accurate. >>> >>> Given that we can easily replace the DTBs on these devices there's no >>> reason to make this any more complicated than it has to be. >> >> Don't you care about normal people at all? Do you assume that everyone >> must to be a kernel developer to be able to use Tegra devices? :/ > > If you know how to install a custom kernel you also know how to replace > the DTB on these devices. > > For everyone else, once these patches are merged upstream and > distributions start shipping the new version, they will get this > automatically by updating their kernel package since most distributions > actually ship the DTB files as part of that. > >> It's not a problem for you to figure out why display is broken, for >> other people it's a problem. Usually nobody will update DTB without a >> well known reason, instead device will be dusted on a shelf. In the end >> you won't have any users at all. > > Most "normal" people aren't even going to notice that their DTB is going > to be updated. They would actually have to do extra work *not* to update > it. My past experience tells that your assumption is incorrect. There are quite a lot of people who will update kernel, but not DTB. ARM devices have endless variations of bootloaders and individual quirks required for a successful installation of a kernel. Kernel update by distro usually isn't a thing on ARM.
RE: [PATCH] drm/ast: Support 1600x900 with 108MHz PCLK
Hi -Original Message- From: Dave Airlie [mailto:airl...@gmail.com] Sent: Wednesday, December 22, 2021 5:56 AM To: Thomas Zimmermann Subject: Re: [PATCH] drm/ast: Support 1600x900 with 108MHz PCLK On Mon, 2 Nov 2020 at 17:57, Thomas Zimmermann wrote: > > Hi > > Am 30.10.20 um 08:42 schrieb KuoHsiang Chou: > > [New] Create the setting for 1600x900 @60Hz refresh rate > > by 108MHz pixel-clock. > > > > Signed-off-by: KuoHsiang Chou > > Acked-by: Thomas Zimmermann > > I'll add your patch to drm-misc-next. > > As Sam mentioned, you should use scripts/get_maintainers.pl to > retrieve the relevant people. These include those in MAINTAINERS, but > also developers that have previously worked on the code. We are seeing a possible report of a regression on an ast2600 server with this patch. I haven't ascertained that reverting it fixes it for the customer yet, but this is a heads up in case anyone else has seen issues. Hi Dave, Yes, you're right, The patch needs to be removed. The patch occurs incorrect timing on CRT and ASTDP when 1600x900 are selected. So, do I need to commit a new patch to remove/revert it from drm/ast? Regards, Kuo-Hsiang Chou Dave.
[PATCH] drm/etnaviv: consider completed fence seqno in hang check
Some GPU heavy test programs manage to trigger the hangcheck quite often.
If there are no other GPU users in the system and the test program
exhibits a very regular structure in the commandstreams that are being
submitted, we can end up with two distinct submits managing to trigger
the hangcheck with the FE in a very similar address range. This leads
the hangcheck to believe that the GPU is stuck, while in reality the GPU
is already busy working on a different job. To avoid those spurious
GPU resets, also remember and consider the last completed fence seqno
in the hang check.
Reported-by: Joerg Albert
Signed-off-by: Lucas Stach
---
drivers/gpu/drm/etnaviv/etnaviv_gpu.h | 1 +
drivers/gpu/drm/etnaviv/etnaviv_sched.c | 4 +++-
2 files changed, 4 insertions(+), 1 deletion(-)
diff --git a/drivers/gpu/drm/etnaviv/etnaviv_gpu.h
b/drivers/gpu/drm/etnaviv/etnaviv_gpu.h
index 1c75c8ed5bce..85eddd492774 100644
--- a/drivers/gpu/drm/etnaviv/etnaviv_gpu.h
+++ b/drivers/gpu/drm/etnaviv/etnaviv_gpu.h
@@ -130,6 +130,7 @@ struct etnaviv_gpu {
/* hang detection */
u32 hangcheck_dma_addr;
+ u32 hangcheck_fence;
void __iomem *mmio;
int irq;
diff --git a/drivers/gpu/drm/etnaviv/etnaviv_sched.c
b/drivers/gpu/drm/etnaviv/etnaviv_sched.c
index 180bb633d5c5..58f593b278c1 100644
--- a/drivers/gpu/drm/etnaviv/etnaviv_sched.c
+++ b/drivers/gpu/drm/etnaviv/etnaviv_sched.c
@@ -107,8 +107,10 @@ static enum drm_gpu_sched_stat
etnaviv_sched_timedout_job(struct drm_sched_job
*/
dma_addr = gpu_read(gpu, VIVS_FE_DMA_ADDRESS);
change = dma_addr - gpu->hangcheck_dma_addr;
- if (change < 0 || change > 16) {
+ if (gpu->completed_fence != gpu->hangcheck_fence ||
+ change < 0 || change > 16) {
gpu->hangcheck_dma_addr = dma_addr;
+ gpu->hangcheck_fence = gpu->completed_fence;
goto out_no_timeout;
}
--
2.30.2
Re: [PATCH v4 3/3] drm/privacy_screen_x86: Add entry for ChromeOS privacy-screen
Hi,
On 12/22/21 01:11, Rajat Jain wrote:
> Add a static entry in the x86 table, to detect and wait for
> privacy-screen on some ChromeOS platforms.
>
> Please note that this means that if CONFIG_CHROMEOS_PRIVACY_SCREEN is
> enabled, and if "GOOG0010" device is found in ACPI, then the i915 probe
> shall return EPROBE_DEFER until a platform driver actually registers the
> privacy-screen: https://hansdegoede.livejournal.com/25948.html
>
> Signed-off-by: Rajat Jain
Thanks, patch looks good to me:
Reviewed-by: Hans de Goede
Regards,
Hans
> ---
> v4: * Simplify the detect_chromeos_privacy_screen() function
> * Don't change the existing print statement
> v3: * Remove the pr_info() from detect_chromeos_privacy_screen(), instead
> enhance the one already present in drm_privacy_screen_lookup_init()
> v2: * Use #if instead of #elif
> * Reorder the patches in the series.
> * Rebased on drm-tip
>
> drivers/gpu/drm/drm_privacy_screen_x86.c | 17 +
> 1 file changed, 17 insertions(+)
>
> diff --git a/drivers/gpu/drm/drm_privacy_screen_x86.c
> b/drivers/gpu/drm/drm_privacy_screen_x86.c
> index a2cafb294ca6..88802cd7a1ee 100644
> --- a/drivers/gpu/drm/drm_privacy_screen_x86.c
> +++ b/drivers/gpu/drm/drm_privacy_screen_x86.c
> @@ -47,6 +47,13 @@ static bool __init detect_thinkpad_privacy_screen(void)
> }
> #endif
>
> +#if IS_ENABLED(CONFIG_CHROMEOS_PRIVACY_SCREEN)
> +static bool __init detect_chromeos_privacy_screen(void)
> +{
> + return acpi_dev_present("GOOG0010", NULL, -1);
> +}
> +#endif
> +
> static const struct arch_init_data arch_init_data[] __initconst = {
> #if IS_ENABLED(CONFIG_THINKPAD_ACPI)
> {
> @@ -58,6 +65,16 @@ static const struct arch_init_data arch_init_data[]
> __initconst = {
> .detect = detect_thinkpad_privacy_screen,
> },
> #endif
> +#if IS_ENABLED(CONFIG_CHROMEOS_PRIVACY_SCREEN)
> + {
> + .lookup = {
> + .dev_id = NULL,
> + .con_id = NULL,
> + .provider = "privacy_screen-GOOG0010:00",
> + },
> + .detect = detect_chromeos_privacy_screen,
> + },
> +#endif
> };
>
> void __init drm_privacy_screen_lookup_init(void)
>
[PATCH v4 3/3] drm/privacy_screen_x86: Add entry for ChromeOS privacy-screen
Add a static entry in the x86 table, to detect and wait for
privacy-screen on some ChromeOS platforms.
Please note that this means that if CONFIG_CHROMEOS_PRIVACY_SCREEN is
enabled, and if "GOOG0010" device is found in ACPI, then the i915 probe
shall return EPROBE_DEFER until a platform driver actually registers the
privacy-screen: https://hansdegoede.livejournal.com/25948.html
Signed-off-by: Rajat Jain
---
v4: * Simplify the detect_chromeos_privacy_screen() function
* Don't change the existing print statement
v3: * Remove the pr_info() from detect_chromeos_privacy_screen(), instead
enhance the one already present in drm_privacy_screen_lookup_init()
v2: * Use #if instead of #elif
* Reorder the patches in the series.
* Rebased on drm-tip
drivers/gpu/drm/drm_privacy_screen_x86.c | 17 +
1 file changed, 17 insertions(+)
diff --git a/drivers/gpu/drm/drm_privacy_screen_x86.c
b/drivers/gpu/drm/drm_privacy_screen_x86.c
index a2cafb294ca6..88802cd7a1ee 100644
--- a/drivers/gpu/drm/drm_privacy_screen_x86.c
+++ b/drivers/gpu/drm/drm_privacy_screen_x86.c
@@ -47,6 +47,13 @@ static bool __init detect_thinkpad_privacy_screen(void)
}
#endif
+#if IS_ENABLED(CONFIG_CHROMEOS_PRIVACY_SCREEN)
+static bool __init detect_chromeos_privacy_screen(void)
+{
+ return acpi_dev_present("GOOG0010", NULL, -1);
+}
+#endif
+
static const struct arch_init_data arch_init_data[] __initconst = {
#if IS_ENABLED(CONFIG_THINKPAD_ACPI)
{
@@ -58,6 +65,16 @@ static const struct arch_init_data arch_init_data[]
__initconst = {
.detect = detect_thinkpad_privacy_screen,
},
#endif
+#if IS_ENABLED(CONFIG_CHROMEOS_PRIVACY_SCREEN)
+ {
+ .lookup = {
+ .dev_id = NULL,
+ .con_id = NULL,
+ .provider = "privacy_screen-GOOG0010:00",
+ },
+ .detect = detect_chromeos_privacy_screen,
+ },
+#endif
};
void __init drm_privacy_screen_lookup_init(void)
--
2.34.1.307.g9b7440fafd-goog
[PATCH v4 2/3] platform/chrome: Add driver for ChromeOS privacy-screen
This adds the ACPI driver for the ChromeOS privacy screen that is
present on some chromeos devices.
Note that ideally, we'd want this privacy screen driver to be probed
BEFORE the drm probe in order to avoid a drm probe deferral:
https://hansdegoede.livejournal.com/25948.html
In practise, I found that ACPI drivers are bound to their devices AFTER
the drm probe on chromebooks. So on chromebooks with privacy-screen,
this patch along with the other one in this series results in a probe
deferral of about 250ms for i915 driver. However, it did not result in
any user noticeable delay of splash screen in my personal experience.
In future if this probe deferral turns out to be an issue, we can
consider turning this ACPI driver into something that is probed
earlier than the drm drivers.
Signed-off-by: Rajat Jain
---
v4: Same as v3 (No changes)
v3: * Renamed everything chromeos_priv_scrn_* to chromeos_privacy_screen_*
(and added line breaks to accommodate longer names within 80 chars)
* Cleanup / Added few comments
* Use the newly added drm_privacy_screen_get_drvdata()
* Provide the cleanup function chromeos_privacy_screen_remove()
v2: * Reword the commit log
* Make the Kconfig into a tristate
* Reorder the patches in the series.
drivers/platform/chrome/Kconfig | 11 ++
drivers/platform/chrome/Makefile | 1 +
.../platform/chrome/chromeos_privacy_screen.c | 152 ++
3 files changed, 164 insertions(+)
create mode 100644 drivers/platform/chrome/chromeos_privacy_screen.c
diff --git a/drivers/platform/chrome/Kconfig b/drivers/platform/chrome/Kconfig
index ccc23d8686e8..75e93efd669f 100644
--- a/drivers/platform/chrome/Kconfig
+++ b/drivers/platform/chrome/Kconfig
@@ -243,6 +243,17 @@ config CROS_USBPD_NOTIFY
To compile this driver as a module, choose M here: the
module will be called cros_usbpd_notify.
+config CHROMEOS_PRIVACY_SCREEN
+ tristate "ChromeOS Privacy Screen support"
+ depends on ACPI
+ depends on DRM
+ select DRM_PRIVACY_SCREEN
+ help
+ This driver provides the support needed for the in-built electronic
+ privacy screen that is present on some ChromeOS devices. When enabled,
+ this should probably always be built into the kernel to avoid or
+ minimize drm probe deferral.
+
source "drivers/platform/chrome/wilco_ec/Kconfig"
endif # CHROMEOS_PLATFORMS
diff --git a/drivers/platform/chrome/Makefile b/drivers/platform/chrome/Makefile
index f901d2e43166..5d4be9735d9d 100644
--- a/drivers/platform/chrome/Makefile
+++ b/drivers/platform/chrome/Makefile
@@ -4,6 +4,7 @@
CFLAGS_cros_ec_trace.o:= -I$(src)
obj-$(CONFIG_CHROMEOS_LAPTOP) += chromeos_laptop.o
+obj-$(CONFIG_CHROMEOS_PRIVACY_SCREEN) += chromeos_privacy_screen.o
obj-$(CONFIG_CHROMEOS_PSTORE) += chromeos_pstore.o
obj-$(CONFIG_CHROMEOS_TBMC)+= chromeos_tbmc.o
obj-$(CONFIG_CROS_EC) += cros_ec.o
diff --git a/drivers/platform/chrome/chromeos_privacy_screen.c
b/drivers/platform/chrome/chromeos_privacy_screen.c
new file mode 100644
index ..07c9f257c723
--- /dev/null
+++ b/drivers/platform/chrome/chromeos_privacy_screen.c
@@ -0,0 +1,152 @@
+// SPDX-License-Identifier: GPL-2.0
+
+/*
+ * ChromeOS Privacy Screen support
+ *
+ * Copyright (C) 2022 Google LLC
+ *
+ * This is the Chromeos privacy screen provider, present on certain
chromebooks,
+ * represented by a GOOG0010 device in the ACPI. This ACPI device, if present,
+ * will cause the i915 drm driver to probe defer until this driver registers
+ * the privacy-screen.
+ */
+
+#include
+#include
+
+/*
+ * The DSM (Device Specific Method) constants below are the agreed API with
+ * the firmware team, on how to control privacy screen using ACPI methods.
+ */
+#define PRIV_SCRN_DSM_REVID1 /* DSM version */
+#define PRIV_SCRN_DSM_FN_GET_STATUS1 /* Get privacy screen status */
+#define PRIV_SCRN_DSM_FN_ENABLE2 /* Enable privacy
screen */
+#define PRIV_SCRN_DSM_FN_DISABLE 3 /* Disable privacy screen */
+
+static const guid_t chromeos_privacy_screen_dsm_guid =
+ GUID_INIT(0xc7033113, 0x8720, 0x4ceb,
+ 0x90, 0x90, 0x9d, 0x52, 0xb3, 0xe5, 0x2d, 0x73);
+
+static void
+chromeos_privacy_screen_get_hw_state(struct drm_privacy_screen
+*drm_privacy_screen)
+{
+ union acpi_object *obj;
+ acpi_handle handle;
+ struct device *privacy_screen =
+ drm_privacy_screen_get_drvdata(drm_privacy_screen);
+
+ handle = acpi_device_handle(to_acpi_device(privacy_screen));
+ obj = acpi_evaluate_dsm(handle, _privacy_screen_dsm_guid,
+ PRIV_SCRN_DSM_REVID,
+ PRIV_SCRN_DSM_FN_GET_STATUS, NULL);
+ if (!obj) {
+ dev_err(privacy_screen,
+
Re: [PATCH v3 0/9] backlight: qcom-wled: fix and solidify handling of enabled-strings
On 2021-11-16 15:42:15, Lee Jones wrote: > On Tue, 16 Nov 2021, Daniel Thompson wrote: > > > Hi Lee > > > > On Mon, Nov 15, 2021 at 09:34:50PM +0100, Marijn Suijten wrote: > > > This patchset fixes WLED's handling of enabled-strings: besides some > > > cleanup it is now actually possible to specify a non-contiguous array of > > > enabled strings (not necessarily starting at zero) and the values from > > > DT are now validated to prevent possible unexpected out-of-bounds > > > register and array element accesses. > > > Off-by-one mistakes in the maximum number of strings, also causing > > > out-of-bounds access, have been addressed as well. > > > > They have arrived piecemeal (during v1, v2 and v3) but all patches on > > the set should now have my R-b: attached to them. > > I can see that. Nothing for you to worry about. > > I'll apply these when I conduct my next sweep, thanks. Thanks for that Lee! Has the next sweep already passed by? Seems everyone is preparing for the 5.17 merge window but these patches haven't yet landed on the backlight tree [1]. I'd appreciate it if we can make them appear in the 5.17 window :) [1]: https://git.kernel.org/pub/scm/linux/kernel/git/lee/backlight.git/ Thanks! - Marijn > -- > Lee Jones [李琼斯] > Senior Technical Lead - Developer Services > Linaro.org │ Open source software for Arm SoCs > Follow Linaro: Facebook | Twitter | Blog
Re: [PATCH v2] drm: fix error found in some cases after the patch d1af5cd86997
On Mon, Dec 20, 2021 at 06:11:31PM +0100, Daniel Vetter wrote:
> On Mon, Dec 20, 2021 at 10:18:38AM +0100, Daniel Vetter wrote:
> > On Thu, Dec 02, 2021 at 10:51:12AM +0100, Claudio Suarez wrote:
> > > The patch d1af5cd86997 ("drm: get rid of DRM_DEBUG_* log
> > > calls in drm core, files drm_a*.c") fails when the drm_device
> > > cannot be found in the parameter plane_state->crtc.
> > > Fix it using plane_state->plane.
> > >
> > > Reported-by: kernel test robot
> > > Fixes: d1af5cd86997 ("drm: get rid of DRM_DEBUG_* log calls in drm core,
> > > files drm_a*.c")
>
> My scrip complained about the fixes line, so I fixed it. I guess you've
> used the sha1 from your tree, not from upstream?
Yes, my bad, sorry. Thanks for the advice.
> Please always use
> upstream sha1 when referencing commits.
>
> Anyway patches are now pushed.
Thank you!
Best regards.
Claudio Suarez.
Re: [Intel-gfx] [PATCH] drm/i915/guc: Log engine resets
On 12/21/2021 05:37, Tvrtko Ursulin wrote:
On 20/12/2021 18:34, John Harrison wrote:
On 12/20/2021 07:00, Tvrtko Ursulin wrote:
On 17/12/2021 16:22, Matthew Brost wrote:
On Fri, Dec 17, 2021 at 12:15:53PM +, Tvrtko Ursulin wrote:
On 14/12/2021 15:07, Tvrtko Ursulin wrote:
From: Tvrtko Ursulin
Log engine resets done by the GuC firmware in the similar way it
is done
by the execlists backend.
This way we have notion of where the hangs are before the GuC gains
support for proper error capture.
Ping - any interest to log this info?
All there currently is a non-descriptive "[drm] GPU HANG: ecode
12:0:".
Yea, this could be helpful. One suggestion below.
Also, will GuC be reporting the reason for the engine reset at any
point?
We are working on the error state capture, presumably the registers
will
give a clue what caused the hang.
As for the GuC providing a reason, that isn't defined in the interface
but that is decent idea to provide a hint in G2H what the issue
was. Let
me run that by the i915 GuC developers / GuC firmware team and see
what
they think.
The GuC does not do any hang analysis. So as far as GuC is concerned,
the reason is pretty much always going to be pre-emption timeout.
There are a few ways the pre-emption itself could be triggered but
basically, if GuC resets an active context then it is because it did
not pre-empt quickly enough when requested.
Regards,
Tvrtko
Signed-off-by: Tvrtko Ursulin
Cc: Matthew Brost
Cc: John Harrison
---
drivers/gpu/drm/i915/gt/uc/intel_guc_submission.c | 12
+++-
1 file changed, 11 insertions(+), 1 deletion(-)
diff --git a/drivers/gpu/drm/i915/gt/uc/intel_guc_submission.c
b/drivers/gpu/drm/i915/gt/uc/intel_guc_submission.c
index 9739da6f..51512123dc1a 100644
--- a/drivers/gpu/drm/i915/gt/uc/intel_guc_submission.c
+++ b/drivers/gpu/drm/i915/gt/uc/intel_guc_submission.c
@@ -11,6 +11,7 @@
#include "gt/intel_context.h"
#include "gt/intel_engine_pm.h"
#include "gt/intel_engine_heartbeat.h"
+#include "gt/intel_engine_user.h"
#include "gt/intel_gpu_commands.h"
#include "gt/intel_gt.h"
#include "gt/intel_gt_clock_utils.h"
@@ -3934,9 +3935,18 @@ static void capture_error_state(struct
intel_guc *guc,
{
struct intel_gt *gt = guc_to_gt(guc);
struct drm_i915_private *i915 = gt->i915;
- struct intel_engine_cs *engine =
__context_to_physical_engine(ce);
+ struct intel_engine_cs *engine = ce->engine;
intel_wakeref_t wakeref;
+ if (intel_engine_is_virtual(engine)) {
+ drm_notice(>drm, "%s class, engines 0x%x; GuC
engine reset\n",
+ intel_engine_class_repr(engine->class),
+ engine->mask);
+ engine = guc_virtual_get_sibling(engine, 0);
+ } else {
+ drm_notice(>drm, "%s GuC engine reset\n",
engine->name);
Probably include the guc_id of the context too then?
Is the guc id stable and useful on its own - who would be the user?
The GuC id is the only thing that matters when trying to correlate
KMD activity with a GuC log. So while it might not be of any use or
interest to an end user, it is extremely important and useful to a
kernel developer attempting to debug an issue. And that includes bug
reports from end users that are hard to repro given that the standard
error capture will include the GuC log.
On the topic of GuC log - is there a tool in IGT (or will be) which
will parse the bit saved in the error capture or how is that supposed
to be used?
Nope.
However, Alan is currently working on supporting the GuC error capture
mechanism. Prior to sending the reset notification to the KMD, the GuC
will save a whole bunch of register state to a memory buffer and send a
notification to the KMD that this is available. When we then get the
actual reset notification, we need to match the two together and include
a parsed, human readable version of the GuC's capture state buffer in
the sysfs error log output.
The GuC log should not be involved in this process. And note that any
register dumps in the GuC log are limited in scope and only enabled at
higher verbosity levels. Whereas, the official state capture is based on
a register list provided by the KMD and is available irrespective of
debug CONFIG settings, verbosity levels, etc.
Also, note that GuC really resets contexts rather than engines. What
it reports back to i915 on a reset is simply the GuC id of the
context. It is up to i915 to work back from that to determine engine
instances/classes if required. And in the case of a virtual context,
it is impossible to extract the actual instance number. So your above
print about resetting all instances within the virtual engine mask is
incorrect/misleading. The reset would have been applied to one and
only one of those engines. If you really need to know exactly which
engine was poked, you need to look inside the GuC log.
I think I understood that part. :) It wasn't my intent to imply in
Re: [PATCH] drm/ast: Support 1600x900 with 108MHz PCLK
On Mon, 2 Nov 2020 at 17:57, Thomas Zimmermann wrote: > > Hi > > Am 30.10.20 um 08:42 schrieb KuoHsiang Chou: > > [New] Create the setting for 1600x900 @60Hz refresh rate > > by 108MHz pixel-clock. > > > > Signed-off-by: KuoHsiang Chou > > Acked-by: Thomas Zimmermann > > I'll add your patch to drm-misc-next. > > As Sam mentioned, you should use scripts/get_maintainers.pl to retrieve > the relevant people. These include those in MAINTAINERS, but also > developers that have previously worked on the code. We are seeing a possible report of a regression on an ast2600 server with this patch. I haven't ascertained that reverting it fixes it for the customer yet, but this is a heads up in case anyone else has seen issues. Dave.
[PATCH v2] drm/i915/guc: Check for wedged before doing stuff
From: John Harrison
A fault injection probe test hit a BUG_ON in a GuC error path. It
showed that the GuC code could potentially attempt to do many things
when the device is actually wedged. So, add a check in to prevent that.
v2: Use intel_gt_is_wedged instead of testing bits directly in the
GuC submission code (review feedback from Tvrtko).
Signed-off-by: John Harrison
---
drivers/gpu/drm/i915/gt/uc/intel_guc_submission.c | 5 +++--
1 file changed, 3 insertions(+), 2 deletions(-)
diff --git a/drivers/gpu/drm/i915/gt/uc/intel_guc_submission.c
b/drivers/gpu/drm/i915/gt/uc/intel_guc_submission.c
index e7517206af82..756b29d8326b 100644
--- a/drivers/gpu/drm/i915/gt/uc/intel_guc_submission.c
+++ b/drivers/gpu/drm/i915/gt/uc/intel_guc_submission.c
@@ -1349,7 +1349,8 @@ submission_disabled(struct intel_guc *guc)
struct i915_sched_engine * const sched_engine = guc->sched_engine;
return unlikely(!sched_engine ||
- !__tasklet_is_enabled(_engine->tasklet));
+ !__tasklet_is_enabled(_engine->tasklet) ||
+ intel_gt_is_wedged(guc_to_gt(guc)));
}
static void disable_submission(struct intel_guc *guc)
@@ -1725,7 +1726,7 @@ void intel_guc_submission_reset_finish(struct intel_guc
*guc)
{
/* Reset called during driver load or during wedge? */
if (unlikely(!guc_submission_initialized(guc) ||
-test_bit(I915_WEDGED, _to_gt(guc)->reset.flags))) {
+intel_gt_is_wedged(guc_to_gt(guc {
return;
}
--
2.25.1
Re: [PATCH v16 08/40] gpu: host1x: Add initial runtime PM and OPP support
Hi,
Thank you for testing it all.
21.12.2021 21:55, Jon Hunter пишет:
> Hi Dmitry, Thierry,
>
> On 30/11/2021 23:23, Dmitry Osipenko wrote:
>> Add runtime PM and OPP support to the Host1x driver. For the starter we
>> will keep host1x always-on because dynamic power management require a
>> major
>> refactoring of the driver code since lot's of code paths are missing the
>> RPM handling and we're going to remove some of these paths in the future.
>
>
> Unfortunately, this change is breaking boot on Tegra186. Bisect points
> to this and reverting on top of -next gets the board booting again.
> Sadly, there is no panic or error reported, it is just a hard hang. I
> will not have time to look at this this week and so we may need to
> revert for the moment.
Only T186 broken? What about T194?
Which board model fails to boot? Is it running in hypervisor mode?
Do you use any additional patches?
Could you please test the below diff? I suspect that
host1x_syncpt_save/restore may be entirely broken for T186 since we
never used these funcs before.
--- >8 ---
diff --git a/drivers/gpu/host1x/dev.c b/drivers/gpu/host1x/dev.c
index f5b4dcded088..fd5dfb875422 100644
--- a/drivers/gpu/host1x/dev.c
+++ b/drivers/gpu/host1x/dev.c
@@ -580,7 +580,6 @@ static int __maybe_unused
host1x_runtime_suspend(struct device *dev)
int err;
host1x_intr_stop(host);
- host1x_syncpt_save(host);
err = reset_control_bulk_assert(host->nresets, host->resets);
if (err) {
@@ -596,9 +595,8 @@ static int __maybe_unused
host1x_runtime_suspend(struct device *dev)
return 0;
resume_host1x:
- host1x_setup_sid_table(host);
- host1x_syncpt_restore(host);
host1x_intr_start(host);
+ host1x_setup_sid_table(host);
return err;
}
@@ -626,9 +624,8 @@ static int __maybe_unused
host1x_runtime_resume(struct device *dev)
goto disable_clk;
}
- host1x_setup_sid_table(host);
- host1x_syncpt_restore(host);
host1x_intr_start(host);
+ host1x_setup_sid_table(host);
return 0;
Re: Re: Re: Re: 回复: Re: [PATCH] drm/amdgpu: fixup bad vram size on gmc v8
Yes, you can either do that, or if amdgpu is loaded, just read the data from /sys/kernel/debug/dri/0/amdgpu_vbios Alex On Mon, Dec 20, 2021 at 3:06 AM 周宗敏 wrote: > > > Dear Alex: > > > I've never tried to get a VBIOS before, so can you tell me how to get a > vbios image copy for you? > > I try to google, just get the message that maybe can get from the > following way: > > echo 1 > /sys/devices/pci:00/:00:02.0/rom > > cat /sys/devices/pci:00/:00:02.0/rom > vbios.dump > > echo 0 > /sys/devices/pci:00/:00:02.0/rom > > > Is that right? > > > Thanks very much. > > > > > > > > > > *主 题：*Re: Re: Re: 回复: Re: [PATCH] drm/amdgpu: fixup bad vram size on gmc > v8 > *日 期：*2021-12-18 05:19 > *发件人：*Alex Deucher > *收件人：*周宗敏 > > > If you could get me a copy of the vbios image from a problematic board, > that would be helpful. In the meantime, I've applied the patch. > > Alex > > > On Thu, Dec 16, 2021 at 9:38 PM 周宗敏 wrote: > >> Dear Alex: >> >> >> >Is the issue reproducible with the same board in bare metal on x86?Or >> does it only happen with passthrough on ARM? >> >> >> Unfortunately, my current environment is not convenient to test this GPU >> board on x86 platform. >> >> but I can tell you the problem still occurs on ARM without passthrough to >> virtual machine. >> >> >> In addition,at end of 2020,my colleagues also found similar problems on >> MIPS platforms with Graphics chips of Radeon R7 340. >> >> So,I may think it can happen to no matter based on x86 ,ARM or mips. >> >> >> I hope the above information is helpful to you，and I also think it will >> be better for user if can root cause this issue. >> >> >> Best regards. >> >> >> >> >> >> >> >> >> >> >> >> *主 题：*Re: Re: 回复: Re: [PATCH] drm/amdgpu: fixup bad vram size on gmc v8 >> >> *日 期：*2021-12-16 23:28 >> *发件人：*Alex Deucher >> *收件人：*周宗敏 >> >> >> Is the issue reproducible with the same board in bare metal on x86? Or >> does it only happen with passthrough on ARM? Looking through the archives, >> the SI patch I made was for an x86 laptop. It would be nice to root cause >> this, but there weren't any gfx8 boards with more than 64G of vram, so I >> think it's safe. That said, if you see similar issues with newer gfx IPs >> then we have an issue since the upper bit will be meaningful, so it would >> be nice to root cause this. >> >> Alex >> >> >> On Thu, Dec 16, 2021 at 4:36 AM 周宗敏 wrote: >> >>> Hi Christian, >>> >>> >>> I'm testing for GPU passthrough feature, so I pass through this GPU to >>> virtual machine to use. It based on arm64 system. >>> >>> As far as i know, Alex had dealt with a similar problems on >>> dri/radeon/si.c . Maybe they have a same reason to cause it? >>> >>> the history commit message is below: >>> >>> >>> https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/commit/?id=0ca223b029a261e82fb2f50c52eb85d510f4260e >>> >>> [image: image.png] >>> >>> >>> Thanks very much. >>> >>> >>> >>> >>> >>> >>> >>> *主 题：*Re: 回复: Re: [PATCH] drm/amdgpu: fixup bad vram size on gmc v8 >>> >>> *日 期：*2021-12-16 16:15 >>> *发件人：*Christian König >>> *收件人：*周宗敏Alex Deucher >>> >>> >>> >>> >>> Hi Zongmin, >>> >>>that strongly sounds like the ASIC is not correctly initialized when >>>trying to read the register. >>> >>>What board and environment are you using this GPU with? Is that a >>> normal x86 system? >>> >>>Regards, >>>Christian. >>> >>> >>> >>> Am 16.12.21 um 04:11 schrieb 周宗敏: >>> >>> >>> >>>1. >>> >>>the problematic boards that I have tested is [AMD/ATI] Lexa >>> PRO [Radeon RX 550/550X] ; and the vbios version : >>> 113-RXF9310-C09-BT >>>2. >>> >>>When an exception occurs I can see the following changes in >>> the values of vram size get from RREG32(mmCONFIG_MEMSIZE) , >>> >>>it seems to have garbage in the upper 16 bits >>> >>>[image: image.png] >>> >>> >>> >>> >>>3. >>> >>>and then I can also see some dmesg like below: >>> >>>when vram size register have garbage,we may see error >>> message like below: >>> >>>amdgpu :09:00.0: VRAM: 4286582784M 0x00F4 - >>> 0x000FF8F4 (4286582784M used) >>> >>>the correct message should like below: >>> >>>amdgpu :09:00.0: VRAM: 4096M 0x00F4 - >>> 0x00F4 (4096M used) >>> >>> >>> >>> >>>if you have any problems,please send me mail. >>> >>>thanks very much. >>> >>> >>> >>> >>> >>> >>> *主 题：*Re: [PATCH] drm/amdgpu: fixup bad vram size on gmc v8 >>> >>>*日 期：*2021-12-16 04:23 >>>*发件人：*Alex Deucher >>>*收件人：*Zongmin Zhou >>> >>> >>> >>> >>> On Wed, Dec 15, 2021 at 10:31 AM Zongmin Zhouwrote: >>> > >>> > Some boards(like RX550) seem to have garbage in the upper >>> > 16 bits of the vram size register. Check for >>> > this and clamp the size properly. Fixes >>> > boards reporting bogus amounts of vram. >>> > >>>
[PATCH 1/3] drm/i915/guc: Temporarily bump the GuC load timeout
From: John Harrison
There is a known (but exceedingly unlikely) race condition where the
asynchronous frequency management code could reduce the GT clock while
a GuC reload is in progress (during a full GT reset). A fix is in
progress but there are complex locking issues to be resolved. In the
meantime bump the timeout to 200ms. Even at slowest clock, this
should be sufficient. And in the working case, a larger timeout makes
no difference.
Signed-off-by: John Harrison
Reviewed-by: Matthew Brost
---
drivers/gpu/drm/i915/gt/uc/intel_guc_fw.c | 13 +++--
1 file changed, 11 insertions(+), 2 deletions(-)
diff --git a/drivers/gpu/drm/i915/gt/uc/intel_guc_fw.c
b/drivers/gpu/drm/i915/gt/uc/intel_guc_fw.c
index 31420ce1ce6b..d09c205b2beb 100644
--- a/drivers/gpu/drm/i915/gt/uc/intel_guc_fw.c
+++ b/drivers/gpu/drm/i915/gt/uc/intel_guc_fw.c
@@ -105,12 +105,21 @@ static int guc_wait_ucode(struct intel_uncore *uncore)
/*
* Wait for the GuC to start up.
* NB: Docs recommend not using the interrupt for completion.
-* Measurements indicate this should take no more than 20ms, so a
+* Measurements indicate this should take no more than 20ms
+* (assuming the GT clock is at maximum frequency). So, a
* timeout here indicates that the GuC has failed and is unusable.
* (Higher levels of the driver may decide to reset the GuC and
* attempt the ucode load again if this happens.)
+*
+* FIXME: There is a known (but exceedingly unlikely) race condition
+* where the asynchronous frequency management code could reduce
+* the GT clock while a GuC reload is in progress (during a full
+* GT reset). A fix is in progress but there are complex locking
+* issues to be resolved. In the meantime bump the timeout to
+* 200ms. Even at slowest clock, this should be sufficient. And
+* in the working case, a larger timeout makes no difference.
*/
- ret = wait_for(guc_ready(uncore, ), 100);
+ ret = wait_for(guc_ready(uncore, ), 200);
if (ret) {
struct drm_device *drm = >i915->drm;
--
2.25.1
[PATCH 3/3] drm/i915/guc: Improve GuC loading status check/error reports
From: John Harrison
If the GuC fails to load, it is useful to know what firmware file /
version was attempted. So move the version info report to before the
load attempt rather than only after a successful load.
If the GuC does fail to load, then make the error messages visible
rather than being 'debug' prints that do not appears in dmesg output
by default.
When waiting for the GuC to load, it used to be necessary to check for
two different states - READY and (LAPIC_DONE | MIA_CORE). Apparently
the second signified init complete on RC6 exit. However, in more
recent GuC versions the RC6 exit sequence now finishes with status
READY as well. So the test can be simplified.
Also, add an enum giving all the current status codes that GuC loading
can report as a reference without having to pull and search through
the GuC source files.
Signed-off-by: John Harrison
Reviewed-by: Matthew Brost
---
.../gpu/drm/i915/gt/uc/abi/guc_errors_abi.h | 23 ++
drivers/gpu/drm/i915/gt/uc/intel_guc_fw.c | 17 +-
drivers/gpu/drm/i915/gt/uc/intel_guc_reg.h| 4 ---
drivers/gpu/drm/i915/gt/uc/intel_huc.c| 1 +
drivers/gpu/drm/i915/gt/uc/intel_uc.c | 31 ++-
5 files changed, 48 insertions(+), 28 deletions(-)
diff --git a/drivers/gpu/drm/i915/gt/uc/abi/guc_errors_abi.h
b/drivers/gpu/drm/i915/gt/uc/abi/guc_errors_abi.h
index 488b6061ee89..c20658ee85a5 100644
--- a/drivers/gpu/drm/i915/gt/uc/abi/guc_errors_abi.h
+++ b/drivers/gpu/drm/i915/gt/uc/abi/guc_errors_abi.h
@@ -11,4 +11,27 @@ enum intel_guc_response_status {
INTEL_GUC_RESPONSE_STATUS_GENERIC_FAIL = 0xF000,
};
+enum intel_guc_load_status {
+ INTEL_GUC_LOAD_STATUS_DEFAULT = 0x00,
+ INTEL_GUC_LOAD_STATUS_START= 0x01,
+ INTEL_GUC_LOAD_STATUS_ERROR_DEVID_BUILD_MISMATCH = 0x02,
+ INTEL_GUC_LOAD_STATUS_GUC_PREPROD_BUILD_MISMATCH = 0x03,
+ INTEL_GUC_LOAD_STATUS_ERROR_DEVID_INVALID_GUCTYPE = 0x04,
+ INTEL_GUC_LOAD_STATUS_GDT_DONE = 0x10,
+ INTEL_GUC_LOAD_STATUS_IDT_DONE = 0x20,
+ INTEL_GUC_LOAD_STATUS_LAPIC_DONE = 0x30,
+ INTEL_GUC_LOAD_STATUS_GUCINT_DONE = 0x40,
+ INTEL_GUC_LOAD_STATUS_DPC_READY= 0x50,
+ INTEL_GUC_LOAD_STATUS_DPC_ERROR= 0x60,
+ INTEL_GUC_LOAD_STATUS_EXCEPTION= 0x70,
+ INTEL_GUC_LOAD_STATUS_INIT_DATA_INVALID= 0x71,
+ INTEL_GUC_LOAD_STATUS_PXP_TEARDOWN_CTRL_ENABLED= 0x72,
+ INTEL_GUC_LOAD_STATUS_INVALID_INIT_DATA_RANGE_START,
+ INTEL_GUC_LOAD_STATUS_MPU_DATA_INVALID = 0x73,
+ INTEL_GUC_LOAD_STATUS_INIT_MMIO_SAVE_RESTORE_INVALID = 0x74,
+ INTEL_GUC_LOAD_STATUS_INVALID_INIT_DATA_RANGE_END,
+
+ INTEL_GUC_LOAD_STATUS_READY= 0xF0,
+};
+
#endif /* _ABI_GUC_ERRORS_ABI_H */
diff --git a/drivers/gpu/drm/i915/gt/uc/intel_guc_fw.c
b/drivers/gpu/drm/i915/gt/uc/intel_guc_fw.c
index d09c205b2beb..f773e7f35bc1 100644
--- a/drivers/gpu/drm/i915/gt/uc/intel_guc_fw.c
+++ b/drivers/gpu/drm/i915/gt/uc/intel_guc_fw.c
@@ -90,11 +90,10 @@ static int guc_xfer_rsa(struct intel_uc_fw *guc_fw,
static inline bool guc_ready(struct intel_uncore *uncore, u32 *status)
{
u32 val = intel_uncore_read(uncore, GUC_STATUS);
- u32 uk_val = val & GS_UKERNEL_MASK;
+ u32 uk_val = REG_FIELD_GET(GS_UKERNEL_MASK, val);
*status = val;
- return (uk_val == GS_UKERNEL_READY) ||
- ((val & GS_MIA_CORE_STATE) && (uk_val ==
GS_UKERNEL_LAPIC_DONE));
+ return uk_val == INTEL_GUC_LOAD_STATUS_READY;
}
static int guc_wait_ucode(struct intel_uncore *uncore)
@@ -123,8 +122,8 @@ static int guc_wait_ucode(struct intel_uncore *uncore)
if (ret) {
struct drm_device *drm = >i915->drm;
- drm_dbg(drm, "GuC load failed: status = 0x%08X\n", status);
- drm_dbg(drm, "GuC load failed: status: Reset = %d, "
+ drm_info(drm, "GuC load failed: status = 0x%08X\n", status);
+ drm_info(drm, "GuC load failed: status: Reset = %d, "
"BootROM = 0x%02X, UKernel = 0x%02X, "
"MIA = 0x%02X, Auth = 0x%02X\n",
REG_FIELD_GET(GS_MIA_IN_RESET, status),
@@ -134,13 +133,13 @@ static int guc_wait_ucode(struct intel_uncore *uncore)
REG_FIELD_GET(GS_AUTH_STATUS_MASK, status));
if ((status & GS_BOOTROM_MASK) == GS_BOOTROM_RSA_FAILED) {
- drm_dbg(drm, "GuC firmware signature verification
failed\n");
+ drm_info(drm, "GuC firmware signature verification
failed\n");
ret = -ENOEXEC;
}
- if ((status & GS_UKERNEL_MASK) ==
[PATCH 2/3] drm/i915/guc: Update to GuC version 69.0.3
From: John Harrison Update to the latest GuC release. The latest GuC firmware introduces a number of interface changes: GuC may return NO_RESPONSE_RETRY message for requests sent over CTB. Add support for this reply and try resending the request again as a new CTB message. A KLV (key-length-value) mechanism is now used for passing configuration data such as CTB management. With the new KLV scheme, the old CTB management actions are no longer used and are removed. Register capture on hang is now supported by GuC. Full i915 support for this will be added by a later patch. A minimum support of providing capture memory and register lists is required though, so add that in. The device id of the current platform needs to be provided at init time. The 'poll CS' w/a (Wa_22012773006) was blanket enabled by previous versions of GuC. It must now be explicitly requested by the KMD. So, add in the code to turn it on when relevant. The GuC log entry format has changed. This requires adding a new field to the log header structure to mark the wrap point at the end of the buffer (as the buffer size is no longer a multiple of the log entry size). New CTB notification messages are now sent for some things that were previously only sent via MMIO notifications. Of these, the crash dump notification was not really being handled by i915. It called the log flush code but that only flushed the regular debug log and then only if relay logging was enabled. So just report an error message instead. The 'exception' notification was just being ignored completely. So add an error message for that as well. Note that in either the crash dump or the exception case, the GuC is basically dead. The KMD will detect this via the heartbeat and trigger both an error log (which will include the crash dump as part of the GuC log) and a GT reset. So no other processing is really required. Signed-off-by: John Harrison Signed-off-by: Michal Wajdeczko Reviewed-by: Matthew Brost --- Documentation/gpu/i915.rst| 1 + .../gpu/drm/i915/gt/uc/abi/guc_actions_abi.h | 80 +- drivers/gpu/drm/i915/gt/uc/abi/guc_klvs_abi.h | 82 ++ drivers/gpu/drm/i915/gt/uc/intel_guc.c| 126 +--- drivers/gpu/drm/i915/gt/uc/intel_guc.h| 4 + drivers/gpu/drm/i915/gt/uc/intel_guc_ads.c| 45 +- drivers/gpu/drm/i915/gt/uc/intel_guc_ct.c | 141 ++ drivers/gpu/drm/i915/gt/uc/intel_guc_fwif.h | 37 - drivers/gpu/drm/i915/gt/uc/intel_guc_log.c| 31 ++-- drivers/gpu/drm/i915/gt/uc/intel_guc_log.h| 3 + .../gpu/drm/i915/gt/uc/intel_guc_submission.c | 18 +++ drivers/gpu/drm/i915/gt/uc/intel_uc_fw.c | 30 ++-- 12 files changed, 434 insertions(+), 164 deletions(-) create mode 100644 drivers/gpu/drm/i915/gt/uc/abi/guc_klvs_abi.h diff --git a/Documentation/gpu/i915.rst b/Documentation/gpu/i915.rst index b7d801993bfa..bcaefc952764 100644 --- a/Documentation/gpu/i915.rst +++ b/Documentation/gpu/i915.rst @@ -539,6 +539,7 @@ GuC ABI .. kernel-doc:: drivers/gpu/drm/i915/gt/uc/abi/guc_communication_mmio_abi.h .. kernel-doc:: drivers/gpu/drm/i915/gt/uc/abi/guc_communication_ctb_abi.h .. kernel-doc:: drivers/gpu/drm/i915/gt/uc/abi/guc_actions_abi.h +.. kernel-doc:: drivers/gpu/drm/i915/gt/uc/abi/guc_klvs_abi.h HuC --- diff --git a/drivers/gpu/drm/i915/gt/uc/abi/guc_actions_abi.h b/drivers/gpu/drm/i915/gt/uc/abi/guc_actions_abi.h index fe5d7d261797..7afdadc7656f 100644 --- a/drivers/gpu/drm/i915/gt/uc/abi/guc_actions_abi.h +++ b/drivers/gpu/drm/i915/gt/uc/abi/guc_actions_abi.h @@ -7,9 +7,9 @@ #define _ABI_GUC_ACTIONS_ABI_H /** - * DOC: HOST2GUC_REGISTER_CTB + * DOC: HOST2GUC_SELF_CFG * - * This message is used as part of the `CTB based communication`_ setup. + * This message is used by Host KMD to setup of the `GuC Self Config KLVs`_. * * This message must be sent as `MMIO HXG Message`_. * @@ -22,20 +22,18 @@ * | +---+--+ * | | 27:16 | DATA0 = MBZ | * | +---+--+ - * | | 15:0 | ACTION = _`GUC_ACTION_HOST2GUC_REGISTER_CTB` = 0x4505 | + * | | 15:0 | ACTION = _`GUC_ACTION_HOST2GUC_SELF_CFG` = 0x0508 | * +---+---+--+ - * | 1 | 31:12 | RESERVED = MBZ | + * | 1 | 31:16 | **KLV_KEY** - KLV key, see `GuC Self Config KLVs`_ | * | +---+--+ - * | | 11:8 | **TYPE** - type for the `CT Buffer`_ | + * | | 15:0 | **KLV_LEN** - KLV length | * | | | | - * | | | - _`GUC_CTB_TYPE_HOST2GUC` = 0
[PATCH 0/3] Update to GuC version 69.0.3
From: John Harrison Update to the latest GuC version. This includes a suite of interface changes and new features with corresponding i915 side changes. Signed-off-by: John Harrison John Harrison (3): drm/i915/guc: Temporarily bump the GuC load timeout drm/i915/guc: Update to GuC version 69.0.3 drm/i915/guc: Improve GuC loading status check/error reports Documentation/gpu/i915.rst| 1 + .../gpu/drm/i915/gt/uc/abi/guc_actions_abi.h | 80 +- .../gpu/drm/i915/gt/uc/abi/guc_errors_abi.h | 23 +++ drivers/gpu/drm/i915/gt/uc/abi/guc_klvs_abi.h | 82 ++ drivers/gpu/drm/i915/gt/uc/intel_guc.c| 126 +--- drivers/gpu/drm/i915/gt/uc/intel_guc.h| 4 + drivers/gpu/drm/i915/gt/uc/intel_guc_ads.c| 45 +- drivers/gpu/drm/i915/gt/uc/intel_guc_ct.c | 141 ++ drivers/gpu/drm/i915/gt/uc/intel_guc_fw.c | 30 ++-- drivers/gpu/drm/i915/gt/uc/intel_guc_fwif.h | 37 - drivers/gpu/drm/i915/gt/uc/intel_guc_log.c| 31 ++-- drivers/gpu/drm/i915/gt/uc/intel_guc_log.h| 3 + drivers/gpu/drm/i915/gt/uc/intel_guc_reg.h| 4 - .../gpu/drm/i915/gt/uc/intel_guc_submission.c | 18 +++ drivers/gpu/drm/i915/gt/uc/intel_huc.c| 1 + drivers/gpu/drm/i915/gt/uc/intel_uc.c | 31 ++-- drivers/gpu/drm/i915/gt/uc/intel_uc_fw.c | 30 ++-- 17 files changed, 493 insertions(+), 194 deletions(-) create mode 100644 drivers/gpu/drm/i915/gt/uc/abi/guc_klvs_abi.h -- 2.25.1
[PATCH v4 4/4] drm/i915: Require the vm mutex for i915_vma_bind()
Protect updates of struct i915_vma flags and async binding / unbinding
with the vm::mutex. This means that i915_vma_bind() needs to assert
vm::mutex held. In order to make that possible drop the caching of
kmap_atomic() maps around i915_vma_bind().
An alternative would be to use kmap_local() but since we block cpu
unplugging during sleeps inside kmap_local() sections this may have
unwanted side-effects. Particularly since we might wait for gpu while
holding the vm mutex.
This change may theoretically increase execbuf cpu-usage on snb, but
at least on non-highmem systems that increase should be very small.
Signed-off-by: Thomas Hellström
Reviewed-by: Matthew Auld
---
.../gpu/drm/i915/gem/i915_gem_execbuffer.c| 50 ++-
drivers/gpu/drm/i915/i915_vma.c | 1 +
2 files changed, 50 insertions(+), 1 deletion(-)
diff --git a/drivers/gpu/drm/i915/gem/i915_gem_execbuffer.c
b/drivers/gpu/drm/i915/gem/i915_gem_execbuffer.c
index ec7c4a29a720..b8330f0bf652 100644
--- a/drivers/gpu/drm/i915/gem/i915_gem_execbuffer.c
+++ b/drivers/gpu/drm/i915/gem/i915_gem_execbuffer.c
@@ -1109,6 +1109,47 @@ static inline struct i915_ggtt *cache_to_ggtt(struct
reloc_cache *cache)
return >ggtt;
}
+static void reloc_cache_unmap(struct reloc_cache *cache)
+{
+ void *vaddr;
+
+ if (!cache->vaddr)
+ return;
+
+ vaddr = unmask_page(cache->vaddr);
+ if (cache->vaddr & KMAP)
+ kunmap_atomic(vaddr);
+ else
+ io_mapping_unmap_atomic((void __iomem *)vaddr);
+}
+
+static void reloc_cache_remap(struct reloc_cache *cache,
+ struct drm_i915_gem_object *obj)
+{
+ void *vaddr;
+
+ if (!cache->vaddr)
+ return;
+
+ if (cache->vaddr & KMAP) {
+ struct page *page = i915_gem_object_get_page(obj, cache->page);
+
+ vaddr = kmap_atomic(page);
+ cache->vaddr = unmask_flags(cache->vaddr) |
+ (unsigned long)vaddr;
+ } else {
+ struct i915_ggtt *ggtt = cache_to_ggtt(cache);
+ unsigned long offset;
+
+ offset = cache->node.start;
+ if (!drm_mm_node_allocated(>node))
+ offset += cache->page << PAGE_SHIFT;
+
+ cache->vaddr = (unsigned long)
+ io_mapping_map_atomic_wc(>iomap, offset);
+ }
+}
+
static void reloc_cache_reset(struct reloc_cache *cache, struct
i915_execbuffer *eb)
{
void *vaddr;
@@ -1373,10 +1414,17 @@ eb_relocate_entry(struct i915_execbuffer *eb,
* batchbuffers.
*/
if (reloc->write_domain == I915_GEM_DOMAIN_INSTRUCTION &&
- GRAPHICS_VER(eb->i915) == 6) {
+ GRAPHICS_VER(eb->i915) == 6 &&
+ !i915_vma_is_bound(target->vma, I915_VMA_GLOBAL_BIND)) {
+ struct i915_vma *vma = target->vma;
+
+ reloc_cache_unmap(>reloc_cache);
+ mutex_lock(>vm->mutex);
err = i915_vma_bind(target->vma,
target->vma->obj->cache_level,
PIN_GLOBAL, NULL);
+ mutex_unlock(>vm->mutex);
+ reloc_cache_remap(>reloc_cache, ev->vma->obj);
if (err)
return err;
}
diff --git a/drivers/gpu/drm/i915/i915_vma.c b/drivers/gpu/drm/i915/i915_vma.c
index de24e4b3b19b..be208a8f1ed0 100644
--- a/drivers/gpu/drm/i915/i915_vma.c
+++ b/drivers/gpu/drm/i915/i915_vma.c
@@ -393,6 +393,7 @@ int i915_vma_bind(struct i915_vma *vma,
u32 bind_flags;
u32 vma_flags;
+ lockdep_assert_held(>vm->mutex);
GEM_BUG_ON(!drm_mm_node_allocated(>node));
GEM_BUG_ON(vma->size > vma->node.size);
--
2.31.1
[PATCH v4 3/4] drm/i915: Break out the i915_deps utility
Since it's starting to be used outside the i915 TTM move code, move it
to a separate set of files.
v2:
- Update the documentation.
v4:
- Rebase.
Signed-off-by: Thomas Hellström
Reviewed-by: Matthew Auld
---
drivers/gpu/drm/i915/Makefile| 1 +
drivers/gpu/drm/i915/gem/i915_gem_ttm_move.c | 171 +
drivers/gpu/drm/i915/gem/i915_gem_ttm_move.h | 17 --
drivers/gpu/drm/i915/i915_deps.c | 237 +++
drivers/gpu/drm/i915/i915_deps.h | 45
drivers/gpu/drm/i915/i915_request.c | 2 +-
6 files changed, 285 insertions(+), 188 deletions(-)
create mode 100644 drivers/gpu/drm/i915/i915_deps.c
create mode 100644 drivers/gpu/drm/i915/i915_deps.h
diff --git a/drivers/gpu/drm/i915/Makefile b/drivers/gpu/drm/i915/Makefile
index 6ddd2d2bbaaf..1b62b9f65196 100644
--- a/drivers/gpu/drm/i915/Makefile
+++ b/drivers/gpu/drm/i915/Makefile
@@ -163,6 +163,7 @@ i915-y += \
i915_active.o \
i915_buddy.o \
i915_cmd_parser.o \
+ i915_deps.o \
i915_gem_evict.o \
i915_gem_gtt.o \
i915_gem_ww.o \
diff --git a/drivers/gpu/drm/i915/gem/i915_gem_ttm_move.c
b/drivers/gpu/drm/i915/gem/i915_gem_ttm_move.c
index 5dbaccf3f201..ee9612a3ee5e 100644
--- a/drivers/gpu/drm/i915/gem/i915_gem_ttm_move.c
+++ b/drivers/gpu/drm/i915/gem/i915_gem_ttm_move.c
@@ -5,6 +5,7 @@
#include
+#include "i915_deps.h"
#include "i915_drv.h"
#include "intel_memory_region.h"
#include "intel_region_ttm.h"
@@ -41,176 +42,6 @@ void i915_ttm_migrate_set_failure_modes(bool gpu_migration,
}
#endif
-/**
- * DOC: Set of utilities to dynamically collect dependencies into a
- * structure which is fed into the GT migration code.
- *
- * Once we can do async unbinding, this is also needed to coalesce
- * the migration fence with the unbind fences if these are coalesced
- * post-migration.
- *
- * While collecting the individual dependencies, we store the refcounted
- * struct dma_fence pointers in a realloc-managed pointer array, since
- * that can be easily fed into a dma_fence_array. Other options are
- * available, like for example an xarray for similarity with drm/sched.
- * Can be changed easily if needed.
- *
- * A struct i915_deps need to be initialized using i915_deps_init().
- * If i915_deps_add_dependency() or i915_deps_add_resv() return an
- * error code they will internally call i915_deps_fini(), which frees
- * all internal references and allocations.
- *
- * We might want to break this out into a separate file as a utility.
- */
-
-#define I915_DEPS_MIN_ALLOC_CHUNK 8U
-
-static void i915_deps_reset_fences(struct i915_deps *deps)
-{
- if (deps->fences != >single)
- kfree(deps->fences);
- deps->num_deps = 0;
- deps->fences_size = 1;
- deps->fences = >single;
-}
-
-static void i915_deps_init(struct i915_deps *deps, gfp_t gfp)
-{
- deps->fences = NULL;
- deps->gfp = gfp;
- i915_deps_reset_fences(deps);
-}
-
-static void i915_deps_fini(struct i915_deps *deps)
-{
- unsigned int i;
-
- for (i = 0; i < deps->num_deps; ++i)
- dma_fence_put(deps->fences[i]);
-
- if (deps->fences != >single)
- kfree(deps->fences);
-}
-
-static int i915_deps_grow(struct i915_deps *deps, struct dma_fence *fence,
- const struct ttm_operation_ctx *ctx)
-{
- int ret;
-
- if (deps->num_deps >= deps->fences_size) {
- unsigned int new_size = 2 * deps->fences_size;
- struct dma_fence **new_fences;
-
- new_size = max(new_size, I915_DEPS_MIN_ALLOC_CHUNK);
- new_fences = kmalloc_array(new_size, sizeof(*new_fences),
deps->gfp);
- if (!new_fences)
- goto sync;
-
- memcpy(new_fences, deps->fences,
- deps->fences_size * sizeof(*new_fences));
- swap(new_fences, deps->fences);
- if (new_fences != >single)
- kfree(new_fences);
- deps->fences_size = new_size;
- }
- deps->fences[deps->num_deps++] = dma_fence_get(fence);
- return 0;
-
-sync:
- if (ctx->no_wait_gpu && !dma_fence_is_signaled(fence)) {
- ret = -EBUSY;
- goto unref;
- }
-
- ret = dma_fence_wait(fence, ctx->interruptible);
- if (ret)
- goto unref;
-
- ret = fence->error;
- if (ret)
- goto unref;
-
- return 0;
-
-unref:
- i915_deps_fini(deps);
- return ret;
-}
-
-static int i915_deps_sync(const struct i915_deps *deps,
- const struct ttm_operation_ctx *ctx)
-{
- struct dma_fence **fences = deps->fences;
- unsigned int i;
- int ret = 0;
-
- for (i = 0; i < deps->num_deps; ++i, ++fences) {
- if (ctx->no_wait_gpu && !dma_fence_is_signaled(*fences)) {
- ret =
[PATCH v4 2/4] drm/i915: remove questionable fence optimization during copy
From: Christian König
First of all as discussed multiple times now kernel copies *must* always
wait for all fences in a BO before actually doing the copy. This is
mandatory.
Additional to that drop the handling when there can't be a shared slot
allocated on the source BO and just properly return an error code.
Otherwise this code path would only be tested under out of memory
conditions.
Signed-off-by: Christian König
Reviewed-by: Thomas Hellström
---
drivers/gpu/drm/i915/gem/i915_gem_ttm_move.c | 43 +++-
1 file changed, 14 insertions(+), 29 deletions(-)
diff --git a/drivers/gpu/drm/i915/gem/i915_gem_ttm_move.c
b/drivers/gpu/drm/i915/gem/i915_gem_ttm_move.c
index 62a6fd2d127d..5dbaccf3f201 100644
--- a/drivers/gpu/drm/i915/gem/i915_gem_ttm_move.c
+++ b/drivers/gpu/drm/i915/gem/i915_gem_ttm_move.c
@@ -195,19 +195,14 @@ static int i915_deps_add_dependency(struct i915_deps
*deps,
}
static int i915_deps_add_resv(struct i915_deps *deps, struct dma_resv *resv,
- bool all, const bool no_excl,
const struct ttm_operation_ctx *ctx)
{
struct dma_resv_iter iter;
struct dma_fence *fence;
+ int ret;
dma_resv_assert_held(resv);
- dma_resv_for_each_fence(, resv, all, fence) {
- int ret;
-
- if (no_excl && dma_resv_iter_is_exclusive())
- continue;
-
+ dma_resv_for_each_fence(, resv, true, fence) {
ret = i915_deps_add_dependency(deps, fence, ctx);
if (ret)
return ret;
@@ -634,11 +629,7 @@ prev_deps(struct ttm_buffer_object *bo, struct
ttm_operation_ctx *ctx,
ret = i915_deps_add_dependency(deps, bo->moving, ctx);
if (!ret)
- /*
-* TODO: Only await excl fence here, and shared fences before
-* signaling the migration fence.
-*/
- ret = i915_deps_add_resv(deps, bo->base.resv, true, false, ctx);
+ ret = i915_deps_add_resv(deps, bo->base.resv, ctx);
return ret;
}
@@ -768,22 +759,21 @@ int i915_gem_obj_copy_ttm(struct drm_i915_gem_object *dst,
struct i915_refct_sgt *dst_rsgt;
struct dma_fence *copy_fence;
struct i915_deps deps;
- int ret, shared_err;
+ int ret;
assert_object_held(dst);
assert_object_held(src);
i915_deps_init(, GFP_KERNEL | __GFP_NORETRY | __GFP_NOWARN);
- /*
-* We plan to add a shared fence only for the source. If that
-* fails, we await all source fences before commencing
-* the copy instead of only the exclusive.
-*/
- shared_err = dma_resv_reserve_shared(src_bo->base.resv, 1);
- ret = i915_deps_add_resv(, dst_bo->base.resv, true, false, );
- if (!ret)
- ret = i915_deps_add_resv(, src_bo->base.resv,
-!!shared_err, false, );
+ ret = dma_resv_reserve_shared(src_bo->base.resv, 1);
+ if (ret)
+ return ret;
+
+ ret = i915_deps_add_resv(, dst_bo->base.resv, );
+ if (ret)
+ return ret;
+
+ ret = i915_deps_add_resv(, src_bo->base.resv, );
if (ret)
return ret;
@@ -798,12 +788,7 @@ int i915_gem_obj_copy_ttm(struct drm_i915_gem_object *dst,
return PTR_ERR_OR_ZERO(copy_fence);
dma_resv_add_excl_fence(dst_bo->base.resv, copy_fence);
-
- /* If we failed to reserve a shared slot, add an exclusive fence */
- if (shared_err)
- dma_resv_add_excl_fence(src_bo->base.resv, copy_fence);
- else
- dma_resv_add_shared_fence(src_bo->base.resv, copy_fence);
+ dma_resv_add_shared_fence(src_bo->base.resv, copy_fence);
dma_fence_put(copy_fence);
--
2.31.1
[PATCH v4 1/4] drm/i915: Avoid using the i915_fence_array when collecting dependencies
Since the gt migration code was using only a single fence for
dependencies, these were collected in a dma_fence_array. However, it
turns out that it's illegal to use some dma_fences in a dma_fence_array,
in particular other dma_fence_arrays and dma_fence_chains, and this
causes trouble for us moving forward.
Have the gt migration code instead take a const struct i915_deps for
dependencies. This means we can skip the dma_fence_array creation
and instead pass the struct i915_deps instead to circumvent the
problem.
v2:
- Make the prev_deps() function static. (kernel test robot )
- Update the struct i915_deps kerneldoc.
v4:
- Rebase.
Fixes: 5652df829b3c ("drm/i915/ttm: Update i915_gem_obj_copy_ttm() to be
asynchronous")
Signed-off-by: Thomas Hellström
Reviewed-by: Matthew Auld
---
drivers/gpu/drm/i915/gem/i915_gem_ttm_move.c | 129 +--
drivers/gpu/drm/i915/gem/i915_gem_ttm_move.h | 17 +++
drivers/gpu/drm/i915/gt/intel_migrate.c | 24 ++--
drivers/gpu/drm/i915/gt/intel_migrate.h | 9 +-
drivers/gpu/drm/i915/i915_request.c | 22
drivers/gpu/drm/i915/i915_request.h | 2 +
6 files changed, 91 insertions(+), 112 deletions(-)
diff --git a/drivers/gpu/drm/i915/gem/i915_gem_ttm_move.c
b/drivers/gpu/drm/i915/gem/i915_gem_ttm_move.c
index 8ad09fcf3698..62a6fd2d127d 100644
--- a/drivers/gpu/drm/i915/gem/i915_gem_ttm_move.c
+++ b/drivers/gpu/drm/i915/gem/i915_gem_ttm_move.c
@@ -3,8 +3,6 @@
* Copyright © 2021 Intel Corporation
*/
-#include
-
#include
#include "i915_drv.h"
@@ -44,17 +42,12 @@ void i915_ttm_migrate_set_failure_modes(bool gpu_migration,
#endif
/**
- * DOC: Set of utilities to dynamically collect dependencies and
- * eventually coalesce them into a single fence which is fed into
- * the GT migration code, since it only accepts a single dependency
- * fence.
- * The single fence returned from these utilities, in the case of
- * dependencies from multiple fence contexts, a struct dma_fence_array,
- * since the i915 request code can break that up and await the individual
- * fences.
+ * DOC: Set of utilities to dynamically collect dependencies into a
+ * structure which is fed into the GT migration code.
*
* Once we can do async unbinding, this is also needed to coalesce
- * the migration fence with the unbind fences.
+ * the migration fence with the unbind fences if these are coalesced
+ * post-migration.
*
* While collecting the individual dependencies, we store the refcounted
* struct dma_fence pointers in a realloc-managed pointer array, since
@@ -65,32 +58,13 @@ void i915_ttm_migrate_set_failure_modes(bool gpu_migration,
* A struct i915_deps need to be initialized using i915_deps_init().
* If i915_deps_add_dependency() or i915_deps_add_resv() return an
* error code they will internally call i915_deps_fini(), which frees
- * all internal references and allocations. After a call to
- * i915_deps_to_fence(), or i915_deps_sync(), the struct should similarly
- * be viewed as uninitialized.
+ * all internal references and allocations.
*
* We might want to break this out into a separate file as a utility.
*/
#define I915_DEPS_MIN_ALLOC_CHUNK 8U
-/**
- * struct i915_deps - Collect dependencies into a single dma-fence
- * @single: Storage for pointer if the collection is a single fence.
- * @fence: Allocated array of fence pointers if more than a single fence;
- * otherwise points to the address of @single.
- * @num_deps: Current number of dependency fences.
- * @fences_size: Size of the @fences array in number of pointers.
- * @gfp: Allocation mode.
- */
-struct i915_deps {
- struct dma_fence *single;
- struct dma_fence **fences;
- unsigned int num_deps;
- unsigned int fences_size;
- gfp_t gfp;
-};
-
static void i915_deps_reset_fences(struct i915_deps *deps)
{
if (deps->fences != >single)
@@ -163,7 +137,7 @@ static int i915_deps_grow(struct i915_deps *deps, struct
dma_fence *fence,
return ret;
}
-static int i915_deps_sync(struct i915_deps *deps,
+static int i915_deps_sync(const struct i915_deps *deps,
const struct ttm_operation_ctx *ctx)
{
struct dma_fence **fences = deps->fences;
@@ -183,7 +157,6 @@ static int i915_deps_sync(struct i915_deps *deps,
break;
}
- i915_deps_fini(deps);
return ret;
}
@@ -221,34 +194,6 @@ static int i915_deps_add_dependency(struct i915_deps *deps,
return i915_deps_grow(deps, fence, ctx);
}
-static struct dma_fence *i915_deps_to_fence(struct i915_deps *deps,
- const struct ttm_operation_ctx *ctx)
-{
- struct dma_fence_array *array;
-
- if (deps->num_deps == 0)
- return NULL;
-
- if (deps->num_deps == 1) {
- deps->num_deps = 0;
- return deps->fences[0];
- }
-
- /*
-* TODO: Alter the allocation mode here to not try
[PATCH v4 0/4] drm/i915: Asynchronous vma unbinding part1
This is the first three already reviewed patches from the patch series titled "Asynchronous vma unbinding", with an additional cleanup patch from Christian, which would otherwise conflict heavily with this series. Christian König (1): drm/i915: remove questionable fence optimization during copy Thomas Hellström (3): drm/i915: Avoid using the i915_fence_array when collecting dependencies drm/i915: Break out the i915_deps utility drm/i915: Require the vm mutex for i915_vma_bind() drivers/gpu/drm/i915/Makefile | 1 + .../gpu/drm/i915/gem/i915_gem_execbuffer.c| 50 ++- drivers/gpu/drm/i915/gem/i915_gem_ttm_move.c | 323 +++--- drivers/gpu/drm/i915/gt/intel_migrate.c | 24 +- drivers/gpu/drm/i915/gt/intel_migrate.h | 9 +- drivers/gpu/drm/i915/i915_deps.c | 237 + drivers/gpu/drm/i915/i915_deps.h | 45 +++ drivers/gpu/drm/i915/i915_request.c | 22 ++ drivers/gpu/drm/i915/i915_request.h | 2 + drivers/gpu/drm/i915/i915_vma.c | 1 + 10 files changed, 412 insertions(+), 302 deletions(-) create mode 100644 drivers/gpu/drm/i915/i915_deps.c create mode 100644 drivers/gpu/drm/i915/i915_deps.h -- 2.31.1
[PATCH v10 1/6] drm/i915/gt: Use to_gt() helper for GGTT accesses
From: Michał Winiarski
GGTT is currently available both through i915->ggtt and gt->ggtt, and we
eventually want to get rid of the i915->ggtt one.
Use to_gt() for all i915->ggtt accesses to help with the future
refactoring.
During the probe of i915 the early intiialization of the gt
(intel_gt_init_hw_early()) is moved prior to any access to the
ggtt. This because it's in that moment we assign the ggtt to the
gt and we want to do that before using it.
Signed-off-by: Michał Winiarski
Cc: Michal Wajdeczko
Signed-off-by: Andi Shyti
Reviewed-by: Sujaritha Sundaresan
Reviewed-by: Matt Roper
---
Hi Matt,
just added a small paragraph in this commit log to highlight the
even erlier early initialization of the gt.
Andi
drivers/gpu/drm/i915/gt/intel_ggtt.c | 14 +++---
drivers/gpu/drm/i915/gt/intel_ggtt_fencing.c | 6 +++---
drivers/gpu/drm/i915/gt/intel_region_lmem.c | 4 ++--
drivers/gpu/drm/i915/gt/selftest_reset.c | 2 +-
drivers/gpu/drm/i915/i915_driver.c | 4 ++--
5 files changed, 15 insertions(+), 15 deletions(-)
diff --git a/drivers/gpu/drm/i915/gt/intel_ggtt.c
b/drivers/gpu/drm/i915/gt/intel_ggtt.c
index 5263dda7f8d5..ab6c4322dc08 100644
--- a/drivers/gpu/drm/i915/gt/intel_ggtt.c
+++ b/drivers/gpu/drm/i915/gt/intel_ggtt.c
@@ -86,7 +86,7 @@ int i915_ggtt_init_hw(struct drm_i915_private *i915)
* beyond the end of the batch buffer, across the page boundary,
* and beyond the end of the GTT if we do not provide a guard.
*/
- ret = ggtt_init_hw(>ggtt);
+ ret = ggtt_init_hw(to_gt(i915)->ggtt);
if (ret)
return ret;
@@ -722,14 +722,14 @@ int i915_init_ggtt(struct drm_i915_private *i915)
{
int ret;
- ret = init_ggtt(>ggtt);
+ ret = init_ggtt(to_gt(i915)->ggtt);
if (ret)
return ret;
if (INTEL_PPGTT(i915) == INTEL_PPGTT_ALIASING) {
- ret = init_aliasing_ppgtt(>ggtt);
+ ret = init_aliasing_ppgtt(to_gt(i915)->ggtt);
if (ret)
- cleanup_init_ggtt(>ggtt);
+ cleanup_init_ggtt(to_gt(i915)->ggtt);
}
return 0;
@@ -772,7 +772,7 @@ static void ggtt_cleanup_hw(struct i915_ggtt *ggtt)
*/
void i915_ggtt_driver_release(struct drm_i915_private *i915)
{
- struct i915_ggtt *ggtt = >ggtt;
+ struct i915_ggtt *ggtt = to_gt(i915)->ggtt;
fini_aliasing_ppgtt(ggtt);
@@ -787,7 +787,7 @@ void i915_ggtt_driver_release(struct drm_i915_private *i915)
*/
void i915_ggtt_driver_late_release(struct drm_i915_private *i915)
{
- struct i915_ggtt *ggtt = >ggtt;
+ struct i915_ggtt *ggtt = to_gt(i915)->ggtt;
GEM_WARN_ON(kref_read(>vm.resv_ref) != 1);
dma_resv_fini(>vm._resv);
@@ -1208,7 +1208,7 @@ int i915_ggtt_probe_hw(struct drm_i915_private *i915)
{
int ret;
- ret = ggtt_probe_hw(>ggtt, to_gt(i915));
+ ret = ggtt_probe_hw(to_gt(i915)->ggtt, to_gt(i915));
if (ret)
return ret;
diff --git a/drivers/gpu/drm/i915/gt/intel_ggtt_fencing.c
b/drivers/gpu/drm/i915/gt/intel_ggtt_fencing.c
index f8948de72036..beabf3bc9b75 100644
--- a/drivers/gpu/drm/i915/gt/intel_ggtt_fencing.c
+++ b/drivers/gpu/drm/i915/gt/intel_ggtt_fencing.c
@@ -728,8 +728,8 @@ static void detect_bit_6_swizzle(struct i915_ggtt *ggtt)
swizzle_y = I915_BIT_6_SWIZZLE_NONE;
}
- i915->ggtt.bit_6_swizzle_x = swizzle_x;
- i915->ggtt.bit_6_swizzle_y = swizzle_y;
+ to_gt(i915)->ggtt->bit_6_swizzle_x = swizzle_x;
+ to_gt(i915)->ggtt->bit_6_swizzle_y = swizzle_y;
}
/*
@@ -896,7 +896,7 @@ void intel_gt_init_swizzling(struct intel_gt *gt)
struct intel_uncore *uncore = gt->uncore;
if (GRAPHICS_VER(i915) < 5 ||
- i915->ggtt.bit_6_swizzle_x == I915_BIT_6_SWIZZLE_NONE)
+ to_gt(i915)->ggtt->bit_6_swizzle_x == I915_BIT_6_SWIZZLE_NONE)
return;
intel_uncore_rmw(uncore, DISP_ARB_CTL, 0, DISP_TILE_SURFACE_SWIZZLING);
diff --git a/drivers/gpu/drm/i915/gt/intel_region_lmem.c
b/drivers/gpu/drm/i915/gt/intel_region_lmem.c
index fde2dcb59809..21215a080088 100644
--- a/drivers/gpu/drm/i915/gt/intel_region_lmem.c
+++ b/drivers/gpu/drm/i915/gt/intel_region_lmem.c
@@ -15,7 +15,7 @@
static int init_fake_lmem_bar(struct intel_memory_region *mem)
{
struct drm_i915_private *i915 = mem->i915;
- struct i915_ggtt *ggtt = >ggtt;
+ struct i915_ggtt *ggtt = to_gt(i915)->ggtt;
unsigned long n;
int ret;
@@ -131,7 +131,7 @@ intel_gt_setup_fake_lmem(struct intel_gt *gt)
if (!i915->params.fake_lmem_start)
return ERR_PTR(-ENODEV);
- GEM_BUG_ON(i915_ggtt_has_aperture(>ggtt));
+ GEM_BUG_ON(i915_ggtt_has_aperture(to_gt(i915)->ggtt));
/* Your mappable aperture belongs to me now! */
mappable_end = pci_resource_len(pdev, 2);
diff --git
Re: [PATCH] dt-bindings: display: bridge: lvds-codec: Document TI DS90CF364A decoder
On Sat, 18 Dec 2021 16:23:09 +0100, Marek Vasut wrote: > Add compatible string for TI DS90CF364A, which is another LVDS to DPI > decoder similar to DS90CF384A, except it is using smaller package and > only provides 18bit DPI bus. > > Signed-off-by: Marek Vasut > Cc: Laurent Pinchart > Cc: Rob Herring > Cc: Sam Ravnborg > Cc: devicet...@vger.kernel.org > To: dri-devel@lists.freedesktop.org > --- > Documentation/devicetree/bindings/display/bridge/lvds-codec.yaml | 1 + > 1 file changed, 1 insertion(+) > Acked-by: Rob Herring
Re: [PATCH v9 2/6] drm/i915: Use to_gt() helper for GGTT accesses
Hi Matt,
> > diff --git a/drivers/gpu/drm/i915/i915_perf.c
> > b/drivers/gpu/drm/i915/i915_perf.c
> > index 170bba913c30..128315aec517 100644
> > --- a/drivers/gpu/drm/i915/i915_perf.c
> > +++ b/drivers/gpu/drm/i915/i915_perf.c
> > @@ -1630,7 +1630,7 @@ static int alloc_noa_wait(struct i915_perf_stream
> > *stream)
> > struct drm_i915_gem_object *bo;
> > struct i915_vma *vma;
> > const u64 delay_ticks = 0x -
> > - intel_gt_ns_to_clock_interval(stream->perf->i915->ggtt.vm.gt,
> > +
> > intel_gt_ns_to_clock_interval(to_gt(stream->perf->i915)->ggtt->vm.gt,
>
> I'm not too familiar with the perf code, but this looks a bit roundabout
> since we're ultimately trying to get to a GT...do we even need to go
> through the ggtt structure here or can we just pass
> "to_gt(stream->perf->i915)" as the first parameter?
>
> >
> > atomic64_read(>perf->noa_programming_delay));
> > const u32 base = stream->engine->mmio_base;
> > #define CS_GPR(x) GEN8_RING_CS_GPR(base, x)
> > @@ -3542,7 +3542,7 @@ i915_perf_open_ioctl_locked(struct i915_perf *perf,
> >
> > static u64 oa_exponent_to_ns(struct i915_perf *perf, int exponent)
> > {
> > - return intel_gt_clock_interval_to_ns(perf->i915->ggtt.vm.gt,
> > + return intel_gt_clock_interval_to_ns(to_gt(perf->i915)->ggtt->vm.gt,
>
> Ditto; this looks like "to_gt(perf->i915)" might be all we need?
I think this function is looking for the GT coming from the VM,
otherwise originally it could have taken it from >gt. In my
first version I proposed a wrapper around this but it was
rejected by Lucas.
Besides, as we discussed earlier when I was proposed the static
allocation, the ggtt might not always be linked to the same gt,
so that I assumed that sometimes:
to_gt(perf->i915)->ggtt->vm.gt != to_gt(perf->i915)
if two GTs are sharing the same ggtt, what would the ggtt->vm.gt
link be?
Thanks,
Andi
Re: [PATCH 2/2] dt-bindings: leds: lp855x: Convert to json-schema
On Fri, Dec 17, 2021 at 06:07:15PM +0100, Thierry Reding wrote:
> From: Thierry Reding
>
> Convert the Texas Instruments LP855x backlight device tree bindings from
> the free-form text format to json-schema.
>
> Signed-off-by: Thierry Reding
> ---
> .../bindings/leds/backlight/lp855x.txt| 72 -
> .../bindings/leds/backlight/ti,lp8550.yaml| 151 ++
> 2 files changed, 151 insertions(+), 72 deletions(-)
> delete mode 100644
> Documentation/devicetree/bindings/leds/backlight/lp855x.txt
> create mode 100644
> Documentation/devicetree/bindings/leds/backlight/ti,lp8550.yaml
>
> diff --git a/Documentation/devicetree/bindings/leds/backlight/lp855x.txt
> b/Documentation/devicetree/bindings/leds/backlight/lp855x.txt
> deleted file mode 100644
> index 88f56641fc28..
> --- a/Documentation/devicetree/bindings/leds/backlight/lp855x.txt
> +++ /dev/null
> @@ -1,72 +0,0 @@
> -lp855x bindings
> -
> -Required properties:
> - - compatible: "ti,lp8550", "ti,lp8551", "ti,lp8552", "ti,lp8553",
> -"ti,lp8555", "ti,lp8556", "ti,lp8557"
> - - reg: I2C slave address (u8)
> - - dev-ctrl: Value of DEVICE CONTROL register (u8). It depends on the
> device.
> -
> -Optional properties:
> - - bl-name: Backlight device name (string)
> - - init-brt: Initial value of backlight brightness (u8)
> - - pwm-period: PWM period value. Set only PWM input mode used (u32)
> - - rom-addr: Register address of ROM area to be updated (u8)
> - - rom-val: Register value to be updated (u8)
> - - power-supply: Regulator which controls the 3V rail
> - - enable-supply: Regulator which controls the EN/VDDIO input
> -
> -Example:
> -
> - /* LP8555 */
> - backlight@2c {
> - compatible = "ti,lp8555";
> - reg = <0x2c>;
> -
> - dev-ctrl = /bits/ 8 <0x00>;
> - pwm-period = <1>;
> -
> - /* 4V OV, 4 output LED0 string enabled */
> - rom_14h {
> - rom-addr = /bits/ 8 <0x14>;
> - rom-val = /bits/ 8 <0xcf>;
> - };
> -
> - /* Heavy smoothing, 24ms ramp time step */
> - rom_15h {
> - rom-addr = /bits/ 8 <0x15>;
> - rom-val = /bits/ 8 <0xc7>;
> - };
> -
> - /* 4 output LED1 string enabled */
> - rom_19h {
> - rom-addr = /bits/ 8 <0x19>;
> - rom-val = /bits/ 8 <0x0f>;
> - };
> - };
> -
> - /* LP8556 */
> - backlight@2c {
> - compatible = "ti,lp8556";
> - reg = <0x2c>;
> -
> - bl-name = "lcd-bl";
> - dev-ctrl = /bits/ 8 <0x85>;
> - init-brt = /bits/ 8 <0x10>;
> - };
> -
> - /* LP8557 */
> - backlight@2c {
> - compatible = "ti,lp8557";
> - reg = <0x2c>;
> - enable-supply = <_vddio>;
> - power-supply = <_vdd>;
> -
> - dev-ctrl = /bits/ 8 <0x41>;
> - init-brt = /bits/ 8 <0x0a>;
> -
> - /* 4V OV, 4 output LED string enabled */
> - rom_14h {
> - rom-addr = /bits/ 8 <0x14>;
> - rom-val = /bits/ 8 <0xcf>;
> - };
> - };
> diff --git a/Documentation/devicetree/bindings/leds/backlight/ti,lp8550.yaml
> b/Documentation/devicetree/bindings/leds/backlight/ti,lp8550.yaml
> new file mode 100644
> index ..412779a5462b
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/leds/backlight/ti,lp8550.yaml
> @@ -0,0 +1,151 @@
> +# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause)
> +%YAML 1.2
> +---
> +$id: http://devicetree.org/schemas/leds/backlight/ti,lp8550.yaml#
> +$schema: http://devicetree.org/meta-schemas/core.yaml#
> +
> +title: Texas Instruments LP855x family devicetree bindings
> +
> +maintainers:
> + - Milo Kim
> + - Rob Herring
I don't know anything about this h/w.
> +
> +properties:
> + compatible:
> +enum:
> + - ti,lp8550
> + - ti,lp8551
> + - ti,lp8552
> + - ti,lp8553
> + - ti,lp8555
> + - ti,lp8556
> + - ti,lp8557
> +
> + reg:
> +maxItems: 1
> +
> + dev-ctrl:
> +$ref: /schemas/types.yaml#/definitions/uint8
> +description: Value of DEVICE CONTROL register. It depends on the device.
> +
> + bl-name:
> +$ref: /schemas/types.yaml#/definitions/string
> +description: Backlight device name
> +
> + init-brt:
> +$ref: /schemas/types.yaml#/definitions/uint8
> +description: Initial value of backlight brightness
> +
> + pwm-period:
> +$ref: /schemas/types.yaml#/definitions/uint32
> +description: PWM period value. Set only PWM input mode used
> +
> + pwm-names:
> +maxItems: 1
> +
> + pwms:
> +maxItems: 1
> +
> + power-supply:
> +description: Regulator which controls the 3V rail
> +
> + enable-supply:
> +description: Regulator which controls the EN/VDDIO input
> +
>
Re: [PATCH 1/2] dt-bindings: leds: Document rfkill* trigger
On Fri, 17 Dec 2021 18:07:14 +0100, Thierry Reding wrote: > From: Thierry Reding > > LEDs can use rfkill events as a trigger source, so document these in the > device tree bindings. > > Signed-off-by: Thierry Reding > --- > .../devicetree/bindings/leds/common.yaml| 17 + > 1 file changed, 9 insertions(+), 8 deletions(-) > Reviewed-by: Rob Herring
[PATCH 2/2] drm/dbi: Use a static inline stub for mipi_dbi_debugfs_init()
From: Ville Syrjälä
Replace the slightly odd "#define NULL" thing with
a standard static inline stub.
Cc: Noralf Trønnes
Cc: Sam Ravnborg
Signed-off-by: Ville Syrjälä
---
include/drm/drm_mipi_dbi.h | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/include/drm/drm_mipi_dbi.h b/include/drm/drm_mipi_dbi.h
index 05e194958265..6fe13cce2670 100644
--- a/include/drm/drm_mipi_dbi.h
+++ b/include/drm/drm_mipi_dbi.h
@@ -194,7 +194,7 @@ int mipi_dbi_buf_copy(void *dst, struct drm_framebuffer *fb,
#ifdef CONFIG_DEBUG_FS
void mipi_dbi_debugfs_init(struct drm_minor *minor);
#else
-#define mipi_dbi_debugfs_init NULL
+static inline void mipi_dbi_debugfs_init(struct drm_minor *minor) {}
#endif
#endif /* __LINUX_MIPI_DBI_H */
--
2.32.0
[PATCH 1/2] drm: Always include the debugfs dentry in drm_crtc
From: Ville Syrjälä
Remove the counterproductive CONFIG_DEBUG_FS ifdef and just include
the debugfs dentry in drm_crtc always. This way we don't need
annoying ifdefs in the actual code with DEBUGFS=n. Also we don't
have these ifdefs around any of the other debugfs dentries either
so can't see why drm_crtc should be special.
This fixes the i915 DEBUGFS=n build because I assumed the dentry
would always be there.
Cc: Jani Nikula
Reported-by: Nathan Chancellor
Tested-by: Nathan Chancellor
Fixes: e74c6aa955ca ("drm/i915/fbc: Register per-crtc debugfs files")
Signed-off-by: Ville Syrjälä
---
include/drm/drm_crtc.h | 2 --
1 file changed, 2 deletions(-)
diff --git a/include/drm/drm_crtc.h b/include/drm/drm_crtc.h
index 13eeba2a750a..4d01b4d89775 100644
--- a/include/drm/drm_crtc.h
+++ b/include/drm/drm_crtc.h
@@ -1135,14 +1135,12 @@ struct drm_crtc {
*/
spinlock_t commit_lock;
-#ifdef CONFIG_DEBUG_FS
/**
* @debugfs_entry:
*
* Debugfs directory for this CRTC.
*/
struct dentry *debugfs_entry;
-#endif
/**
* @crc:
--
2.32.0
Re: [PATCH] dt-bindings: display: Add SPI peripheral schema to SPI based displays
On Tue, Dec 21, 2021 at 08:52:09AM -0400, Rob Herring wrote:
> With 'unevaluatedProperties' support enabled, several SPI based display
> binding examples have warnings:
>
> Documentation/devicetree/bindings/display/panel/samsung,ld9040.example.dt.yaml:
> lcd@0: Unevaluated properties are not allowed ('#address-cells',
> '#size-cells', 'spi-max-frequency', 'spi-cpol', 'spi-cpha' were unexpected)
> Documentation/devicetree/bindings/display/panel/kingdisplay,kd035g6-54nt.example.dt.yaml:
> panel@0: Unevaluated properties are not allowed ('spi-max-frequency',
> 'spi-3wire' were unexpected)
> Documentation/devicetree/bindings/display/panel/ilitek,ili9322.example.dt.yaml:
> display@0: Unevaluated properties are not allowed ('reg' was unexpected)
> Documentation/devicetree/bindings/display/panel/samsung,s6e63m0.example.dt.yaml:
> display@0: Unevaluated properties are not allowed ('spi-max-frequency' was
> unexpected)
> Documentation/devicetree/bindings/display/panel/abt,y030xx067a.example.dt.yaml:
> panel@0: Unevaluated properties are not allowed ('spi-max-frequency' was
> unexpected)
> Documentation/devicetree/bindings/display/panel/sony,acx565akm.example.dt.yaml:
> panel@2: Unevaluated properties are not allowed ('spi-max-frequency', 'reg'
> were unexpected)
> Documentation/devicetree/bindings/display/panel/tpo,td.example.dt.yaml:
> panel@0: Unevaluated properties are not allowed ('spi-max-frequency',
> 'spi-cpol', 'spi-cpha' were unexpected)
> Documentation/devicetree/bindings/display/panel/lgphilips,lb035q02.example.dt.yaml:
> panel@0: Unevaluated properties are not allowed ('reg', 'spi-max-frequency',
> 'spi-cpol', 'spi-cpha' were unexpected)
> Documentation/devicetree/bindings/display/panel/innolux,ej030na.example.dt.yaml:
> panel@0: Unevaluated properties are not allowed ('spi-max-frequency' was
> unexpected)
> Documentation/devicetree/bindings/display/panel/sitronix,st7789v.example.dt.yaml:
> panel@0: Unevaluated properties are not allowed ('spi-max-frequency',
> 'spi-cpol', 'spi-cpha' were unexpected)
>
> Fix all of these by adding a reference to spi-peripheral-props.yaml.
> With this, the description that the binding must follow
> spi-controller.yaml is both a bit out of date and redundant, so remove
> it.
>
> Signed-off-by: Rob Herring
Acked-by: Sam Ravnborg
Re: [PATCH] dt-bindings: display: novatek,nt36672a: Fix unevaluated properties warning
On Tue, Dec 21, 2021 at 08:51:26AM -0400, Rob Herring wrote:
> With 'unevaluatedProperties' support enabled, the novatek,nt36672a
> binding has a new warning:
>
> Documentation/devicetree/bindings/display/panel/novatek,nt36672a.example.dt.yaml:
> panel@0: Unevaluated properties are not allowed ('vddi0-supply',
> '#address-cells', '#size-cells' were unexpected)
>
> Based on dts files, 'vddi0-supply' does appear to be the correct name.
> Drop '#address-cells' and '#size-cells' which aren't needed.
>
> Signed-off-by: Rob Herring
Acked-by: Sam Ravnborg
> ---
> .../devicetree/bindings/display/panel/novatek,nt36672a.yaml | 4 +---
> 1 file changed, 1 insertion(+), 3 deletions(-)
>
> diff --git
> a/Documentation/devicetree/bindings/display/panel/novatek,nt36672a.yaml
> b/Documentation/devicetree/bindings/display/panel/novatek,nt36672a.yaml
> index ef4c0a24512d..563766d283f6 100644
> --- a/Documentation/devicetree/bindings/display/panel/novatek,nt36672a.yaml
> +++ b/Documentation/devicetree/bindings/display/panel/novatek,nt36672a.yaml
> @@ -34,7 +34,7 @@ properties:
> description: phandle of gpio for reset line - This should be 8mA, gpio
>can be configured using mux, pinctrl, pinctrl-names (active high)
>
> - vddio-supply:
> + vddi0-supply:
> description: phandle of the regulator that provides the supply voltage
>Power IC supply
>
> @@ -75,8 +75,6 @@ examples:
>
> reset-gpios = < 6 GPIO_ACTIVE_HIGH>;
>
> -#address-cells = <1>;
> -#size-cells = <0>;
> port {
> tianma_nt36672a_in_0: endpoint {
> remote-endpoint = <_out>;
> --
> 2.32.0
Re: [PATCH v16 08/40] gpu: host1x: Add initial runtime PM and OPP support
Hi Dmitry, Thierry, On 30/11/2021 23:23, Dmitry Osipenko wrote: Add runtime PM and OPP support to the Host1x driver. For the starter we will keep host1x always-on because dynamic power management require a major refactoring of the driver code since lot's of code paths are missing the RPM handling and we're going to remove some of these paths in the future. Unfortunately, this change is breaking boot on Tegra186. Bisect points to this and reverting on top of -next gets the board booting again. Sadly, there is no panic or error reported, it is just a hard hang. I will not have time to look at this this week and so we may need to revert for the moment. Jon -- nvpublic
Re: [PATCH v3 3/3] drm/privacy_screen_x86: Add entry for ChromeOS privacy-screen
Hi,
On 12/20/21 23:28, Rajat Jain wrote:
> Add a static entry in the x86 table, to detect and wait for
> privacy-screen on some ChromeOS platforms.
>
> Please note that this means that if CONFIG_CHROMEOS_PRIVACY_SCREEN is
> enabled, and if "GOOG0010" device is found in ACPI, then the i915 probe
> shall return EPROBE_DEFER until a platform driver actually registers the
> privacy-screen: https://hansdegoede.livejournal.com/25948.html
>
> Signed-off-by: Rajat Jain
> ---
> v3: * Remove the pr_info() from detect_chromeos_privacy_screen(), instead
> enhance the one already present in drm_privacy_screen_lookup_init()
> v2: * Use #if instead of #elif
> * Reorder the patches in the series.
> * Rebased on drm-tip
>
> drivers/gpu/drm/drm_privacy_screen_x86.c | 23 ++-
> 1 file changed, 22 insertions(+), 1 deletion(-)
>
> diff --git a/drivers/gpu/drm/drm_privacy_screen_x86.c
> b/drivers/gpu/drm/drm_privacy_screen_x86.c
> index a2cafb294ca6..0fdd2b500e6d 100644
> --- a/drivers/gpu/drm/drm_privacy_screen_x86.c
> +++ b/drivers/gpu/drm/drm_privacy_screen_x86.c
> @@ -47,6 +47,16 @@ static bool __init detect_thinkpad_privacy_screen(void)
> }
> #endif
>
> +#if IS_ENABLED(CONFIG_CHROMEOS_PRIVACY_SCREEN)
> +static bool __init detect_chromeos_privacy_screen(void)
> +{
> + if (!acpi_dev_present("GOOG0010", NULL, -1))
> + return false;
> +
> + return true;
This can be simplified to just:
return acpi_dev_present("GOOG0010", NULL, -1);
> +}
> +#endif
> +
> static const struct arch_init_data arch_init_data[] __initconst = {
> #if IS_ENABLED(CONFIG_THINKPAD_ACPI)
> {
> @@ -58,6 +68,16 @@ static const struct arch_init_data arch_init_data[]
> __initconst = {
> .detect = detect_thinkpad_privacy_screen,
> },
> #endif
> +#if IS_ENABLED(CONFIG_CHROMEOS_PRIVACY_SCREEN)
> + {
> + .lookup = {
> + .dev_id = NULL,
> + .con_id = NULL,
> + .provider = "privacy_screen-GOOG0010:00",
> + },
> + .detect = detect_chromeos_privacy_screen,
> + },
> +#endif
> };
>
> void __init drm_privacy_screen_lookup_init(void)
> @@ -68,7 +88,8 @@ void __init drm_privacy_screen_lookup_init(void)
> if (!arch_init_data[i].detect())
> continue;
>
> - pr_info("Found '%s' privacy-screen provider\n",
> + pr_info("Found '%s' privacy-screen provider."
> + "Might have to defer probe for it...\n",
> arch_init_data[i].lookup.provider);
I'm afraid this change in the log message will only confuse users,
and for your goal of checking if a privacy-screen provider has
been detected, the original message is good enough.
Please drop this part of the patch.
Regards,
Hans
>
> /* Make a copy because arch_init_data is __initconst */
>
Re: [PATCH v3 1/3] drm/privacy_screen: Add drvdata in drm_privacy_screen
Hi,
On 12/20/21 23:28, Rajat Jain wrote:
> Allow a privacy screen provider to stash its private data pointer in the
> drm_privacy_screen, and update the drm_privacy_screen_register() call to
> accept that. Also introduce a *_get_drvdata() so that it can retrieved
> back when needed.
>
> This also touches the IBM Thinkpad platform driver, the only user of
> privacy screen today, to pass NULL for now to the updated API.
>
> Signed-off-by: Rajat Jain
Thanks, patch looks good to me:
Reviewed-by: Hans de Goede
Regards,
Hans
> ---
> v3: Initial version. Came up due to review comments on v2 of other patches.
> v2: No v2
> v1: No v1
>
> drivers/gpu/drm/drm_privacy_screen.c| 5 -
> drivers/platform/x86/thinkpad_acpi.c| 2 +-
> include/drm/drm_privacy_screen_driver.h | 13 -
> 3 files changed, 17 insertions(+), 3 deletions(-)
>
> diff --git a/drivers/gpu/drm/drm_privacy_screen.c
> b/drivers/gpu/drm/drm_privacy_screen.c
> index beaf99e9120a..03b149cc455b 100644
> --- a/drivers/gpu/drm/drm_privacy_screen.c
> +++ b/drivers/gpu/drm/drm_privacy_screen.c
> @@ -387,7 +387,8 @@ static void drm_privacy_screen_device_release(struct
> device *dev)
> * * An ERR_PTR(errno) on failure.
> */
> struct drm_privacy_screen *drm_privacy_screen_register(
> - struct device *parent, const struct drm_privacy_screen_ops *ops)
> + struct device *parent, const struct drm_privacy_screen_ops *ops,
> + void *data)
> {
> struct drm_privacy_screen *priv;
> int ret;
> @@ -404,6 +405,7 @@ struct drm_privacy_screen *drm_privacy_screen_register(
> priv->dev.parent = parent;
> priv->dev.release = drm_privacy_screen_device_release;
> dev_set_name(>dev, "privacy_screen-%s", dev_name(parent));
> + priv->drvdata = data;
> priv->ops = ops;
>
> priv->ops->get_hw_state(priv);
> @@ -439,6 +441,7 @@ void drm_privacy_screen_unregister(struct
> drm_privacy_screen *priv)
> mutex_unlock(_privacy_screen_devs_lock);
>
> mutex_lock(>lock);
> + priv->drvdata = NULL;
> priv->ops = NULL;
> mutex_unlock(>lock);
>
> diff --git a/drivers/platform/x86/thinkpad_acpi.c
> b/drivers/platform/x86/thinkpad_acpi.c
> index 341655d711ce..ccbfda2b0095 100644
> --- a/drivers/platform/x86/thinkpad_acpi.c
> +++ b/drivers/platform/x86/thinkpad_acpi.c
> @@ -9782,7 +9782,7 @@ static int tpacpi_lcdshadow_init(struct ibm_init_struct
> *iibm)
> return 0;
>
> lcdshadow_dev = drm_privacy_screen_register(_pdev->dev,
> - _ops);
> + _ops, NULL);
> if (IS_ERR(lcdshadow_dev))
> return PTR_ERR(lcdshadow_dev);
>
> diff --git a/include/drm/drm_privacy_screen_driver.h
> b/include/drm/drm_privacy_screen_driver.h
> index 24591b607675..4ef246d5706f 100644
> --- a/include/drm/drm_privacy_screen_driver.h
> +++ b/include/drm/drm_privacy_screen_driver.h
> @@ -73,10 +73,21 @@ struct drm_privacy_screen {
>* for more info.
>*/
> enum drm_privacy_screen_status hw_state;
> + /**
> + * @drvdata: Private data owned by the privacy screen provider
> + */
> + void *drvdata;
> };
>
> +static inline
> +void *drm_privacy_screen_get_drvdata(struct drm_privacy_screen *priv)
> +{
> + return priv->drvdata;
> +}
> +
> struct drm_privacy_screen *drm_privacy_screen_register(
> - struct device *parent, const struct drm_privacy_screen_ops *ops);
> + struct device *parent, const struct drm_privacy_screen_ops *ops,
> + void *data);
> void drm_privacy_screen_unregister(struct drm_privacy_screen *priv);
>
> void drm_privacy_screen_call_notifier_chain(struct drm_privacy_screen *priv);
>
[PATCH v8 2/2] drm/msm/dp: do not initialize phy until plugin interrupt received
Current DP drivers have regulators, clocks, irq and phy are grouped
together within a function and executed not in a symmetric manner.
This increase difficulty of code maintenance and limited code scalability.
This patch divides the driver life cycle of operation into four states,
resume (including booting up), dongle plugin, dongle unplugged and suspend.
Regulators, core clocks and irq are grouped together and enabled at resume
(or booting up) so that the DP controller is armed and ready to receive HPD
plugin interrupts. HPD plugin interrupt is generated when a dongle plugs
into DUT (device under test). Once HPD plugin interrupt is received, DP
controller will initialize phy so that dpcd read/write will function and
following link training can be proceeded successfully. DP phy will be
disabled after main link is teared down at end of unplugged HPD interrupt
handle triggered by dongle unplugged out of DUT. Finally regulators, code
clocks and irq are disabled at corresponding suspension.
Changes in V2:
-- removed unnecessary dp_ctrl NULL check
-- removed unnecessary phy init_count and power_count DRM_DEBUG_DP logs
-- remove flip parameter out of dp_ctrl_irq_enable()
-- add fixes tag
Changes in V3:
-- call dp_display_host_phy_init() instead of dp_ctrl_phy_init() at
dp_display_host_init() for eDP
Changes in V4:
-- rewording commit text to match this commit changes
Changes in V5:
-- rebase on top of msm-next branch
Changes in V6:
-- delete flip variable
Changes in V7:
-- dp_ctrl_irq_enable/disabe() merged into dp_ctrl_reset_irq_ctrl()
Changes in V8:
-- add more detail comment regrading dp phy at dp_display_host_init()
Fixes: 8ede2ecc3e5e ("drm/msm/dp: Add DP compliance tests on Snapdragon
Chipsets")
Signed-off-by: Kuogee Hsieh
---
drivers/gpu/drm/msm/dp/dp_ctrl.c| 77
drivers/gpu/drm/msm/dp/dp_ctrl.h| 8 ++--
drivers/gpu/drm/msm/dp/dp_display.c | 87 +++--
3 files changed, 95 insertions(+), 77 deletions(-)
diff --git a/drivers/gpu/drm/msm/dp/dp_ctrl.c b/drivers/gpu/drm/msm/dp/dp_ctrl.c
index c724cb0..39558a2 100644
--- a/drivers/gpu/drm/msm/dp/dp_ctrl.c
+++ b/drivers/gpu/drm/msm/dp/dp_ctrl.c
@@ -1365,60 +1365,44 @@ static int dp_ctrl_enable_stream_clocks(struct
dp_ctrl_private *ctrl)
return ret;
}
-int dp_ctrl_host_init(struct dp_ctrl *dp_ctrl, bool flip, bool reset)
+void dp_ctrl_reset_irq_ctrl(struct dp_ctrl *dp_ctrl, bool enable)
+{
+ struct dp_ctrl_private *ctrl;
+
+ ctrl = container_of(dp_ctrl, struct dp_ctrl_private, dp_ctrl);
+
+ dp_catalog_ctrl_reset(ctrl->catalog);
+
+ if (enable)
+ dp_catalog_ctrl_enable_irq(ctrl->catalog, enable);
+}
+
+void dp_ctrl_phy_init(struct dp_ctrl *dp_ctrl)
{
struct dp_ctrl_private *ctrl;
struct dp_io *dp_io;
struct phy *phy;
- if (!dp_ctrl) {
- DRM_ERROR("Invalid input data\n");
- return -EINVAL;
- }
-
ctrl = container_of(dp_ctrl, struct dp_ctrl_private, dp_ctrl);
dp_io = >parser->io;
phy = dp_io->phy;
- ctrl->dp_ctrl.orientation = flip;
-
- if (reset)
- dp_catalog_ctrl_reset(ctrl->catalog);
-
- DRM_DEBUG_DP("flip=%d\n", flip);
dp_catalog_ctrl_phy_reset(ctrl->catalog);
phy_init(phy);
- dp_catalog_ctrl_enable_irq(ctrl->catalog, true);
-
- return 0;
}
-/**
- * dp_ctrl_host_deinit() - Uninitialize DP controller
- * @dp_ctrl: Display Port Driver data
- *
- * Perform required steps to uninitialize DP controller
- * and its resources.
- */
-void dp_ctrl_host_deinit(struct dp_ctrl *dp_ctrl)
+void dp_ctrl_phy_exit(struct dp_ctrl *dp_ctrl)
{
struct dp_ctrl_private *ctrl;
struct dp_io *dp_io;
struct phy *phy;
- if (!dp_ctrl) {
- DRM_ERROR("Invalid input data\n");
- return;
- }
-
ctrl = container_of(dp_ctrl, struct dp_ctrl_private, dp_ctrl);
dp_io = >parser->io;
phy = dp_io->phy;
- dp_catalog_ctrl_enable_irq(ctrl->catalog, false);
+ dp_catalog_ctrl_phy_reset(ctrl->catalog);
phy_exit(phy);
-
- DRM_DEBUG_DP("Host deinitialized successfully\n");
}
static bool dp_ctrl_use_fixed_nvid(struct dp_ctrl_private *ctrl)
@@ -1893,8 +1877,14 @@ int dp_ctrl_off_link_stream(struct dp_ctrl *dp_ctrl)
return ret;
}
+ DRM_DEBUG_DP("Before, phy=%x init_count=%d power_on=%d\n",
+ (u32)(uintptr_t)phy, phy->init_count, phy->power_count);
+
phy_power_off(phy);
+ DRM_DEBUG_DP("After, phy=%x init_count=%d power_on=%d\n",
+ (u32)(uintptr_t)phy, phy->init_count, phy->power_count);
+
/* aux channel down, reinit phy */
phy_exit(phy);
phy_init(phy);
@@ -1903,23 +1893,6 @@ int dp_ctrl_off_link_stream(struct dp_ctrl *dp_ctrl)
return ret;
}
-void dp_ctrl_off_phy(struct dp_ctrl *dp_ctrl)
-{
-
[PATCH v8 1/2] drm/msm/dp: dp_link_parse_sink_count() return immediately if aux read failed
Add checking aux read/write status at both dp_link_parse_sink_count()
and dp_link_parse_sink_status_filed() to avoid long timeout delay if
dp aux read/write failed at timeout due to cable unplugged. Also make
sure dp controller had been initialized before start dpcd read and write.
Changes in V4:
-- split this patch as stand alone patch
Changes in v5:
-- rebase on msm-next branch
Changes in v6:
-- add more details commit text
Signed-off-by: Kuogee Hsieh
Reviewed-by: Stephen Boyd
Tested-by: Stephen Boyd
---
drivers/gpu/drm/msm/dp/dp_display.c | 12 +---
drivers/gpu/drm/msm/dp/dp_link.c| 19 ++-
2 files changed, 23 insertions(+), 8 deletions(-)
diff --git a/drivers/gpu/drm/msm/dp/dp_display.c
b/drivers/gpu/drm/msm/dp/dp_display.c
index 3d61459..0766752 100644
--- a/drivers/gpu/drm/msm/dp/dp_display.c
+++ b/drivers/gpu/drm/msm/dp/dp_display.c
@@ -692,9 +692,15 @@ static int dp_irq_hpd_handle(struct dp_display_private
*dp, u32 data)
return 0;
}
- ret = dp_display_usbpd_attention_cb(>pdev->dev);
- if (ret == -ECONNRESET) { /* cable unplugged */
- dp->core_initialized = false;
+ /*
+* dp core (ahb/aux clks) must be initialized before
+* irq_hpd be handled
+*/
+ if (dp->core_initialized) {
+ ret = dp_display_usbpd_attention_cb(>pdev->dev);
+ if (ret == -ECONNRESET) { /* cable unplugged */
+ dp->core_initialized = false;
+ }
}
DRM_DEBUG_DP("hpd_state=%d\n", state);
diff --git a/drivers/gpu/drm/msm/dp/dp_link.c b/drivers/gpu/drm/msm/dp/dp_link.c
index a5bdfc5..d4d31e5 100644
--- a/drivers/gpu/drm/msm/dp/dp_link.c
+++ b/drivers/gpu/drm/msm/dp/dp_link.c
@@ -737,18 +737,25 @@ static int dp_link_parse_sink_count(struct dp_link
*dp_link)
return 0;
}
-static void dp_link_parse_sink_status_field(struct dp_link_private *link)
+static int dp_link_parse_sink_status_field(struct dp_link_private *link)
{
int len = 0;
link->prev_sink_count = link->dp_link.sink_count;
- dp_link_parse_sink_count(>dp_link);
+ len = dp_link_parse_sink_count(>dp_link);
+ if (len < 0) {
+ DRM_ERROR("DP parse sink count failed\n");
+ return len;
+ }
len = drm_dp_dpcd_read_link_status(link->aux,
link->link_status);
- if (len < DP_LINK_STATUS_SIZE)
+ if (len < DP_LINK_STATUS_SIZE) {
DRM_ERROR("DP link status read failed\n");
- dp_link_parse_request(link);
+ return len;
+ }
+
+ return dp_link_parse_request(link);
}
/**
@@ -1023,7 +1030,9 @@ int dp_link_process_request(struct dp_link *dp_link)
dp_link_reset_data(link);
- dp_link_parse_sink_status_field(link);
+ ret = dp_link_parse_sink_status_field(link);
+ if (ret)
+ return ret;
if (link->request.test_requested == DP_TEST_LINK_EDID_READ) {
dp_link->sink_request |= DP_TEST_LINK_EDID_READ;
--
The Qualcomm Innovation Center, Inc. is a member of the Code Aurora Forum,
a Linux Foundation Collaborative Project
Re: [PATCH v4] dt-bindings: display: tegra: Convert to json-schema
On Fri, 17 Dec 2021 17:43:20 +0100, Thierry Reding 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. > > Signed-off-by: Thierry Reding > --- > Changes in v4: > - add interconnect, interconnect-names, operating-points-v2 and > power-domains property to match latest DT updates > - drop stale reference to DPAUX pad controller bindings > - include dsi-controller.yaml and drop $nodename, #address-cells, > #size-cells and patternProperties from DSI bindings > - integrate #sound-dai-cells addition from published patch > - drop some generic, useless comments > > Changes in v3: > - split into separate YAML files for simplicity > - add display-hub DT bindings > > Changes in v2: > - use additionalProperties instead of unevaluatedProperties where > sufficient > - drop redundant $ref and add missing maxItems properties > - drop documentation for standard properties > - remove status properties from example > - drop spurious comments > > .../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 | 46 ++ > .../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, 2524 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 > Reviewed-by: Rob Herring
Re: [Intel-gfx] [PATCH] drm/i915/guc: Request RP0 before loading firmware
On 12/21/2021 10:11 AM, Ewins, Jon wrote:
On 12/20/2021 3:52 PM, Sundaresan, Sujaritha wrote:
On 12/16/2021 3:30 PM, Vinay Belgaumkar wrote:
By default, GT (and GuC) run at RPn. Requesting for RP0
before firmware load can speed up DMA and HuC auth as well.
In addition to writing to 0xA008, we also need to enable
swreq in 0xA024 so that Punit will pay heed to our request.
SLPC will restore the frequency back to RPn after initialization,
but we need to manually do that for the non-SLPC path.
We don't need a manual override in the SLPC disabled case, just
use the intel_rps_set function to ensure consistent RPS state.
Signed-off-by: Vinay Belgaumkar
---
drivers/gpu/drm/i915/gt/intel_rps.c | 59
+++
drivers/gpu/drm/i915/gt/intel_rps.h | 2 +
drivers/gpu/drm/i915/gt/uc/intel_uc.c | 9
drivers/gpu/drm/i915/i915_reg.h | 4 ++
4 files changed, 74 insertions(+)
diff --git a/drivers/gpu/drm/i915/gt/intel_rps.c
b/drivers/gpu/drm/i915/gt/intel_rps.c
index 07ff7ba7b2b7..d576b34c7d6f 100644
--- a/drivers/gpu/drm/i915/gt/intel_rps.c
+++ b/drivers/gpu/drm/i915/gt/intel_rps.c
@@ -2226,6 +2226,65 @@ u32 intel_rps_read_state_cap(struct intel_rps
*rps)
return intel_uncore_read(uncore, GEN6_RP_STATE_CAP);
}
+static void intel_rps_set_manual(struct intel_rps *rps, bool enable)
+{
+ struct intel_uncore *uncore = rps_to_uncore(rps);
+ u32 state = enable ? GEN9_RPSWCTL_ENABLE : GEN9_RPSWCTL_DISABLE;
+
+ /* Allow punit to process software requests */
+ intel_uncore_write(uncore, GEN6_RP_CONTROL, state);
+}
Was there a specific reason to remove the set/clear timer functions ?
Replying on behalf of Vinay Belguamkar:
We are now using the intel_rps_set() function which handles more state
update in the correct rps path. We also obtain an rps lock which
guarantees not clobbering rps data. The set/clear timers were being
done when we were modifying the frequency outside of the rps paths.
rps_set_manual is now only called in the SLPC path where the rps
timers are not even running.
Got it.
Reviewed-by: Sujaritha Sundaresan
+
+void intel_rps_raise_unslice(struct intel_rps *rps)
+{
+ struct intel_uncore *uncore = rps_to_uncore(rps);
+ u32 rp0_unslice_req;
+
+ mutex_lock(>lock);
+
+ if (rps_uses_slpc(rps)) {
+ /* RP limits have not been initialized yet for SLPC path */
+ rp0_unslice_req = ((intel_rps_read_state_cap(rps) >> 0)
+ & 0xff) * GEN9_FREQ_SCALER;
+
+ intel_rps_set_manual(rps, true);
+ intel_uncore_write(uncore, GEN6_RPNSWREQ,
+ ((rp0_unslice_req <<
+ GEN9_SW_REQ_UNSLICE_RATIO_SHIFT) |
+ GEN9_IGNORE_SLICE_RATIO));
+ intel_rps_set_manual(rps, false);
+ } else {
+ intel_rps_set(rps, rps->rp0_freq);
+ }
+
+ mutex_unlock(>lock);
+}
+
+void intel_rps_lower_unslice(struct intel_rps *rps)
+{
+ struct intel_uncore *uncore = rps_to_uncore(rps);
+ u32 rpn_unslice_req;
+
+ mutex_lock(>lock);
+
+ if (rps_uses_slpc(rps)) {
+ /* RP limits have not been initialized yet for SLPC path */
+ rpn_unslice_req = ((intel_rps_read_state_cap(rps) >> 16)
+ & 0xff) * GEN9_FREQ_SCALER;
+
+ intel_rps_set_manual(rps, true);
+ intel_uncore_write(uncore, GEN6_RPNSWREQ,
+ ((rpn_unslice_req <<
+ GEN9_SW_REQ_UNSLICE_RATIO_SHIFT) |
+ GEN9_IGNORE_SLICE_RATIO));
+ intel_rps_set_manual(rps, false);
+ } else {
+ intel_rps_set(rps, rps->min_freq);
+ }
+
+ mutex_unlock(>lock);
+}
+
Small function name nitpick maybe unslice_freq ? Just a suggestion.
/* External interface for intel_ips.ko */
static struct drm_i915_private __rcu *ips_mchdev;
diff --git a/drivers/gpu/drm/i915/gt/intel_rps.h
b/drivers/gpu/drm/i915/gt/intel_rps.h
index aee12f37d38a..c6d76a3d1331 100644
--- a/drivers/gpu/drm/i915/gt/intel_rps.h
+++ b/drivers/gpu/drm/i915/gt/intel_rps.h
@@ -45,6 +45,8 @@ u32 intel_rps_get_rpn_frequency(struct intel_rps
*rps);
u32 intel_rps_read_punit_req(struct intel_rps *rps);
u32 intel_rps_read_punit_req_frequency(struct intel_rps *rps);
u32 intel_rps_read_state_cap(struct intel_rps *rps);
+void intel_rps_raise_unslice(struct intel_rps *rps);
+void intel_rps_lower_unslice(struct intel_rps *rps);
void gen5_rps_irq_handler(struct intel_rps *rps);
void gen6_rps_irq_handler(struct intel_rps *rps, u32 pm_iir);
diff --git a/drivers/gpu/drm/i915/gt/uc/intel_uc.c
b/drivers/gpu/drm/i915/gt/uc/intel_uc.c
index 2fef3b0bbe95..3693c4e7dad0 100644
--- a/drivers/gpu/drm/i915/gt/uc/intel_uc.c
+++ b/drivers/gpu/drm/i915/gt/uc/intel_uc.c
@@ -8,6 +8,7 @@
#include "intel_guc.h"
#include "intel_guc_ads.h"
#include "intel_guc_submission.h"
+#include "gt/intel_rps.h"
#include "intel_uc.h"
#include "i915_drv.h"
@@ -462,6 +463,8 @@ static int __uc_init_hw(struct intel_uc *uc)
else
Re: [Intel-gfx] [PATCH] drm/i915/guc: Request RP0 before loading firmware
On 12/20/2021 3:52 PM, Sundaresan, Sujaritha wrote:
On 12/16/2021 3:30 PM, Vinay Belgaumkar wrote:
By default, GT (and GuC) run at RPn. Requesting for RP0
before firmware load can speed up DMA and HuC auth as well.
In addition to writing to 0xA008, we also need to enable
swreq in 0xA024 so that Punit will pay heed to our request.
SLPC will restore the frequency back to RPn after initialization,
but we need to manually do that for the non-SLPC path.
We don't need a manual override in the SLPC disabled case, just
use the intel_rps_set function to ensure consistent RPS state.
Signed-off-by: Vinay Belgaumkar
---
drivers/gpu/drm/i915/gt/intel_rps.c | 59 +++
drivers/gpu/drm/i915/gt/intel_rps.h | 2 +
drivers/gpu/drm/i915/gt/uc/intel_uc.c | 9
drivers/gpu/drm/i915/i915_reg.h | 4 ++
4 files changed, 74 insertions(+)
diff --git a/drivers/gpu/drm/i915/gt/intel_rps.c
b/drivers/gpu/drm/i915/gt/intel_rps.c
index 07ff7ba7b2b7..d576b34c7d6f 100644
--- a/drivers/gpu/drm/i915/gt/intel_rps.c
+++ b/drivers/gpu/drm/i915/gt/intel_rps.c
@@ -2226,6 +2226,65 @@ u32 intel_rps_read_state_cap(struct intel_rps
*rps)
return intel_uncore_read(uncore, GEN6_RP_STATE_CAP);
}
+static void intel_rps_set_manual(struct intel_rps *rps, bool enable)
+{
+ struct intel_uncore *uncore = rps_to_uncore(rps);
+ u32 state = enable ? GEN9_RPSWCTL_ENABLE : GEN9_RPSWCTL_DISABLE;
+
+ /* Allow punit to process software requests */
+ intel_uncore_write(uncore, GEN6_RP_CONTROL, state);
+}
Was there a specific reason to remove the set/clear timer functions ?
Replying on behalf of Vinay Belguamkar:
We are now using the intel_rps_set() function which handles more state
update in the correct rps path. We also obtain an rps lock which
guarantees not clobbering rps data. The set/clear timers were being
done when we were modifying the frequency outside of the rps paths.
rps_set_manual is now only called in the SLPC path where the rps timers
are not even running.
+
+void intel_rps_raise_unslice(struct intel_rps *rps)
+{
+ struct intel_uncore *uncore = rps_to_uncore(rps);
+ u32 rp0_unslice_req;
+
+ mutex_lock(>lock);
+
+ if (rps_uses_slpc(rps)) {
+ /* RP limits have not been initialized yet for SLPC path */
+ rp0_unslice_req = ((intel_rps_read_state_cap(rps) >> 0)
+ & 0xff) * GEN9_FREQ_SCALER;
+
+ intel_rps_set_manual(rps, true);
+ intel_uncore_write(uncore, GEN6_RPNSWREQ,
+ ((rp0_unslice_req <<
+ GEN9_SW_REQ_UNSLICE_RATIO_SHIFT) |
+ GEN9_IGNORE_SLICE_RATIO));
+ intel_rps_set_manual(rps, false);
+ } else {
+ intel_rps_set(rps, rps->rp0_freq);
+ }
+
+ mutex_unlock(>lock);
+}
+
+void intel_rps_lower_unslice(struct intel_rps *rps)
+{
+ struct intel_uncore *uncore = rps_to_uncore(rps);
+ u32 rpn_unslice_req;
+
+ mutex_lock(>lock);
+
+ if (rps_uses_slpc(rps)) {
+ /* RP limits have not been initialized yet for SLPC path */
+ rpn_unslice_req = ((intel_rps_read_state_cap(rps) >> 16)
+ & 0xff) * GEN9_FREQ_SCALER;
+
+ intel_rps_set_manual(rps, true);
+ intel_uncore_write(uncore, GEN6_RPNSWREQ,
+ ((rpn_unslice_req <<
+ GEN9_SW_REQ_UNSLICE_RATIO_SHIFT) |
+ GEN9_IGNORE_SLICE_RATIO));
+ intel_rps_set_manual(rps, false);
+ } else {
+ intel_rps_set(rps, rps->min_freq);
+ }
+
+ mutex_unlock(>lock);
+}
+
Small function name nitpick maybe unslice_freq ? Just a suggestion.
/* External interface for intel_ips.ko */
static struct drm_i915_private __rcu *ips_mchdev;
diff --git a/drivers/gpu/drm/i915/gt/intel_rps.h
b/drivers/gpu/drm/i915/gt/intel_rps.h
index aee12f37d38a..c6d76a3d1331 100644
--- a/drivers/gpu/drm/i915/gt/intel_rps.h
+++ b/drivers/gpu/drm/i915/gt/intel_rps.h
@@ -45,6 +45,8 @@ u32 intel_rps_get_rpn_frequency(struct intel_rps
*rps);
u32 intel_rps_read_punit_req(struct intel_rps *rps);
u32 intel_rps_read_punit_req_frequency(struct intel_rps *rps);
u32 intel_rps_read_state_cap(struct intel_rps *rps);
+void intel_rps_raise_unslice(struct intel_rps *rps);
+void intel_rps_lower_unslice(struct intel_rps *rps);
void gen5_rps_irq_handler(struct intel_rps *rps);
void gen6_rps_irq_handler(struct intel_rps *rps, u32 pm_iir);
diff --git a/drivers/gpu/drm/i915/gt/uc/intel_uc.c
b/drivers/gpu/drm/i915/gt/uc/intel_uc.c
index 2fef3b0bbe95..3693c4e7dad0 100644
--- a/drivers/gpu/drm/i915/gt/uc/intel_uc.c
+++ b/drivers/gpu/drm/i915/gt/uc/intel_uc.c
@@ -8,6 +8,7 @@
#include "intel_guc.h"
#include "intel_guc_ads.h"
#include "intel_guc_submission.h"
+#include "gt/intel_rps.h"
#include "intel_uc.h"
#include "i915_drv.h"
@@ -462,6 +463,8 @@ static int __uc_init_hw(struct intel_uc *uc)
else
attempts = 1;
+ intel_rps_raise_unslice(_to_gt(uc)->rps);
+
while (attempts--)
Re: [PATCH v7 2/2] drm/msm/dp: do not initialize phy until plugin interrupt received
On 12/14/2021 5:50 PM, Stephen Boyd wrote:
Quoting Kuogee Hsieh (2021-12-09 13:35:07)
diff --git a/drivers/gpu/drm/msm/dp/dp_display.c
b/drivers/gpu/drm/msm/dp/dp_display.c
index 0766752..cfbc5e4 100644
--- a/drivers/gpu/drm/msm/dp/dp_display.c
+++ b/drivers/gpu/drm/msm/dp/dp_display.c
@@ -83,6 +83,7 @@ struct dp_display_private {
/* state variables */
bool core_initialized;
+ bool phy_initialized;
bool hpd_irq_on;
bool audio_supported;
@@ -371,21 +372,46 @@ static int dp_display_process_hpd_high(struct
dp_display_private *dp)
return rc;
}
-static void dp_display_host_init(struct dp_display_private *dp, int reset)
+static void dp_display_host_phy_init(struct dp_display_private *dp)
{
- bool flip = false;
+ DRM_DEBUG_DP("core_init=%d phy_init=%d\n",
+ dp->core_initialized, dp->phy_initialized);
+ if (!dp->phy_initialized) {
Is this check only here because we don't know when this function is
getting called? I see in the DP case we get here from
dp_display_usbpd_configure_cb() but in the eDP case we get here from
dp_display_host_init() and presumably again from
dp_display_usbpd_configure_cb() called by dp_hpd_plug_handle().
If at all possible, I'd prefer to not have another tracking variable and
call dp_display_host_phy_init() from the same place regardless of DP or
eDP. Doing that would make it symmetric, per the commit text.
Agree, but dp->phy_initialized variable used to track some other
conditions.
For example, phy required to be re inited if dpcd read failed due to
cable unplugged.
other dpcd read failed will not required to re inited.
Therefore I think dp->phy_initialized variable is still required.
+ dp_ctrl_phy_init(dp->ctrl);
+ dp->phy_initialized = true;
+ }
+}
+
+static void dp_display_host_phy_exit(struct dp_display_private *dp)
+{
+ DRM_DEBUG_DP("core_init=%d phy_init=%d\n",
+ dp->core_initialized, dp->phy_initialized);
+
+ if (dp->phy_initialized) {
+ dp_ctrl_phy_exit(dp->ctrl);
+ dp->phy_initialized = false;
+ }
+}
+
+static void dp_display_host_init(struct dp_display_private *dp)
+{
DRM_DEBUG_DP("core_initialized=%d\n", dp->core_initialized);
if (dp->core_initialized) {
DRM_DEBUG_DP("DP core already initialized\n");
return;
}
- if (dp->usbpd->orientation == ORIENTATION_CC2)
- flip = true;
+ dp_power_init(dp->power, false);
+ dp_ctrl_reset_irq_ctrl(dp->ctrl, true);
+
+ /*
+* eDP is the embedded primary display and has its own phy
+* initialize phy immediately
Can we get some more details here? Why is it better to initialize the
phy here vs. when HPD goes high on the panel? The comment says what the
code is doing but it isn't telling us why that's OK.
Will do.
+*/
+ if (dp->dp_display.connector_type == DRM_MODE_CONNECTOR_eDP)
+ dp_display_host_phy_init(dp);
- dp_power_init(dp->power, flip);
- dp_ctrl_host_init(dp->ctrl, flip, reset);
dp_aux_init(dp->aux);
dp->core_initialized = true;
}
Re: [PATCH v1 0/5] Improvements for TC358768 DSI bridge driver
21.12.2021 21:10, Robert Foss пишет: > Hey Dmitry, > > On Sun, 19 Dec 2021 at 17:02, Dmitry Osipenko wrote: >> >> 19.10.2021 23:37, Dmitry Osipenko пишет: >>> 19.10.2021 12:47, Robert Foss пишет: Applied to drm-misc-next On Sun, 3 Oct 2021 at 01:35, Dmitry Osipenko wrote: > > This series adds couple improvements to the TC358768 DSI bridge driver, > enabling Panasonic VVX10F004B00 DSI panel support. This panel is used by > ASUS Transformer TF700T tablet, which is ready for upstream kernel and > display panel support is the biggest missing part. > > Dmitry Osipenko (5): > drm/bridge: tc358768: Enable reference clock > drm/bridge: tc358768: Support pulse mode > drm/bridge: tc358768: Calculate video start delay > drm/bridge: tc358768: Disable non-continuous clock mode > drm/bridge: tc358768: Correct BTACNTRL1 programming > > drivers/gpu/drm/bridge/tc358768.c | 94 +++ > 1 file changed, 71 insertions(+), 23 deletions(-) > > -- > 2.32.0 > >>> >>> Robert, thank you for taking care of these patches! Now nothing is >>> holding us from upstreaming the device-tree of the Transformer tablet. >>> >> >> Hello Robert, >> >> These patches spent 2 months in drm-misc-next, will they graduate into >> v5.17 or something special needs to be done for that? > > They series has landed in linux-next, and will be in v5.17 if nothing > catastrophic happens. Very good to know, thank you!
Re: [PATCH] dt-bindings: display: Add SPI peripheral schema to SPI based displays
Hi Rob,
Le mar., déc. 21 2021 at 08:52:09 -0400, Rob Herring
a écrit :
With 'unevaluatedProperties' support enabled, several SPI based
display
binding examples have warnings:
Documentation/devicetree/bindings/display/panel/samsung,ld9040.example.dt.yaml:
lcd@0: Unevaluated properties are not allowed ('#address-cells',
'#size-cells', 'spi-max-frequency', 'spi-cpol', 'spi-cpha' were
unexpected)
Documentation/devicetree/bindings/display/panel/kingdisplay,kd035g6-54nt.example.dt.yaml:
panel@0: Unevaluated properties are not allowed ('spi-max-frequency',
'spi-3wire' were unexpected)
Documentation/devicetree/bindings/display/panel/ilitek,ili9322.example.dt.yaml:
display@0: Unevaluated properties are not allowed ('reg' was
unexpected)
Documentation/devicetree/bindings/display/panel/samsung,s6e63m0.example.dt.yaml:
display@0: Unevaluated properties are not allowed
('spi-max-frequency' was unexpected)
Documentation/devicetree/bindings/display/panel/abt,y030xx067a.example.dt.yaml:
panel@0: Unevaluated properties are not allowed ('spi-max-frequency'
was unexpected)
Documentation/devicetree/bindings/display/panel/sony,acx565akm.example.dt.yaml:
panel@2: Unevaluated properties are not allowed ('spi-max-frequency',
'reg' were unexpected)
Documentation/devicetree/bindings/display/panel/tpo,td.example.dt.yaml:
panel@0: Unevaluated properties are not allowed ('spi-max-frequency',
'spi-cpol', 'spi-cpha' were unexpected)
Documentation/devicetree/bindings/display/panel/lgphilips,lb035q02.example.dt.yaml:
panel@0: Unevaluated properties are not allowed ('reg',
'spi-max-frequency', 'spi-cpol', 'spi-cpha' were unexpected)
Documentation/devicetree/bindings/display/panel/innolux,ej030na.example.dt.yaml:
panel@0: Unevaluated properties are not allowed ('spi-max-frequency'
was unexpected)
Documentation/devicetree/bindings/display/panel/sitronix,st7789v.example.dt.yaml:
panel@0: Unevaluated properties are not allowed ('spi-max-frequency',
'spi-cpol', 'spi-cpha' were unexpected)
Fix all of these by adding a reference to spi-peripheral-props.yaml.
With this, the description that the binding must follow
spi-controller.yaml is both a bit out of date and redundant, so remove
it.
Signed-off-by: Rob Herring
Acked-by: Paul Cercueil
Cheers,
-Paul
Re: [PATCH v1 0/5] Improvements for TC358768 DSI bridge driver
Hey Dmitry, On Sun, 19 Dec 2021 at 17:02, Dmitry Osipenko wrote: > > 19.10.2021 23:37, Dmitry Osipenko пишет: > > 19.10.2021 12:47, Robert Foss пишет: > >> Applied to drm-misc-next > >> > >> On Sun, 3 Oct 2021 at 01:35, Dmitry Osipenko wrote: > >>> > >>> This series adds couple improvements to the TC358768 DSI bridge driver, > >>> enabling Panasonic VVX10F004B00 DSI panel support. This panel is used by > >>> ASUS Transformer TF700T tablet, which is ready for upstream kernel and > >>> display panel support is the biggest missing part. > >>> > >>> Dmitry Osipenko (5): > >>> drm/bridge: tc358768: Enable reference clock > >>> drm/bridge: tc358768: Support pulse mode > >>> drm/bridge: tc358768: Calculate video start delay > >>> drm/bridge: tc358768: Disable non-continuous clock mode > >>> drm/bridge: tc358768: Correct BTACNTRL1 programming > >>> > >>> drivers/gpu/drm/bridge/tc358768.c | 94 +++ > >>> 1 file changed, 71 insertions(+), 23 deletions(-) > >>> > >>> -- > >>> 2.32.0 > >>> > > > > Robert, thank you for taking care of these patches! Now nothing is > > holding us from upstreaming the device-tree of the Transformer tablet. > > > > Hello Robert, > > These patches spent 2 months in drm-misc-next, will they graduate into > v5.17 or something special needs to be done for that? They series has landed in linux-next, and will be in v5.17 if nothing catastrophic happens. Rob.
Re: [PATCH 0/2] drm/tegra: Fix panel support on Venice 2 and Nyan
On Tue, Dec 21, 2021 at 07:45:31PM +0300, Dmitry Osipenko wrote: > 21.12.2021 19:17, Thierry Reding пишет: > > On Tue, Dec 21, 2021 at 06:47:31PM +0300, Dmitry Osipenko wrote: > >> 21.12.2021 13:58, Thierry Reding пишет: > >> .. > >> The panel->ddc isn't used by the new panel-edp driver unless panel is > >> compatible with "edp-panel". Hence the generic_edp_panel_probe() should > >> either fail or crash for a such "edp-panel" since panel->ddc isn't > >> fully > >> instantiated, AFAICS. > > > > I've tested this and it works fine on Venice 2. Since that was the > > reference design for Nyan, I suspect that Nyan's will also work. > > > > It'd be great if Thomas or anyone else with access to a Nyan could > > test this to verify that. > > There is no panel-edp driver in the v5.15. The EOL of v5.15 is Oct, > 2023, hence we need to either use: > >>> > >>> All the (at least relevant) functionality that is in panel-edp was in > >>> panel-simple before it was moved to panel-edp. I've backported this set > >>> of patches to v5.15 and it works just fine there. > >> > >> Will we be able to add patch to bypass the panel's DT ddc-i2c-bus on > >> Nyan to keep the older DTBs working? > > > > I don't see why we would want to do that. It's quite clear that the DTB > > is buggy in this case and we have a more accurate way to describe what's > > really there in hardware. In addition that more accurate representation > > also gets rid of a bug. Obviously because the bug is caused by the > > previous representation that was not accurate. > > > > Given that we can easily replace the DTBs on these devices there's no > > reason to make this any more complicated than it has to be. > > Don't you care about normal people at all? Do you assume that everyone > must to be a kernel developer to be able to use Tegra devices? :/ If you know how to install a custom kernel you also know how to replace the DTB on these devices. For everyone else, once these patches are merged upstream and distributions start shipping the new version, they will get this automatically by updating their kernel package since most distributions actually ship the DTB files as part of that. > It's not a problem for you to figure out why display is broken, for > other people it's a problem. Usually nobody will update DTB without a > well known reason, instead device will be dusted on a shelf. In the end > you won't have any users at all. Most "normal" people aren't even going to notice that their DTB is going to be updated. They would actually have to do extra work *not* to update it. Thierry signature.asc Description: PGP signature
Re: [PATCH v5 0/4] ti-sn65dsi83 patches
Applied to drm-misc-next
Re: [PATCH 20/22] drm/encoder: Add of_graph port to struct drm_encoder
Am Montag, 20. Dezember 2021, 12:06:28 CET schrieb Sascha Hauer:
> Add a device node to drm_encoder which corresponds with the port node
> in the DT description of the encoder. This allows drivers to find the
> of_graph link between a crtc and an encoder.
>
> Signed-off-by: Sascha Hauer
> ---
> include/drm/drm_encoder.h | 2 ++
> 1 file changed, 2 insertions(+)
>
> diff --git a/include/drm/drm_encoder.h b/include/drm/drm_encoder.h
> index 6e91a0280f31b..3acd054b1eb3e 100644
> --- a/include/drm/drm_encoder.h
> +++ b/include/drm/drm_encoder.h
> @@ -99,6 +99,8 @@ struct drm_encoder {
> struct drm_device *dev;
> struct list_head head;
>
> + struct device_node *port;
> +
> struct drm_mode_object base;
> char *name;
> /**
>
Is this the port that gets used in patch 3/22?
It looks like it.
So this would break bisectability. Can we order patches
sequentially so that git bisect keeps working.
Thanks
Heiko
Re: [PATCH v3 6/7] drm/i915: Use vma resources for async unbinding
On 21/12/2021 16:07, Thomas Hellström wrote:
On Tue, 2021-12-21 at 14:02 +, Matthew Auld wrote:
On 17/12/2021 14:52, Thomas Hellström wrote:
Implement async (non-blocking) unbinding by not syncing the vma
before
calling unbind on the vma_resource.
Add the resulting unbind fence to the object's dma_resv from where
it is
picked up by the ttm migration code.
Ideally these unbind fences should be coalesced with the migration
blit
fence to avoid stalling the migration blit waiting for unbind, as
they
can certainly go on in parallel, but since we don't yet have a
reasonable data structure to use to coalesce fences and attach the
resulting fence to a timeline, we defer that for now.
Note that with async unbinding, even while the unbind waits for the
preceding bind to complete before unbinding, the vma itself might
have been
destroyed in the process, clearing the vma pages. Therefore we can
only allow async unbinding if we have a refcounted sg-list and keep
a
refcount on that for the vma resource pages to stay intact until
binding occurs. If this condition is not met, a request for an
async
unbind is diverted to a sync unbind.
v2:
- Use a separate kmem_cache for vma resources for now to isolate
their
memory allocation and aid debugging.
- Move the check for vm closed to the actual unbinding thread.
Regardless
of whether the vm is closed, we need the unbind fence to
properly wait
for capture.
- Clear vma_res::vm on unbind and update its documentation.
Signed-off-by: Thomas Hellström
@@ -416,6 +420,7 @@ int i915_vma_bind(struct i915_vma *vma,
{
u32 bind_flags;
u32 vma_flags;
+ int ret;
lockdep_assert_held(>vm->mutex);
GEM_BUG_ON(!drm_mm_node_allocated(>node));
@@ -424,12 +429,12 @@ int i915_vma_bind(struct i915_vma *vma,
if (GEM_DEBUG_WARN_ON(range_overflows(vma->node.start,
vma->node.size,
vma->vm->total))) {
- kfree(vma_res);
+ i915_vma_resource_free(vma_res);
return -ENODEV;
}
if (GEM_DEBUG_WARN_ON(!flags)) {
- kfree(vma_res);
+ i915_vma_resource_free(vma_res);
return -EINVAL;
}
@@ -441,12 +446,30 @@ int i915_vma_bind(struct i915_vma *vma,
bind_flags &= ~vma_flags;
if (bind_flags == 0) {
- kfree(vma_res);
+ i915_vma_resource_free(vma_res);
return 0;
}
GEM_BUG_ON(!vma->pages);
+ /* Wait for or await async unbinds touching our range */
+ if (work && bind_flags & vma->vm->bind_async_flags)
+ ret = i915_vma_resource_bind_dep_await(vma->vm,
+
base.chain,
+ vma-
node.start,
+ vma-
node.size,
+ true,
+ GFP_NOWAIT |
+
__GFP_RETRY_MAYFAIL |
+
__GFP_NOWARN);
+ else
+ ret = i915_vma_resource_bind_dep_sync(vma->vm, vma-
node.start,
+ vma-
node.size, true);
Is there nothing scary here with coloring? Say with cache coloring,
to
ensure we unbind the neighbouring nodes(if they are conflicting)
before
doing the bind, or is async unbinding only ever going to be used for
the
ppGTT?
And then I guess there might also be memory coloring where we likely
need to ensure that all the unbinds within the overlapping PT(s) have
been completed before doing the bind, since the bind will also
increment
the usage count of the PT, potentially preventing it from being
destroyed, which will skip nuking the PDE state, AFAIK. Previously
the
drm_mm node(s) would still be present, which would trigger the
eviction.
Although it might be that we just end up aligning everything to 2M,
and
so drop the memory coloring anyway, so maybe no need to worry about
this
yet...
Hmm. This indeed sounds that there were some important considerations
left out. I was under the impression that only previously scheduled
unbinds touching the same range would have need to have finished.
Currently there's only ppGTT async unbinding, but I figure moving
forward we don't want to restrict it.
I wonder whether instead of keeping an interval tree of pending unbinds
we should keep just a single fence per VM of the last pending unbind,
and move to the RB tree as a separate optimization step if needed. That
would AFAICT keep the current semantics of all unbinds that were
scheduled before the current bind are completed before the bind. Do you
think that would be sufficient?
Single fence should work I think. Or alternatively keep the interval
tree and then add a 4K chunk at the beginning and end of the search
Re: [PATCH v9 2/6] drm/i915: Use to_gt() helper for GGTT accesses
On Sun, Dec 19, 2021 at 11:24:56PM +0200, Andi Shyti wrote:
> From: Michał Winiarski
>
> GGTT is currently available both through i915->ggtt and gt->ggtt, and we
> eventually want to get rid of the i915->ggtt one.
> Use to_gt() for all i915->ggtt accesses to help with the future
> refactoring.
>
> Signed-off-by: Michał Winiarski
> Cc: Michal Wajdeczko
> Signed-off-by: Andi Shyti
> ---
> drivers/gpu/drm/i915/gvt/dmabuf.c| 2 +-
> drivers/gpu/drm/i915/i915_debugfs.c | 4 ++--
> drivers/gpu/drm/i915/i915_driver.c | 4 ++--
> drivers/gpu/drm/i915/i915_drv.h | 2 +-
> drivers/gpu/drm/i915/i915_gem.c | 23 ---
> drivers/gpu/drm/i915/i915_gem_gtt.c | 6 +++---
> drivers/gpu/drm/i915/i915_getparam.c | 2 +-
> drivers/gpu/drm/i915/i915_perf.c | 4 ++--
> 8 files changed, 24 insertions(+), 23 deletions(-)
>
> diff --git a/drivers/gpu/drm/i915/gvt/dmabuf.c
> b/drivers/gpu/drm/i915/gvt/dmabuf.c
> index 8e65cd8258b9..94c3eb1586b0 100644
> --- a/drivers/gpu/drm/i915/gvt/dmabuf.c
> +++ b/drivers/gpu/drm/i915/gvt/dmabuf.c
> @@ -84,7 +84,7 @@ static int vgpu_gem_get_pages(
> kfree(st);
> return ret;
> }
> - gtt_entries = (gen8_pte_t __iomem *)dev_priv->ggtt.gsm +
> + gtt_entries = (gen8_pte_t __iomem *)to_gt(dev_priv)->ggtt->gsm +
> (fb_info->start >> PAGE_SHIFT);
> for_each_sg(st->sgl, sg, page_num, i) {
> dma_addr_t dma_addr =
> diff --git a/drivers/gpu/drm/i915/i915_debugfs.c
> b/drivers/gpu/drm/i915/i915_debugfs.c
> index e0e052cdf8b8..6966fe08df92 100644
> --- a/drivers/gpu/drm/i915/i915_debugfs.c
> +++ b/drivers/gpu/drm/i915/i915_debugfs.c
> @@ -390,9 +390,9 @@ static int i915_swizzle_info(struct seq_file *m, void
> *data)
> intel_wakeref_t wakeref;
>
> seq_printf(m, "bit6 swizzle for X-tiling = %s\n",
> -swizzle_string(dev_priv->ggtt.bit_6_swizzle_x));
> +swizzle_string(to_gt(dev_priv)->ggtt->bit_6_swizzle_x));
> seq_printf(m, "bit6 swizzle for Y-tiling = %s\n",
> -swizzle_string(dev_priv->ggtt.bit_6_swizzle_y));
> +swizzle_string(to_gt(dev_priv)->ggtt->bit_6_swizzle_y));
>
> if (dev_priv->quirks & QUIRK_PIN_SWIZZLED_PAGES)
> seq_puts(m, "L-shaped memory detected\n");
> diff --git a/drivers/gpu/drm/i915/i915_driver.c
> b/drivers/gpu/drm/i915/i915_driver.c
> index 60f8cbf24de7..3c984553d86f 100644
> --- a/drivers/gpu/drm/i915/i915_driver.c
> +++ b/drivers/gpu/drm/i915/i915_driver.c
> @@ -1146,7 +1146,7 @@ static int i915_drm_suspend(struct drm_device *dev)
>
> /* Must be called before GGTT is suspended. */
> intel_dpt_suspend(dev_priv);
> - i915_ggtt_suspend(_priv->ggtt);
> + i915_ggtt_suspend(to_gt(dev_priv)->ggtt);
>
> i915_save_display(dev_priv);
>
> @@ -1270,7 +1270,7 @@ static int i915_drm_resume(struct drm_device *dev)
> if (ret)
> drm_err(_priv->drm, "failed to re-enable GGTT\n");
>
> - i915_ggtt_resume(_priv->ggtt);
> + i915_ggtt_resume(to_gt(dev_priv)->ggtt);
> /* Must be called after GGTT is resumed. */
> intel_dpt_resume(dev_priv);
>
> diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h
> index 471be2716abe..524025790fe0 100644
> --- a/drivers/gpu/drm/i915/i915_drv.h
> +++ b/drivers/gpu/drm/i915/i915_drv.h
> @@ -1749,7 +1749,7 @@ static inline bool
> i915_gem_object_needs_bit17_swizzle(struct drm_i915_gem_objec
> {
> struct drm_i915_private *i915 = to_i915(obj->base.dev);
>
> - return i915->ggtt.bit_6_swizzle_x == I915_BIT_6_SWIZZLE_9_10_17 &&
> + return to_gt(i915)->ggtt->bit_6_swizzle_x == I915_BIT_6_SWIZZLE_9_10_17
> &&
> i915_gem_object_is_tiled(obj);
> }
>
> diff --git a/drivers/gpu/drm/i915/i915_gem.c b/drivers/gpu/drm/i915/i915_gem.c
> index 8ba2119092f2..45e3b4c540a1 100644
> --- a/drivers/gpu/drm/i915/i915_gem.c
> +++ b/drivers/gpu/drm/i915/i915_gem.c
> @@ -88,7 +88,8 @@ int
> i915_gem_get_aperture_ioctl(struct drm_device *dev, void *data,
> struct drm_file *file)
> {
> - struct i915_ggtt *ggtt = _i915(dev)->ggtt;
> + struct drm_i915_private *i915 = to_i915(dev);
> + struct i915_ggtt *ggtt = to_gt(i915)->ggtt;
> struct drm_i915_gem_get_aperture *args = data;
> struct i915_vma *vma;
> u64 pinned;
> @@ -289,7 +290,7 @@ static struct i915_vma *i915_gem_gtt_prepare(struct
> drm_i915_gem_object *obj,
>bool write)
> {
> struct drm_i915_private *i915 = to_i915(obj->base.dev);
> - struct i915_ggtt *ggtt = >ggtt;
> + struct i915_ggtt *ggtt = to_gt(i915)->ggtt;
> struct i915_vma *vma;
> struct i915_gem_ww_ctx ww;
> int ret;
> @@ -350,7 +351,7 @@ static void i915_gem_gtt_cleanup(struct
> drm_i915_gem_object *obj,
>struct i915_vma *vma)
> {
> struct
Re: [PATCH v9 1/6] drm/i915/gt: Use to_gt() helper for GGTT accesses
On Sun, Dec 19, 2021 at 11:24:55PM +0200, Andi Shyti wrote:
> From: Michał Winiarski
>
> GGTT is currently available both through i915->ggtt and gt->ggtt, and we
> eventually want to get rid of the i915->ggtt one.
> Use to_gt() for all i915->ggtt accesses to help with the future
> refactoring.
>
> Signed-off-by: Michał Winiarski
> Cc: Michal Wajdeczko
> Signed-off-by: Andi Shyti
> Reviewed-by: Sujaritha Sundaresan
> Reviewed-by: Matt Roper
> ---
> drivers/gpu/drm/i915/gt/intel_ggtt.c | 14 +++---
> drivers/gpu/drm/i915/gt/intel_ggtt_fencing.c | 6 +++---
> drivers/gpu/drm/i915/gt/intel_region_lmem.c | 4 ++--
> drivers/gpu/drm/i915/gt/selftest_reset.c | 2 +-
> drivers/gpu/drm/i915/i915_driver.c | 4 ++--
> 5 files changed, 15 insertions(+), 15 deletions(-)
>
> diff --git a/drivers/gpu/drm/i915/gt/intel_ggtt.c
> b/drivers/gpu/drm/i915/gt/intel_ggtt.c
> index 971e737b37b2..ec3b998392ff 100644
> --- a/drivers/gpu/drm/i915/gt/intel_ggtt.c
> +++ b/drivers/gpu/drm/i915/gt/intel_ggtt.c
> @@ -89,7 +89,7 @@ int i915_ggtt_init_hw(struct drm_i915_private *i915)
>* beyond the end of the batch buffer, across the page boundary,
>* and beyond the end of the GTT if we do not provide a guard.
>*/
> - ret = ggtt_init_hw(>ggtt);
> + ret = ggtt_init_hw(to_gt(i915)->ggtt);
> if (ret)
> return ret;
>
> @@ -725,14 +725,14 @@ int i915_init_ggtt(struct drm_i915_private *i915)
> {
> int ret;
>
> - ret = init_ggtt(>ggtt);
> + ret = init_ggtt(to_gt(i915)->ggtt);
> if (ret)
> return ret;
>
> if (INTEL_PPGTT(i915) == INTEL_PPGTT_ALIASING) {
> - ret = init_aliasing_ppgtt(>ggtt);
> + ret = init_aliasing_ppgtt(to_gt(i915)->ggtt);
> if (ret)
> - cleanup_init_ggtt(>ggtt);
> + cleanup_init_ggtt(to_gt(i915)->ggtt);
> }
>
> return 0;
> @@ -775,7 +775,7 @@ static void ggtt_cleanup_hw(struct i915_ggtt *ggtt)
> */
> void i915_ggtt_driver_release(struct drm_i915_private *i915)
> {
> - struct i915_ggtt *ggtt = >ggtt;
> + struct i915_ggtt *ggtt = to_gt(i915)->ggtt;
>
> fini_aliasing_ppgtt(ggtt);
>
> @@ -790,7 +790,7 @@ void i915_ggtt_driver_release(struct drm_i915_private
> *i915)
> */
> void i915_ggtt_driver_late_release(struct drm_i915_private *i915)
> {
> - struct i915_ggtt *ggtt = >ggtt;
> + struct i915_ggtt *ggtt = to_gt(i915)->ggtt;
>
> GEM_WARN_ON(kref_read(>vm.resv_ref) != 1);
> dma_resv_fini(>vm._resv);
> @@ -1232,7 +1232,7 @@ int i915_ggtt_probe_hw(struct drm_i915_private *i915)
> {
> int ret;
>
> - ret = ggtt_probe_hw(>ggtt, to_gt(i915));
> + ret = ggtt_probe_hw(to_gt(i915)->ggtt, to_gt(i915));
> if (ret)
> return ret;
>
> diff --git a/drivers/gpu/drm/i915/gt/intel_ggtt_fencing.c
> b/drivers/gpu/drm/i915/gt/intel_ggtt_fencing.c
> index f8948de72036..beabf3bc9b75 100644
> --- a/drivers/gpu/drm/i915/gt/intel_ggtt_fencing.c
> +++ b/drivers/gpu/drm/i915/gt/intel_ggtt_fencing.c
> @@ -728,8 +728,8 @@ static void detect_bit_6_swizzle(struct i915_ggtt *ggtt)
> swizzle_y = I915_BIT_6_SWIZZLE_NONE;
> }
>
> - i915->ggtt.bit_6_swizzle_x = swizzle_x;
> - i915->ggtt.bit_6_swizzle_y = swizzle_y;
> + to_gt(i915)->ggtt->bit_6_swizzle_x = swizzle_x;
> + to_gt(i915)->ggtt->bit_6_swizzle_y = swizzle_y;
> }
>
> /*
> @@ -896,7 +896,7 @@ void intel_gt_init_swizzling(struct intel_gt *gt)
> struct intel_uncore *uncore = gt->uncore;
>
> if (GRAPHICS_VER(i915) < 5 ||
> - i915->ggtt.bit_6_swizzle_x == I915_BIT_6_SWIZZLE_NONE)
> + to_gt(i915)->ggtt->bit_6_swizzle_x == I915_BIT_6_SWIZZLE_NONE)
> return;
>
> intel_uncore_rmw(uncore, DISP_ARB_CTL, 0, DISP_TILE_SURFACE_SWIZZLING);
> diff --git a/drivers/gpu/drm/i915/gt/intel_region_lmem.c
> b/drivers/gpu/drm/i915/gt/intel_region_lmem.c
> index fde2dcb59809..21215a080088 100644
> --- a/drivers/gpu/drm/i915/gt/intel_region_lmem.c
> +++ b/drivers/gpu/drm/i915/gt/intel_region_lmem.c
> @@ -15,7 +15,7 @@
> static int init_fake_lmem_bar(struct intel_memory_region *mem)
> {
> struct drm_i915_private *i915 = mem->i915;
> - struct i915_ggtt *ggtt = >ggtt;
> + struct i915_ggtt *ggtt = to_gt(i915)->ggtt;
> unsigned long n;
> int ret;
>
> @@ -131,7 +131,7 @@ intel_gt_setup_fake_lmem(struct intel_gt *gt)
> if (!i915->params.fake_lmem_start)
> return ERR_PTR(-ENODEV);
>
> - GEM_BUG_ON(i915_ggtt_has_aperture(>ggtt));
> + GEM_BUG_ON(i915_ggtt_has_aperture(to_gt(i915)->ggtt));
>
> /* Your mappable aperture belongs to me now! */
> mappable_end = pci_resource_len(pdev, 2);
> diff --git a/drivers/gpu/drm/i915/gt/selftest_reset.c
> b/drivers/gpu/drm/i915/gt/selftest_reset.c
> index 8a873f6bda7f..37c38bdd5f47 100644
> ---
Re: [PATCH 0/2] drm/tegra: Fix panel support on Venice 2 and Nyan
21.12.2021 19:17, Thierry Reding пишет: > On Tue, Dec 21, 2021 at 06:47:31PM +0300, Dmitry Osipenko wrote: >> 21.12.2021 13:58, Thierry Reding пишет: >> .. >> The panel->ddc isn't used by the new panel-edp driver unless panel is >> compatible with "edp-panel". Hence the generic_edp_panel_probe() should >> either fail or crash for a such "edp-panel" since panel->ddc isn't fully >> instantiated, AFAICS. > > I've tested this and it works fine on Venice 2. Since that was the > reference design for Nyan, I suspect that Nyan's will also work. > > It'd be great if Thomas or anyone else with access to a Nyan could > test this to verify that. There is no panel-edp driver in the v5.15. The EOL of v5.15 is Oct, 2023, hence we need to either use: >>> >>> All the (at least relevant) functionality that is in panel-edp was in >>> panel-simple before it was moved to panel-edp. I've backported this set >>> of patches to v5.15 and it works just fine there. >> >> Will we be able to add patch to bypass the panel's DT ddc-i2c-bus on >> Nyan to keep the older DTBs working? > > I don't see why we would want to do that. It's quite clear that the DTB > is buggy in this case and we have a more accurate way to describe what's > really there in hardware. In addition that more accurate representation > also gets rid of a bug. Obviously because the bug is caused by the > previous representation that was not accurate. > > Given that we can easily replace the DTBs on these devices there's no > reason to make this any more complicated than it has to be. Don't you care about normal people at all? Do you assume that everyone must to be a kernel developer to be able to use Tegra devices? :/ It's not a problem for you to figure out why display is broken, for other people it's a problem. Usually nobody will update DTB without a well known reason, instead device will be dusted on a shelf. In the end you won't have any users at all.
Re: [PATCH v3 0/2] Use "ref" clocks from firmware for DSI PLL VCO parent
On 2021-12-15 16:43:45, Stephen Boyd wrote: > Quoting Dmitry Baryshkov (2021-12-15 12:02:37) > > On 14/12/2021 22:46, Marijn Suijten wrote: > > > Hi all, > > > > > > On 2021-09-18 16:40:38, Marijn Suijten wrote: > > >> On 2021-09-14 14:44:01, Stephen Boyd wrote: > > >>> Quoting Marijn Suijten (2021-09-11 06:19:19) > > All DSI PHY/PLL drivers were referencing their VCO parent clock by a > > global name, most of which don't exist or have been renamed. These > > clock drivers seem to function fine without that except the 14nm driver > > for sdm6xx [1]. > > > > At the same time all DTs provide a "ref" clock as per the requirements > > of dsi-phy-common.yaml, but the clock is never used. This patchset > > puts > > that clock to use without relying on a global clock name, so that all > > dependencies are explicitly defined in DT (the firmware) in the end. > > >>> > > >>> I can take this through clk tree if it helps avoid conflicts. There are > > >>> some other patches to sdm660.c in the clk tree already. > > >> > > >> Might be useful to maintain proper ordering of these dependent patches > > >> but it's up to Dmitry and Rob to decide, whom I'm sending this mail > > >> directly to so that they can chime in. > > > > > > Dependent patch [3] landed in 5.15 and [2] made it into 5.16 rc's - is > > > it time to pick this series up and if so through what tree? > > > > I'd also second the idea of merging these two patches into 5.17. > > Most probably it'd be easier to merge both of them through the clk tree. > > Or we can take the first patch into drm-msm (but then we'd have a > > dependency between msm-next and clk-qcom-next). > > > > Bjorn, Stephen? > > > > Sounds fine to take through clk tree. Thanks Stephen, would be great to take this in through the clk tree for 5.17. I don't have anything to add that could possibly warrant a v3, only msm8996 remains with the "xo" clock but that needs more work in other drivers and is best dealt with separately. Please take v2, assuming there are enough acks/reviews :) - Marijn
Re: [PATCH 0/2] drm/tegra: Fix panel support on Venice 2 and Nyan
On Tue, Dec 21, 2021 at 06:47:31PM +0300, Dmitry Osipenko wrote: > 21.12.2021 13:58, Thierry Reding пишет: > .. > The panel->ddc isn't used by the new panel-edp driver unless panel is > compatible with "edp-panel". Hence the generic_edp_panel_probe() should > either fail or crash for a such "edp-panel" since panel->ddc isn't fully > instantiated, AFAICS. > >>> > >>> I've tested this and it works fine on Venice 2. Since that was the > >>> reference design for Nyan, I suspect that Nyan's will also work. > >>> > >>> It'd be great if Thomas or anyone else with access to a Nyan could > >>> test this to verify that. > >> > >> There is no panel-edp driver in the v5.15. The EOL of v5.15 is Oct, > >> 2023, hence we need to either use: > > > > All the (at least relevant) functionality that is in panel-edp was in > > panel-simple before it was moved to panel-edp. I've backported this set > > of patches to v5.15 and it works just fine there. > > Will we be able to add patch to bypass the panel's DT ddc-i2c-bus on > Nyan to keep the older DTBs working? I don't see why we would want to do that. It's quite clear that the DTB is buggy in this case and we have a more accurate way to describe what's really there in hardware. In addition that more accurate representation also gets rid of a bug. Obviously because the bug is caused by the previous representation that was not accurate. Given that we can easily replace the DTBs on these devices there's no reason to make this any more complicated than it has to be. Thierry signature.asc Description: PGP signature
Re: [RFC 4/6] drm/amdgpu: Serialize non TDR gpu recovery with TDRs
On 2021-12-21 2:59 a.m., Christian König wrote:
Am 20.12.21 um 23:17 schrieb Andrey Grodzovsky:
On 2021-12-20 2:20 a.m., Christian König wrote:
Am 17.12.21 um 23:27 schrieb Andrey Grodzovsky:
Use reset domain wq also for non TDR gpu recovery trigers
such as sysfs and RAS. We must serialize all possible
GPU recoveries to gurantee no concurrency there.
For TDR call the original recovery function directly since
it's already executed from within the wq. For others just
use a wrapper to qeueue work and wait on it to finish.
Signed-off-by: Andrey Grodzovsky
---
drivers/gpu/drm/amd/amdgpu/amdgpu.h | 2 ++
drivers/gpu/drm/amd/amdgpu/amdgpu_device.c | 33
+-
drivers/gpu/drm/amd/amdgpu/amdgpu_job.c | 2 +-
3 files changed, 35 insertions(+), 2 deletions(-)
diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu.h
b/drivers/gpu/drm/amd/amdgpu/amdgpu.h
index b5ff76aae7e0..8e96b9a14452 100644
--- a/drivers/gpu/drm/amd/amdgpu/amdgpu.h
+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu.h
@@ -1296,6 +1296,8 @@ bool amdgpu_device_has_job_running(struct
amdgpu_device *adev);
bool amdgpu_device_should_recover_gpu(struct amdgpu_device *adev);
int amdgpu_device_gpu_recover(struct amdgpu_device *adev,
struct amdgpu_job* job);
+int amdgpu_device_gpu_recover_imp(struct amdgpu_device *adev,
+ struct amdgpu_job *job);
void amdgpu_device_pci_config_reset(struct amdgpu_device *adev);
int amdgpu_device_pci_reset(struct amdgpu_device *adev);
bool amdgpu_device_need_post(struct amdgpu_device *adev);
diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_device.c
b/drivers/gpu/drm/amd/amdgpu/amdgpu_device.c
index b595e6d699b5..55cd67b9ede2 100644
--- a/drivers/gpu/drm/amd/amdgpu/amdgpu_device.c
+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_device.c
@@ -4979,7 +4979,7 @@ static void amdgpu_device_recheck_guilty_jobs(
* Returns 0 for success or an error on failure.
*/
-int amdgpu_device_gpu_recover(struct amdgpu_device *adev,
+int amdgpu_device_gpu_recover_imp(struct amdgpu_device *adev,
struct amdgpu_job *job)
{
struct list_head device_list, *device_list_handle = NULL;
@@ -5236,6 +5236,37 @@ int amdgpu_device_gpu_recover(struct
amdgpu_device *adev,
return r;
}
+struct recover_work_struct {
Please add an amdgpu_ prefix to the name.
+ struct work_struct base;
+ struct amdgpu_device *adev;
+ struct amdgpu_job *job;
+ int ret;
+};
+
+static void amdgpu_device_queue_gpu_recover_work(struct
work_struct *work)
+{
+ struct recover_work_struct *recover_work = container_of(work,
struct recover_work_struct, base);
+
+ recover_work->ret =
amdgpu_device_gpu_recover_imp(recover_work->adev, recover_work->job);
+}
+/*
+ * Serialize gpu recover into reset domain single threaded wq
+ */
+int amdgpu_device_gpu_recover(struct amdgpu_device *adev,
+ struct amdgpu_job *job)
+{
+ struct recover_work_struct work = {.adev = adev, .job = job};
+
+ INIT_WORK(, amdgpu_device_queue_gpu_recover_work);
+
+ if (!queue_work(adev->reset_domain.wq, ))
+ return -EAGAIN;
+
+ flush_work();
+
+ return work.ret;
+}
Maybe that should be part of the scheduler code? Not really sure,
just an idea.
Seems to me that since the reset domain is almost always above a
single scheduler granularity then it wouldn't feet very well there.
Yeah, but what if we introduce an drm_sched_recover_queue and
drm_sched_recover_work object?
It's probably ok to go forward with that for now, but this handling
makes quite some sense to have independent of which driver is using
it. So as soon as we see a second similar implementation we should
move it into common code.
Regards,
Christian.
Agree, the only point i would stress is that we need an entity separate
from from drm_gpu_scheduler, something like
drm_gpu_reset_domain which should point to or be pointed by a set of
schedulers that should go through
reset together.
Andrey
Andrey
Apart from that looks good to me,
Christian.
+
/**
* amdgpu_device_get_pcie_info - fence pcie info about the PCIE slot
*
diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_job.c
b/drivers/gpu/drm/amd/amdgpu/amdgpu_job.c
index bfc47bea23db..38c9fd7b7ad4 100644
--- a/drivers/gpu/drm/amd/amdgpu/amdgpu_job.c
+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_job.c
@@ -63,7 +63,7 @@ static enum drm_gpu_sched_stat
amdgpu_job_timedout(struct drm_sched_job *s_job)
ti.process_name, ti.tgid, ti.task_name, ti.pid);
if (amdgpu_device_should_recover_gpu(ring->adev)) {
- amdgpu_device_gpu_recover(ring->adev, job);
+ amdgpu_device_gpu_recover_imp(ring->adev, job);
} else {
drm_sched_suspend_timeout(>sched);
if (amdgpu_sriov_vf(adev))
Re: [PATCH v3 6/7] drm/i915: Use vma resources for async unbinding
On Tue, 2021-12-21 at 14:02 +, Matthew Auld wrote:
> On 17/12/2021 14:52, Thomas Hellström wrote:
> > Implement async (non-blocking) unbinding by not syncing the vma
> > before
> > calling unbind on the vma_resource.
> > Add the resulting unbind fence to the object's dma_resv from where
> > it is
> > picked up by the ttm migration code.
> > Ideally these unbind fences should be coalesced with the migration
> > blit
> > fence to avoid stalling the migration blit waiting for unbind, as
> > they
> > can certainly go on in parallel, but since we don't yet have a
> > reasonable data structure to use to coalesce fences and attach the
> > resulting fence to a timeline, we defer that for now.
> >
> > Note that with async unbinding, even while the unbind waits for the
> > preceding bind to complete before unbinding, the vma itself might
> > have been
> > destroyed in the process, clearing the vma pages. Therefore we can
> > only allow async unbinding if we have a refcounted sg-list and keep
> > a
> > refcount on that for the vma resource pages to stay intact until
> > binding occurs. If this condition is not met, a request for an
> > async
> > unbind is diverted to a sync unbind.
> >
> > v2:
> > - Use a separate kmem_cache for vma resources for now to isolate
> > their
> > memory allocation and aid debugging.
> > - Move the check for vm closed to the actual unbinding thread.
> > Regardless
> > of whether the vm is closed, we need the unbind fence to
> > properly wait
> > for capture.
> > - Clear vma_res::vm on unbind and update its documentation.
> >
> > Signed-off-by: Thomas Hellström
>
>
>
> > @@ -416,6 +420,7 @@ int i915_vma_bind(struct i915_vma *vma,
> > {
> > u32 bind_flags;
> > u32 vma_flags;
> > + int ret;
> >
> > lockdep_assert_held(>vm->mutex);
> > GEM_BUG_ON(!drm_mm_node_allocated(>node));
> > @@ -424,12 +429,12 @@ int i915_vma_bind(struct i915_vma *vma,
> > if (GEM_DEBUG_WARN_ON(range_overflows(vma->node.start,
> > vma->node.size,
> > vma->vm->total))) {
> > - kfree(vma_res);
> > + i915_vma_resource_free(vma_res);
> > return -ENODEV;
> > }
> >
> > if (GEM_DEBUG_WARN_ON(!flags)) {
> > - kfree(vma_res);
> > + i915_vma_resource_free(vma_res);
> > return -EINVAL;
> > }
> >
> > @@ -441,12 +446,30 @@ int i915_vma_bind(struct i915_vma *vma,
> >
> > bind_flags &= ~vma_flags;
> > if (bind_flags == 0) {
> > - kfree(vma_res);
> > + i915_vma_resource_free(vma_res);
> > return 0;
> > }
> >
> > GEM_BUG_ON(!vma->pages);
> >
> > + /* Wait for or await async unbinds touching our range */
> > + if (work && bind_flags & vma->vm->bind_async_flags)
> > + ret = i915_vma_resource_bind_dep_await(vma->vm,
> > +
> > >base.chain,
> > + vma-
> > >node.start,
> > + vma-
> > >node.size,
> > + true,
> > + GFP_NOWAIT |
> > +
> > __GFP_RETRY_MAYFAIL |
> > +
> > __GFP_NOWARN);
> > + else
> > + ret = i915_vma_resource_bind_dep_sync(vma->vm, vma-
> > >node.start,
> > + vma-
> > >node.size, true);
>
> Is there nothing scary here with coloring? Say with cache coloring,
> to
> ensure we unbind the neighbouring nodes(if they are conflicting)
> before
> doing the bind, or is async unbinding only ever going to be used for
> the
> ppGTT?
>
> And then I guess there might also be memory coloring where we likely
> need to ensure that all the unbinds within the overlapping PT(s) have
> been completed before doing the bind, since the bind will also
> increment
> the usage count of the PT, potentially preventing it from being
> destroyed, which will skip nuking the PDE state, AFAIK. Previously
> the
> drm_mm node(s) would still be present, which would trigger the
> eviction.
> Although it might be that we just end up aligning everything to 2M,
> and
> so drop the memory coloring anyway, so maybe no need to worry about
> this
> yet...
Hmm. This indeed sounds that there were some important considerations
left out. I was under the impression that only previously scheduled
unbinds touching the same range would have need to have finished.
Currently there's only ppGTT async unbinding, but I figure moving
forward we don't want to restrict it.
I wonder whether instead of keeping an interval tree of pending unbinds
we should
Re: [RFC 3/6] drm/amdgpu: Fix crash on modprobe
On 2021-12-21 2:02 a.m., Christian König wrote:
Am 20.12.21 um 20:22 schrieb Andrey Grodzovsky:
On 2021-12-20 2:17 a.m., Christian König wrote:
Am 17.12.21 um 23:27 schrieb Andrey Grodzovsky:
Restrict jobs resubmission to suspend case
only since schedulers not initialised yet on
probe.
Signed-off-by: Andrey Grodzovsky
---
drivers/gpu/drm/amd/amdgpu/amdgpu_fence.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_fence.c
b/drivers/gpu/drm/amd/amdgpu/amdgpu_fence.c
index 5527c68c51de..8ebd954e06c6 100644
--- a/drivers/gpu/drm/amd/amdgpu/amdgpu_fence.c
+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_fence.c
@@ -582,7 +582,7 @@ void amdgpu_fence_driver_hw_init(struct
amdgpu_device *adev)
if (!ring || !ring->fence_drv.initialized)
continue;
- if (!ring->no_scheduler) {
+ if (adev->in_suspend && !ring->no_scheduler) {
Uff, why is that suddenly necessary? Because of the changed order?
Christian.
Yes.
Mhm, that's quite bad design then.
If you look at the original patch for this
https://www.spinics.net/lists/amd-gfx/msg67560.html you will
see that that restarting scheduler here is only relevant for
suspend/resume case because there was
a race to fix. There is no point in this code on driver init because
nothing was submitted to scheduler yet
and so it seems to me ok to add condition that this code run only
in_suspend case.
How about we keep the order as is and allow specifying the reset work
queue with drm_sched_start() ?
As i mentioned above, the fact we even have drm_sched_start there is
just part of a solution to resolve a race
during suspend/resume. It is not for device initialization and indeed,
other client drivers of gpu shcheduler never call
drm_sched_start on device init. We must guarantee that reset work queue
already initialized before any job submission to scheduler
and because of that IMHO the right place for this is drm_sched_init.
Andrey
Christian.
Andrey
drm_sched_resubmit_jobs(>sched);
drm_sched_start(>sched, true);
}
Re: [PATCH 0/2] drm/tegra: Fix panel support on Venice 2 and Nyan
21.12.2021 13:58, Thierry Reding пишет: .. The panel->ddc isn't used by the new panel-edp driver unless panel is compatible with "edp-panel". Hence the generic_edp_panel_probe() should either fail or crash for a such "edp-panel" since panel->ddc isn't fully instantiated, AFAICS. >>> >>> I've tested this and it works fine on Venice 2. Since that was the >>> reference design for Nyan, I suspect that Nyan's will also work. >>> >>> It'd be great if Thomas or anyone else with access to a Nyan could >>> test this to verify that. >> >> There is no panel-edp driver in the v5.15. The EOL of v5.15 is Oct, >> 2023, hence we need to either use: > > All the (at least relevant) functionality that is in panel-edp was in > panel-simple before it was moved to panel-edp. I've backported this set > of patches to v5.15 and it works just fine there. Will we be able to add patch to bypass the panel's DT ddc-i2c-bus on Nyan to keep the older DTBs working?
[PATCH] drm/msm: replace DEFINE_SIMPLE_ATTRIBUTE with DEFINE_DEBUGFS_ATTRIBUTE
From: Changcheng Deng Fix the following coccicheck warning: ./drivers/gpu/drm/msm/msm_debugfs.c: 132: 0-23: WARNING: shrink_fops should be defined with DEFINE_DEBUGFS_ATTRIBUTE Use DEFINE_DEBUGFS_ATTRIBUTE rather than DEFINE_SIMPLE_ATTRIBUTE for debugfs files. Reported-by: Zeal Robot Signed-off-by: Changcheng Deng --- drivers/gpu/drm/msm/msm_debugfs.c | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/drivers/gpu/drm/msm/msm_debugfs.c b/drivers/gpu/drm/msm/msm_debugfs.c index 956b1efc3721..91fb0c83b600 100644 --- a/drivers/gpu/drm/msm/msm_debugfs.c +++ b/drivers/gpu/drm/msm/msm_debugfs.c @@ -129,9 +129,9 @@ shrink_set(void *data, u64 val) return 0; } -DEFINE_SIMPLE_ATTRIBUTE(shrink_fops, - shrink_get, shrink_set, - "0x%08llx\n"); +DEFINE_DEBUGFS_ATTRIBUTE(shrink_fops, +shrink_get, shrink_set, +"0x%08llx\n"); static int msm_gem_show(struct seq_file *m, void *arg) -- 2.25.1
Re: [PATCH 11/22] dt-bindings: display: rockchip: Add binding for VOP2
On Mon, Dec 20, 2021 at 12:06:19PM +0100, Sascha Hauer wrote:
> The VOP2 is found on newer Rockchip SoCs like the rk3568 or the rk3566.
> The binding differs slightly from the existing VOP binding, so add a new
> binding file for it.
>
> Signed-off-by: Sascha Hauer
> ---
> .../display/rockchip/rockchip-vop2.yaml | 146 ++
> 1 file changed, 146 insertions(+)
> create mode 100644
> Documentation/devicetree/bindings/display/rockchip/rockchip-vop2.yaml
>
> diff --git
> a/Documentation/devicetree/bindings/display/rockchip/rockchip-vop2.yaml
> b/Documentation/devicetree/bindings/display/rockchip/rockchip-vop2.yaml
> new file mode 100644
> index 0..df14d5aa85c85
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/display/rockchip/rockchip-vop2.yaml
> @@ -0,0 +1,146 @@
> +# SPDX-License-Identifier: GPL-2.0-only or BSD-2-Clause
> +%YAML 1.2
> +---
> +$id: http://devicetree.org/schemas/display/rockchip/rockchip-vop2.yaml#
> +$schema: http://devicetree.org/meta-schemas/core.yaml#
> +
> +title: Rockchip SoC display controller (VOP2)
> +
> +description:
> + VOP2 (Video Output Processor v2) is the display controller for the Rockchip
> + series of SoCs which transfers the image data from a video memory
> + buffer to an external LCD interface.
> +
> +maintainers:
> + - Sandy Huang
> + - Heiko Stuebner
> +
> +properties:
> + compatible:
> +enum:
> + - rockchip,rk3566-vop
> + - rockchip,rk3568-vop
> +
> + reg:
> +minItems: 1
> +items:
> + - description:
> + Must contain one entry corresponding to the base address and length
> + of the register space.
> + - description:
> + Can optionally contain a second entry corresponding to
> + the CRTC gamma LUT address.
> +
> + interrupts:
> +maxItems: 1
> +description:
> + The VOP interrupt is shared by several interrupt sources, such as
> + frame start (VSYNC), line flag and other status interrupts.
> +
> + clocks:
> +items:
> + - description: Clock for ddr buffer transfer.
> + - description: Clock for the ahb bus to R/W the phy regs.
> + - description: Pixel clock for video port 0.
> + - description: Pixel clock for video port 1.
> + - description: Pixel clock for video port 2.
> +
> + clock-names:
> +items:
> + - const: aclk_vop
> + - const: hclk_vop
_vop is redundant.
> + - const: dclk_vp0
> + - const: dclk_vp1
> + - const: dclk_vp2
> +
> + rockchip,grf:
> +$ref: /schemas/types.yaml#/definitions/phandle
> +description:
> + Phandle to GRF regs used for misc control
> +
> + ports:
> +$ref: /schemas/graph.yaml#/properties/port
s/port/ports/
> +
> +properties:
> + port@0:
> +$ref: /schemas/graph.yaml#/properties/port
> +description:
> + Output endpoint of VP0
> +
> + port@1:
> +$ref: /schemas/graph.yaml#/properties/port
> +description:
> + Output endpoint of VP1
> +
> + port@:
port@2
> +$ref: /schemas/graph.yaml#/properties/port
> +description:
> + Output endpoint of VP2
> +
> + assigned-clocks: true
> +
> + assigned-clock-rates: true
> +
> + assigned-clock-parents: true
These are automatically added.
> +
> + iommus:
> +maxItems: 1
> +
> + power-domains:
> +maxItems: 1
> +
> +required:
> + - compatible
> + - reg
> + - interrupts
> + - clocks
> + - clock-names
> + - ports
> +
> +additionalProperties: false
> +
> +examples:
> + - |
> +#include
> +#include
> +#include
> +bus {
> +#address-cells = <2>;
> +#size-cells = <2>;
> +vop: vop@fe04 {
> +compatible = "rockchip,rk3568-vop";
> +reg = <0x0 0xfe04 0x0 0x3000>, <0x0 0xfe044000 0x0
> 0x1000>;
> +interrupts = ;
> +clocks = < ACLK_VOP>,
> + < HCLK_VOP>,
> + < DCLK_VOP0>,
> + < DCLK_VOP1>,
> + < DCLK_VOP2>;
> +clock-names = "aclk_vop",
> + "hclk_vop",
> + "dclk_vp0",
> + "dclk_vp1",
> + "dclk_vp2";
> +power-domains = < RK3568_PD_VO>;
> +iommus = <_mmu>;
> +vop_out: ports {
> +#address-cells = <1>;
> +#size-cells = <0>;
> +vp0: port@0 {
> +reg = <0>;
> +#address-cells = <1>;
> +#size-cells = <0>;
> +};
> +vp1: port@1 {
> +reg = <1>;
> +#address-cells = <1>;
> +#size-cells = <0>;
> +};
> +vp2: port@2 {
> +
Re: [PATCH 08/22] dt-bindings: display: rockchip: dw-hdmi: use "ref" as clock name
On Mon, Dec 20, 2021 at 12:06:16PM +0100, Sascha Hauer wrote: > "vpll" is a misnomer. A clock input to a device should be named after > the usage in the device, not after the clock that drives it. On the > rk3568 the same clock is driven by the HPLL. > To fix that, this patch renames the vpll clock to ref clock. The problem with this series is it breaks an old kernel with new dt. You can partially mitigate that with stable kernel backport, but IMO keeping the old name is not a burden to maintain. And given RK3399 is widely used including by me, we should not be breaking compatibility. So allow for ref in addition to vpll if you like, but only use 'ref' for new users. And add a comment in the schema to that effect. Rob > > Signed-off-by: Sascha Hauer > --- > .../bindings/display/rockchip/rockchip,dw-hdmi.yaml| 7 --- > 1 file changed, 4 insertions(+), 3 deletions(-) > > diff --git > a/Documentation/devicetree/bindings/display/rockchip/rockchip,dw-hdmi.yaml > b/Documentation/devicetree/bindings/display/rockchip/rockchip,dw-hdmi.yaml > index 6e09dd2ee05ac..3b40219e3ea60 100644 > --- a/Documentation/devicetree/bindings/display/rockchip/rockchip,dw-hdmi.yaml > +++ b/Documentation/devicetree/bindings/display/rockchip/rockchip,dw-hdmi.yaml > @@ -47,11 +47,12 @@ properties: >- enum: >- cec >- grf > - - vpll > + - ref >- enum: >- grf > - - vpll > - - const: vpll > + - ref > + - const: > + - ref > >ddc-i2c-bus: > $ref: /schemas/types.yaml#/definitions/phandle > -- > 2.30.2 > >
Re: [PATCH v3 6/7] drm/i915: Use vma resources for async unbinding
On 17/12/2021 14:52, Thomas Hellström wrote:
Implement async (non-blocking) unbinding by not syncing the vma before
calling unbind on the vma_resource.
Add the resulting unbind fence to the object's dma_resv from where it is
picked up by the ttm migration code.
Ideally these unbind fences should be coalesced with the migration blit
fence to avoid stalling the migration blit waiting for unbind, as they
can certainly go on in parallel, but since we don't yet have a
reasonable data structure to use to coalesce fences and attach the
resulting fence to a timeline, we defer that for now.
Note that with async unbinding, even while the unbind waits for the
preceding bind to complete before unbinding, the vma itself might have been
destroyed in the process, clearing the vma pages. Therefore we can
only allow async unbinding if we have a refcounted sg-list and keep a
refcount on that for the vma resource pages to stay intact until
binding occurs. If this condition is not met, a request for an async
unbind is diverted to a sync unbind.
v2:
- Use a separate kmem_cache for vma resources for now to isolate their
memory allocation and aid debugging.
- Move the check for vm closed to the actual unbinding thread. Regardless
of whether the vm is closed, we need the unbind fence to properly wait
for capture.
- Clear vma_res::vm on unbind and update its documentation.
Signed-off-by: Thomas Hellström
@@ -416,6 +420,7 @@ int i915_vma_bind(struct i915_vma *vma,
{
u32 bind_flags;
u32 vma_flags;
+ int ret;
lockdep_assert_held(>vm->mutex);
GEM_BUG_ON(!drm_mm_node_allocated(>node));
@@ -424,12 +429,12 @@ int i915_vma_bind(struct i915_vma *vma,
if (GEM_DEBUG_WARN_ON(range_overflows(vma->node.start,
vma->node.size,
vma->vm->total))) {
- kfree(vma_res);
+ i915_vma_resource_free(vma_res);
return -ENODEV;
}
if (GEM_DEBUG_WARN_ON(!flags)) {
- kfree(vma_res);
+ i915_vma_resource_free(vma_res);
return -EINVAL;
}
@@ -441,12 +446,30 @@ int i915_vma_bind(struct i915_vma *vma,
bind_flags &= ~vma_flags;
if (bind_flags == 0) {
- kfree(vma_res);
+ i915_vma_resource_free(vma_res);
return 0;
}
GEM_BUG_ON(!vma->pages);
+ /* Wait for or await async unbinds touching our range */
+ if (work && bind_flags & vma->vm->bind_async_flags)
+ ret = i915_vma_resource_bind_dep_await(vma->vm,
+ >base.chain,
+ vma->node.start,
+ vma->node.size,
+ true,
+ GFP_NOWAIT |
+ __GFP_RETRY_MAYFAIL |
+ __GFP_NOWARN);
+ else
+ ret = i915_vma_resource_bind_dep_sync(vma->vm, vma->node.start,
+ vma->node.size, true);
Is there nothing scary here with coloring? Say with cache coloring, to
ensure we unbind the neighbouring nodes(if they are conflicting) before
doing the bind, or is async unbinding only ever going to be used for the
ppGTT?
And then I guess there might also be memory coloring where we likely
need to ensure that all the unbinds within the overlapping PT(s) have
been completed before doing the bind, since the bind will also increment
the usage count of the PT, potentially preventing it from being
destroyed, which will skip nuking the PDE state, AFAIK. Previously the
drm_mm node(s) would still be present, which would trigger the eviction.
Although it might be that we just end up aligning everything to 2M, and
so drop the memory coloring anyway, so maybe no need to worry about this
yet...
Re: [PATCH 22/22] drm: rockchip: Add VOP2 driver
On Montag, 20. Dezember 2021 12:06:30 CET Sascha Hauer wrote:
> From: Andy Yan
>
> The VOP2 unit is found on Rockchip SoCs beginning with rk3566/rk3568.
> It replaces the VOP unit found in the older Rockchip SoCs.
>
> This driver has been derived from the downstream Rockchip Kernel and
> heavily modified:
>
> - All nonstandard DRM properties have been removed
> - dropped struct vop2_plane_state and pass around less data between
> functions
> - Dropped all DRM_FORMAT_* not known on upstream
> - rework register access to get rid of excessively used macros
> - Drop all waiting for framesyncs
>
> The driver is tested with HDMI and MIPI-DSI display on a RK3568-EVB
> board. Overlay support is tested with the modetest utility. AFBC support
> on the cluster windows is tested with weston-simple-dmabuf-egl on
> weston using the (yet to be upstreamed) panfrost driver support.
>
> Signed-off-by: Sascha Hauer
> ---
Hi Sascha,
quick partial review of the code in-line.
For reference, I debugged locking issues with the kernel lock
debug config options and assert_spin_locked in the reg write
functions, as well as some manual deduction.
> drivers/gpu/drm/rockchip/Kconfig |6 +
> drivers/gpu/drm/rockchip/Makefile|1 +
> drivers/gpu/drm/rockchip/rockchip_drm_drv.c |1 +
> drivers/gpu/drm/rockchip/rockchip_drm_drv.h |7 +-
> drivers/gpu/drm/rockchip/rockchip_drm_fb.c |2 +
> drivers/gpu/drm/rockchip/rockchip_drm_vop.h | 15 +
> drivers/gpu/drm/rockchip/rockchip_drm_vop2.c | 2768 ++
> drivers/gpu/drm/rockchip/rockchip_drm_vop2.h | 480 +++
> drivers/gpu/drm/rockchip/rockchip_vop2_reg.c | 285 ++
> 9 files changed, 3564 insertions(+), 1 deletion(-)
> create mode 100644 drivers/gpu/drm/rockchip/rockchip_drm_vop2.c
> create mode 100644 drivers/gpu/drm/rockchip/rockchip_drm_vop2.h
> create mode 100644 drivers/gpu/drm/rockchip/rockchip_vop2_reg.c
>
> diff --git a/drivers/gpu/drm/rockchip/Kconfig
> b/drivers/gpu/drm/rockchip/Kconfig
> index b9b156308460a..4ff0043f0ee70 100644
> --- a/drivers/gpu/drm/rockchip/Kconfig
> +++ b/drivers/gpu/drm/rockchip/Kconfig
> @@ -28,6 +28,12 @@ config ROCKCHIP_VOP
> This selects support for the VOP driver. You should enable it
> on all older SoCs up to RK3399.
>
> +config ROCKCHIP_VOP2
> + bool "Rockchip VOP2 driver"
> + help
> + This selects support for the VOP2 driver. You should enable it
> + on all newer SoCs beginning form RK3568.
> +
> config ROCKCHIP_ANALOGIX_DP
> bool "Rockchip specific extensions for Analogix DP driver"
> depends on ROCKCHIP_VOP
> diff --git a/drivers/gpu/drm/rockchip/Makefile
> b/drivers/gpu/drm/rockchip/Makefile
> index cd6e7bb5ce9c5..29848caef5c21 100644
> --- a/drivers/gpu/drm/rockchip/Makefile
> +++ b/drivers/gpu/drm/rockchip/Makefile
> @@ -7,6 +7,7 @@ rockchipdrm-y := rockchip_drm_drv.o rockchip_drm_fb.o \
> rockchip_drm_gem.o
> rockchipdrm-$(CONFIG_DRM_FBDEV_EMULATION) += rockchip_drm_fbdev.o
>
> +rockchipdrm-$(CONFIG_ROCKCHIP_VOP2) += rockchip_drm_vop2.o
> rockchip_vop2_reg.o
> rockchipdrm-$(CONFIG_ROCKCHIP_VOP) += rockchip_drm_vop.o rockchip_vop_reg.o
> rockchipdrm-$(CONFIG_ROCKCHIP_ANALOGIX_DP) += analogix_dp-rockchip.o
> rockchipdrm-$(CONFIG_ROCKCHIP_CDN_DP) += cdn-dp-core.o cdn-dp-reg.o
> diff --git a/drivers/gpu/drm/rockchip/rockchip_drm_drv.c
> b/drivers/gpu/drm/rockchip/rockchip_drm_drv.c
> index 64fa5fd62c01a..2bd9acb265e5a 100644
> --- a/drivers/gpu/drm/rockchip/rockchip_drm_drv.c
> +++ b/drivers/gpu/drm/rockchip/rockchip_drm_drv.c
> @@ -474,6 +474,7 @@ static int __init rockchip_drm_init(void)
>
> num_rockchip_sub_drivers = 0;
> ADD_ROCKCHIP_SUB_DRIVER(vop_platform_driver, CONFIG_ROCKCHIP_VOP);
> + ADD_ROCKCHIP_SUB_DRIVER(vop2_platform_driver, CONFIG_ROCKCHIP_VOP2);
> ADD_ROCKCHIP_SUB_DRIVER(rockchip_lvds_driver,
> CONFIG_ROCKCHIP_LVDS);
> ADD_ROCKCHIP_SUB_DRIVER(rockchip_dp_driver,
> diff --git a/drivers/gpu/drm/rockchip/rockchip_drm_drv.h
> b/drivers/gpu/drm/rockchip/rockchip_drm_drv.h
> index aa0909e8edf93..fd6994f21817e 100644
> --- a/drivers/gpu/drm/rockchip/rockchip_drm_drv.h
> +++ b/drivers/gpu/drm/rockchip/rockchip_drm_drv.h
> @@ -18,7 +18,7 @@
>
> #define ROCKCHIP_MAX_FB_BUFFER 3
> #define ROCKCHIP_MAX_CONNECTOR 2
> -#define ROCKCHIP_MAX_CRTC2
> +#define ROCKCHIP_MAX_CRTC4
>
> struct drm_device;
> struct drm_connector;
> @@ -31,6 +31,9 @@ struct rockchip_crtc_state {
> int output_bpc;
> int output_flags;
> bool enable_afbc;
> + uint32_t bus_format;
> + u32 bus_flags;
> + int color_space;
> };
> #define to_rockchip_crtc_state(s) \
> container_of(s, struct rockchip_crtc_state, base)
> @@ -65,4 +68,6 @@ extern struct platform_driver rockchip_dp_driver;
> extern struct platform_driver rockchip_lvds_driver;
> extern struct platform_driver vop_platform_driver;
Re: [Intel-gfx] [PATCH] drm/i915/guc: Log engine resets
On 20/12/2021 18:34, John Harrison wrote:
On 12/20/2021 07:00, Tvrtko Ursulin wrote:
On 17/12/2021 16:22, Matthew Brost wrote:
On Fri, Dec 17, 2021 at 12:15:53PM +, Tvrtko Ursulin wrote:
On 14/12/2021 15:07, Tvrtko Ursulin wrote:
From: Tvrtko Ursulin
Log engine resets done by the GuC firmware in the similar way it is
done
by the execlists backend.
This way we have notion of where the hangs are before the GuC gains
support for proper error capture.
Ping - any interest to log this info?
All there currently is a non-descriptive "[drm] GPU HANG: ecode
12:0:".
Yea, this could be helpful. One suggestion below.
Also, will GuC be reporting the reason for the engine reset at any
point?
We are working on the error state capture, presumably the registers will
give a clue what caused the hang.
As for the GuC providing a reason, that isn't defined in the interface
but that is decent idea to provide a hint in G2H what the issue was. Let
me run that by the i915 GuC developers / GuC firmware team and see what
they think.
The GuC does not do any hang analysis. So as far as GuC is concerned,
the reason is pretty much always going to be pre-emption timeout. There
are a few ways the pre-emption itself could be triggered but basically,
if GuC resets an active context then it is because it did not pre-empt
quickly enough when requested.
Regards,
Tvrtko
Signed-off-by: Tvrtko Ursulin
Cc: Matthew Brost
Cc: John Harrison
---
drivers/gpu/drm/i915/gt/uc/intel_guc_submission.c | 12 +++-
1 file changed, 11 insertions(+), 1 deletion(-)
diff --git a/drivers/gpu/drm/i915/gt/uc/intel_guc_submission.c
b/drivers/gpu/drm/i915/gt/uc/intel_guc_submission.c
index 9739da6f..51512123dc1a 100644
--- a/drivers/gpu/drm/i915/gt/uc/intel_guc_submission.c
+++ b/drivers/gpu/drm/i915/gt/uc/intel_guc_submission.c
@@ -11,6 +11,7 @@
#include "gt/intel_context.h"
#include "gt/intel_engine_pm.h"
#include "gt/intel_engine_heartbeat.h"
+#include "gt/intel_engine_user.h"
#include "gt/intel_gpu_commands.h"
#include "gt/intel_gt.h"
#include "gt/intel_gt_clock_utils.h"
@@ -3934,9 +3935,18 @@ static void capture_error_state(struct
intel_guc *guc,
{
struct intel_gt *gt = guc_to_gt(guc);
struct drm_i915_private *i915 = gt->i915;
- struct intel_engine_cs *engine =
__context_to_physical_engine(ce);
+ struct intel_engine_cs *engine = ce->engine;
intel_wakeref_t wakeref;
+ if (intel_engine_is_virtual(engine)) {
+ drm_notice(>drm, "%s class, engines 0x%x; GuC engine
reset\n",
+ intel_engine_class_repr(engine->class),
+ engine->mask);
+ engine = guc_virtual_get_sibling(engine, 0);
+ } else {
+ drm_notice(>drm, "%s GuC engine reset\n",
engine->name);
Probably include the guc_id of the context too then?
Is the guc id stable and useful on its own - who would be the user?
The GuC id is the only thing that matters when trying to correlate KMD
activity with a GuC log. So while it might not be of any use or interest
to an end user, it is extremely important and useful to a kernel
developer attempting to debug an issue. And that includes bug reports
from end users that are hard to repro given that the standard error
capture will include the GuC log.
On the topic of GuC log - is there a tool in IGT (or will be) which will
parse the bit saved in the error capture or how is that supposed to be used?
Also, note that GuC really resets contexts rather than engines. What it
reports back to i915 on a reset is simply the GuC id of the context. It
is up to i915 to work back from that to determine engine
instances/classes if required. And in the case of a virtual context, it
is impossible to extract the actual instance number. So your above print
about resetting all instances within the virtual engine mask is
incorrect/misleading. The reset would have been applied to one and only
one of those engines. If you really need to know exactly which engine
was poked, you need to look inside the GuC log.
I think I understood that part. :) It wasn't my intent to imply in the
message multiple engines have been reset, but in the case of veng, log
the class and mask and the fact there was an engine reset (singular).
Clearer message can probably be written.
However, the follow up point is to ask why you need to report the exact
class/instance? The end user doesn't care about which specific engine
got reset. They only care that their context was reset. Even a KMD
developer doesn't really care unless the concern is about a hardware bug
rather than a software bug.
I was simply aligning both backends to log as similar information as
possible. Information is there, just not logged.
Concerning the wider topic, my thinking is end user is mainly interested
to know there are any engine resets happening (to tie with the
experience of UI/video glitching or whatever). Going for deeper analysis
Re: [PATCH] dt-bindings: display: Add SPI peripheral schema to SPI based displays
On Tue, Dec 21, 2021 at 1:52 PM Rob Herring wrote: > With 'unevaluatedProperties' support enabled, several SPI based display > binding examples have warnings: (...) > Fix all of these by adding a reference to spi-peripheral-props.yaml. > With this, the description that the binding must follow > spi-controller.yaml is both a bit out of date and redundant, so remove > it. > > Signed-off-by: Rob Herring Excellent patch. Reviewed-by: Linus Walleij Yours, Linus Walleij
[PATCH] dt-bindings: display: Add SPI peripheral schema to SPI based displays
With 'unevaluatedProperties' support enabled, several SPI based display
binding examples have warnings:
Documentation/devicetree/bindings/display/panel/samsung,ld9040.example.dt.yaml:
lcd@0: Unevaluated properties are not allowed ('#address-cells', '#size-cells',
'spi-max-frequency', 'spi-cpol', 'spi-cpha' were unexpected)
Documentation/devicetree/bindings/display/panel/kingdisplay,kd035g6-54nt.example.dt.yaml:
panel@0: Unevaluated properties are not allowed ('spi-max-frequency',
'spi-3wire' were unexpected)
Documentation/devicetree/bindings/display/panel/ilitek,ili9322.example.dt.yaml:
display@0: Unevaluated properties are not allowed ('reg' was unexpected)
Documentation/devicetree/bindings/display/panel/samsung,s6e63m0.example.dt.yaml:
display@0: Unevaluated properties are not allowed ('spi-max-frequency' was
unexpected)
Documentation/devicetree/bindings/display/panel/abt,y030xx067a.example.dt.yaml:
panel@0: Unevaluated properties are not allowed ('spi-max-frequency' was
unexpected)
Documentation/devicetree/bindings/display/panel/sony,acx565akm.example.dt.yaml:
panel@2: Unevaluated properties are not allowed ('spi-max-frequency', 'reg'
were unexpected)
Documentation/devicetree/bindings/display/panel/tpo,td.example.dt.yaml:
panel@0: Unevaluated properties are not allowed ('spi-max-frequency',
'spi-cpol', 'spi-cpha' were unexpected)
Documentation/devicetree/bindings/display/panel/lgphilips,lb035q02.example.dt.yaml:
panel@0: Unevaluated properties are not allowed ('reg', 'spi-max-frequency',
'spi-cpol', 'spi-cpha' were unexpected)
Documentation/devicetree/bindings/display/panel/innolux,ej030na.example.dt.yaml:
panel@0: Unevaluated properties are not allowed ('spi-max-frequency' was
unexpected)
Documentation/devicetree/bindings/display/panel/sitronix,st7789v.example.dt.yaml:
panel@0: Unevaluated properties are not allowed ('spi-max-frequency',
'spi-cpol', 'spi-cpha' were unexpected)
Fix all of these by adding a reference to spi-peripheral-props.yaml.
With this, the description that the binding must follow
spi-controller.yaml is both a bit out of date and redundant, so remove
it.
Signed-off-by: Rob Herring
---
This is dependent on spi-peripheral-props.yaml landing in 5.17-rc1.
---
.../devicetree/bindings/display/panel/abt,y030xx067a.yaml | 5 +
.../devicetree/bindings/display/panel/ilitek,ili9322.yaml | 4 +---
.../devicetree/bindings/display/panel/innolux,ej030na.yaml | 5 +
.../bindings/display/panel/kingdisplay,kd035g6-54nt.yaml | 5 +
.../bindings/display/panel/lgphilips,lb035q02.yaml | 5 +
.../devicetree/bindings/display/panel/samsung,ld9040.yaml | 7 +--
.../devicetree/bindings/display/panel/samsung,s6e63m0.yaml | 1 +
.../bindings/display/panel/sitronix,st7789v.yaml | 5 +
.../devicetree/bindings/display/panel/sony,acx565akm.yaml | 5 +
.../devicetree/bindings/display/panel/tpo,td.yaml | 5 +
10 files changed, 10 insertions(+), 37 deletions(-)
diff --git
a/Documentation/devicetree/bindings/display/panel/abt,y030xx067a.yaml
b/Documentation/devicetree/bindings/display/panel/abt,y030xx067a.yaml
index a108029ecfab..acd2f3faa6b9 100644
--- a/Documentation/devicetree/bindings/display/panel/abt,y030xx067a.yaml
+++ b/Documentation/devicetree/bindings/display/panel/abt,y030xx067a.yaml
@@ -6,15 +6,12 @@ $schema: http://devicetree.org/meta-schemas/core.yaml#
title: Asia Better Technology 3.0" (320x480 pixels) 24-bit IPS LCD panel
-description: |
- The panel must obey the rules for a SPI slave device as specified in
- spi/spi-controller.yaml
-
maintainers:
- Paul Cercueil
allOf:
- $ref: panel-common.yaml#
+ - $ref: /schemas/spi/spi-peripheral-props.yaml#
properties:
compatible:
diff --git
a/Documentation/devicetree/bindings/display/panel/ilitek,ili9322.yaml
b/Documentation/devicetree/bindings/display/panel/ilitek,ili9322.yaml
index e89c1ea62ffa..7d221ef35443 100644
--- a/Documentation/devicetree/bindings/display/panel/ilitek,ili9322.yaml
+++ b/Documentation/devicetree/bindings/display/panel/ilitek,ili9322.yaml
@@ -15,11 +15,9 @@ description: |
960 TFT source driver pins and 240 TFT gate driver pins, VCOM, VCOML and
VCOMH outputs.
- The panel must obey the rules for a SPI slave device as specified in
- spi/spi-controller.yaml
-
allOf:
- $ref: panel-common.yaml#
+ - $ref: /schemas/spi/spi-peripheral-props.yaml#
properties:
compatible:
diff --git
a/Documentation/devicetree/bindings/display/panel/innolux,ej030na.yaml
b/Documentation/devicetree/bindings/display/panel/innolux,ej030na.yaml
index cda36c04e85c..72788e3e6c59 100644
--- a/Documentation/devicetree/bindings/display/panel/innolux,ej030na.yaml
+++ b/Documentation/devicetree/bindings/display/panel/innolux,ej030na.yaml
@@ -6,15 +6,12 @@ $schema: http://devicetree.org/meta-schemas/core.yaml#
title: Innolux EJ030NA 3.0" (320x480 pixels) 24-bit TFT LCD panel
-description: |
- The panel must obey the rules for a SPI
[PATCH] dt-bindings: display: st, stm32-dsi: Fix panel node name in example
With 'unevaluatedProperties' support enabled, the st,stm32-dsi binding
has a new warning:
Documentation/devicetree/bindings/display/st,stm32-dsi.example.dt.yaml:
dsi@5a00: Unevaluated properties are not allowed ('panel-dsi@0' was
unexpected)
The documented child node name is 'panel', so update the example.
Signed-off-by: Rob Herring
---
Documentation/devicetree/bindings/display/st,stm32-dsi.yaml | 3 +--
1 file changed, 1 insertion(+), 2 deletions(-)
diff --git a/Documentation/devicetree/bindings/display/st,stm32-dsi.yaml
b/Documentation/devicetree/bindings/display/st,stm32-dsi.yaml
index ce1ef93cce93..54f67cb51040 100644
--- a/Documentation/devicetree/bindings/display/st,stm32-dsi.yaml
+++ b/Documentation/devicetree/bindings/display/st,stm32-dsi.yaml
@@ -110,7 +110,7 @@ examples:
};
};
-panel-dsi@0 {
+panel@0 {
compatible = "orisetech,otm8009a";
reg = <0>;
reset-gpios = < 4 GPIO_ACTIVE_LOW>;
@@ -125,4 +125,3 @@ examples:
};
...
-
--
2.32.0
[PATCH] dt-bindings: display: novatek, nt36672a: Fix unevaluated properties warning
With 'unevaluatedProperties' support enabled, the novatek,nt36672a
binding has a new warning:
Documentation/devicetree/bindings/display/panel/novatek,nt36672a.example.dt.yaml:
panel@0: Unevaluated properties are not allowed ('vddi0-supply',
'#address-cells', '#size-cells' were unexpected)
Based on dts files, 'vddi0-supply' does appear to be the correct name.
Drop '#address-cells' and '#size-cells' which aren't needed.
Signed-off-by: Rob Herring
---
.../devicetree/bindings/display/panel/novatek,nt36672a.yaml | 4 +---
1 file changed, 1 insertion(+), 3 deletions(-)
diff --git
a/Documentation/devicetree/bindings/display/panel/novatek,nt36672a.yaml
b/Documentation/devicetree/bindings/display/panel/novatek,nt36672a.yaml
index ef4c0a24512d..563766d283f6 100644
--- a/Documentation/devicetree/bindings/display/panel/novatek,nt36672a.yaml
+++ b/Documentation/devicetree/bindings/display/panel/novatek,nt36672a.yaml
@@ -34,7 +34,7 @@ properties:
description: phandle of gpio for reset line - This should be 8mA, gpio
can be configured using mux, pinctrl, pinctrl-names (active high)
- vddio-supply:
+ vddi0-supply:
description: phandle of the regulator that provides the supply voltage
Power IC supply
@@ -75,8 +75,6 @@ examples:
reset-gpios = < 6 GPIO_ACTIVE_HIGH>;
-#address-cells = <1>;
-#size-cells = <0>;
port {
tianma_nt36672a_in_0: endpoint {
remote-endpoint = <_out>;
--
2.32.0
Re: [PATCH 1/3] drm/plane: Mention format_mod_supported in IN_FORMATS docs
On Tuesday, December 21st, 2021 at 13:33, Dmitry Baryshkov wrote: > I'd still suggest to fix create_in_format_blob() Yeah, I agree. I thought there were a good reason why create_in_format_blob() behaves this way but can't find anything in the Git history, so fixing it to behave as the documentation says would be best.
Re: [PATCH 1/3] drm/plane: Mention format_mod_supported in IN_FORMATS docs
On Tue, 21 Dec 2021 at 13:50, Simon Ser wrote: > > On Tuesday, December 21st, 2021 at 11:48, Dmitry Baryshkov > wrote: > > > I think the fix should be applied to the generic code. > > Related: > https://lore.kernel.org/dri-devel/t1g_xNE6hgj1nTQfx2UWob1wbsCAxElBvWRwNSY_EzmlEe_9WWpq8-vQKyJPK6wZY8q8BqHl-KoGwS5V91VgN8lGIl3PJt7s2fkdsRd3y70=@emersion.fr/T/#u I'd still suggest to fix create_in_format_blob() -- With best wishes Dmitry
Re: [PATCH] drm: adv7511: override i2c address of cec before accessing it
Quoting Antonio Borneo (2021-12-20 15:53:12)
> On Mon, 2021-12-20 at 14:54 +, Kieran Bingham wrote:
> > Hi Antonio,
> >
> > Quoting Antonio Borneo (2021-12-18 18:28:04)
> > > Commit 680532c50bca ("drm: adv7511: Add support for
> > > i2c_new_secondary_device") allows a device tree node to override
> > > the default addresses of the secondary i2c devices. This is useful
> > > for solving address conflicts on the i2c bus.
> > >
> > > In adv7511_init_cec_regmap() the new i2c address of cec device is
> > > read from device tree and immediately accessed, well before it is
> > > written in the proper register to override the default address.
> > > This can cause an i2c error during probe and a consequent probe
> > > failure.
> >
> > Ouch, it does seem that way. I guess no one has used the CEC for
> > quite
> > some time, as it must have been like this for a while?
>
> Using the default i2c address for cec works without problem; apparently
> everyone is happy with such default. The issue appears only when you
> have to override the default cec address.
> The commit 680532c50bca landed in v4.18.
Ok, phew - so the 'normal' case still worked. That makes sense.
Sorry for getting it wrong, and I hope it didn't take too long to find
and fix. I'm sure we'll see it percolate down the stable trees once
integrated.
--
Kieran
> > > Once the new i2c address is read from the device tree, override
> > > the default address before any attempt to access the cec.
> >
> > Reviewed-by: Kieran Bingham
>
> Thanks!
> Antonio
>
> > > Tested with adv7533 and stm32mp157f.
> > >
> > > Signed-off-by: Antonio Borneo
> > > Fixes: 680532c50bca ("drm: adv7511: Add support for
> > > i2c_new_secondary_device")
> > > ---
> > > To: Andrzej Hajda
> > > To: Neil Armstrong
> > > To: Robert Foss
> > > To: Laurent Pinchart
> > > To: Jonas Karlman
> > > To: Jernej Skrabec
> > > To: David Airlie
> > > To: Daniel Vetter
> > > To: Kieran Bingham
> > > To: dri-devel@lists.freedesktop.org
> > > Cc: linux-ker...@vger.kernel.org
> > > Cc: linux-st...@st-md-mailman.stormreply.com
> > > ---
> > > drivers/gpu/drm/bridge/adv7511/adv7511_drv.c | 7 ---
> > > 1 file changed, 4 insertions(+), 3 deletions(-)
> > >
> > > diff --git a/drivers/gpu/drm/bridge/adv7511/adv7511_drv.c
> > > b/drivers/gpu/drm/bridge/adv7511/adv7511_drv.c
> > > index 76555ae64e9c..629e05286fd9 100644
> > > --- a/drivers/gpu/drm/bridge/adv7511/adv7511_drv.c
> > > +++ b/drivers/gpu/drm/bridge/adv7511/adv7511_drv.c
> > > @@ -1048,6 +1048,10 @@ static int adv7511_init_cec_regmap(struct
> > > adv7511 *adv)
> > >
> > > ADV7511_CEC_I2C_ADDR_DEFAULT);
> > > if (IS_ERR(adv->i2c_cec))
> > > return PTR_ERR(adv->i2c_cec);
> > > +
> > > + regmap_write(adv->regmap, ADV7511_REG_CEC_I2C_ADDR,
> > > + adv->i2c_cec->addr << 1);
> > > +
> > > i2c_set_clientdata(adv->i2c_cec, adv);
> > >
> > > adv->regmap_cec = devm_regmap_init_i2c(adv->i2c_cec,
> > > @@ -1252,9 +1256,6 @@ static int adv7511_probe(struct i2c_client
> > > *i2c, const struct i2c_device_id *id)
> > > if (ret)
> > > goto err_i2c_unregister_packet;
> > >
> > > - regmap_write(adv7511->regmap, ADV7511_REG_CEC_I2C_ADDR,
> > > - adv7511->i2c_cec->addr << 1);
> > > -
> > > INIT_WORK(>hpd_work, adv7511_hpd_work);
> > >
> > > if (i2c->irq) {
> > >
> > > base-commit: fc74881c28d314b10efac016ef49df4ff40b8b97
> > > --
> > > 2.34.1
> > >
>
Re: [PATCH v3 3/7] drm/i915: Require the vm mutex for i915_vma_bind()
On 17/12/2021 14:52, Thomas Hellström wrote: Protect updates of struct i915_vma flags and async binding / unbinding with the vm::mutex. This means that i915_vma_bind() needs to assert vm::mutex held. In order to make that possible drop the caching of kmap_atomic() maps around i915_vma_bind(). An alternative would be to use kmap_local() but since we block cpu unplugging during sleeps inside kmap_local() sections this may have unwanted side-effects. Particularly since we might wait for gpu while holding the vm mutex. This change may theoretically increase execbuf cpu-usage on snb, but at least on non-highmem systems that increase should be very small. Signed-off-by: Thomas Hellström Reviewed-by: Matthew Auld
Re: [PATCH v3 2/7] drm/i915: Break out the i915_deps utility
On 17/12/2021 14:52, Thomas Hellström wrote: Since it's starting to be used outside the i915 TTM move code, move it to a separate set of files. v2: - Update the documentation. Signed-off-by: Thomas Hellström Reviewed-by: Matthew Auld
Re: [PATCH v3 1/7] drm/i915: Avoid using the i915_fence_array when collecting dependencies
On 17/12/2021 14:52, Thomas Hellström wrote:
Since the gt migration code was using only a single fence for
dependencies, these were collected in a dma_fence_array. However, it
turns out that it's illegal to use some dma_fences in a dma_fence_array,
in particular other dma_fence_arrays and dma_fence_chains, and this
causes trouble for us moving forward.
Have the gt migration code instead take a const struct i915_deps for
dependencies. This means we can skip the dma_fence_array creation
and instead pass the struct i915_deps instead to circumvent the
problem.
v2:
- Make the prev_deps() function static. (kernel test robot )
- Update the struct i915_deps kerneldoc.
Fixes: 5652df829b3c ("drm/i915/ttm: Update i915_gem_obj_copy_ttm() to be
asynchronous")
Signed-off-by: Thomas Hellström
Reviewed-by: Matthew Auld
Re: [PATCH 0/2] drm/tegra: Fix panel support on Venice 2 and Nyan
On Mon, Dec 20, 2021 at 07:12:03PM +0300, Dmitry Osipenko wrote: > 20.12.2021 18:27, Thierry Reding пишет: > > On Mon, Dec 20, 2021 at 05:45:41PM +0300, Dmitry Osipenko wrote: > >> 20.12.2021 13:48, Thierry Reding пишет: > >>> From: Thierry Reding > >>> > >>> Hi, > >>> > >>> this is an alternative proposal to fix panel support on Venice 2 and > >>> Nyan. Dmitry had proposed a different solution that involved reverting > >>> the I2C/DDC registration order and would complicate things by breaking > >>> the encapsulation of the driver by introducing a global (though locally > >>> scoped) variable[0]. > >>> > >>> This set of patches avoids that by using the recently introduced DP AUX > >>> bus infrastructure. The result is that the changes are actually less > >>> intrusive and not a step back. Instead they nicely remove the circular > >>> dependency that previously existed and caused these issues in the first > >>> place. > >>> > >>> To be fair, this is not perfect either because it requires a device tree > >>> change and hence isn't technically backwards-compatible. However, given > >>> that the original device tree was badly broken in the first place, I > >>> think we can make an exception, especially since it is not generally a > >>> problem to update device trees on the affected devices. > >>> > >>> Secondly, this relies on infrastructure that was introduced in v5.15 and > >>> therefore will be difficult to backport beyond that. However, since this > >>> functionality has been broken since v5.13 and all of the kernel versions > >>> between that and v5.15 are EOL anyway, there isn't much that we can do > >>> to fix the interim versions anyway. > >>> > >>> Adding Doug and Laurent since they originally designed the AUX bus > >>> patches in case they see anything in here that would be objectionable. > >>> > >>> Thierry > >>> > >>> [0]: > >>> https://lore.kernel.org/dri-devel/20211130230957.30213-1-dig...@gmail.com/ > >>> > >>> Thierry Reding (2): > >>> drm/tegra: dpaux: Populate AUX bus > >>> ARM: tegra: Move panels to AUX bus > >>> > >>> arch/arm/boot/dts/tegra124-nyan-big.dts | 15 +-- > >>> arch/arm/boot/dts/tegra124-nyan-blaze.dts | 15 +-- > >>> arch/arm/boot/dts/tegra124-venice2.dts| 14 +++--- > >>> drivers/gpu/drm/tegra/Kconfig | 1 + > >>> drivers/gpu/drm/tegra/dpaux.c | 7 +++ > >>> 5 files changed, 33 insertions(+), 19 deletions(-) > >>> > >> > >> It should "work" since you removed the ddc-i2c-bus phandle from the > >> panel nodes, and thus, panel->ddc won't be used during panel-edp driver > >> probe. But this looks like a hack rather than a fix. > > > > The AUX ->ddc will be used for panel->ddc if the ddc-i2c-bus property is > > not specified. And that makes perfect sense because we'd basically just > > be pointing back to the AUX node anyway. > > > >> I'm not sure why and how devm_of_dp_aux_populate_ep_devices() usage > >> should be relevant here. The drm_dp_aux_register() still should to > >> invoked before devm_of_dp_aux_populate_ep_devices(), otherwise > >> panel->ddc adapter won't be registered. > > > > drm_dp_aux_register() is only needed to expose the device to userspace > > and make the I2C adapter available to the rest of the system. But since > > we already know the AUX and I2C adapter, we can use it directly without > > doing a separate lookup. drm_dp_aux_init() should be enough to set the > > adapter up to work for what we need. > > > > See also the kerneldoc for drm_dp_aux_register() where this is described > > in a bit more detail. > > Alright, so you fixed it by removing the ddc-i2c-bus phandles and I2C > DDC will work properly anyways with that on v5.16. > > But the aux-bus usage still is irrelevant for the fix. Let's not use it > then. Yes, it's completely relevant because it essentially replaces the I2C DDC. With the AUX bus, the AUX and hence the I2C DDC can be implied from the bus' parent. > >> The panel->ddc isn't used by the new panel-edp driver unless panel is > >> compatible with "edp-panel". Hence the generic_edp_panel_probe() should > >> either fail or crash for a such "edp-panel" since panel->ddc isn't fully > >> instantiated, AFAICS. > > > > I've tested this and it works fine on Venice 2. Since that was the > > reference design for Nyan, I suspect that Nyan's will also work. > > > > It'd be great if Thomas or anyone else with access to a Nyan could > > test this to verify that. > > There is no panel-edp driver in the v5.15. The EOL of v5.15 is Oct, > 2023, hence we need to either use: All the (at least relevant) functionality that is in panel-edp was in panel-simple before it was moved to panel-edp. I've backported this set of patches to v5.15 and it works just fine there. Thierry signature.asc Description: PGP signature
Re: [PATCH 1/3] drm/plane: Mention format_mod_supported in IN_FORMATS docs
On Tuesday, December 21st, 2021 at 11:48, Dmitry Baryshkov wrote: > I think the fix should be applied to the generic code. Related: https://lore.kernel.org/dri-devel/t1g_xNE6hgj1nTQfx2UWob1wbsCAxElBvWRwNSY_EzmlEe_9WWpq8-vQKyJPK6wZY8q8BqHl-KoGwS5V91VgN8lGIl3PJt7s2fkdsRd3y70=@emersion.fr/T/#u
Re: [PATCH 0/3] Add missing format_mod_supported functions
On Tue, 21 Dec 2021 at 13:13, José Expósito wrote: > > Hi all, > > When setting IN_FORMATS, implementing the > "drm_plane_funcs.format_mod_supported" function is mandatory to avoid > exposing a bogus blob. > > This patchset adds a bit of documentation and fixes the issue in a > couple of drivers affected by the bug. > > I reviewed all the other drivers and I didn't find more instances of > the issue. > > Jose > > José Expósito (3): > drm/plane: Mention format_mod_supported in IN_FORMATS docs > drm/msm/mdp4: Add format_mod_supported function > drm/sun4i: Add format_mod_supported function > > drivers/gpu/drm/drm_plane.c| 7 +-- > drivers/gpu/drm/msm/disp/mdp4/mdp4_plane.c | 8 You missed mdp5_plane.c here. But I assume that the proposed fix is not correct, see my comments on patch 1. > drivers/gpu/drm/sun4i/sun8i_ui_layer.c | 7 +++ > drivers/gpu/drm/sun4i/sun8i_vi_layer.c | 7 +++ > 4 files changed, 27 insertions(+), 2 deletions(-) > > -- > 2.25.1 > -- With best wishes Dmitry
Re: [PATCH 1/3] drm/plane: Mention format_mod_supported in IN_FORMATS docs
On Tue, 21 Dec 2021 at 13:13, José Expósito wrote: > > Adding format modifiers without implementing the function > "drm_plane_funcs.format_mod_supported" exposes an invalid IN_FORMATS > blob with modifiers but no formats to user-space. I think the fix should be applied to the generic code. The docs at drm_plane.h clearly state that this callback is optional: * This optional hook is used for the DRM to determine if the given * format/modifier combination is valid for the plane. This allows the * DRM to generate the correct format bitmask (which formats apply to * which modifier), and to valdiate modifiers at atomic_check time. * * If not present, then any modifier in the plane's modifier * list is allowed with any of the plane's formats. So, one should fix the core code in create_in_format_blob() to include all combinations. > > This breaks the latest Weston [1]. For testing purposes, I extracted the > affected code to a standalone program [2]. > > Make clear in the IN_FORMATS documentation that implementing > "format_mod_supported" is mandatory. format_mod_supported() being optional or mandatory should be documented in the format_mod_supported() docs, not in the IN_FORMAT docs. > > [1] > https://gitlab.freedesktop.org/wayland/weston/-/blob/9.0/libweston/backend-drm/kms.c#L431 > [2] https://github.com/JoseExposito/drm-sandbox/blob/main/in_formats.c > > Signed-off-by: José Expósito > --- > drivers/gpu/drm/drm_plane.c | 7 +-- > 1 file changed, 5 insertions(+), 2 deletions(-) > > diff --git a/drivers/gpu/drm/drm_plane.c b/drivers/gpu/drm/drm_plane.c > index 82afb854141b..347571f8909a 100644 > --- a/drivers/gpu/drm/drm_plane.c > +++ b/drivers/gpu/drm/drm_plane.c > @@ -126,8 +126,11 @@ > * IN_FORMATS: > * Blob property which contains the set of buffer format and modifier > * pairs supported by this plane. The blob is a struct > - * drm_format_modifier_blob. Without this property the plane doesn't > - * support buffers with modifiers. Userspace cannot change this property. > + * drm_format_modifier_blob and the drm_plane_funcs.format_mod_supported > + * function must be implemented by the driver to determine if the given > + * format/modifier combination is valid for the plane. Without this > property > + * the plane doesn't support buffers with modifiers. Userspace cannot > change > + * this property. > * > * Note that userspace can check the _CAP_ADDFB2_MODIFIERS driver > * capability for general modifier support. If this flag is set then > every > -- > 2.25.1 > -- With best wishes Dmitry
Re: [PATCH v2 2/2] drm/vkms: set plane modifiers
Overall looks good, but it is a bit repetitive to copy & paste this in all
drivers. It'd be nice to provide a core helper to do this, and then drivers
can just set format_mod_supported to the helper if they don't have more
involved logic. Thoughts?
See drm_plane_check_pixel_format, where the logic is already implemented.
Alternatively… We can just support a missing format_mod_supported in
create_in_format_blob. This sounds like this was the original intention of
db1689aa61bd ("drm: Create a format/modifier blob") and
drm_plane_check_pixel_format.
Re: [PATCH 1/3] drm/plane: Mention format_mod_supported in IN_FORMATS docs
Reviewed-by: Simon Ser Please ping me in a week or so if nobody objected and this isn't merged.
Re: [Nouveau] [PATCH] drm/nouveau: wait for the exclusive fence after the shared ones v2
Am 21.12.21 um 11:11 schrieb Thorsten Leemhuis:
Hi, this is your Linux kernel regression tracker speaking.
CCing Dave and Daniel.
On 15.12.21 23:32, Ben Skeggs wrote:
On Tue, 14 Dec 2021 at 19:19, Christian König
wrote:
Am 11.12.21 um 10:59 schrieb Stefan Fritsch:
On 09.12.21 11:23, Christian König wrote:
Always waiting for the exclusive fence resulted on some performance
regressions. So try to wait for the shared fences first, then the
exclusive fence should always be signaled already.
v2: fix incorrectly placed "(", add some comment why we do this.
Signed-off-by: Christian König
Tested-by: Stefan Fritsch
Thanks.
Please also add a cc for linux-stable, so that this is fixed in 5.15.x
Sure, but I still need some acked-by or rb from one of the Nouveau guys.
So gentle ping on that.
Acked-by: Ben Skeggs
What's the status of this patch? I checked a few git trees, but either
it's not there or it missed it.
You missed it. I've pushed it to drm-misc-fixes about 2 hours ago:
https://cgit.freedesktop.org/drm/drm-misc/log/?h=drm-misc-fixes
Regards,
Christian.
Reminder, it's a regression already introduced in v5.15, hence all users
of the current stable kernel are affected by it, so it would be nice to
get the fix on its way now that Ben acked it and Dan tested it.
Ciao, Thorsten
P.S.: As a Linux kernel regression tracker I'm getting a lot of reports
on my table. I can only look briefly into most of them. Unfortunately
therefore I sometimes will get things wrong or miss something important.
I hope that's not the case here; if you think it is, don't hesitate to
tell me about it in a public reply. That's in everyone's interest, as
what I wrote above might be misleading to everyone reading this; any
suggestion I gave thus might sent someone reading this down the wrong
rabbit hole, which none of us wants.
BTW, I have no personal interest in this issue, which is tracked using
regzbot, my Linux kernel regression tracking bot
(https://linux-regtracking.leemhuis.info/regzbot/). I'm only posting
this mail to get things rolling again and hence don't need to be CC on
all further activities wrt to this regression.
#regzbot poke
---
drivers/gpu/drm/nouveau/nouveau_fence.c | 28 +
1 file changed, 15 insertions(+), 13 deletions(-)
diff --git a/drivers/gpu/drm/nouveau/nouveau_fence.c
b/drivers/gpu/drm/nouveau/nouveau_fence.c
index 05d0b3eb3690..0ae416aa76dc 100644
--- a/drivers/gpu/drm/nouveau/nouveau_fence.c
+++ b/drivers/gpu/drm/nouveau/nouveau_fence.c
@@ -353,15 +353,22 @@ nouveau_fence_sync(struct nouveau_bo *nvbo,
struct nouveau_channel *chan, bool e
if (ret)
return ret;
-}
-fobj = dma_resv_shared_list(resv);
-fence = dma_resv_excl_fence(resv);
+fobj = NULL;
+} else {
+fobj = dma_resv_shared_list(resv);
+}
-if (fence) {
+/* Waiting for the exclusive fence first causes performance
regressions
+ * under some circumstances. So manually wait for the shared
ones first.
+ */
+for (i = 0; i < (fobj ? fobj->shared_count : 0) && !ret; ++i) {
struct nouveau_channel *prev = NULL;
bool must_wait = true;
+fence = rcu_dereference_protected(fobj->shared[i],
+dma_resv_held(resv));
+
f = nouveau_local_fence(fence, chan->drm);
if (f) {
rcu_read_lock();
@@ -373,20 +380,13 @@ nouveau_fence_sync(struct nouveau_bo *nvbo,
struct nouveau_channel *chan, bool e
if (must_wait)
ret = dma_fence_wait(fence, intr);
-
-return ret;
}
-if (!exclusive || !fobj)
-return ret;
-
-for (i = 0; i < fobj->shared_count && !ret; ++i) {
+fence = dma_resv_excl_fence(resv);
+if (fence) {
struct nouveau_channel *prev = NULL;
bool must_wait = true;
-fence = rcu_dereference_protected(fobj->shared[i],
-dma_resv_held(resv));
-
f = nouveau_local_fence(fence, chan->drm);
if (f) {
rcu_read_lock();
@@ -398,6 +398,8 @@ nouveau_fence_sync(struct nouveau_bo *nvbo,
struct nouveau_channel *chan, bool e
if (must_wait)
ret = dma_fence_wait(fence, intr);
+
+return ret;
}
return ret;
[PATCH 2/3] drm/msm/mdp4: Add format_mod_supported function
Implement the missing "drm_plane_funcs.format_mod_supported" function
to avoid exposing an invalid IN_FORMATS blob with modifiers but no
formats.
Signed-off-by: José Expósito
---
drivers/gpu/drm/msm/disp/mdp4/mdp4_plane.c | 8
1 file changed, 8 insertions(+)
diff --git a/drivers/gpu/drm/msm/disp/mdp4/mdp4_plane.c
b/drivers/gpu/drm/msm/disp/mdp4/mdp4_plane.c
index 49bdabea8ed5..8809f1633786 100644
--- a/drivers/gpu/drm/msm/disp/mdp4/mdp4_plane.c
+++ b/drivers/gpu/drm/msm/disp/mdp4/mdp4_plane.c
@@ -80,6 +80,13 @@ static int mdp4_plane_set_property(struct drm_plane *plane,
return -EINVAL;
}
+static bool mdp4_plane_format_mod_supported(struct drm_plane *plane, u32
format,
+ u64 modifier)
+{
+ return (modifier == DRM_FORMAT_MOD_SAMSUNG_64_32_TILE) ||
+ (modifier == DRM_FORMAT_MOD_LINEAR);
+}
+
static const struct drm_plane_funcs mdp4_plane_funcs = {
.update_plane = drm_atomic_helper_update_plane,
.disable_plane = drm_atomic_helper_disable_plane,
@@ -88,6 +95,7 @@ static const struct drm_plane_funcs mdp4_plane_funcs = {
.reset = drm_atomic_helper_plane_reset,
.atomic_duplicate_state =
drm_atomic_helper_plane_duplicate_state,
.atomic_destroy_state = drm_atomic_helper_plane_destroy_state,
+ .format_mod_supported = mdp4_plane_format_mod_supported,
};
static void mdp4_plane_cleanup_fb(struct drm_plane *plane,
--
2.25.1
[PATCH 3/3] drm/sun4i: Add format_mod_supported function
Implement the missing "drm_plane_funcs.format_mod_supported" function
to avoid exposing an invalid IN_FORMATS blob with modifiers but no
formats.
Signed-off-by: José Expósito
---
drivers/gpu/drm/sun4i/sun8i_ui_layer.c | 7 +++
drivers/gpu/drm/sun4i/sun8i_vi_layer.c | 7 +++
2 files changed, 14 insertions(+)
diff --git a/drivers/gpu/drm/sun4i/sun8i_ui_layer.c
b/drivers/gpu/drm/sun4i/sun8i_ui_layer.c
index 7845c2a53a7f..728563f23cd6 100644
--- a/drivers/gpu/drm/sun4i/sun8i_ui_layer.c
+++ b/drivers/gpu/drm/sun4i/sun8i_ui_layer.c
@@ -331,6 +331,12 @@ static void sun8i_ui_layer_atomic_update(struct drm_plane
*plane,
true, zpos, old_zpos);
}
+static bool sun8i_ui_layer_format_mod_supported(struct drm_plane *plane,
+ u32 format, u64 modifier)
+{
+ return (modifier == DRM_FORMAT_MOD_LINEAR);
+}
+
static const struct drm_plane_helper_funcs sun8i_ui_layer_helper_funcs = {
.atomic_check = sun8i_ui_layer_atomic_check,
.atomic_disable = sun8i_ui_layer_atomic_disable,
@@ -344,6 +350,7 @@ static const struct drm_plane_funcs sun8i_ui_layer_funcs = {
.disable_plane = drm_atomic_helper_disable_plane,
.reset = drm_atomic_helper_plane_reset,
.update_plane = drm_atomic_helper_update_plane,
+ .format_mod_supported = sun8i_ui_layer_format_mod_supported,
};
static const u32 sun8i_ui_layer_formats[] = {
diff --git a/drivers/gpu/drm/sun4i/sun8i_vi_layer.c
b/drivers/gpu/drm/sun4i/sun8i_vi_layer.c
index bb7c43036dfa..d17813a7cac3 100644
--- a/drivers/gpu/drm/sun4i/sun8i_vi_layer.c
+++ b/drivers/gpu/drm/sun4i/sun8i_vi_layer.c
@@ -435,6 +435,12 @@ static void sun8i_vi_layer_atomic_update(struct drm_plane
*plane,
true, zpos, old_zpos);
}
+static bool sun8i_vi_layer_format_mod_supported(struct drm_plane *plane,
+ u32 format, u64 modifier)
+{
+ return (modifier == DRM_FORMAT_MOD_LINEAR);
+}
+
static const struct drm_plane_helper_funcs sun8i_vi_layer_helper_funcs = {
.atomic_check = sun8i_vi_layer_atomic_check,
.atomic_disable = sun8i_vi_layer_atomic_disable,
@@ -448,6 +454,7 @@ static const struct drm_plane_funcs sun8i_vi_layer_funcs = {
.disable_plane = drm_atomic_helper_disable_plane,
.reset = drm_atomic_helper_plane_reset,
.update_plane = drm_atomic_helper_update_plane,
+ .format_mod_supported = sun8i_vi_layer_format_mod_supported,
};
/*
--
2.25.1
[PATCH 1/3] drm/plane: Mention format_mod_supported in IN_FORMATS docs
Adding format modifiers without implementing the function "drm_plane_funcs.format_mod_supported" exposes an invalid IN_FORMATS blob with modifiers but no formats to user-space. This breaks the latest Weston [1]. For testing purposes, I extracted the affected code to a standalone program [2]. Make clear in the IN_FORMATS documentation that implementing "format_mod_supported" is mandatory. [1] https://gitlab.freedesktop.org/wayland/weston/-/blob/9.0/libweston/backend-drm/kms.c#L431 [2] https://github.com/JoseExposito/drm-sandbox/blob/main/in_formats.c Signed-off-by: José Expósito --- drivers/gpu/drm/drm_plane.c | 7 +-- 1 file changed, 5 insertions(+), 2 deletions(-) diff --git a/drivers/gpu/drm/drm_plane.c b/drivers/gpu/drm/drm_plane.c index 82afb854141b..347571f8909a 100644 --- a/drivers/gpu/drm/drm_plane.c +++ b/drivers/gpu/drm/drm_plane.c @@ -126,8 +126,11 @@ * IN_FORMATS: * Blob property which contains the set of buffer format and modifier * pairs supported by this plane. The blob is a struct - * drm_format_modifier_blob. Without this property the plane doesn't - * support buffers with modifiers. Userspace cannot change this property. + * drm_format_modifier_blob and the drm_plane_funcs.format_mod_supported + * function must be implemented by the driver to determine if the given + * format/modifier combination is valid for the plane. Without this property + * the plane doesn't support buffers with modifiers. Userspace cannot change + * this property. * * Note that userspace can check the _CAP_ADDFB2_MODIFIERS driver * capability for general modifier support. If this flag is set then every -- 2.25.1
[PATCH 0/3] Add missing format_mod_supported functions
Hi all, When setting IN_FORMATS, implementing the "drm_plane_funcs.format_mod_supported" function is mandatory to avoid exposing a bogus blob. This patchset adds a bit of documentation and fixes the issue in a couple of drivers affected by the bug. I reviewed all the other drivers and I didn't find more instances of the issue. Jose José Expósito (3): drm/plane: Mention format_mod_supported in IN_FORMATS docs drm/msm/mdp4: Add format_mod_supported function drm/sun4i: Add format_mod_supported function drivers/gpu/drm/drm_plane.c| 7 +-- drivers/gpu/drm/msm/disp/mdp4/mdp4_plane.c | 8 drivers/gpu/drm/sun4i/sun8i_ui_layer.c | 7 +++ drivers/gpu/drm/sun4i/sun8i_vi_layer.c | 7 +++ 4 files changed, 27 insertions(+), 2 deletions(-) -- 2.25.1
Re: [Nouveau] [PATCH] drm/nouveau: wait for the exclusive fence after the shared ones v2
Hi, this is your Linux kernel regression tracker speaking.
CCing Dave and Daniel.
On 15.12.21 23:32, Ben Skeggs wrote:
> On Tue, 14 Dec 2021 at 19:19, Christian König
> wrote:
>>
>> Am 11.12.21 um 10:59 schrieb Stefan Fritsch:
>>> On 09.12.21 11:23, Christian König wrote:
Always waiting for the exclusive fence resulted on some performance
regressions. So try to wait for the shared fences first, then the
exclusive fence should always be signaled already.
v2: fix incorrectly placed "(", add some comment why we do this.
Signed-off-by: Christian König
>>>
>>> Tested-by: Stefan Fritsch
>>
>> Thanks.
>>
>>>
>>> Please also add a cc for linux-stable, so that this is fixed in 5.15.x
>>
>> Sure, but I still need some acked-by or rb from one of the Nouveau guys.
>> So gentle ping on that.
> Acked-by: Ben Skeggs
What's the status of this patch? I checked a few git trees, but either
it's not there or it missed it.
Reminder, it's a regression already introduced in v5.15, hence all users
of the current stable kernel are affected by it, so it would be nice to
get the fix on its way now that Ben acked it and Dan tested it.
Ciao, Thorsten
P.S.: As a Linux kernel regression tracker I'm getting a lot of reports
on my table. I can only look briefly into most of them. Unfortunately
therefore I sometimes will get things wrong or miss something important.
I hope that's not the case here; if you think it is, don't hesitate to
tell me about it in a public reply. That's in everyone's interest, as
what I wrote above might be misleading to everyone reading this; any
suggestion I gave thus might sent someone reading this down the wrong
rabbit hole, which none of us wants.
BTW, I have no personal interest in this issue, which is tracked using
regzbot, my Linux kernel regression tracking bot
(https://linux-regtracking.leemhuis.info/regzbot/). I'm only posting
this mail to get things rolling again and hence don't need to be CC on
all further activities wrt to this regression.
#regzbot poke
---
drivers/gpu/drm/nouveau/nouveau_fence.c | 28 +
1 file changed, 15 insertions(+), 13 deletions(-)
diff --git a/drivers/gpu/drm/nouveau/nouveau_fence.c
b/drivers/gpu/drm/nouveau/nouveau_fence.c
index 05d0b3eb3690..0ae416aa76dc 100644
--- a/drivers/gpu/drm/nouveau/nouveau_fence.c
+++ b/drivers/gpu/drm/nouveau/nouveau_fence.c
@@ -353,15 +353,22 @@ nouveau_fence_sync(struct nouveau_bo *nvbo,
struct nouveau_channel *chan, bool e
if (ret)
return ret;
-}
-fobj = dma_resv_shared_list(resv);
-fence = dma_resv_excl_fence(resv);
+fobj = NULL;
+} else {
+fobj = dma_resv_shared_list(resv);
+}
-if (fence) {
+/* Waiting for the exclusive fence first causes performance
regressions
+ * under some circumstances. So manually wait for the shared
ones first.
+ */
+for (i = 0; i < (fobj ? fobj->shared_count : 0) && !ret; ++i) {
struct nouveau_channel *prev = NULL;
bool must_wait = true;
+fence = rcu_dereference_protected(fobj->shared[i],
+dma_resv_held(resv));
+
f = nouveau_local_fence(fence, chan->drm);
if (f) {
rcu_read_lock();
@@ -373,20 +380,13 @@ nouveau_fence_sync(struct nouveau_bo *nvbo,
struct nouveau_channel *chan, bool e
if (must_wait)
ret = dma_fence_wait(fence, intr);
-
-return ret;
}
-if (!exclusive || !fobj)
-return ret;
-
-for (i = 0; i < fobj->shared_count && !ret; ++i) {
+fence = dma_resv_excl_fence(resv);
+if (fence) {
struct nouveau_channel *prev = NULL;
bool must_wait = true;
-fence = rcu_dereference_protected(fobj->shared[i],
-dma_resv_held(resv));
-
f = nouveau_local_fence(fence, chan->drm);
if (f) {
rcu_read_lock();
@@ -398,6 +398,8 @@ nouveau_fence_sync(struct nouveau_bo *nvbo,
struct nouveau_channel *chan, bool e
if (must_wait)
ret = dma_fence_wait(fence, intr);
+
+return ret;
}
return ret;
>>
Re: [PATCH v4 01/34] component: Introduce struct aggregate_device
On Thu, Dec 02, 2021 at 02:26:59PM -0800, Stephen Boyd wrote:
> Replace 'struct master' with 'struct aggregate_device' and then rename
> 'master' to 'adev' everywhere in the code. While we're here, put a
> struct device inside the aggregate device so that we can register it
> with a bus_type in the next patch.
>
> The diff is large but that's because this is mostly a rename, where
> sometimes 'master' is replaced with 'adev' and other times it is
> replaced with 'parent' to indicate that the struct device that was being
> used is actually the parent of the aggregate device and driver.
>
> Cc: Daniel Vetter
> Cc: Greg Kroah-Hartman
> Cc: Laurent Pinchart
> Cc: "Rafael J. Wysocki"
> Cc: Rob Clark
> Cc: Russell King
> Cc: Saravana Kannan
> Signed-off-by: Stephen Boyd
> ---
> drivers/base/component.c | 16 +++-
> 1 file changed, 15 insertions(+), 1 deletion(-)
>
> diff --git a/drivers/base/component.c b/drivers/base/component.c
> index 2d25a6416587..d25048d04b70 100644
> --- a/drivers/base/component.c
> +++ b/drivers/base/component.c
> @@ -9,6 +9,7 @@
> */
> #include
> #include
> +#include
> #include
> #include
> #include
> @@ -63,7 +64,10 @@ struct master {
>
> const struct component_master_ops *ops;
> struct device *parent;
> + struct device dev;
Who initializes this new structure?
> struct component_match *match;
> +
> + int id;
> };
>
> struct component {
> @@ -79,6 +83,7 @@ struct component {
> static DEFINE_MUTEX(component_mutex);
> static LIST_HEAD(component_list);
> static LIST_HEAD(masters);
> +static DEFINE_IDA(aggregate_ida);
>
> #ifdef CONFIG_DEBUG_FS
>
> @@ -440,6 +445,7 @@ static void free_master(struct master *master)
> }
> }
>
> + ida_free(_ida, master->id);
> kfree(master);
> }
>
> @@ -460,7 +466,7 @@ int component_master_add_with_match(struct device *parent,
> struct component_match *match)
> {
> struct master *master;
> - int ret;
> + int ret, id;
>
> /* Reallocate the match array for its true size */
> ret = component_match_realloc(match, match->num);
> @@ -471,9 +477,17 @@ int component_master_add_with_match(struct device
> *parent,
> if (!master)
> return -ENOMEM;
>
> + id = ida_alloc(_ida, GFP_KERNEL);
> + if (id < 0) {
> + kfree(master);
> + return id;
> + }
> +
> + master->id = id;
> master->parent = parent;
> master->ops = ops;
> master->match = match;
> + dev_set_name(>dev, "aggregate%d", id);
You set the name yet the device is not "real"?
I don't understand this patch at all, sorry.
greg k-h
Re: [PATCH v4 01/34] component: Introduce struct aggregate_device
On Thu, Dec 02, 2021 at 02:26:59PM -0800, Stephen Boyd wrote: > Replace 'struct master' with 'struct aggregate_device' and then rename > 'master' to 'adev' everywhere in the code. While we're here, put a > struct device inside the aggregate device so that we can register it > with a bus_type in the next patch. Do not do a "while we are here" type change please. Do the rename/replace first, and then make the other change. > The diff is large but that's because this is mostly a rename, where > sometimes 'master' is replaced with 'adev' and other times it is > replaced with 'parent' to indicate that the struct device that was being > used is actually the parent of the aggregate device and driver. The diff is 15 lines, how is that "large"? thanks, greg k-h
[PATCH] video: fbdev: mb862xx: remove redundant assignment to pointer ptr
The pointer ptr is being assigned a value that is never read. The
pointer is being re-assigned later in a loop. The assignment is
redundant and can be removed.
Signed-off-by: Colin Ian King
---
drivers/video/fbdev/mb862xx/mb862xxfb_accel.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/drivers/video/fbdev/mb862xx/mb862xxfb_accel.c
b/drivers/video/fbdev/mb862xx/mb862xxfb_accel.c
index d40b806461ca..61aed7fc0b8d 100644
--- a/drivers/video/fbdev/mb862xx/mb862xxfb_accel.c
+++ b/drivers/video/fbdev/mb862xx/mb862xxfb_accel.c
@@ -132,7 +132,7 @@ static void mb86290fb_imageblit8(u32 *cmd, u16 step, u16
dx, u16 dy,
cmd[2] = (height << 16) | width;
i = 0;
- line = ptr = image->data;
+ line = image->data;
bytes = image->width;
while (i < height) {
--
2.32.0
Re: [PATCH v2 1/2] platform/chrome: Add driver for ChromeOS privacy-screen
On Mon, Dec 20, 2021 at 12:21:47PM -0800, Rajat Jain wrote: > Hi Dmitry, > > Thanks for the review. Please see inline. > > On Mon, Dec 20, 2021 at 11:42 AM Dmitry Torokhov > wrote: > > > > Hi Rajat, > > > > On Fri, Dec 17, 2021 at 12:28:49PM -0800, Rajat Jain wrote: > > > This adds the ACPI driver for the ChromeOS privacy screen that is > > > present on some chromeos devices. > > > > > > Note that ideally, we'd want this privacy screen driver to be probed > > > BEFORE the drm probe in order to avoid a drm probe deferral: > > > https://hansdegoede.livejournal.com/25948.html > > > > > > In practise, I found that ACPI drivers are bound to their devices AFTER > > > the drm probe on chromebooks. So on chromebooks with privacy-screen, > > > this patch along with the next one in this series results in a probe > > > deferral of about 250ms for i915 driver. However, it did not result in > > > any user noticeable delay of splash screen in my personal experience. > > > > > > In future if this probe deferral turns out to be an issue, we can > > > consider turning this ACPI driver into something that is probed > > > earlier than the drm drivers. > > > > > > Signed-off-by: Rajat Jain > > > --- > > > v2: * Reword the commit log > > > * Make the Kconfig into a tristate > > > * Reorder the patches in the series. > > > > > > drivers/platform/chrome/Kconfig | 9 ++ > > > drivers/platform/chrome/Makefile | 1 + > > > drivers/platform/chrome/chromeos_priv_scrn.c | 132 +++ > > > 3 files changed, 142 insertions(+) > > > create mode 100644 drivers/platform/chrome/chromeos_priv_scrn.c > > > > > > diff --git a/drivers/platform/chrome/Kconfig > > > b/drivers/platform/chrome/Kconfig > > > index ccc23d8686e8..d1c209a45a62 100644 > > > --- a/drivers/platform/chrome/Kconfig > > > +++ b/drivers/platform/chrome/Kconfig > > > @@ -243,6 +243,15 @@ config CROS_USBPD_NOTIFY > > > To compile this driver as a module, choose M here: the > > > module will be called cros_usbpd_notify. > > > > > > +config CHROMEOS_PRIVACY_SCREEN > > > + tristate "ChromeOS Privacy Screen support" > > > + depends on ACPI > > > + depends on DRM > > > + select DRM_PRIVACY_SCREEN > > > + help > > > + This driver provides the support needed for the in-built > > > electronic > > > + privacy screen that is present on some ChromeOS devices. > > > + > > > source "drivers/platform/chrome/wilco_ec/Kconfig" > > > > > > endif # CHROMEOS_PLATFORMS > > > diff --git a/drivers/platform/chrome/Makefile > > > b/drivers/platform/chrome/Makefile > > > index f901d2e43166..cfa0bb4e9e34 100644 > > > --- a/drivers/platform/chrome/Makefile > > > +++ b/drivers/platform/chrome/Makefile > > > @@ -4,6 +4,7 @@ > > > CFLAGS_cros_ec_trace.o:= -I$(src) > > > > > > obj-$(CONFIG_CHROMEOS_LAPTOP)+= chromeos_laptop.o > > > +obj-$(CONFIG_CHROMEOS_PRIVACY_SCREEN)+= chromeos_priv_scrn.o > > > obj-$(CONFIG_CHROMEOS_PSTORE)+= chromeos_pstore.o > > > obj-$(CONFIG_CHROMEOS_TBMC) += chromeos_tbmc.o > > > obj-$(CONFIG_CROS_EC)+= cros_ec.o > > > diff --git a/drivers/platform/chrome/chromeos_priv_scrn.c > > > b/drivers/platform/chrome/chromeos_priv_scrn.c > > > new file mode 100644 > > > index ..a4cbf5c79c2a > > > --- /dev/null > > > +++ b/drivers/platform/chrome/chromeos_priv_scrn.c > > > > I think we can spare a few more characters :) chromeos_privacy_screen.c > > maybe? > > > > And also see if maybe variables in the code are not that unseemly long > > even if not abbreviated? > > Sure, I can certainly replace "chromeos_priv_scrn" with > "chromeos_privacy_screen" everywhere. Some of the variables may be a > little long, but I think that should be OK (my main concern was > > chromeos_privacy_screen_device_ids > chromeos_privacy_screen_get_hw_state() > > Let me know if that doesn't sound right (in which case, I can probably > omit "chromeos" from the local variable and function names) Another option to go all the way into different direction, and use "cps_" prefix for everything. It is probably just me but combination of "priv" "scrn" just grates on me ;) > > > > > > @@ -0,0 +1,132 @@ > > > +// SPDX-License-Identifier: GPL-2.0 > > > + > > > +/* > > > + * chromeos_priv_scrn.c - ChromeOS Privacy Screen support > > > > I'd avoid mentioning file name as those tend to change. > > Ack, will do. > > > > > > + * > > > + * Copyright (C) 2022 The Chromium OS Authors > > > > This is not correct copyright for kernel contributions. It should be > > attributed to "Google LLC". Note that it is different from CrOS > > userspace. > > > > Ack, will do. > > > > + * > > > + */ > > > + > > > +#include > > > +#include > > > + > > > +/* > > > + * The DSM (Define Specific Method) constants below are the agreed API > > > with > > > + * the firmware team, on how to control privacy screen using ACPI > > >
