date:20231124


Hi Christian,

do you think it makes sense to handle this in drm_exec_prepare_obj() or
even dma_resv_reserve_fences() even?

- Danilo

On 11/25/23 00:36, Danilo Krummrich wrote:

Fall back to drm_exec_lock_obj() if num_fences is zero for the
drm_gpuvm_prepare_* function family.

Otherwise dma_resv_reserve_fences() would actually allocate slots even
though num_fences is zero.

Cc: Christian König 
Signed-off-by: Danilo Krummrich 
---
  drivers/gpu/drm/drm_gpuvm.c | 36 +---
  include/drm/drm_gpuvm.h | 23 +++
  2 files changed, 36 insertions(+), 23 deletions(-)

diff --git a/drivers/gpu/drm/drm_gpuvm.c b/drivers/gpu/drm/drm_gpuvm.c
index 54f5e8851de5..d1d1c2379e44 100644
--- a/drivers/gpu/drm/drm_gpuvm.c
+++ b/drivers/gpu/drm/drm_gpuvm.c
@@ -1085,6 +1085,36 @@ drm_gpuvm_put(struct drm_gpuvm *gpuvm)
  }
  EXPORT_SYMBOL_GPL(drm_gpuvm_put);
  
+static int

+exec_prepare_obj(struct drm_exec *exec, struct drm_gem_object *obj,
+unsigned int num_fences)
+{
+   return num_fences ? drm_exec_prepare_obj(exec, obj, num_fences) :
+   drm_exec_lock_obj(exec, obj);
+}
+
+/**
+ * drm_gpuvm_prepare_vm() - prepare the GPUVMs common dma-resv
+ * @gpuvm: the &drm_gpuvm
+ * @exec: the &drm_exec context
+ * @num_fences: the amount of &dma_fences to reserve
+ *
+ * Calls drm_exec_prepare_obj() for the GPUVMs dummy &drm_gem_object.
+ *
+ * Using this function directly, it is the drivers responsibility to call
+ * drm_exec_init() and drm_exec_fini() accordingly.
+ *
+ * Returns: 0 on success, negative error code on failure.
+ */
+int
+drm_gpuvm_prepare_vm(struct drm_gpuvm *gpuvm,
+struct drm_exec *exec,
+unsigned int num_fences)
+{
+   return exec_prepare_obj(exec, gpuvm->r_obj, num_fences);
+}
+EXPORT_SYMBOL_GPL(drm_gpuvm_prepare_vm);
+
  static int
  __drm_gpuvm_prepare_objects(struct drm_gpuvm *gpuvm,
struct drm_exec *exec,
@@ -1095,7 +1125,7 @@ __drm_gpuvm_prepare_objects(struct drm_gpuvm *gpuvm,
int ret = 0;
  
  	for_each_vm_bo_in_list(gpuvm, extobj, &extobjs, vm_bo) {

-   ret = drm_exec_prepare_obj(exec, vm_bo->obj, num_fences);
+   ret = exec_prepare_obj(exec, vm_bo->obj, num_fences);
if (ret)
break;
}
@@ -1116,7 +1146,7 @@ drm_gpuvm_prepare_objects_locked(struct drm_gpuvm *gpuvm,
  
  	drm_gpuvm_resv_assert_held(gpuvm);

list_for_each_entry(vm_bo, &gpuvm->extobj.list, list.entry.extobj) {
-   ret = drm_exec_prepare_obj(exec, vm_bo->obj, num_fences);
+   ret = exec_prepare_obj(exec, vm_bo->obj, num_fences);
if (ret)
break;
  
@@ -1186,7 +1216,7 @@ drm_gpuvm_prepare_range(struct drm_gpuvm *gpuvm, struct drm_exec *exec,

drm_gpuvm_for_each_va_range(va, gpuvm, addr, end) {
struct drm_gem_object *obj = va->gem.obj;
  
-		ret = drm_exec_prepare_obj(exec, obj, num_fences);

+   ret = exec_prepare_obj(exec, obj, num_fences);
if (ret)
return ret;
}
diff --git a/include/drm/drm_gpuvm.h b/include/drm/drm_gpuvm.h
index f94fec9a8517..b3f82ec7fb17 100644
--- a/include/drm/drm_gpuvm.h
+++ b/include/drm/drm_gpuvm.h
@@ -544,26 +544,9 @@ struct drm_gpuvm_exec {
} extra;
  };
  
-/**

- * drm_gpuvm_prepare_vm() - prepare the GPUVMs common dma-resv
- * @gpuvm: the &drm_gpuvm
- * @exec: the &drm_exec context
- * @num_fences: the amount of &dma_fences to reserve
- *
- * Calls drm_exec_prepare_obj() for the GPUVMs dummy &drm_gem_object.
- *
- * Using this function directly, it is the drivers responsibility to call
- * drm_exec_init() and drm_exec_fini() accordingly.
- *
- * Returns: 0 on success, negative error code on failure.
- */
-static inline int
-drm_gpuvm_prepare_vm(struct drm_gpuvm *gpuvm,
-struct drm_exec *exec,
-unsigned int num_fences)
-{
-   return drm_exec_prepare_obj(exec, gpuvm->r_obj, num_fences);
-}
+int drm_gpuvm_prepare_vm(struct drm_gpuvm *gpuvm,
+struct drm_exec *exec,
+unsigned int num_fences);
  
  int drm_gpuvm_prepare_objects(struct drm_gpuvm *gpuvm,

  struct drm_exec *exec,

[PATCH drm-misc-next 5/5] drm/imagination: vm: make use of GPUVM's drm_exec helper

Make use of GPUVM's drm_exec helper functions preventing direct access
to GPUVM internal data structures, such as the external object list.

This is especially important to ensure following the locking rules
around the GPUVM external object list.

Fixes: ff5f643de0bf ("drm/imagination: Add GEM and VM related code")
Signed-off-by: Danilo Krummrich 
---
 drivers/gpu/drm/imagination/pvr_vm.c | 16 +---
 1 file changed, 5 insertions(+), 11 deletions(-)

diff --git a/drivers/gpu/drm/imagination/pvr_vm.c 
b/drivers/gpu/drm/imagination/pvr_vm.c
index e0d74d9a6190..3f7888f5cc53 100644
--- a/drivers/gpu/drm/imagination/pvr_vm.c
+++ b/drivers/gpu/drm/imagination/pvr_vm.c
@@ -337,27 +337,21 @@ static int
 pvr_vm_bind_op_lock_resvs(struct drm_exec *exec, struct pvr_vm_bind_op 
*bind_op)
 {
drm_exec_until_all_locked(exec) {
-   struct drm_gem_object *r_obj = &bind_op->vm_ctx->dummy_gem;
struct drm_gpuvm *gpuvm = &bind_op->vm_ctx->gpuvm_mgr;
struct pvr_gem_object *pvr_obj = bind_op->pvr_obj;
-   struct drm_gpuvm_bo *gpuvm_bo;
 
/* Acquire lock on the vm_context's reserve object. */
-   int err = drm_exec_lock_obj(exec, r_obj);
+   int err = drm_gpuvm_prepare_vm(gpuvm, exec, 0);
 
drm_exec_retry_on_contention(exec);
if (err)
return err;
 
/* Acquire lock on all BOs in the context. */
-   list_for_each_entry(gpuvm_bo, &gpuvm->extobj.list,
-   list.entry.extobj) {
-   err = drm_exec_lock_obj(exec, gpuvm_bo->obj);
-
-   drm_exec_retry_on_contention(exec);
-   if (err)
-   return err;
-   }
+   err = drm_gpuvm_prepare_objects(gpuvm, exec, 0);
+   drm_exec_retry_on_contention(exec);
+   if (err)
+   return err;
 
/* Unmap operations don't have an object to lock. */
if (!pvr_obj)
-- 
2.42.0

[PATCH drm-misc-next 4/5] drm/gpuvm: fall back to drm_exec_lock_obj()

Fall back to drm_exec_lock_obj() if num_fences is zero for the
drm_gpuvm_prepare_* function family.

Otherwise dma_resv_reserve_fences() would actually allocate slots even
though num_fences is zero.

Cc: Christian König 
Signed-off-by: Danilo Krummrich 
---
 drivers/gpu/drm/drm_gpuvm.c | 36 +---
 include/drm/drm_gpuvm.h | 23 +++
 2 files changed, 36 insertions(+), 23 deletions(-)

diff --git a/drivers/gpu/drm/drm_gpuvm.c b/drivers/gpu/drm/drm_gpuvm.c
index 54f5e8851de5..d1d1c2379e44 100644
--- a/drivers/gpu/drm/drm_gpuvm.c
+++ b/drivers/gpu/drm/drm_gpuvm.c
@@ -1085,6 +1085,36 @@ drm_gpuvm_put(struct drm_gpuvm *gpuvm)
 }
 EXPORT_SYMBOL_GPL(drm_gpuvm_put);
 
+static int
+exec_prepare_obj(struct drm_exec *exec, struct drm_gem_object *obj,
+unsigned int num_fences)
+{
+   return num_fences ? drm_exec_prepare_obj(exec, obj, num_fences) :
+   drm_exec_lock_obj(exec, obj);
+}
+
+/**
+ * drm_gpuvm_prepare_vm() - prepare the GPUVMs common dma-resv
+ * @gpuvm: the &drm_gpuvm
+ * @exec: the &drm_exec context
+ * @num_fences: the amount of &dma_fences to reserve
+ *
+ * Calls drm_exec_prepare_obj() for the GPUVMs dummy &drm_gem_object.
+ *
+ * Using this function directly, it is the drivers responsibility to call
+ * drm_exec_init() and drm_exec_fini() accordingly.
+ *
+ * Returns: 0 on success, negative error code on failure.
+ */
+int
+drm_gpuvm_prepare_vm(struct drm_gpuvm *gpuvm,
+struct drm_exec *exec,
+unsigned int num_fences)
+{
+   return exec_prepare_obj(exec, gpuvm->r_obj, num_fences);
+}
+EXPORT_SYMBOL_GPL(drm_gpuvm_prepare_vm);
+
 static int
 __drm_gpuvm_prepare_objects(struct drm_gpuvm *gpuvm,
struct drm_exec *exec,
@@ -1095,7 +1125,7 @@ __drm_gpuvm_prepare_objects(struct drm_gpuvm *gpuvm,
int ret = 0;
 
for_each_vm_bo_in_list(gpuvm, extobj, &extobjs, vm_bo) {
-   ret = drm_exec_prepare_obj(exec, vm_bo->obj, num_fences);
+   ret = exec_prepare_obj(exec, vm_bo->obj, num_fences);
if (ret)
break;
}
@@ -1116,7 +1146,7 @@ drm_gpuvm_prepare_objects_locked(struct drm_gpuvm *gpuvm,
 
drm_gpuvm_resv_assert_held(gpuvm);
list_for_each_entry(vm_bo, &gpuvm->extobj.list, list.entry.extobj) {
-   ret = drm_exec_prepare_obj(exec, vm_bo->obj, num_fences);
+   ret = exec_prepare_obj(exec, vm_bo->obj, num_fences);
if (ret)
break;
 
@@ -1186,7 +1216,7 @@ drm_gpuvm_prepare_range(struct drm_gpuvm *gpuvm, struct 
drm_exec *exec,
drm_gpuvm_for_each_va_range(va, gpuvm, addr, end) {
struct drm_gem_object *obj = va->gem.obj;
 
-   ret = drm_exec_prepare_obj(exec, obj, num_fences);
+   ret = exec_prepare_obj(exec, obj, num_fences);
if (ret)
return ret;
}
diff --git a/include/drm/drm_gpuvm.h b/include/drm/drm_gpuvm.h
index f94fec9a8517..b3f82ec7fb17 100644
--- a/include/drm/drm_gpuvm.h
+++ b/include/drm/drm_gpuvm.h
@@ -544,26 +544,9 @@ struct drm_gpuvm_exec {
} extra;
 };
 
-/**
- * drm_gpuvm_prepare_vm() - prepare the GPUVMs common dma-resv
- * @gpuvm: the &drm_gpuvm
- * @exec: the &drm_exec context
- * @num_fences: the amount of &dma_fences to reserve
- *
- * Calls drm_exec_prepare_obj() for the GPUVMs dummy &drm_gem_object.
- *
- * Using this function directly, it is the drivers responsibility to call
- * drm_exec_init() and drm_exec_fini() accordingly.
- *
- * Returns: 0 on success, negative error code on failure.
- */
-static inline int
-drm_gpuvm_prepare_vm(struct drm_gpuvm *gpuvm,
-struct drm_exec *exec,
-unsigned int num_fences)
-{
-   return drm_exec_prepare_obj(exec, gpuvm->r_obj, num_fences);
-}
+int drm_gpuvm_prepare_vm(struct drm_gpuvm *gpuvm,
+struct drm_exec *exec,
+unsigned int num_fences);
 
 int drm_gpuvm_prepare_objects(struct drm_gpuvm *gpuvm,
  struct drm_exec *exec,
-- 
2.42.0

[PATCH drm-misc-next 3/5] drm/imagination: vm: fix drm_gpuvm reference count

The driver specific reference count indicates whether the VM should be
teared down, whereas GPUVM's reference count indicates whether the VM
structure can finally be freed.

Hence, free the VM structure in pvr_gpuvm_free() and drop the last
GPUVM reference after tearing down the VM. Generally, this prevents
lifetime issues such as the VM being freed as long as drm_gpuvm_bo
structures still hold references to the VM.

Fixes: ff5f643de0bf ("drm/imagination: Add GEM and VM related code")
Signed-off-by: Danilo Krummrich 
---
 drivers/gpu/drm/imagination/pvr_vm.c | 11 ---
 1 file changed, 8 insertions(+), 3 deletions(-)

diff --git a/drivers/gpu/drm/imagination/pvr_vm.c 
b/drivers/gpu/drm/imagination/pvr_vm.c
index 1e89092c3dcc..e0d74d9a6190 100644
--- a/drivers/gpu/drm/imagination/pvr_vm.c
+++ b/drivers/gpu/drm/imagination/pvr_vm.c
@@ -64,6 +64,12 @@ struct pvr_vm_context {
struct drm_gem_object dummy_gem;
 };
 
+static inline
+struct pvr_vm_context *to_pvr_vm_context(struct drm_gpuvm *gpuvm)
+{
+   return container_of(gpuvm, struct pvr_vm_context, gpuvm_mgr);
+}
+
 struct pvr_vm_context *pvr_vm_context_get(struct pvr_vm_context *vm_ctx)
 {
if (vm_ctx)
@@ -535,7 +541,7 @@ pvr_device_addr_and_size_are_valid(struct pvr_vm_context 
*vm_ctx,
 
 void pvr_gpuvm_free(struct drm_gpuvm *gpuvm)
 {
-
+   kfree(to_pvr_vm_context(gpuvm));
 }
 
 static const struct drm_gpuvm_ops pvr_vm_gpuva_ops = {
@@ -655,12 +661,11 @@ pvr_vm_context_release(struct kref *ref_count)
WARN_ON(pvr_vm_unmap(vm_ctx, vm_ctx->gpuvm_mgr.mm_start,
 vm_ctx->gpuvm_mgr.mm_range));
 
-   drm_gpuvm_put(&vm_ctx->gpuvm_mgr);
pvr_mmu_context_destroy(vm_ctx->mmu_ctx);
drm_gem_private_object_fini(&vm_ctx->dummy_gem);
mutex_destroy(&vm_ctx->lock);
 
-   kfree(vm_ctx);
+   drm_gpuvm_put(&vm_ctx->gpuvm_mgr);
 }
 
 /**
-- 
2.42.0

[PATCH drm-misc-next 0/5] PowerVR VM fixes

Hi,

Some major GPUVM changes landed just before v8 of the PowerVR series. Since v8
went in rather quickly (review process was finished otherwise) I haven't had the
chance to review the subsequent code changes.

Hence, this series with a few fixes in this context. Plus a minor GPUVM patch to
make the drm_gpuvm_prepare_* helpers useful for PowerVR.

- Danilo


Danilo Krummrich (5):
  drm/imagination: vm: prevent duplicate drm_gpuvm_bo instances
  drm/imagination: vm: check for drm_gpuvm_range_valid()
  drm/imagination: vm: fix drm_gpuvm reference count
  drm/gpuvm: fall back to drm_exec_lock_obj()
  drm/imagination: vm: make use of GPUVM's drm_exec helper

 drivers/gpu/drm/drm_gpuvm.c  | 36 --
 drivers/gpu/drm/imagination/pvr_vm.c | 46 +++-
 drivers/gpu/drm/imagination/pvr_vm.h |  3 +-
 include/drm/drm_gpuvm.h  | 23 ++
 4 files changed, 63 insertions(+), 45 deletions(-)


base-commit: 46990918f35c1bf6e367cf8e0423e7344fec9fcb
-- 
2.42.0

[PATCH drm-misc-next 2/5] drm/imagination: vm: check for drm_gpuvm_range_valid()

Extend pvr_device_addr_and_size_are_valid() by the corresponding GPUVM
sanity checks. This includes a, previously missing, overflow check for
the base address and size of the requested mapping.

Fixes: ff5f643de0bf ("drm/imagination: Add GEM and VM related code")
Signed-off-by: Danilo Krummrich 
---
 drivers/gpu/drm/imagination/pvr_vm.c | 9 ++---
 drivers/gpu/drm/imagination/pvr_vm.h | 3 ++-
 2 files changed, 8 insertions(+), 4 deletions(-)

diff --git a/drivers/gpu/drm/imagination/pvr_vm.c 
b/drivers/gpu/drm/imagination/pvr_vm.c
index 09d481c575b0..1e89092c3dcc 100644
--- a/drivers/gpu/drm/imagination/pvr_vm.c
+++ b/drivers/gpu/drm/imagination/pvr_vm.c
@@ -239,7 +239,7 @@ pvr_vm_bind_op_map_init(struct pvr_vm_bind_op *bind_op,
return -EINVAL;
}
 
-   if (!pvr_device_addr_and_size_are_valid(device_addr, size) ||
+   if (!pvr_device_addr_and_size_are_valid(vm_ctx, device_addr, size) ||
offset & ~PAGE_MASK || size & ~PAGE_MASK ||
offset >= pvr_obj_size || offset_plus_size > pvr_obj_size)
return -EINVAL;
@@ -295,7 +295,7 @@ pvr_vm_bind_op_unmap_init(struct pvr_vm_bind_op *bind_op,
 {
int err;
 
-   if (!pvr_device_addr_and_size_are_valid(device_addr, size))
+   if (!pvr_device_addr_and_size_are_valid(vm_ctx, device_addr, size))
return -EINVAL;
 
bind_op->type = PVR_VM_BIND_TYPE_UNMAP;
@@ -505,6 +505,7 @@ pvr_device_addr_is_valid(u64 device_addr)
 /**
  * pvr_device_addr_and_size_are_valid() - Tests whether a device-virtual
  * address and associated size are both valid.
+ * @vm_ctx: Target VM context.
  * @device_addr: Virtual device address to test.
  * @size: Size of the range based at @device_addr to test.
  *
@@ -523,9 +524,11 @@ pvr_device_addr_is_valid(u64 device_addr)
  *  * %false otherwise.
  */
 bool
-pvr_device_addr_and_size_are_valid(u64 device_addr, u64 size)
+pvr_device_addr_and_size_are_valid(struct pvr_vm_context *vm_ctx,
+  u64 device_addr, u64 size)
 {
return pvr_device_addr_is_valid(device_addr) &&
+  drm_gpuvm_range_valid(&vm_ctx->gpuvm_mgr, device_addr, size) &&
   size != 0 && (size & ~PVR_DEVICE_PAGE_MASK) == 0 &&
   (device_addr + size <= PVR_PAGE_TABLE_ADDR_SPACE_SIZE);
 }
diff --git a/drivers/gpu/drm/imagination/pvr_vm.h 
b/drivers/gpu/drm/imagination/pvr_vm.h
index cf8b97553dc8..f2a6463f2b05 100644
--- a/drivers/gpu/drm/imagination/pvr_vm.h
+++ b/drivers/gpu/drm/imagination/pvr_vm.h
@@ -29,7 +29,8 @@ struct drm_exec;
 /* Functions defined in pvr_vm.c */
 
 bool pvr_device_addr_is_valid(u64 device_addr);
-bool pvr_device_addr_and_size_are_valid(u64 device_addr, u64 size);
+bool pvr_device_addr_and_size_are_valid(struct pvr_vm_context *vm_ctx,
+   u64 device_addr, u64 size);
 
 struct pvr_vm_context *pvr_vm_create_context(struct pvr_device *pvr_dev,
 bool is_userspace_context);
-- 
2.42.0

[PATCH drm-misc-next 1/5] drm/imagination: vm: prevent duplicate drm_gpuvm_bo instances

Use drm_gpuvm_bo_obtain() instead of drm_gpuvm_bo_create(). The latter
should only be used in conjunction with drm_gpuvm_bo_obtain_prealloc().

drm_gpuvm_bo_obtain() re-uses existing instances of a given VM and BO
combination, whereas drm_gpuvm_bo_create() would always create a new
instance of struct drm_gpuvm_bo and hence leave us with duplicates.

Fixes: ff5f643de0bf ("drm/imagination: Add GEM and VM related code")
Signed-off-by: Danilo Krummrich 
---
 drivers/gpu/drm/imagination/pvr_vm.c | 10 ++
 1 file changed, 6 insertions(+), 4 deletions(-)

diff --git a/drivers/gpu/drm/imagination/pvr_vm.c 
b/drivers/gpu/drm/imagination/pvr_vm.c
index 3ad1366294b9..09d481c575b0 100644
--- a/drivers/gpu/drm/imagination/pvr_vm.c
+++ b/drivers/gpu/drm/imagination/pvr_vm.c
@@ -224,6 +224,7 @@ pvr_vm_bind_op_map_init(struct pvr_vm_bind_op *bind_op,
struct pvr_gem_object *pvr_obj, u64 offset,
u64 device_addr, u64 size)
 {
+   struct drm_gem_object *obj = gem_from_pvr_gem(pvr_obj);
const bool is_user = vm_ctx == vm_ctx->pvr_dev->kernel_vm_ctx;
const u64 pvr_obj_size = pvr_gem_object_size(pvr_obj);
struct sg_table *sgt;
@@ -245,10 +246,11 @@ pvr_vm_bind_op_map_init(struct pvr_vm_bind_op *bind_op,
 
bind_op->type = PVR_VM_BIND_TYPE_MAP;
 
-   bind_op->gpuvm_bo = drm_gpuvm_bo_create(&vm_ctx->gpuvm_mgr,
-   gem_from_pvr_gem(pvr_obj));
-   if (!bind_op->gpuvm_bo)
-   return -ENOMEM;
+   dma_resv_lock(obj->resv, NULL);
+   bind_op->gpuvm_bo = drm_gpuvm_bo_obtain(&vm_ctx->gpuvm_mgr, obj);
+   dma_resv_unlock(obj->resv);
+   if (IS_ERR(bind_op->gpuvm_bo))
+   return PTR_ERR(bind_op->gpuvm_bo);
 
bind_op->new_va = kzalloc(sizeof(*bind_op->new_va), GFP_KERNEL);
bind_op->prev_va = kzalloc(sizeof(*bind_op->prev_va), GFP_KERNEL);
-- 
2.42.0

Re: linux-next: Signed-off-by missing for commit in the drm-misc tree

2023-11-24 Thread Luben Tuikov

On 2023-11-24 08:20, Jani Nikula wrote:
> On Wed, 22 Nov 2023, Luben Tuikov  wrote:
>> On 2023-11-22 07:00, Maxime Ripard wrote:
>>> Hi Luben,
>>>
>>> On Thu, Nov 16, 2023 at 09:27:58AM +0100, Daniel Vetter wrote:
 On Thu, Nov 16, 2023 at 09:11:43AM +0100, Maxime Ripard wrote:
> On Tue, Nov 14, 2023 at 06:46:21PM -0500, Luben Tuikov wrote:
>> On 2023-11-13 22:08, Stephen Rothwell wrote:
>>> BTW, cherry picking commits does not avoid conflicts - in fact it can
>>> cause conflicts if there are further changes to the files affected by
>>> the cherry picked commit in either the tree/branch the commit was
>>> cheery picked from or the destination tree/branch (I have to deal with
>>> these all the time when merging the drm trees in linux-next).  Much
>>> better is to cross merge the branches so that the patch only appears
>>> once or have a shared branches that are merged by any other branch that
>>> needs the changes.
>>>
>>> I understand that things are not done like this in the drm trees :-(
>>
>> Hi Stephen,
>>
>> Thank you for the clarification--understood. I'll be more careful in the 
>> future.
>> Thanks again! :-)
>
> In this case, the best thing to do would indeed have been to ask the
> drm-misc maintainers to merge drm-misc-fixes into drm-misc-next.
>
> We're doing that all the time, but we're not ubiquitous so you need to
> ask us :)
>
> Also, dim should have caught that when you pushed the branch. Did you
> use it?

 Yeah dim must be used, exactly to avoid these issues. Both for applying
 patches (so not git am directly, or cherry-picking from your own
 development branch), and for pushing. The latter is even checked for by
 the server (dim sets a special push flag which is very long and contains a
 very clear warning if you bypass it).

 If dim was used, this would be a bug in the dim script that we need to
 fix.
>>>
>>> It would be very useful for you to explain what happened here so we
>>> improve the tooling or doc and can try to make sure it doesn't happen
>>> again
>>>
>>> Maxime
>>
>> There is no problem with the tooling--I just forced the commit in.
> 
> Wait what?
> 
> What do you mean by forcing the commit in? Bypass dim?
> 
> If yes, please *never* do that when you're dealing with dim managed
> branches. That's part of the deal for getting commit access, along with
> following all the other maintainer tools documentation.

Hi Jani,

I only use dim, ever.
-- 
Regards,
Luben


OpenPGP_0x4C15479431A334AF.asc
Description: OpenPGP public key


OpenPGP_signature.asc
Description: OpenPGP digital signature

Re: [PATCH 4/6] input/vmmouse: Use vmware_hypercall API

2023-11-24 Thread Simon Horman

On Wed, Nov 22, 2023 at 03:30:49PM -0800, Alexey Makhalov wrote:
> Switch from VMWARE_HYPERCALL macro to vmware_hypercall API.
> Eliminate arch specific code. No functional changes intended.
> 
> Signed-off-by: Alexey Makhalov 

Hi Alexey,

it is not strictly related to this patch, but I notice than an x86_64
allmodconfig build with W=1 using gcc-13 fails to compile this file.

It appears that the problem relates to both priv->phys and
psmouse->ps2dev.serio->phys being 32 bytes.


drivers/input/mouse/vmmouse.c: In function ‘vmmouse_init’:
drivers/input/mouse/vmmouse.c:455:53: error: ‘/input1’ directive output may be 
truncated writing 7 bytes into a region of size between 1 and 32 
[-Werror=format-truncation=]
  455 | snprintf(priv->phys, sizeof(priv->phys), "%s/input1",
  | ^~~
drivers/input/mouse/vmmouse.c:455:9: note: ‘snprintf’ output between 8 and 39 
bytes into a destination of size 32
  455 | snprintf(priv->phys, sizeof(priv->phys), "%s/input1",
  | ^
  456 |  psmouse->ps2dev.serio->phys);
  |  

...

[PATCH 6.1 324/372] drm/mediatek/dp: fix memory leak on ->get_edid callback audio detection

6.1-stable review patch.  If anyone has any objections, please let me know.

--

From: Jani Nikula 

commit dab12fa8d2bd3868cf2de485ed15a3feef28a13d upstream.

The sads returned by drm_edid_to_sad() needs to be freed.

Fixes: e71a8ebbe086 ("drm/mediatek: dp: Audio support for MT8195")
Cc: Guillaume Ranquet 
Cc: Bo-Chen Chen 
Cc: AngeloGioacchino Del Regno 
Cc: Dmitry Osipenko 
Cc: Chun-Kuang Hu 
Cc: Philipp Zabel 
Cc: Matthias Brugger 
Cc: dri-devel@lists.freedesktop.org
Cc: linux-media...@lists.infradead.org
Cc: linux-ker...@vger.kernel.org
Cc: linux-arm-ker...@lists.infradead.org
Cc:  # v6.1+
Signed-off-by: Jani Nikula 
Reviewed-by: Chen-Yu Tsai 
Link: 
https://patchwork.kernel.org/project/dri-devel/patch/20230914155317.2511876-1-jani.nik...@intel.com/
Signed-off-by: Chun-Kuang Hu 
Signed-off-by: Greg Kroah-Hartman 
---
 drivers/gpu/drm/mediatek/mtk_dp.c |5 -
 1 file changed, 4 insertions(+), 1 deletion(-)

--- a/drivers/gpu/drm/mediatek/mtk_dp.c
+++ b/drivers/gpu/drm/mediatek/mtk_dp.c
@@ -1983,7 +1983,6 @@ static struct edid *mtk_dp_get_edid(stru
bool enabled = mtk_dp->enabled;
struct edid *new_edid = NULL;
struct mtk_dp_audio_cfg *audio_caps = &mtk_dp->info.audio_cur_cfg;
-   struct cea_sad *sads;
 
if (!enabled) {
drm_bridge_chain_pre_enable(bridge);
@@ -2010,7 +2009,11 @@ static struct edid *mtk_dp_get_edid(stru
}
 
if (new_edid) {
+   struct cea_sad *sads;
+
audio_caps->sad_count = drm_edid_to_sad(new_edid, &sads);
+   kfree(sads);
+
audio_caps->detect_monitor = drm_detect_monitor_audio(new_edid);
}

[PATCH 6.1 325/372] drm/mediatek/dp: fix memory leak on ->get_edid callback error path

6.1-stable review patch.  If anyone has any objections, please let me know.

--

From: Jani Nikula 

commit fcaf9761fd5884a64eaac48536f8c27ecfd2e6bc upstream.

Setting new_edid to NULL leaks the buffer.

Fixes: f70ac097a2cf ("drm/mediatek: Add MT8195 Embedded DisplayPort driver")
Cc: Markus Schneider-Pargmann 
Cc: Guillaume Ranquet 
Cc: Bo-Chen Chen 
Cc: CK Hu 
Cc: AngeloGioacchino Del Regno 
Cc: Dmitry Osipenko 
Cc: Chun-Kuang Hu 
Cc: Philipp Zabel 
Cc: Matthias Brugger 
Cc: dri-devel@lists.freedesktop.org
Cc: linux-media...@lists.infradead.org
Cc: linux-ker...@vger.kernel.org
Cc: linux-arm-ker...@lists.infradead.org
Cc:  # v6.1+
Signed-off-by: Jani Nikula 
Reviewed-by: Guillaume Ranquet 
Link: 
https://patchwork.kernel.org/project/dri-devel/patch/20230914131058.2472260-1-jani.nik...@intel.com/
Signed-off-by: Chun-Kuang Hu 
Signed-off-by: Greg Kroah-Hartman 
---
 drivers/gpu/drm/mediatek/mtk_dp.c |1 +
 1 file changed, 1 insertion(+)

--- a/drivers/gpu/drm/mediatek/mtk_dp.c
+++ b/drivers/gpu/drm/mediatek/mtk_dp.c
@@ -2005,6 +2005,7 @@ static struct edid *mtk_dp_get_edid(stru
 */
if (mtk_dp_parse_capabilities(mtk_dp)) {
drm_err(mtk_dp->drm_dev, "Can't parse capabilities\n");
+   kfree(new_edid);
new_edid = NULL;
}

[PATCH 6.5 428/491] drm/mediatek/dp: fix memory leak on ->get_edid callback error path

6.5-stable review patch.  If anyone has any objections, please let me know.

--

From: Jani Nikula 

commit fcaf9761fd5884a64eaac48536f8c27ecfd2e6bc upstream.

Setting new_edid to NULL leaks the buffer.

Fixes: f70ac097a2cf ("drm/mediatek: Add MT8195 Embedded DisplayPort driver")
Cc: Markus Schneider-Pargmann 
Cc: Guillaume Ranquet 
Cc: Bo-Chen Chen 
Cc: CK Hu 
Cc: AngeloGioacchino Del Regno 
Cc: Dmitry Osipenko 
Cc: Chun-Kuang Hu 
Cc: Philipp Zabel 
Cc: Matthias Brugger 
Cc: dri-devel@lists.freedesktop.org
Cc: linux-media...@lists.infradead.org
Cc: linux-ker...@vger.kernel.org
Cc: linux-arm-ker...@lists.infradead.org
Cc:  # v6.1+
Signed-off-by: Jani Nikula 
Reviewed-by: Guillaume Ranquet 
Link: 
https://patchwork.kernel.org/project/dri-devel/patch/20230914131058.2472260-1-jani.nik...@intel.com/
Signed-off-by: Chun-Kuang Hu 
Signed-off-by: Greg Kroah-Hartman 
---
 drivers/gpu/drm/mediatek/mtk_dp.c |1 +
 1 file changed, 1 insertion(+)

--- a/drivers/gpu/drm/mediatek/mtk_dp.c
+++ b/drivers/gpu/drm/mediatek/mtk_dp.c
@@ -2005,6 +2005,7 @@ static struct edid *mtk_dp_get_edid(stru
 */
if (mtk_dp_parse_capabilities(mtk_dp)) {
drm_err(mtk_dp->drm_dev, "Can't parse capabilities\n");
+   kfree(new_edid);
new_edid = NULL;
}

[PATCH 6.5 427/491] drm/mediatek/dp: fix memory leak on ->get_edid callback audio detection

6.5-stable review patch.  If anyone has any objections, please let me know.

--

From: Jani Nikula 

commit dab12fa8d2bd3868cf2de485ed15a3feef28a13d upstream.

The sads returned by drm_edid_to_sad() needs to be freed.

Fixes: e71a8ebbe086 ("drm/mediatek: dp: Audio support for MT8195")
Cc: Guillaume Ranquet 
Cc: Bo-Chen Chen 
Cc: AngeloGioacchino Del Regno 
Cc: Dmitry Osipenko 
Cc: Chun-Kuang Hu 
Cc: Philipp Zabel 
Cc: Matthias Brugger 
Cc: dri-devel@lists.freedesktop.org
Cc: linux-media...@lists.infradead.org
Cc: linux-ker...@vger.kernel.org
Cc: linux-arm-ker...@lists.infradead.org
Cc:  # v6.1+
Signed-off-by: Jani Nikula 
Reviewed-by: Chen-Yu Tsai 
Link: 
https://patchwork.kernel.org/project/dri-devel/patch/20230914155317.2511876-1-jani.nik...@intel.com/
Signed-off-by: Chun-Kuang Hu 
Signed-off-by: Greg Kroah-Hartman 
---
 drivers/gpu/drm/mediatek/mtk_dp.c |5 -
 1 file changed, 4 insertions(+), 1 deletion(-)

--- a/drivers/gpu/drm/mediatek/mtk_dp.c
+++ b/drivers/gpu/drm/mediatek/mtk_dp.c
@@ -1983,7 +1983,6 @@ static struct edid *mtk_dp_get_edid(stru
bool enabled = mtk_dp->enabled;
struct edid *new_edid = NULL;
struct mtk_dp_audio_cfg *audio_caps = &mtk_dp->info.audio_cur_cfg;
-   struct cea_sad *sads;
 
if (!enabled) {
drm_atomic_bridge_chain_pre_enable(bridge, 
connector->state->state);
@@ -2010,7 +2009,11 @@ static struct edid *mtk_dp_get_edid(stru
}
 
if (new_edid) {
+   struct cea_sad *sads;
+
audio_caps->sad_count = drm_edid_to_sad(new_edid, &sads);
+   kfree(sads);
+
audio_caps->detect_monitor = drm_detect_monitor_audio(new_edid);
}

Re: [git pull] drm fixes for 6.7-rc3

2023-11-24 Thread pr-tracker-bot

The pull request you sent on Fri, 24 Nov 2023 11:38:52 +1000:

> git://anongit.freedesktop.org/drm/drm tags/drm-fixes-2023-11-24

has been merged into torvalds/linux.git:
https://git.kernel.org/torvalds/c/afa0f6ee000abd220a8160f0375b5b8d3e4284f2

Thank you!

-- 
Deet-doot-dot, I am a bot.
https://korg.docs.kernel.org/prtracker.html

[PATCH 6.6 461/530] drm/mediatek/dp: fix memory leak on ->get_edid callback error path

6.6-stable review patch.  If anyone has any objections, please let me know.

--

From: Jani Nikula 

commit fcaf9761fd5884a64eaac48536f8c27ecfd2e6bc upstream.

Setting new_edid to NULL leaks the buffer.

Fixes: f70ac097a2cf ("drm/mediatek: Add MT8195 Embedded DisplayPort driver")
Cc: Markus Schneider-Pargmann 
Cc: Guillaume Ranquet 
Cc: Bo-Chen Chen 
Cc: CK Hu 
Cc: AngeloGioacchino Del Regno 
Cc: Dmitry Osipenko 
Cc: Chun-Kuang Hu 
Cc: Philipp Zabel 
Cc: Matthias Brugger 
Cc: dri-devel@lists.freedesktop.org
Cc: linux-media...@lists.infradead.org
Cc: linux-ker...@vger.kernel.org
Cc: linux-arm-ker...@lists.infradead.org
Cc:  # v6.1+
Signed-off-by: Jani Nikula 
Reviewed-by: Guillaume Ranquet 
Link: 
https://patchwork.kernel.org/project/dri-devel/patch/20230914131058.2472260-1-jani.nik...@intel.com/
Signed-off-by: Chun-Kuang Hu 
Signed-off-by: Greg Kroah-Hartman 
---
 drivers/gpu/drm/mediatek/mtk_dp.c |1 +
 1 file changed, 1 insertion(+)

--- a/drivers/gpu/drm/mediatek/mtk_dp.c
+++ b/drivers/gpu/drm/mediatek/mtk_dp.c
@@ -2048,6 +2048,7 @@ static struct edid *mtk_dp_get_edid(stru
 */
if (mtk_dp_parse_capabilities(mtk_dp)) {
drm_err(mtk_dp->drm_dev, "Can't parse capabilities\n");
+   kfree(new_edid);
new_edid = NULL;
}

[PATCH 6.6 460/530] drm/mediatek/dp: fix memory leak on ->get_edid callback audio detection

6.6-stable review patch.  If anyone has any objections, please let me know.

--

From: Jani Nikula 

commit dab12fa8d2bd3868cf2de485ed15a3feef28a13d upstream.

The sads returned by drm_edid_to_sad() needs to be freed.

Fixes: e71a8ebbe086 ("drm/mediatek: dp: Audio support for MT8195")
Cc: Guillaume Ranquet 
Cc: Bo-Chen Chen 
Cc: AngeloGioacchino Del Regno 
Cc: Dmitry Osipenko 
Cc: Chun-Kuang Hu 
Cc: Philipp Zabel 
Cc: Matthias Brugger 
Cc: dri-devel@lists.freedesktop.org
Cc: linux-media...@lists.infradead.org
Cc: linux-ker...@vger.kernel.org
Cc: linux-arm-ker...@lists.infradead.org
Cc:  # v6.1+
Signed-off-by: Jani Nikula 
Reviewed-by: Chen-Yu Tsai 
Link: 
https://patchwork.kernel.org/project/dri-devel/patch/20230914155317.2511876-1-jani.nik...@intel.com/
Signed-off-by: Chun-Kuang Hu 
Signed-off-by: Greg Kroah-Hartman 
---
 drivers/gpu/drm/mediatek/mtk_dp.c |5 -
 1 file changed, 4 insertions(+), 1 deletion(-)

--- a/drivers/gpu/drm/mediatek/mtk_dp.c
+++ b/drivers/gpu/drm/mediatek/mtk_dp.c
@@ -2034,7 +2034,6 @@ static struct edid *mtk_dp_get_edid(stru
bool enabled = mtk_dp->enabled;
struct edid *new_edid = NULL;
struct mtk_dp_audio_cfg *audio_caps = &mtk_dp->info.audio_cur_cfg;
-   struct cea_sad *sads;
 
if (!enabled) {
drm_atomic_bridge_chain_pre_enable(bridge, 
connector->state->state);
@@ -2053,7 +2052,11 @@ static struct edid *mtk_dp_get_edid(stru
}
 
if (new_edid) {
+   struct cea_sad *sads;
+
audio_caps->sad_count = drm_edid_to_sad(new_edid, &sads);
+   kfree(sads);
+
audio_caps->detect_monitor = drm_detect_monitor_audio(new_edid);
}

RE: [Intel-gfx] [PATCH] drm/i915/irq: Improve error logging for unexpected DE Misc interrupts

2023-11-24 Thread Rahul Rameshbabu

On Thursday, November 23rd, 2023 at 11:54 PM, Borah, Chaitanya Kumar 
 wrote:
> > -Original Message-
> > From: Intel-gfx  On Behalf Of Rahul
> > Rameshbabu
> > Sent: Thursday, November 23, 2023 11:27 PM
> > To: intel-...@lists.freedesktop.org
> > Cc: Nikula, Jani ; dri-devel@lists.freedesktop.org;
> > Rahul Rameshbabu 
> > Subject: [Intel-gfx] [PATCH] drm/i915/irq: Improve error logging for
> > unexpected DE Misc interrupts
> > 
> > Dump the iir value in hex when the interrupt is unexpected.
> > 
> > Link: https://gitlab.freedesktop.org/drm/intel/-/issues/9652#note_2178501
> > Cc: Jani Nikula 
> > Signed-off-by: Rahul Rameshbabu 
> > ---
> > drivers/gpu/drm/i915/display/intel_display_irq.c | 2 +-
> > 1 file changed, 1 insertion(+), 1 deletion(-)
> > 
> > diff --git a/drivers/gpu/drm/i915/display/intel_display_irq.c
> > b/drivers/gpu/drm/i915/display/intel_display_irq.c
> > index bff4a76310c0..1a5a9e0fc01e 100644
> > --- a/drivers/gpu/drm/i915/display/intel_display_irq.c
> > +++ b/drivers/gpu/drm/i915/display/intel_display_irq.c
> > @@ -896,7 +896,7 @@ gen8_de_misc_irq_handler(struct drm_i915_private
> > *dev_priv, u32 iir)
> > }
> > 
> > if (!found)
> > - drm_err(&dev_priv->drm, "Unexpected DE Misc
> > interrupt\n");
> > + drm_err(&dev_priv->drm, "Unexpected DE Misc interrupt:
> > %#x\n", iir);
> 
> 
> Nit: It could use a format specifier like "0x%08x" (like other instances in 
> the file) to maintain uniform width.

Agreed. I made this change initially for debugging without actually checking 
the practices used in the file.
Will send a v2 with this change and your Reviewed-by tag soon. Away from my 
development machine right now.

> Other than that. LGTM.
> 
> Reviewed-by: Chaitanya Kumar Borah 

--
Thanks,

Rahul Rameshbabu

Re: (subset) [PATCH v9 00/12] drm/meson: add support for MIPI DSI Display

Applied to clk-meson (v6.8/drivers), thanks!

[01/12] dt-bindings: clk: g12a-clkc: add CTS_ENCL clock ids
https://github.com/BayLibre/clk-meson/commit/bd5ef3f21d17
[06/12] clk: meson: g12a: add CTS_ENCL & CTS_ENCL_SEL clocks
https://github.com/BayLibre/clk-meson/commit/5de4e8353e32

Best regards,
--
Jerome

[PATCH][next] drm/imagination: Fix a couple of spelling mistakes in literal strings

2023-11-24 Thread Colin Ian King

There are a couple of spelling mistakes in literal strings in the
stid_fmts array. Fix these.

Signed-off-by: Colin Ian King 
---
 drivers/gpu/drm/imagination/pvr_rogue_fwif_sf.h | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/drivers/gpu/drm/imagination/pvr_rogue_fwif_sf.h 
b/drivers/gpu/drm/imagination/pvr_rogue_fwif_sf.h
index 571954182f33..56e11009e123 100644
--- a/drivers/gpu/drm/imagination/pvr_rogue_fwif_sf.h
+++ b/drivers/gpu/drm/imagination/pvr_rogue_fwif_sf.h
@@ -497,7 +497,7 @@ static const struct rogue_km_stid_fmt stid_fmts[] = {
{ ROGUE_FW_LOG_CREATESFID(213, ROGUE_FW_GROUP_MAIN, 1),
  "Safety Watchdog threshold period set to 0x%x clock cycles" },
{ ROGUE_FW_LOG_CREATESFID(214, ROGUE_FW_GROUP_MAIN, 0),
- "MTS Safety Event trigged by the safety watchdog." },
+ "MTS Safety Event triggered by the safety watchdog." },
{ ROGUE_FW_LOG_CREATESFID(215, ROGUE_FW_GROUP_MAIN, 3),
  "DM%d USC tasks range limit 0 - %d, stride %d" },
{ ROGUE_FW_LOG_CREATESFID(216, ROGUE_FW_GROUP_MAIN, 1),
@@ -1114,7 +1114,7 @@ static const struct rogue_km_stid_fmt stid_fmts[] = {
{ ROGUE_FW_LOG_CREATESFID(39, ROGUE_FW_GROUP_SPM, 2),
  "3DMemFree matches freelist 0x%08x (FL type = %u)" },
{ ROGUE_FW_LOG_CREATESFID(40, ROGUE_FW_GROUP_SPM, 0),
- "Raise the 3DMemFreeDedected flag" },
+ "Raise the 3DMemFreeDetected flag" },
{ ROGUE_FW_LOG_CREATESFID(41, ROGUE_FW_GROUP_SPM, 1),
  "Wait for pending grow on Freelist 0x%08x" },
{ ROGUE_FW_LOG_CREATESFID(42, ROGUE_FW_GROUP_SPM, 1),
-- 
2.39.2

Re: [PATCH v1 0/2] Add waveshare 7inch touchscreen panel support

2023-11-24 Thread Dave Stevenson

On Fri, 24 Nov 2023 at 15:00, Stefan Wahren  wrote:
>
> Hi Shengyang,
>
> [fix address of Emma]

Not merged to master yet, but Emma has stepped back from maintenance.
https://lists.freedesktop.org/archives/dri-devel/2023-October/428829.html
Dropped from the cc.

> Am 24.11.23 um 11:44 schrieb Shengyang Chen:
> > This patchset adds waveshare 7inch touchscreen panel support
> > for the StarFive JH7110 SoC.
> >
> > Patch 1 add new compatible for the raspberrypi panel driver and its 
> > dt-binding.
> > Patch 2 add new display mode and new probing process for raspberrypi panel 
> > driver.
> >
> > Waveshare 7inch touchscreen panel is a kind of raspberrypi panel
> > which can be drived by raspberrypi panel driver.
> >
> > The series has been tested on the VisionFive 2 board.
> surprisingly i was recently working on the official Raspberry Pi
> touchscreen and was able to get it running the new way.
>
> What do i mean with the new way. There is almost nothing special to the
> Raspberry Pi touchscreen, so we should try to use/extend existing
> components like:
>
> CONFIG_DRM_PANEL_SIMPLE
> CONFIG_TOUCHSCREEN_EDT_FT5X06
> CONFIG_DRM_TOSHIBA_TC358762
>
> The only special part is the Attiny on the connector PCB which requires:
>
> CONFIG_REGULATOR_RASPBERRYPI_TOUCHSCREEN_ATTINY
>
> So the whole point is to avoid writing monolitic drivers for simple
> panel like that.
>
> There is a WIP branch based on top of Linux 6.7-rcX, which should
> demonstrate this approach [1]. Unfortunately it is not ready for
> upstreaming, but it has been tested on a Raspberry Pi 3 B Plus. Maybe
> this is helpful for your case.
>
> Actually i consider panel-raspberrypi-touchscreen.c as a dead end, which
> shouldn't be extended.

Agreed.

The panel control being bound in with the Atmel control has no hook
for the EDT5x06 touch driver to hook in and keep the power to the
touch controller active. When the panel disable gets called, bye bye
touch overlay :-(

And I'm reading the driver change as more of a hack to get it to work
on your platform, not as adding support for the Waveshare panel
variant.
Waveshare deliberately cloned the behaviour of the Pi 7" panel in
order to make it work with the old Pi firmware drivers, so it
shouldn't need any significant changes. Where did the new timings come
from?

  Dave

> Btw there are already DT overlays in mainline which seems to use the
> Raspberry Pi 7inch panel (without touch function yet) [2].
>
> [1] - https://github.com/lategoodbye/rpi-zero/commits/v6.7-7inch-ts
> [2] -
> https://git.kernel.org/pub/scm/linux/kernel/git/stable/linux.git/commit/arch/arm64/boot/dts/freescale/imx8mm-venice-gw72xx-0x-rpidsi.dtso?h=v6.6.2&id=6b4da1354fd81adace0cda448c77d8f2a47d8474
>
> >
> > Shengyang Chen (2):
> >dt-bindings: display: panel: raspberrypi: Add compatible property for
> >  waveshare 7inch touchscreen panel
> >gpu: drm: panel: raspberrypi: add new display mode and new probing
> >  process
> >
> >   .../panel/raspberrypi,7inch-touchscreen.yaml  |  4 +-
> >   .../drm/panel/panel-raspberrypi-touchscreen.c | 99 ---
> >   2 files changed, 91 insertions(+), 12 deletions(-)
> >
>

Re: [PATCH v9 08/12] clk: meson: g12a: make VCLK2 and ENCL clock path configurable by CCF



On Fri 24 Nov 2023 at 16:15, Neil Armstrong  wrote:

> On 24/11/2023 15:12, Jerome Brunet wrote:
>> On Fri 24 Nov 2023 at 09:41, Neil Armstrong 
>> wrote:
>> 
>>> In order to setup the DSI clock, let's make the unused VCLK2 clock path
>>> configuration via CCF.
>>>
>>> The nocache option is removed from following clocks:
>>> - vclk2_sel
>>> - vclk2_input
>>> - vclk2_div
>>> - vclk2
>>> - vclk_div1
>>> - vclk2_div2_en
>>> - vclk2_div4_en
>>> - vclk2_div6_en
>>> - vclk2_div12_en
>>> - vclk2_div2
>>> - vclk2_div4
>>> - vclk2_div6
>>> - vclk2_div12
>>> - cts_encl_sel
>>>
>>> vclk2 and vclk2_div uses the newly introduced vclk regmap driver
>>> to handle the enable and reset bits.
>>>
>>> In order to set a rate on cts_encl via the vclk2 clock path,
>>> the NO_REPARENT flag is set on cts_encl_sel & vclk2_sel in order
>>> to keep CCF from selection a parent.
>>> The parents of cts_encl_sel & vclk2_sel are expected to be defined
>>> in DT.
>>>
>>> The following clock scheme is to be used for DSI:
>>>
>>> xtal
>>> \_ gp0_pll_dco
>>> \_ gp0_pll
>>>|- vclk2_sel
>>>|  \_ vclk2_input
>>>| \_ vclk2_div
>>>|\_ vclk2
>>>|   \_ vclk2_div1
>>>|  \_ cts_encl_sel
>>>| \_ cts_encl-> to VPU LCD Encoder
>>>|- mipi_dsi_pxclk_sel
>>>\_ mipi_dsi_pxclk_div
>>>   \_ mipi_dsi_pxclk -> to DSI controller
>>>
>>> The mipi_dsi_pxclk_div is set as RO in order to use the same GP0
>>> for mipi_dsi_pxclk and vclk2_input.
>> Could you explain a bit more this part of about the RO ops ?
>> Maybe I'm missing something.
>> You would be relying on the reset being always the way it. It is
>> probable but not safe.
>> A way to deal with the shared GP0 would be to:
>> * cut rate propagation at mipi_dsi_pxclk_sel (already done) and
>>(vclk2_sel - TBD) ...
>> * Set GP0 base rate through assigned-clock-rate (which you already in
>>patch 11)
>> With this, I'm not sure anything needs to be RO for the rates to be set
>> properly for each subtree.
>> Also, with the subtree above and your example in patch 11, it looks odd
>> that
>> PXCLK is manually set through DT while ENCL is not. Both are input of
>> dsi driver.
>
> So the deal is about dynamic setup of clocks for DSI bridges, not really
> for panels where we can probably know in advance the clock setup.
>
> In this particular case, we need to keep a ratio between the vclk and the
> DSI bitclk, the DSI bitclk is taken from mipi_dsi_pxclk and vclk is derived
> from gp0 via vclk2.
>
> If we set the bitclk rate via mipi_dsi_pxclk, CCF will try to use 
> mipi_dsi_pxclk_div
> to achieve the rate,

If you have CLK_RATE_PARENT on mipi_dsi_pxclk_sel, I'm not surprised it
does that, but you don't :/ I'm quite surprised it would do that, or
even could.

>From your example setting 96Mhz on both gp0 and mipi_dsi_pxclk, since
you've proposed RO-OPS, I suppose the divider is assumed to be 1 and
stay like that forever.

With rate propagation disabled mipi_dsi_pxclk_sel and GP0 is 96Mhz,
CCF would have no choice but picking 1 as divider, so I don't understand
how CCF would pick anything else and how RO-OPS help

> and it does it everytime I tried, breaking the vclk/bitclk ratio,
> and we have no way to know the gp0 rate in this case.

If you really want to ensure the divider value is always 1, why not use a
divider table allowing only 1 ? Adding a comment in the g12 clock driver
would nice because this not obvious. It would be safer than relying on
the reset value.

>
> I suspect mipi_dsi_pxclk_div was added to achieve fractional vclk/bitclk 
> ratios,
> since it doesn't exist on AXG. Not sure we would ever need it... and none
> of the other upstream DSI drivers supports such setups.
>
> The main reasons I set only mipi_dsi_pxclk in DT is because :
> 1) the DSI controller requires a bitclk to respond, pclk is not enough
> 2) GP0 is disabled with an invalid config at cold boot, thus we cannot
> rely on a default/safe rate on an initial prepare_enable().
> This permits setting initial valid state for the DSI controller, while
> the actual bitclk and vclk are calculated dynamically with panel/bridge
> runtime parameters.

Nothing against setting rate in DT when it is static. Setting it then
overriding it is not easy to follow.

To work around GP0 not being set, assuming you want to keep rate
propagation as it is, you could call clk_set_rate() on cts_encl (possibly w/o
enabling it) to force a setup on gp0 then clk_prepare_enable() on
pxclk. You'd get a your safe rate on GP0 and the clock you need on pxclk.

It is a bit hackish. Might be better to claim gp0 in your driver to
manage it directly, cutting rate propagation above it to control each
branch of the subtree as you need. It seems you need to have control over
that anyway and it would be clear GP0 is expected to belong to DSI.

>
> For the record, the samsung-dsim used fixed rate set from DT, and they moved
> from t

Bug in the error handling in TTMs pool implementation

2023-11-24 Thread Christian König


Hi guys,

some users ran into an OOM situation and discovered another problem in 
the error handling in TTMs page pool implementation.


@Karolina do you of hand know a way how we could exercise this in a TTM 
unit test? Basically we would need to redirect the alloc_pages_node() 
symbol to an unit test internal function and let it return an error (or 
something like that).


@Thomas you recently discovered and fixed bugs in that. I've just gone 
over the code once more, but can't find anything. Any idea what might be 
wrong?


Regards,
Christian.

Re: [PATCH v9 11/20] drm/imagination: Add GEM and VM related code

2023-11-24 Thread kernel test robot

Hi Donald Sarah,

kernel test robot noticed the following build warnings:

[auto build test WARNING on drm-misc/drm-misc-next]
[also build test WARNING on drm-exynos/exynos-drm-next drm-intel/for-linux-next 
drm-intel/for-linux-next-fixes linus/master v6.7-rc2]
[cannot apply to drm/drm-next drm-tip/drm-tip next-20231124]
[If your patch is applied to the wrong git tree, kindly drop us a note.
And when submitting patch, we suggest to use '--base' as documented in
https://git-scm.com/docs/git-format-patch#_base_tree_information]

url:
https://github.com/intel-lab-lkp/linux/commits/Donald-Robson/drm-gpuvm-Helper-to-get-range-of-unmap-from-a-remap-op/20231123-013514
base:   git://anongit.freedesktop.org/drm/drm-misc drm-misc-next
patch link:
https://lore.kernel.org/r/3c96dd170efe759b73897e3675d7310a7c4b06d0.1700668843.git.donald.robson%40imgtec.com
patch subject: [PATCH v9 11/20] drm/imagination: Add GEM and VM related code
config: arm64-allmodconfig 
(https://download.01.org/0day-ci/archive/20231124/202311242159.hh8mwiam-...@intel.com/config)
compiler: clang version 17.0.0 (https://github.com/llvm/llvm-project.git 
4a5ac14ee968ff0ad5d2cc1ffa0299048db4c88a)
reproduce (this is a W=1 build): 
(https://download.01.org/0day-ci/archive/20231124/202311242159.hh8mwiam-...@intel.com/reproduce)

If you fix the issue in a separate patch/commit (i.e. not just a new version of
the same patch/commit), kindly add following tags
| Reported-by: kernel test robot 
| Closes: 
https://lore.kernel.org/oe-kbuild-all/202311242159.hh8mwiam-...@intel.com/

All warnings (new ones prefixed by >>):

>> drivers/gpu/drm/imagination/pvr_vm.c:531:6: warning: no previous prototype 
>> for function 'pvr_gpuvm_free' [-Wmissing-prototypes]
 531 | void pvr_gpuvm_free(struct drm_gpuvm *gpuvm)
 |  ^
   drivers/gpu/drm/imagination/pvr_vm.c:531:1: note: declare 'static' if the 
function is not intended to be used outside of this translation unit
 531 | void pvr_gpuvm_free(struct drm_gpuvm *gpuvm)
 | ^
 | static 
   drivers/gpu/drm/imagination/pvr_vm.c:112:22: warning: unused function 
'to_pvr_vm_gpuva' [-Wunused-function]
 112 | struct pvr_vm_gpuva *to_pvr_vm_gpuva(struct drm_gpuva *gpuva)
 |  ^
   2 warnings generated.
--
>> drivers/gpu/drm/imagination/pvr_mmu.c:285: warning: Function parameter or 
>> member 'flags' not described in 'pvr_mmu_backing_page_sync'
--
>> drivers/gpu/drm/imagination/pvr_vm.c:65: warning: Function parameter or 
>> member 'gpuvm_mgr' not described in 'pvr_vm_context'


vim +/pvr_gpuvm_free +531 drivers/gpu/drm/imagination/pvr_vm.c

   530  
 > 531  void pvr_gpuvm_free(struct drm_gpuvm *gpuvm)
   532  {
   533  

-- 
0-DAY CI Kernel Test Service
https://github.com/intel/lkp-tests/wiki

Re: [PATCH] video: fbdev: mmp: Fix typo in code comment

2023-11-24 Thread Randy Dunlap

Hi,

On 11/24/23 01:52, Dario Binacchi wrote:
> s/singals/signals/
> 
> Fixes: 641b4b1b6a7c ("video: mmpdisp: add spi port in display controller")
> Signed-off-by: Dario Binacchi 
> ---
> 
>  drivers/video/fbdev/mmp/hw/mmp_spi.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/drivers/video/fbdev/mmp/hw/mmp_spi.c 
> b/drivers/video/fbdev/mmp/hw/mmp_spi.c
> index 16401eb95c6c..64e34b7e739e 100644
> --- a/drivers/video/fbdev/mmp/hw/mmp_spi.c
> +++ b/drivers/video/fbdev/mmp/hw/mmp_spi.c
> @@ -91,7 +91,7 @@ static int lcd_spi_setup(struct spi_device *spi)
>   writel(tmp, reg_base + LCD_SPU_SPI_CTRL);
>  
>   /*
> -  * After set mode it need a time to pull up the spi singals,
> +  * After set mode it need a time to pull up the spi signals,

Also:
  it needs time
or
  it needs some time

>* or it would cause the wrong waveform when send spi command,
>* especially on pxa910h
>*/

thanks.
-- 
~Randy

[PATCH v2 0/2] drm/bridge: adv7511: get edid in hpd_work to update CEC phys address

This series fixes a small bug where the CEC adapter could have an
invalid CEC address even though we got a hotplug connect and could have
read it.

Signed-off-by: Alvin Šipraga 
---
Changes in v2:
- Rearrange driver code to avoid the previous prototype of
  adv7511_get_edid(), per Laurent's feedback
- Free the returned EDID to prevent a memory leak, per Jani's comment
- Link to v1: 
https://lore.kernel.org/r/20231014-adv7511-cec-edid-v1-1-a58ceae0b...@bang-olufsen.dk

---
Alvin Šipraga (2):
  drm/bridge: adv7511: rearrange hotplug work code
  drm/bridge: adv7511: get edid in hpd_work to update CEC phys address

 drivers/gpu/drm/bridge/adv7511/adv7511_drv.c | 154 ---
 1 file changed, 90 insertions(+), 64 deletions(-)
---
base-commit: ab93edb2f94c3c0d5965be3815782472adbe3f52
change-id: 20231014-adv7511-cec-edid-ff75bd3ac929

[PATCH v2 1/2] drm/bridge: adv7511: rearrange hotplug work code

From: Alvin Šipraga 

In preparation for calling EDID helpers from the hotplug work, move the
hotplug work below the EDID helper section. No functional change.

Signed-off-by: Alvin Šipraga 
---
 drivers/gpu/drm/bridge/adv7511/adv7511_drv.c | 120 ++-
 1 file changed, 62 insertions(+), 58 deletions(-)

diff --git a/drivers/gpu/drm/bridge/adv7511/adv7511_drv.c 
b/drivers/gpu/drm/bridge/adv7511/adv7511_drv.c
index 8be235144f6d..5ffc5904bd59 100644
--- a/drivers/gpu/drm/bridge/adv7511/adv7511_drv.c
+++ b/drivers/gpu/drm/bridge/adv7511/adv7511_drv.c
@@ -406,64 +406,6 @@ static void adv7511_power_off(struct adv7511 *adv7511)
  * Interrupt and hotplug detection
  */
 
-static bool adv7511_hpd(struct adv7511 *adv7511)
-{
-   unsigned int irq0;
-   int ret;
-
-   ret = regmap_read(adv7511->regmap, ADV7511_REG_INT(0), &irq0);
-   if (ret < 0)
-   return false;
-
-   if (irq0 & ADV7511_INT0_HPD) {
-   regmap_write(adv7511->regmap, ADV7511_REG_INT(0),
-ADV7511_INT0_HPD);
-   return true;
-   }
-
-   return false;
-}
-
-static void adv7511_hpd_work(struct work_struct *work)
-{
-   struct adv7511 *adv7511 = container_of(work, struct adv7511, hpd_work);
-   enum drm_connector_status status;
-   unsigned int val;
-   int ret;
-
-   ret = regmap_read(adv7511->regmap, ADV7511_REG_STATUS, &val);
-   if (ret < 0)
-   status = connector_status_disconnected;
-   else if (val & ADV7511_STATUS_HPD)
-   status = connector_status_connected;
-   else
-   status = connector_status_disconnected;
-
-   /*
-* The bridge resets its registers on unplug. So when we get a plug
-* event and we're already supposed to be powered, cycle the bridge to
-* restore its state.
-*/
-   if (status == connector_status_connected &&
-   adv7511->connector.status == connector_status_disconnected &&
-   adv7511->powered) {
-   regcache_mark_dirty(adv7511->regmap);
-   adv7511_power_on(adv7511);
-   }
-
-   if (adv7511->connector.status != status) {
-   adv7511->connector.status = status;
-
-   if (adv7511->connector.dev) {
-   if (status == connector_status_disconnected)
-   cec_phys_addr_invalidate(adv7511->cec_adap);
-   drm_kms_helper_hotplug_event(adv7511->connector.dev);
-   } else {
-   drm_bridge_hpd_notify(&adv7511->bridge, status);
-   }
-   }
-}
-
 static int adv7511_irq_process(struct adv7511 *adv7511, bool process_hpd)
 {
unsigned int irq0, irq1;
@@ -600,6 +542,68 @@ static int adv7511_get_edid_block(void *data, u8 *buf, 
unsigned int block,
return 0;
 }
 
+/* 
-
+ * Hotplug handling
+ */
+
+static bool adv7511_hpd(struct adv7511 *adv7511)
+{
+   unsigned int irq0;
+   int ret;
+
+   ret = regmap_read(adv7511->regmap, ADV7511_REG_INT(0), &irq0);
+   if (ret < 0)
+   return false;
+
+   if (irq0 & ADV7511_INT0_HPD) {
+   regmap_write(adv7511->regmap, ADV7511_REG_INT(0),
+ADV7511_INT0_HPD);
+   return true;
+   }
+
+   return false;
+}
+
+static void adv7511_hpd_work(struct work_struct *work)
+{
+   struct adv7511 *adv7511 = container_of(work, struct adv7511, hpd_work);
+   enum drm_connector_status status;
+   unsigned int val;
+   int ret;
+
+   ret = regmap_read(adv7511->regmap, ADV7511_REG_STATUS, &val);
+   if (ret < 0)
+   status = connector_status_disconnected;
+   else if (val & ADV7511_STATUS_HPD)
+   status = connector_status_connected;
+   else
+   status = connector_status_disconnected;
+
+   /*
+* The bridge resets its registers on unplug. So when we get a plug
+* event and we're already supposed to be powered, cycle the bridge to
+* restore its state.
+*/
+   if (status == connector_status_connected &&
+   adv7511->connector.status == connector_status_disconnected &&
+   adv7511->powered) {
+   regcache_mark_dirty(adv7511->regmap);
+   adv7511_power_on(adv7511);
+   }
+
+   if (adv7511->connector.status != status) {
+   adv7511->connector.status = status;
+
+   if (adv7511->connector.dev) {
+   if (status == connector_status_disconnected)
+   cec_phys_addr_invalidate(adv7511->cec_adap);
+   drm_kms_helper_hotplug_event(adv7511->connector.dev);
+   } else {
+   drm_bridge_hpd_notify(&adv7511->bridge, status);
+   }
+   }
+}
+
 /*

[PATCH v2 2/2] drm/bridge: adv7511: get edid in hpd_work to update CEC phys address

From: Alvin Šipraga 

The adv7511 driver is solely responsible for setting the physical
address of its CEC adapter. To do this, it must read the EDID. However,
EDID is only read when either the drm_bridge_funcs :: get_edid or
drm_connector_helper_funcs :: get_modes ops are called. Without loss of
generality, it cannot be assumed that these ops are called when a sink
gets attached. Therefore there exist scenarios in which the CEC physical
address will be invalid (f.f.f.f), rendering the CEC adapter inoperable.

Address this problem by always fetching the EDID in the HPD work when we
detect a connection. The CEC physical address is set in the process.
This is done by moving the EDID DRM helper into an internal helper
function so that it can be cleanly called from an earlier section of
the code. The EDID getter has not changed in practice.

Signed-off-by: Alvin Šipraga 
---
 drivers/gpu/drm/bridge/adv7511/adv7511_drv.c | 74 ++--
 1 file changed, 48 insertions(+), 26 deletions(-)

diff --git a/drivers/gpu/drm/bridge/adv7511/adv7511_drv.c 
b/drivers/gpu/drm/bridge/adv7511/adv7511_drv.c
index 5ffc5904bd59..1f1d3a440895 100644
--- a/drivers/gpu/drm/bridge/adv7511/adv7511_drv.c
+++ b/drivers/gpu/drm/bridge/adv7511/adv7511_drv.c
@@ -542,6 +542,36 @@ static int adv7511_get_edid_block(void *data, u8 *buf, 
unsigned int block,
return 0;
 }
 
+static struct edid *__adv7511_get_edid(struct adv7511 *adv7511,
+  struct drm_connector *connector)
+{
+   struct edid *edid;
+
+   /* Reading the EDID only works if the device is powered */
+   if (!adv7511->powered) {
+   unsigned int edid_i2c_addr =
+   (adv7511->i2c_edid->addr << 1);
+
+   __adv7511_power_on(adv7511);
+
+   /* Reset the EDID_I2C_ADDR register as it might be cleared */
+   regmap_write(adv7511->regmap, ADV7511_REG_EDID_I2C_ADDR,
+edid_i2c_addr);
+   }
+
+   edid = drm_do_get_edid(connector, adv7511_get_edid_block, adv7511);
+
+   if (!adv7511->powered)
+   __adv7511_power_off(adv7511);
+
+   adv7511_set_config_csc(adv7511, connector, adv7511->rgb,
+  drm_detect_hdmi_monitor(edid));
+
+   cec_s_phys_addr_from_edid(adv7511->cec_adap, edid);
+
+   return edid;
+}
+
 /* 
-
  * Hotplug handling
  */
@@ -595,8 +625,24 @@ static void adv7511_hpd_work(struct work_struct *work)
adv7511->connector.status = status;
 
if (adv7511->connector.dev) {
-   if (status == connector_status_disconnected)
+   if (status == connector_status_disconnected) {
cec_phys_addr_invalidate(adv7511->cec_adap);
+   } else {
+   struct edid *edid;
+
+   /*
+* Get the updated EDID so that the CEC
+* subsystem gets informed of any change in CEC
+* address. The helper returns a newly allocated
+* edid structure, so free it to prevent
+* leakage.
+*/
+   edid = __adv7511_get_edid(adv7511,
+ &adv7511->connector);
+   if (edid)
+   kfree(edid);
+   }
+
drm_kms_helper_hotplug_event(adv7511->connector.dev);
} else {
drm_bridge_hpd_notify(&adv7511->bridge, status);
@@ -611,31 +657,7 @@ static void adv7511_hpd_work(struct work_struct *work)
 static struct edid *adv7511_get_edid(struct adv7511 *adv7511,
 struct drm_connector *connector)
 {
-   struct edid *edid;
-
-   /* Reading the EDID only works if the device is powered */
-   if (!adv7511->powered) {
-   unsigned int edid_i2c_addr =
-   (adv7511->i2c_edid->addr << 1);
-
-   __adv7511_power_on(adv7511);
-
-   /* Reset the EDID_I2C_ADDR register as it might be cleared */
-   regmap_write(adv7511->regmap, ADV7511_REG_EDID_I2C_ADDR,
-edid_i2c_addr);
-   }
-
-   edid = drm_do_get_edid(connector, adv7511_get_edid_block, adv7511);
-
-   if (!adv7511->powered)
-   __adv7511_power_off(adv7511);
-
-   adv7511_set_config_csc(adv7511, connector, adv7511->rgb,
-  drm_detect_hdmi_monitor(edid));
-
-   cec_s_phys_addr_from_edid(adv7511->cec_adap, edid);
-
-   return edid;
+   return __adv7511_get_edid(adv7511, connector);
 }

Re: [Intel-gfx] [PATCH 1/3] drm: Add new DRM_IOCTL_MODE_GETPLANE2

2023-11-24 Thread Andy Shevchenko

On Thu, Jan 12, 2017 at 12:23:16PM +0200, Ville Syrjälä wrote:
> On Wed, Jan 11, 2017 at 04:51:16PM -0800, Ben Widawsky wrote:

...

> > +   memcpy(plane->modifiers, format_modifiers,
> > +  format_modifier_count * sizeof(format_modifiers[0]));
> 
> Looks all right since we do the same for formats anyway. But it did
> occur to me (twice at least) that a kmemdup_array() might a nice thing
> to have for things like this. But that's a separate topic.

JFYI:
https://lore.kernel.org/all/20231017052322.2636-2-kkar...@nvidia.com/

-- 
With Best Regards,
Andy Shevchenko

Re: [PATCH v9 08/12] clk: meson: g12a: make VCLK2 and ENCL clock path configurable by CCF


On 24/11/2023 15:12, Jerome Brunet wrote:


On Fri 24 Nov 2023 at 09:41, Neil Armstrong  wrote:


In order to setup the DSI clock, let's make the unused VCLK2 clock path
configuration via CCF.

The nocache option is removed from following clocks:
- vclk2_sel
- vclk2_input
- vclk2_div
- vclk2
- vclk_div1
- vclk2_div2_en
- vclk2_div4_en
- vclk2_div6_en
- vclk2_div12_en
- vclk2_div2
- vclk2_div4
- vclk2_div6
- vclk2_div12
- cts_encl_sel

vclk2 and vclk2_div uses the newly introduced vclk regmap driver
to handle the enable and reset bits.

In order to set a rate on cts_encl via the vclk2 clock path,
the NO_REPARENT flag is set on cts_encl_sel & vclk2_sel in order
to keep CCF from selection a parent.
The parents of cts_encl_sel & vclk2_sel are expected to be defined
in DT.

The following clock scheme is to be used for DSI:

xtal
\_ gp0_pll_dco
\_ gp0_pll
   |- vclk2_sel
   |  \_ vclk2_input
   | \_ vclk2_div
   |\_ vclk2
   |   \_ vclk2_div1
   |  \_ cts_encl_sel
   | \_ cts_encl-> to VPU LCD Encoder
   |- mipi_dsi_pxclk_sel
   \_ mipi_dsi_pxclk_div
  \_ mipi_dsi_pxclk -> to DSI controller

The mipi_dsi_pxclk_div is set as RO in order to use the same GP0
for mipi_dsi_pxclk and vclk2_input.


Could you explain a bit more this part of about the RO ops ?
Maybe I'm missing something.

You would be relying on the reset being always the way it. It is
probable but not safe.

A way to deal with the shared GP0 would be to:
* cut rate propagation at mipi_dsi_pxclk_sel (already done) and
   (vclk2_sel - TBD) ...
* Set GP0 base rate through assigned-clock-rate (which you already in
   patch 11)

With this, I'm not sure anything needs to be RO for the rates to be set
properly for each subtree.

Also, with the subtree above and your example in patch 11, it looks odd that
PXCLK is manually set through DT while ENCL is not. Both are input of
dsi driver.


So the deal is about dynamic setup of clocks for DSI bridges, not really
for panels where we can probably know in advance the clock setup.

In this particular case, we need to keep a ratio between the vclk and the
DSI bitclk, the DSI bitclk is taken from mipi_dsi_pxclk and vclk is derived
from gp0 via vclk2.

If we set the bitclk rate via mipi_dsi_pxclk, CCF will try to use 
mipi_dsi_pxclk_div
to achieve the rate, and it does it everytime I tried, breaking the vclk/bitclk 
ratio,
and we have no way to know the gp0 rate in this case.

I suspect mipi_dsi_pxclk_div was added to achieve fractional vclk/bitclk ratios,
since it doesn't exist on AXG. Not sure we would ever need it... and none
of the other upstream DSI drivers supports such setups.

The main reasons I set only mipi_dsi_pxclk in DT is because :
1) the DSI controller requires a bitclk to respond, pclk is not enough
2) GP0 is disabled with an invalid config at cold boot, thus we cannot
rely on a default/safe rate on an initial prepare_enable().
This permits setting initial valid state for the DSI controller, while
the actual bitclk and vclk are calculated dynamically with panel/bridge
runtime parameters.

For the record, the samsung-dsim used fixed rate set from DT, and they moved
from that in order to support more panel and bridges.

But they're quite lucky because usually the DSI PLL is included in the PHY,
this makes the Amlogic design quite unusual (like most multimedia stuf...).

Neil





Signed-off-by: Neil Armstrong 
---
  drivers/clk/meson/g12a.c | 68 +---
  1 file changed, 47 insertions(+), 21 deletions(-)

diff --git a/drivers/clk/meson/g12a.c b/drivers/clk/meson/g12a.c
index cadd824336ad..fb3d9196a1fd 100644
--- a/drivers/clk/meson/g12a.c
+++ b/drivers/clk/meson/g12a.c
@@ -22,6 +22,7 @@
  #include "clk-regmap.h"
  #include "clk-cpu-dyndiv.h"
  #include "vid-pll-div.h"
+#include "vclk.h"
  #include "meson-eeclk.h"
  #include "g12a.h"
  
@@ -3165,7 +3166,7 @@ static struct clk_regmap g12a_vclk2_sel = {

.ops = &clk_regmap_mux_ops,
.parent_hws = g12a_vclk_parent_hws,
.num_parents = ARRAY_SIZE(g12a_vclk_parent_hws),
-   .flags = CLK_SET_RATE_NO_REPARENT | CLK_GET_RATE_NOCACHE,
+   .flags = CLK_SET_RATE_PARENT | CLK_SET_RATE_NO_REPARENT,


No sure CLK_SET_RATE_PARENT is wise here.
What you manually set in DT for the GP0, is likely to change because of
this, isn't it ?


},
  };
  
@@ -3193,7 +3194,7 @@ static struct clk_regmap g12a_vclk2_input = {

.ops = &clk_regmap_gate_ops,
.parent_hws = (const struct clk_hw *[]) { &g12a_vclk2_sel.hw },
.num_parents = 1,
-   .flags = CLK_SET_RATE_PARENT | CLK_IGNORE_UNUSED,
+   .flags = CLK_SET_RATE_PARENT,
},
  };
  
@@ -3215,19 +3216,32 @@ static struct clk_regmap g12a_vclk_div = {

  };
  
  static struct clk_regmap g12a_vclk2_div = {

-   .data = &(struct cl

Re: [PATCH v1 0/2] Add waveshare 7inch touchscreen panel support

2023-11-24 Thread Stefan Wahren

Hi Shengyang,

[fix address of Emma]

Am 24.11.23 um 11:44 schrieb Shengyang Chen:

This patchset adds waveshare 7inch touchscreen panel support
for the StarFive JH7110 SoC.

Patch 1 add new compatible for the raspberrypi panel driver and its dt-binding.
Patch 2 add new display mode and new probing process for raspberrypi panel
driver.

Waveshare 7inch touchscreen panel is a kind of raspberrypi panel
which can be drived by raspberrypi panel driver.

The series has been tested on the VisionFive 2 board.

surprisingly i was recently working on the official Raspberry Pi
touchscreen and was able to get it running the new way.

What do i mean with the new way. There is almost nothing special to the
Raspberry Pi touchscreen, so we should try to use/extend existing
components like:

CONFIG_DRM_PANEL_SIMPLE
CONFIG_TOUCHSCREEN_EDT_FT5X06
CONFIG_DRM_TOSHIBA_TC358762

The only special part is the Attiny on the connector PCB which requires:

CONFIG_REGULATOR_RASPBERRYPI_TOUCHSCREEN_ATTINY

So the whole point is to avoid writing monolitic drivers for simple
panel like that.

There is a WIP branch based on top of Linux 6.7-rcX, which should
demonstrate this approach [1]. Unfortunately it is not ready for
upstreaming, but it has been tested on a Raspberry Pi 3 B Plus. Maybe
this is helpful for your case.

Actually i consider panel-raspberrypi-touchscreen.c as a dead end, which
shouldn't be extended.

Btw there are already DT overlays in mainline which seems to use the
Raspberry Pi 7inch panel (without touch function yet) [2].

[1] - https://github.com/lategoodbye/rpi-zero/commits/v6.7-7inch-ts
[2] -
https://git.kernel.org/pub/scm/linux/kernel/git/stable/linux.git/commit/arch/arm64/boot/dts/freescale/imx8mm-venice-gw72xx-0x-rpidsi.dtso?h=v6.6.2&id=6b4da1354fd81adace0cda448c77d8f2a47d8474

Shengyang Chen (2):
dt-bindings: display: panel: raspberrypi: Add compatible property for
waveshare 7inch touchscreen panel
gpu: drm: panel: raspberrypi: add new display mode and new probing
process

.../panel/raspberrypi,7inch-touchscreen.yaml | 4 +-
.../drm/panel/panel-raspberrypi-touchscreen.c | 99 ---
2 files changed, 91 insertions(+), 12 deletions(-)

Re: [PATCH v9 07/12] clk: meson: add vclk driver



On Fri 24 Nov 2023 at 09:41, Neil Armstrong  wrote:

> The VCLK and VCLK_DIV clocks have supplementary bits.
>
> The VCLK has a "SOFT RESET" bit to toggle after the whole
> VCLK sub-tree rate has been set, this is implemented in
> the gate enable callback.
>
> The VCLK_DIV clocks as enable and reset bits used to disable
> and reset the divider, associated with CLK_SET_RATE_GATE it ensures
> the rate is set while the divider is disabled and in reset mode.
>
> The VCLK_DIV enable bit isn't implemented as a gate since it's part
> of the divider logic and vendor does this exact sequence to ensure
> the divider is correctly set.
>
> Signed-off-by: Neil Armstrong 
> ---
>  drivers/clk/meson/Kconfig  |   5 ++
>  drivers/clk/meson/Makefile |   1 +
>  drivers/clk/meson/vclk.c   | 141 
> +
>  drivers/clk/meson/vclk.h   |  51 
>  4 files changed, 198 insertions(+)
>
> diff --git a/drivers/clk/meson/Kconfig b/drivers/clk/meson/Kconfig
> index 29ffd14d267b..59a40a49f8e1 100644
> --- a/drivers/clk/meson/Kconfig
> +++ b/drivers/clk/meson/Kconfig
> @@ -30,6 +30,10 @@ config COMMON_CLK_MESON_VID_PLL_DIV
>   tristate
>   select COMMON_CLK_MESON_REGMAP
>  
> +config COMMON_CLK_MESON_VCLK
> + tristate
> + select COMMON_CLK_MESON_REGMAP
> +
>  config COMMON_CLK_MESON_CLKC_UTILS
>   tristate
>  
> @@ -140,6 +144,7 @@ config COMMON_CLK_G12A
>   select COMMON_CLK_MESON_EE_CLKC
>   select COMMON_CLK_MESON_CPU_DYNDIV
>   select COMMON_CLK_MESON_VID_PLL_DIV
> + select COMMON_CLK_MESON_VCLK

This particular line belong in the next patch

>   select MFD_SYSCON
>   help
> Support for the clock controller on Amlogic S905D2, S905X2 and S905Y2
> diff --git a/drivers/clk/meson/Makefile b/drivers/clk/meson/Makefile
> index 9ee4b954c896..9ba43fe7a07a 100644
> --- a/drivers/clk/meson/Makefile
> +++ b/drivers/clk/meson/Makefile
> @@ -12,6 +12,7 @@ obj-$(CONFIG_COMMON_CLK_MESON_PLL) += clk-pll.o
>  obj-$(CONFIG_COMMON_CLK_MESON_REGMAP) += clk-regmap.o
>  obj-$(CONFIG_COMMON_CLK_MESON_SCLK_DIV) += sclk-div.o
>  obj-$(CONFIG_COMMON_CLK_MESON_VID_PLL_DIV) += vid-pll-div.o
> +obj-$(CONFIG_COMMON_CLK_MESON_VCLK) += vclk.o
>  
>  # Amlogic Clock controllers
>  
> diff --git a/drivers/clk/meson/vclk.c b/drivers/clk/meson/vclk.c
> new file mode 100644
> index ..47f08a52b49f
> --- /dev/null
> +++ b/drivers/clk/meson/vclk.c
> @@ -0,0 +1,141 @@
> +// SPDX-License-Identifier: GPL-2.0
> +/*
> + * Copyright (c) 2023 Neil Armstrong 
> + */
> +
> +#include 
> +#include "vclk.h"
> +
> +/* The VCLK gate has a supplementary reset bit to pulse after ungating */
> +
> +static inline struct clk_regmap_vclk_data *
> +clk_get_regmap_vclk_data(struct clk_regmap *clk)
> +{
> + return (struct clk_regmap_vclk_data *)clk->data;
> +}
> +
> +static int clk_regmap_vclk_enable(struct clk_hw *hw)
> +{
> + struct clk_regmap *clk = to_clk_regmap(hw);
> + struct clk_regmap_vclk_data *vclk = clk_get_regmap_vclk_data(clk);
> +
> + meson_parm_write(clk->map, &vclk->enable, 1);
> +
> + /* Do a reset pulse */
> + meson_parm_write(clk->map, &vclk->reset, 1);
> + meson_parm_write(clk->map, &vclk->reset, 0);
> +
> + return 0;
> +}
> +
> +static void clk_regmap_vclk_disable(struct clk_hw *hw)
> +{
> + struct clk_regmap *clk = to_clk_regmap(hw);
> + struct clk_regmap_vclk_data *vclk = clk_get_regmap_vclk_data(clk);
> +
> + meson_parm_write(clk->map, &vclk->enable, 0);
> +}
> +
> +static int clk_regmap_vclk_is_enabled(struct clk_hw *hw)
> +{
> + struct clk_regmap *clk = to_clk_regmap(hw);
> + struct clk_regmap_vclk_data *vclk = clk_get_regmap_vclk_data(clk);
> +
> + return meson_parm_read(clk->map, &vclk->enable);
> +}
> +
> +const struct clk_ops clk_regmap_vclk_ops = {
> + .enable = clk_regmap_vclk_enable,
> + .disable = clk_regmap_vclk_disable,
> + .is_enabled = clk_regmap_vclk_is_enabled,
> +};
> +EXPORT_SYMBOL_GPL(clk_regmap_vclk_ops);

s/clk_regmap_vclk/meson_vclk at least for what is exported, ideally most
all the code.

I get clk_regmap_ comes from code copied from clk_regmap.c.
The reason the this part is different (and not using parm) if that when
I converted amlogic to regmap, I hope we could make this generic,
possibly converging between aml and qcom (which was the only other
platform using regmap for clock at the time). This is why clk_regmap.c
is a bit different from the other driver.

For the aml specific drivers, best to look at the mpll or cpu-dyndiv one.

> +
> +/* The VCLK Divider has supplementary reset & enable bits */
> +
> +static inline struct clk_regmap_vclk_div_data *
> +clk_get_regmap_vclk_div_data(struct clk_regmap *clk)
> +{
> + return (struct clk_regmap_vclk_div_data *)clk->data;
> +}
> +
> +static unsigned long clk_regmap_vclk_div_recalc_rate(struct clk_hw *hw,
> +  unsigned long prate)
> +{
> + struct clk_regmap *cl

Re: [PATCH v9 11/12] DONOTMERGE: arm64: meson: khadas-vim3l: add DSI panel


Hi,

On 24/11/2023 11:52, Maxime Ripard wrote:

Hi,

On Fri, Nov 24, 2023 at 09:41:22AM +0100, Neil Armstrong wrote:

This add nodes to support the Khadas TS050 panel on the
Khadas VIM3 & VIM3L boards.

Signed-off-by: Neil Armstrong 
---
  .../boot/dts/amlogic/meson-g12b-khadas-vim3.dtsi   |  2 +-
  arch/arm64/boot/dts/amlogic/meson-khadas-vim3.dtsi | 74 ++
  .../boot/dts/amlogic/meson-sm1-khadas-vim3l.dts|  2 +-
  3 files changed, 76 insertions(+), 2 deletions(-)


Generally, those kind of patches still have value. Now that we are
accepting overlays, could this be converted to one and merged maybe?


Yep I was thinking about that, I'll probably do that,
some new boards will also need overlays for DSI panels.

I'll probably switch to an overlay on v10.

Neil



Maxime

Re: [PATCH v9 04/12] dt-bindings: phy: amlogic, g12a-mipi-dphy-analog: drop unneeded reg property and example


On 24/11/2023 15:41, Conor Dooley wrote:

On Fri, Nov 24, 2023 at 02:50:58PM +0100, Neil Armstrong wrote:

Hi Conor,

On 24/11/2023 13:36, Conor Dooley wrote:

On Fri, Nov 24, 2023 at 09:41:15AM +0100, Neil Armstrong wrote:

The amlogic,g12a-mipi-dphy-analog is a feature of the simple-mfd
amlogic,meson-axg-hhi-sysctrl system control register zone which is an
intermixed registers zone, thus it's very hard to define clear ranges for
each SoC controlled features even if possible.

The amlogic,g12a-mipi-dphy-analog was wrongly documented as an independent
register range, which is not the reality, thus fix the bindings by dropping
the reg property now it's referred from amlogic,meson-gx-hhi-sysctrl.yaml
and documented as a subnode of amlogic,meson-axg-hhi-sysctrl.

Also drop the unnecessary example, the top level bindings example should
be enough.

Fixes: 76ab79f9726c ("dt-bindings: phy: add Amlogic G12A Analog MIPI D-PHY 
bindings")
Signed-off-by: Neil Armstrong 


I feel like I left a tag on this one before, but I can't remember.
Perhaps I missed the conclusion to the discussion to the discussion with
Rob about whether having "reg" was desirable that lead to a tag being
dropped?


I checked again and nope, not tag, but Rob's question was legitimate and I 
reworded
and clarified the commit message following your reviews.
On the other side you suggested a Fixes tag, which I added.

The rewording is about why reg doesn't make sense on the nature of the memory
region and it doesn't make sense here like other similar nodes.


Okay, I thought that I had given you one. Perhaps I forgot to send, or
Rob's message came in between me asking about the Fixes tag & replying
with an Ack. Sorry about that,
Acked-by: Conor Dooley 


No problem thanks for your review.

Neil



Cheers,
Conor.

Re: [PATCH v4 0/5] drm: Allow the damage helpers to handle buffer damage

2023-11-24 Thread Javier Martinez Canillas

Javier Martinez Canillas  writes:

> Hello,
>
> This series is to fix an issue that surfaced after damage clipping was
> enabled for the virtio-gpu by commit 01f05940a9a7 ("drm/virtio: Enable
> fb damage clips property for the primary plane").
>
> After that change, flickering artifacts was reported to be present with
> both weston and wlroots wayland compositors when running in a virtual
> machine. The cause was identified by Sima Vetter, who pointed out that
> virtio-gpu does per-buffer uploads and for this reason it needs to do
> a buffer damage handling, instead of frame damage handling.
>
> Their suggestion was to extend the damage helpers to cover that case
> and given that there's isn't a buffer damage accumulation algorithm
> (e.g: buffer age), just do a full plane update if the framebuffer that
> is attached to a plane changed since the last plane update (page-flip).
>
> It is a v4 that addresses issues pointed out by Sima Vetter in v3:
>
> https://lists.freedesktop.org/archives/dri-devel/2023-November/431409.html
>
> Patch #1 adds a ignore_damage_clips field to struct drm_plane_state to be
> set by drivers that want the damage helpers to ignore the damage clips.
>
> Patch #2 fixes the virtio-gpu damage handling logic by asking the damage
> helper to ignore the damage clips if the framebuffer attached to a plane
> has changed since the last page-flip.
>
> Patch #3 does the same but for the vmwgfx driver that also needs to handle
> buffer damage and should have the same issue (although I haven't tested it
> due not having a VMWare setup).
>
> Patch #4 adds to the KMS damage tracking kernel-doc some paragraphs about
> damage tracking types and references to links that explain frame damage vs
> buffer damage.
>
> Finally patch #5 adds an item to the DRM todo, about the need to implement
> some buffer damage accumulation algorithm instead of just doing full plane
> updates in this case.
>
> Because commit 01f05940a9a7 landed in v6.4, the first 2 patches are marked
> as Fixes and Cc stable.
>
> I've tested this on a VM with weston, was able to reproduce the issue
> reported and the patches did fix the problem.
>
> Best regards,
> Javier
>
> Changes in v4:
> - Refer in ignore_damage_clips kernel-doc to "Damage Tracking Properties"
>   KMS documentation section (Sima Vetter).
> - Add another paragraph to "Damage Tracking Properties" section to mention
>   the fields that drivers with per-buffer upload target should check to set
>   drm_plane_state.ignore_damage_clips (Sima Vetter).
> - Reference the &drm_plane_state.ignore_damage_clips and the damage helpers
>   in the buffer damage TODO entry (Sima Vetter).
>
> Changes in v3:
> - Fix typo in the kernel-doc (Simon Ser).
> - Add a paragraph explaining what the problem in the kernel is and
>   make it clear that the refeference documents are related to how
>   user-space handles this case (Thomas Zimmermann).
>
> Changes in v2:
> - Add a struct drm_plane_state .ignore_damage_clips to set in the plane's
>   .atomic_check, instead of having different helpers (Thomas Zimmermann).
> - Set struct drm_plane_state .ignore_damage_clips in virtio-gpu plane's
>   .atomic_check instead of using a different helpers (Thomas Zimmermann).
> - Set struct drm_plane_state .ignore_damage_clips in vmwgfx plane's
>   .atomic_check instead of using a different helpers (Thomas Zimmermann).
>
> Javier Martinez Canillas (5):
>   drm: Allow drivers to indicate the damage helpers to ignore damage
> clips
>   drm/virtio: Disable damage clipping if FB changed since last page-flip
>   drm/vmwgfx: Disable damage clipping if FB changed since last page-flip
>   drm/plane: Extend damage tracking kernel-doc
>   drm/todo: Add entry about implementing buffer age for damage tracking
>

Pushed to drm-misc (drm-misc-next). Thanks!

-- 
Best regards,

Javier Martinez Canillas
Core Platforms
Red Hat

Re: [PATCH v9 04/12] dt-bindings: phy: amlogic,g12a-mipi-dphy-analog: drop unneeded reg property and example

2023-11-24 Thread Conor Dooley

On Fri, Nov 24, 2023 at 02:50:58PM +0100, Neil Armstrong wrote:
> Hi Conor,
> 
> On 24/11/2023 13:36, Conor Dooley wrote:
> > On Fri, Nov 24, 2023 at 09:41:15AM +0100, Neil Armstrong wrote:
> > > The amlogic,g12a-mipi-dphy-analog is a feature of the simple-mfd
> > > amlogic,meson-axg-hhi-sysctrl system control register zone which is an
> > > intermixed registers zone, thus it's very hard to define clear ranges for
> > > each SoC controlled features even if possible.
> > > 
> > > The amlogic,g12a-mipi-dphy-analog was wrongly documented as an independent
> > > register range, which is not the reality, thus fix the bindings by 
> > > dropping
> > > the reg property now it's referred from amlogic,meson-gx-hhi-sysctrl.yaml
> > > and documented as a subnode of amlogic,meson-axg-hhi-sysctrl.
> > > 
> > > Also drop the unnecessary example, the top level bindings example should
> > > be enough.
> > > 
> > > Fixes: 76ab79f9726c ("dt-bindings: phy: add Amlogic G12A Analog MIPI 
> > > D-PHY bindings")
> > > Signed-off-by: Neil Armstrong 
> > 
> > I feel like I left a tag on this one before, but I can't remember.
> > Perhaps I missed the conclusion to the discussion to the discussion with
> > Rob about whether having "reg" was desirable that lead to a tag being
> > dropped?
> 
> I checked again and nope, not tag, but Rob's question was legitimate and I 
> reworded
> and clarified the commit message following your reviews.
> On the other side you suggested a Fixes tag, which I added.
> 
> The rewording is about why reg doesn't make sense on the nature of the memory
> region and it doesn't make sense here like other similar nodes.

Okay, I thought that I had given you one. Perhaps I forgot to send, or
Rob's message came in between me asking about the Fixes tag & replying
with an Ack. Sorry about that,
Acked-by: Conor Dooley 

Cheers,
Conor.


signature.asc
Description: PGP signature

Re: [PATCH v6 0/9] Fix cursor planes with virtualized drivers

2023-11-24 Thread Javier Martinez Canillas

Albert Esteve  writes:

> v6: Shift DRIVER_CURSOR_HOTSPOT flag bit to BIT(9), since BIT(8)
> was already taken by DRIVER_GEM_GPUVA.
>
> v5: Add a change with documentation from Michael, based on his discussion
> with Pekka and bump the kernel version DRM_CLIENT_CAP_CURSOR_PLANE_HOTSPOT
> might be introduced with to 6.6.
>
> v4: Make drm_plane_create_hotspot_properties static, rename
> DRM_CLIENT_CAP_VIRTUALIZED_CURSOR_PLANE to DRM_CLIENT_CAP_CURSOR_PLANE_HOTSPOT
> and some minor stylistic fixes for things found by Javier and Pekka
> in v3.
>
> v3: Renames, fixes and cleanups suggested by Daniel, Simon and Pekka
> after v2. There's no major changes in functionality. Please let me know
> if I missed anything, it's been a while since v2.
>
> Virtualized drivers have had a lot of issues with cursor support on top
> of atomic modesetting. This set both fixes the long standing problems
> with atomic kms and virtualized drivers and adds code to let userspace
> use atomic kms on virtualized drivers while preserving functioning
> seamless cursors between the host and guest.
>
> The first change in the set is one that should be backported as far as
> possible, likely 5.4 stable, because earlier stable kernels do not have
> virtualbox driver. The change makes virtualized drivers stop exposing
> a cursor plane for atomic clients, this fixes mouse cursor on all well
> formed compositors which will automatically fallback to software cursor.
>
> The rest of the changes until the last one ports the legacy hotspot code
> to atomic plane properties.
>
> Finally the last change introduces userspace API to let userspace
> clients advertise the fact that they are aware of additional restrictions
> placed upon the cursor plane by virtualized drivers and lets them use
> atomic kms with virtualized drivers (the clients are expected to set
> hotspots correctly when advertising support for virtual cursor plane).
>
> Link to the IGT test covering this patch (already merged):
> https://lists.freedesktop.org/archives/igt-dev/2023-July/058427.html
>
> Mutter patch:
> https://lists.freedesktop.org/archives/igt-dev/2023-July/058427.html
>
> Michael Banack (1):
>   drm: Introduce documentation for hotspot properties
>
> Zack Rusin (8):
>   drm: Disable the cursor plane on atomic contexts with virtualized
> drivers
>   drm/atomic: Add support for mouse hotspots
>   drm/vmwgfx: Use the hotspot properties from cursor planes
>   drm/qxl: Use the hotspot properties from cursor planes
>   drm/vboxvideo: Use the hotspot properties from cursor planes
>   drm/virtio: Use the hotspot properties from cursor planes
>   drm: Remove legacy cursor hotspot code
>   drm: Introduce DRM_CLIENT_CAP_CURSOR_PLANE_HOTSPOT
>

Pushed to drm-misc (drm-misc-next). Thanks!

-- 
Best regards,

Javier Martinez Canillas
Core Platforms
Red Hat

Re: [PATCH v9 08/12] clk: meson: g12a: make VCLK2 and ENCL clock path configurable by CCF



On Fri 24 Nov 2023 at 09:41, Neil Armstrong  wrote:

> In order to setup the DSI clock, let's make the unused VCLK2 clock path
> configuration via CCF.
>
> The nocache option is removed from following clocks:
> - vclk2_sel
> - vclk2_input
> - vclk2_div
> - vclk2
> - vclk_div1
> - vclk2_div2_en
> - vclk2_div4_en
> - vclk2_div6_en
> - vclk2_div12_en
> - vclk2_div2
> - vclk2_div4
> - vclk2_div6
> - vclk2_div12
> - cts_encl_sel
>
> vclk2 and vclk2_div uses the newly introduced vclk regmap driver
> to handle the enable and reset bits.
>
> In order to set a rate on cts_encl via the vclk2 clock path,
> the NO_REPARENT flag is set on cts_encl_sel & vclk2_sel in order
> to keep CCF from selection a parent.
> The parents of cts_encl_sel & vclk2_sel are expected to be defined
> in DT.
>
> The following clock scheme is to be used for DSI:
>
> xtal
> \_ gp0_pll_dco
>\_ gp0_pll
>   |- vclk2_sel
>   |  \_ vclk2_input
>   | \_ vclk2_div
>   |\_ vclk2
>   |   \_ vclk2_div1
>   |  \_ cts_encl_sel
>   | \_ cts_encl   -> to VPU LCD Encoder
>   |- mipi_dsi_pxclk_sel
>   \_ mipi_dsi_pxclk_div
>  \_ mipi_dsi_pxclk-> to DSI controller
>
> The mipi_dsi_pxclk_div is set as RO in order to use the same GP0
> for mipi_dsi_pxclk and vclk2_input.

Could you explain a bit more this part of about the RO ops ?
Maybe I'm missing something.

You would be relying on the reset being always the way it. It is
probable but not safe.

A way to deal with the shared GP0 would be to:
* cut rate propagation at mipi_dsi_pxclk_sel (already done) and
  (vclk2_sel - TBD) ... 
* Set GP0 base rate through assigned-clock-rate (which you already in
  patch 11)

With this, I'm not sure anything needs to be RO for the rates to be set
properly for each subtree.

Also, with the subtree above and your example in patch 11, it looks odd that
PXCLK is manually set through DT while ENCL is not. Both are input of
dsi driver.

>
> Signed-off-by: Neil Armstrong 
> ---
>  drivers/clk/meson/g12a.c | 68 
> +---
>  1 file changed, 47 insertions(+), 21 deletions(-)
>
> diff --git a/drivers/clk/meson/g12a.c b/drivers/clk/meson/g12a.c
> index cadd824336ad..fb3d9196a1fd 100644
> --- a/drivers/clk/meson/g12a.c
> +++ b/drivers/clk/meson/g12a.c
> @@ -22,6 +22,7 @@
>  #include "clk-regmap.h"
>  #include "clk-cpu-dyndiv.h"
>  #include "vid-pll-div.h"
> +#include "vclk.h"
>  #include "meson-eeclk.h"
>  #include "g12a.h"
>  
> @@ -3165,7 +3166,7 @@ static struct clk_regmap g12a_vclk2_sel = {
>   .ops = &clk_regmap_mux_ops,
>   .parent_hws = g12a_vclk_parent_hws,
>   .num_parents = ARRAY_SIZE(g12a_vclk_parent_hws),
> - .flags = CLK_SET_RATE_NO_REPARENT | CLK_GET_RATE_NOCACHE,
> + .flags = CLK_SET_RATE_PARENT | CLK_SET_RATE_NO_REPARENT,

No sure CLK_SET_RATE_PARENT is wise here.
What you manually set in DT for the GP0, is likely to change because of
this, isn't it ?

>   },
>  };
>  
> @@ -3193,7 +3194,7 @@ static struct clk_regmap g12a_vclk2_input = {
>   .ops = &clk_regmap_gate_ops,
>   .parent_hws = (const struct clk_hw *[]) { &g12a_vclk2_sel.hw },
>   .num_parents = 1,
> - .flags = CLK_SET_RATE_PARENT | CLK_IGNORE_UNUSED,
> + .flags = CLK_SET_RATE_PARENT,
>   },
>  };
>  
> @@ -3215,19 +3216,32 @@ static struct clk_regmap g12a_vclk_div = {
>  };
>  
>  static struct clk_regmap g12a_vclk2_div = {
> - .data = &(struct clk_regmap_div_data){
> - .offset = HHI_VIID_CLK_DIV,
> - .shift = 0,
> - .width = 8,
> + .data = &(struct clk_regmap_vclk_div_data){
> + .div = {
> + .reg_off = HHI_VIID_CLK_DIV,
> + .shift   = 0,
> + .width   = 8,
> + },
> + .enable = {
> + .reg_off = HHI_VIID_CLK_DIV,
> + .shift   = 16,
> + .width   = 1,
> + },
> + .reset = {
> + .reg_off = HHI_VIID_CLK_DIV,
> + .shift   = 17,
> + .width   = 1,
> + },
> + .flags = CLK_DIVIDER_ROUND_CLOSEST,
>   },
>   .hw.init = &(struct clk_init_data){
>   .name = "vclk2_div",
> - .ops = &clk_regmap_divider_ops,
> + .ops = &clk_regmap_vclk_div_ops,
>   .parent_hws = (const struct clk_hw *[]) {
>   &g12a_vclk2_input.hw
>   },
>   .num_parents = 1,
> - .flags = CLK_GET_RATE_NOCACHE,
> + .flags = CLK_SET_RATE_PARENT | CLK_SET_RATE_GATE,
>   },
>  };
>  
> @@ -3246,16 +3260,24 @@ static struct clk_regmap g12a_vclk = {
>  };
>  
>  static struct clk_regmap g12a_vclk2 = {
> - .data = &(struct clk_regmap_gate_data){
> - .

Re: [PATCH v9 04/12] dt-bindings: phy: amlogic, g12a-mipi-dphy-analog: drop unneeded reg property and example


Hi Conor,

On 24/11/2023 13:36, Conor Dooley wrote:

On Fri, Nov 24, 2023 at 09:41:15AM +0100, Neil Armstrong wrote:

The amlogic,g12a-mipi-dphy-analog is a feature of the simple-mfd
amlogic,meson-axg-hhi-sysctrl system control register zone which is an
intermixed registers zone, thus it's very hard to define clear ranges for
each SoC controlled features even if possible.

The amlogic,g12a-mipi-dphy-analog was wrongly documented as an independent
register range, which is not the reality, thus fix the bindings by dropping
the reg property now it's referred from amlogic,meson-gx-hhi-sysctrl.yaml
and documented as a subnode of amlogic,meson-axg-hhi-sysctrl.

Also drop the unnecessary example, the top level bindings example should
be enough.

Fixes: 76ab79f9726c ("dt-bindings: phy: add Amlogic G12A Analog MIPI D-PHY 
bindings")
Signed-off-by: Neil Armstrong 


I feel like I left a tag on this one before, but I can't remember.
Perhaps I missed the conclusion to the discussion to the discussion with
Rob about whether having "reg" was desirable that lead to a tag being
dropped?


I checked again and nope, not tag, but Rob's question was legitimate and I 
reworded
and clarified the commit message following your reviews.
On the other side you suggested a Fixes tag, which I added.

The rewording is about why reg doesn't make sense on the nature of the memory
region and it doesn't make sense here like other similar nodes.

Neil




---
  .../bindings/phy/amlogic,g12a-mipi-dphy-analog.yaml  | 12 
  1 file changed, 12 deletions(-)

diff --git 
a/Documentation/devicetree/bindings/phy/amlogic,g12a-mipi-dphy-analog.yaml 
b/Documentation/devicetree/bindings/phy/amlogic,g12a-mipi-dphy-analog.yaml
index c8c83acfb871..81c2654b7e57 100644
--- a/Documentation/devicetree/bindings/phy/amlogic,g12a-mipi-dphy-analog.yaml
+++ b/Documentation/devicetree/bindings/phy/amlogic,g12a-mipi-dphy-analog.yaml
@@ -16,20 +16,8 @@ properties:
"#phy-cells":
  const: 0
  
-  reg:

-maxItems: 1
-
  required:
- compatible
-  - reg
- "#phy-cells"
  
  additionalProperties: false

-
-examples:
-  - |
-phy@0 {
-  compatible = "amlogic,g12a-mipi-dphy-analog";
-  reg = <0x0 0xc>;
-  #phy-cells = <0>;
-};

--
2.34.1

dri-devel@lists.freedesktop.org

2023-11-24 Thread Thomas Zimmermann


Hi

Am 25.10.23 um 12:39 schrieb Keith Zhao:
[...]

+
+static struct drm_crtc_state *
+vs_crtc_atomic_duplicate_state(struct drm_crtc *crtc)
+{
+   struct vs_crtc_state *ori_state;


I have not yet seen 'ori_' being used anywhere. Typical names are 
'state' for the current state and 'new_state' for the newly created 
state.  Would be nice for consistency with other drivers.



+   struct vs_crtc_state *state;
+
+   if (!crtc->state)
+   return NULL;
+
+   ori_state = to_vs_crtc_state(crtc->state);
+   state = kzalloc(sizeof(*state), GFP_KERNEL);
+   if (!state)
+   return NULL;
+
+   __drm_atomic_helper_crtc_duplicate_state(crtc, &state->base);
+
+   state->output_fmt = ori_state->output_fmt;
+   state->encoder_type = ori_state->encoder_type;
+   state->bpp = ori_state->bpp;
+   state->underflow = ori_state->underflow;
+
+   return &state->base;
+}
+
+static void vs_crtc_atomic_destroy_state(struct drm_crtc *crtc,
+struct drm_crtc_state *state)
+{
+   __drm_atomic_helper_crtc_destroy_state(state);
+   kfree(to_vs_crtc_state(state));
+}
+
+static int vs_crtc_enable_vblank(struct drm_crtc *crtc)
+{
+   struct vs_crtc *vs_crtc = to_vs_crtc(crtc);
+   struct vs_dc *dc = dev_get_drvdata(vs_crtc->dev);
+
+   vs_dc_enable_vblank(dc, true);
+
+   return 0;
+}
+
+static void vs_crtc_disable_vblank(struct drm_crtc *crtc)
+{
+   struct vs_crtc *vs_crtc = to_vs_crtc(crtc);
+   struct vs_dc *dc = dev_get_drvdata(vs_crtc->dev);
+
+   vs_dc_enable_vblank(dc, false);
+}
+
+static const struct drm_crtc_funcs vs_crtc_funcs = {
+   .set_config = drm_atomic_helper_set_config,
+   .page_flip  = drm_atomic_helper_page_flip,
+   .reset  = vs_crtc_reset,
+   .atomic_duplicate_state = vs_crtc_atomic_duplicate_state,
+   .atomic_destroy_state   = vs_crtc_atomic_destroy_state,
+   .enable_vblank  = vs_crtc_enable_vblank,
+   .disable_vblank = vs_crtc_disable_vblank,
+};
+
+static u8 cal_pixel_bits(u32 bus_format)
+{
+   u8 bpp;
+
+   switch (bus_format) {
+   case MEDIA_BUS_FMT_RGB565_1X16:
+   case MEDIA_BUS_FMT_UYVY8_1X16:
+   bpp = 16;
+   break;
+   case MEDIA_BUS_FMT_RGB666_1X18:
+   case MEDIA_BUS_FMT_RGB666_1X24_CPADHI:
+   bpp = 18;
+   break;
+   case MEDIA_BUS_FMT_UYVY10_1X20:
+   bpp = 20;
+   break;
+   case MEDIA_BUS_FMT_BGR888_1X24:
+   case MEDIA_BUS_FMT_UYYVYY8_0_5X24:
+   case MEDIA_BUS_FMT_YUV8_1X24:
+   bpp = 24;
+   break;
+   case MEDIA_BUS_FMT_RGB101010_1X30:
+   case MEDIA_BUS_FMT_UYYVYY10_0_5X30:
+   case MEDIA_BUS_FMT_YUV10_1X30:
+   bpp = 30;
+   break;
+   default:
+   bpp = 24;
+   break;
+   }
+
+   return bpp;
+}
+
+static void vs_crtc_atomic_enable(struct drm_crtc *crtc,
+ struct drm_atomic_state *state)
+{
+   struct vs_crtc *vs_crtc = to_vs_crtc(crtc);
+   struct vs_dc *dc = dev_get_drvdata(vs_crtc->dev);
+   struct vs_crtc_state *vs_crtc_state = to_vs_crtc_state(crtc->state);
+
+   vs_crtc_state->bpp = cal_pixel_bits(vs_crtc_state->output_fmt);
+
+   vs_dc_enable(dc, crtc);
+   drm_crtc_vblank_on(crtc);
+}
+
+static void vs_crtc_atomic_disable(struct drm_crtc *crtc,
+  struct drm_atomic_state *state)
+{
+   struct vs_crtc *vs_crtc = to_vs_crtc(crtc);
+   struct vs_dc *dc = dev_get_drvdata(vs_crtc->dev);
+
+   drm_crtc_vblank_off(crtc);
+
+   vs_dc_disable(dc, crtc);
+
+   if (crtc->state->event && !crtc->state->active) {
+   spin_lock_irq(&crtc->dev->event_lock);
+   drm_crtc_send_vblank_event(crtc, crtc->state->event);
+   spin_unlock_irq(&crtc->dev->event_lock);
+
+   crtc->state->event = NULL;
+   }
+}
+
+static void vs_crtc_atomic_begin(struct drm_crtc *crtc,
+struct drm_atomic_state *state)
+{
+   struct drm_crtc_state *crtc_state = drm_atomic_get_new_crtc_state(state,
+ crtc);
+
+   struct vs_crtc *vs_crtc = to_vs_crtc(crtc);
+   struct device *dev = vs_crtc->dev;
+   struct drm_property_blob *blob = crtc->state->gamma_lut;
+   struct drm_color_lut *lut;
+   struct vs_dc *dc = dev_get_drvdata(dev);
+
+   if (crtc_state->color_mgmt_changed) {
+   if (blob && blob->length) {
+   lut = blob->data;
+   vs_dc_set_gamma(dc, crtc, lut,
+   blob->length / sizeof(*lut));
+   vs_dc_enable_gamma(dc, crtc, true);
+   } else {
+   vs_dc_enable_gamma(

Re: [PATCH] drm/bridge: adv7511: fix crash on irq during probe

Hi Laurent,

This is a friendly ping to get your feedback on my reply below. I don't think
the Fixes tag is incorrect here. Please could you take another look and let me
know if I can resend with your Reviewed-by?

Kind regards,
Alvin

On Mon, Oct 16, 2023 at 10:42:27AM +0200, Alvin Šipraga wrote:
> Hi Laurent,
> 
> Thanks for the quick review!
> 
> On Mon, Oct 16, 2023 at 11:14:44AM +0300, Laurent Pinchart wrote:
> > Hello Alvin,
> > 
> > On Sat, Oct 14, 2023 at 08:46:12PM +0200, Alvin Šipraga wrote:
> > > From: Mads Bligaard Nielsen 
> > > 
> > > Moved IRQ registration down to end of adv7511_probe().
> > > 
> > > If an IRQ already is pending during adv7511_probe
> > > (before adv7511_cec_init) then cec_received_msg_ts
> > > could crash using uninitialized data:
> > > 
> > > Unable to handle kernel read from unreadable memory at virtual 
> > > address 03d5
> > > Internal error: Oops: 9604 [#1] PREEMPT_RT SMP
> > > Call trace:
> > >  cec_received_msg_ts+0x48/0x990 [cec]
> > >  adv7511_cec_irq_process+0x1cc/0x308 [adv7511]
> > >  adv7511_irq_process+0xd8/0x120 [adv7511]
> > >  adv7511_irq_handler+0x1c/0x30 [adv7511]
> > >  irq_thread_fn+0x30/0xa0
> > >  irq_thread+0x14c/0x238
> > >  kthread+0x190/0x1a8
> > > 
> > > Fixes: 3b1b975003e4 ("drm: adv7511/33: add HDMI CEC support")
> > 
> > Isn't the issue older than that ?
> 
> I don't think so. The stacktrace shows the crash is in CEC handling code, 
> which
> was added in the blamed commit. A static analysis of adv7511_irq_process()
> suggests that the only other place where data could be uninitialized is if the
> hpd_work is scheduled:
> 
>   if (process_hpd && irq0 & ADV7511_INT0_HPD && adv7511->bridge.encoder)
>   schedule_work(&adv7511->hpd_work);
> 
> ... but this has a check on bridge.encoder, which seems to have been 
> introduced
> in a similar fix here:
> 
> | commit a1d0503d26ea2ef04f3f013d379e8f4d29c27127
> | Author: Laurent Pinchart 
> | Date:   Thu May 14 00:31:07 2015 +0300
> | 
> | drm: adv7511: Fix crash in IRQ handler when no encoder is associated
> | 
> | The ADV7511 is probed before its slave encoder init function associates
> | it with an encoder. This creates a time window during which hot plug
> | detection interrupts can occur with an encoder, resulting in a crash in
> | the IRQ handler.
> | 
> | Fix this by ignoring hot plug detection IRQs when no encoder is
> | associated yet.
> | 
> | Signed-off-by: Laurent Pinchart 
> 
> | Acked-by: Lars-Peter Clausen 
> | 
> | diff --git a/drivers/gpu/drm/i2c/adv7511.c b/drivers/gpu/drm/i2c/adv7511.c
> | index b728523e194f..2aaa3c88999e 100644
> | --- a/drivers/gpu/drm/i2c/adv7511.c
> | +++ b/drivers/gpu/drm/i2c/adv7511.c
> | @@ -438,7 +438,7 @@ static int adv7511_irq_process(struct adv7511 *adv7511)
> | regmap_write(adv7511->regmap, ADV7511_REG_INT(0), irq0);
> | regmap_write(adv7511->regmap, ADV7511_REG_INT(1), irq1);
> |  
> | -   if (irq0 & ADV7511_INT0_HDP)
> | +   if (irq0 & ADV7511_INT0_HDP && adv7511->encoder)
> | drm_helper_hpd_irq_event(adv7511->encoder->dev);
> |  
> | if (irq0 & ADV7511_INT0_EDID_READY || irq1 & 
> ADV7511_INT1_DDC_ERROR) {
> 
> So assuming that is the case, I am not sure which Fixes: tag I ought to
> otherwise use. What do you think?
> 
> > 
> > > Signed-off-by: Mads Bligaard Nielsen 
> > > Signed-off-by: Alvin Šipraga 
> > 
> > With the Fixes: tag updated,
> > 
> > Reviewed-by: Laurent Pinchart 
> 
> Kind regards,
> Alvin

Re: [PATCH v5 1/4] pwm: rename pwm_apply_state() to pwm_apply_cansleep()

2023-11-24 Thread Thierry Reding

On Sat, Nov 18, 2023 at 04:16:17PM +, Sean Young wrote:
> In order to introduce a pwm api which can be used from atomic context,
> we will need two functions for applying pwm changes:
> 
>   int pwm_apply_cansleep(struct pwm *, struct pwm_state *);
>   int pwm_apply_atomic(struct pwm *, struct pwm_state *);
> 
> This commit just deals with renaming pwm_apply_state(), a following
> commit will introduce the pwm_apply_atomic() function.

Sorry, I still don't agree with that _cansleep suffix. I think it's the
wrong terminology. Just because something can sleep doesn't mean that it
ever will. "Might sleep" is much more accurate because it says exactly
what might happen and indicates what we're guarding against.

Thierry

signature.asc
Description: PGP signature

Patch "drm/mediatek/dp: fix memory leak on ->get_edid callback error path" has been added to the 6.6-stable tree



This is a note to let you know that I've just added the patch titled

drm/mediatek/dp: fix memory leak on ->get_edid callback error path

to the 6.6-stable tree which can be found at:

http://www.kernel.org/git/?p=linux/kernel/git/stable/stable-queue.git;a=summary

The filename of the patch is:
 drm-mediatek-dp-fix-memory-leak-on-get_edid-callback-error-path.patch
and it can be found in the queue-6.6 subdirectory.

If you, or anyone else, feels it should not be added to the stable tree,
please let  know about it.


>From fcaf9761fd5884a64eaac48536f8c27ecfd2e6bc Mon Sep 17 00:00:00 2001
From: Jani Nikula 
Date: Thu, 14 Sep 2023 16:10:58 +0300
Subject: drm/mediatek/dp: fix memory leak on ->get_edid callback error path

From: Jani Nikula 

commit fcaf9761fd5884a64eaac48536f8c27ecfd2e6bc upstream.

Setting new_edid to NULL leaks the buffer.

Fixes: f70ac097a2cf ("drm/mediatek: Add MT8195 Embedded DisplayPort driver")
Cc: Markus Schneider-Pargmann 
Cc: Guillaume Ranquet 
Cc: Bo-Chen Chen 
Cc: CK Hu 
Cc: AngeloGioacchino Del Regno 
Cc: Dmitry Osipenko 
Cc: Chun-Kuang Hu 
Cc: Philipp Zabel 
Cc: Matthias Brugger 
Cc: dri-devel@lists.freedesktop.org
Cc: linux-media...@lists.infradead.org
Cc: linux-ker...@vger.kernel.org
Cc: linux-arm-ker...@lists.infradead.org
Cc:  # v6.1+
Signed-off-by: Jani Nikula 
Reviewed-by: Guillaume Ranquet 
Link: 
https://patchwork.kernel.org/project/dri-devel/patch/20230914131058.2472260-1-jani.nik...@intel.com/
Signed-off-by: Chun-Kuang Hu 
Signed-off-by: Greg Kroah-Hartman 
---
 drivers/gpu/drm/mediatek/mtk_dp.c |1 +
 1 file changed, 1 insertion(+)

--- a/drivers/gpu/drm/mediatek/mtk_dp.c
+++ b/drivers/gpu/drm/mediatek/mtk_dp.c
@@ -2048,6 +2048,7 @@ static struct edid *mtk_dp_get_edid(stru
 */
if (mtk_dp_parse_capabilities(mtk_dp)) {
drm_err(mtk_dp->drm_dev, "Can't parse capabilities\n");
+   kfree(new_edid);
new_edid = NULL;
}
 


Patches currently in stable-queue which might be from jani.nik...@intel.com are

queue-6.6/drm-msm-dp-skip-validity-check-for-dp-cts-edid-check.patch
queue-6.6/i915-perf-fix-null-deref-bugs-with-drm_dbg-calls.patch
queue-6.6/drm-mediatek-dp-fix-memory-leak-on-get_edid-callback-error-path.patch
queue-6.6/drm-edid-fixup-h-vsync_end-instead-of-h-vtotal.patch
queue-6.6/drm-mediatek-dp-fix-memory-leak-on-get_edid-callback-audio-detection.patch
queue-6.6/drm-i915-mtl-avoid-stringop-overflow-warning.patch
queue-6.6/drm-i915-tc-fix-wformat-truncation-in-intel_tc_port_.patch

Patch "drm/mediatek/dp: fix memory leak on ->get_edid callback audio detection" has been added to the 6.6-stable tree



This is a note to let you know that I've just added the patch titled

drm/mediatek/dp: fix memory leak on ->get_edid callback audio detection

to the 6.6-stable tree which can be found at:

http://www.kernel.org/git/?p=linux/kernel/git/stable/stable-queue.git;a=summary

The filename of the patch is:
 drm-mediatek-dp-fix-memory-leak-on-get_edid-callback-audio-detection.patch
and it can be found in the queue-6.6 subdirectory.

If you, or anyone else, feels it should not be added to the stable tree,
please let  know about it.


>From dab12fa8d2bd3868cf2de485ed15a3feef28a13d Mon Sep 17 00:00:00 2001
From: Jani Nikula 
Date: Thu, 14 Sep 2023 18:53:17 +0300
Subject: drm/mediatek/dp: fix memory leak on ->get_edid callback audio detection

From: Jani Nikula 

commit dab12fa8d2bd3868cf2de485ed15a3feef28a13d upstream.

The sads returned by drm_edid_to_sad() needs to be freed.

Fixes: e71a8ebbe086 ("drm/mediatek: dp: Audio support for MT8195")
Cc: Guillaume Ranquet 
Cc: Bo-Chen Chen 
Cc: AngeloGioacchino Del Regno 
Cc: Dmitry Osipenko 
Cc: Chun-Kuang Hu 
Cc: Philipp Zabel 
Cc: Matthias Brugger 
Cc: dri-devel@lists.freedesktop.org
Cc: linux-media...@lists.infradead.org
Cc: linux-ker...@vger.kernel.org
Cc: linux-arm-ker...@lists.infradead.org
Cc:  # v6.1+
Signed-off-by: Jani Nikula 
Reviewed-by: Chen-Yu Tsai 
Link: 
https://patchwork.kernel.org/project/dri-devel/patch/20230914155317.2511876-1-jani.nik...@intel.com/
Signed-off-by: Chun-Kuang Hu 
Signed-off-by: Greg Kroah-Hartman 
---
 drivers/gpu/drm/mediatek/mtk_dp.c |5 -
 1 file changed, 4 insertions(+), 1 deletion(-)

--- a/drivers/gpu/drm/mediatek/mtk_dp.c
+++ b/drivers/gpu/drm/mediatek/mtk_dp.c
@@ -2034,7 +2034,6 @@ static struct edid *mtk_dp_get_edid(stru
bool enabled = mtk_dp->enabled;
struct edid *new_edid = NULL;
struct mtk_dp_audio_cfg *audio_caps = &mtk_dp->info.audio_cur_cfg;
-   struct cea_sad *sads;
 
if (!enabled) {
drm_atomic_bridge_chain_pre_enable(bridge, 
connector->state->state);
@@ -2053,7 +2052,11 @@ static struct edid *mtk_dp_get_edid(stru
}
 
if (new_edid) {
+   struct cea_sad *sads;
+
audio_caps->sad_count = drm_edid_to_sad(new_edid, &sads);
+   kfree(sads);
+
audio_caps->detect_monitor = drm_detect_monitor_audio(new_edid);
}
 


Patches currently in stable-queue which might be from jani.nik...@intel.com are

queue-6.6/drm-msm-dp-skip-validity-check-for-dp-cts-edid-check.patch
queue-6.6/i915-perf-fix-null-deref-bugs-with-drm_dbg-calls.patch
queue-6.6/drm-mediatek-dp-fix-memory-leak-on-get_edid-callback-error-path.patch
queue-6.6/drm-edid-fixup-h-vsync_end-instead-of-h-vtotal.patch
queue-6.6/drm-mediatek-dp-fix-memory-leak-on-get_edid-callback-audio-detection.patch
queue-6.6/drm-i915-mtl-avoid-stringop-overflow-warning.patch
queue-6.6/drm-i915-tc-fix-wformat-truncation-in-intel_tc_port_.patch

Re: linux-next: Signed-off-by missing for commit in the drm-misc tree

On Wed, 22 Nov 2023, Luben Tuikov  wrote:
> On 2023-11-22 07:00, Maxime Ripard wrote:
>> Hi Luben,
>> 
>> On Thu, Nov 16, 2023 at 09:27:58AM +0100, Daniel Vetter wrote:
>>> On Thu, Nov 16, 2023 at 09:11:43AM +0100, Maxime Ripard wrote:
 On Tue, Nov 14, 2023 at 06:46:21PM -0500, Luben Tuikov wrote:
> On 2023-11-13 22:08, Stephen Rothwell wrote:
>> BTW, cherry picking commits does not avoid conflicts - in fact it can
>> cause conflicts if there are further changes to the files affected by
>> the cherry picked commit in either the tree/branch the commit was
>> cheery picked from or the destination tree/branch (I have to deal with
>> these all the time when merging the drm trees in linux-next).  Much
>> better is to cross merge the branches so that the patch only appears
>> once or have a shared branches that are merged by any other branch that
>> needs the changes.
>>
>> I understand that things are not done like this in the drm trees :-(
>
> Hi Stephen,
>
> Thank you for the clarification--understood. I'll be more careful in the 
> future.
> Thanks again! :-)

 In this case, the best thing to do would indeed have been to ask the
 drm-misc maintainers to merge drm-misc-fixes into drm-misc-next.

 We're doing that all the time, but we're not ubiquitous so you need to
 ask us :)

 Also, dim should have caught that when you pushed the branch. Did you
 use it?
>>>
>>> Yeah dim must be used, exactly to avoid these issues. Both for applying
>>> patches (so not git am directly, or cherry-picking from your own
>>> development branch), and for pushing. The latter is even checked for by
>>> the server (dim sets a special push flag which is very long and contains a
>>> very clear warning if you bypass it).
>>>
>>> If dim was used, this would be a bug in the dim script that we need to
>>> fix.
>> 
>> It would be very useful for you to explain what happened here so we
>> improve the tooling or doc and can try to make sure it doesn't happen
>> again
>> 
>> Maxime
>
> There is no problem with the tooling--I just forced the commit in.

Wait what?

What do you mean by forcing the commit in? Bypass dim?

If yes, please *never* do that when you're dealing with dim managed
branches. That's part of the deal for getting commit access, along with
following all the other maintainer tools documentation.


BR,
Jani.


-- 
Jani Nikula, Intel

Re: [PATCH] drm/amdgpu: Fix cat debugfs amdgpu_regs_didt causes kernel null pointer

2023-11-24 Thread kernel test robot

Hi Lu,

kernel test robot noticed the following build errors:

[auto build test ERROR on drm-misc/drm-misc-next]
[also build test ERROR on linus/master v6.7-rc2 next-20231124]
[If your patch is applied to the wrong git tree, kindly drop us a note.
And when submitting patch, we suggest to use '--base' as documented in
https://git-scm.com/docs/git-format-patch#_base_tree_information]

url:
https://github.com/intel-lab-lkp/linux/commits/Lu-Yao/drm-amdgpu-Fix-cat-debugfs-amdgpu_regs_didt-causes-kernel-null-pointer/20231122-203138
base:   git://anongit.freedesktop.org/drm/drm-misc drm-misc-next
patch link:
https://lore.kernel.org/r/20231122093509.34302-1-yaolu%40kylinos.cn
patch subject: [PATCH] drm/amdgpu: Fix cat debugfs amdgpu_regs_didt causes 
kernel null pointer
config: x86_64-randconfig-001-20231123 
(https://download.01.org/0day-ci/archive/20231124/202311241442.f0s4bazk-...@intel.com/config)
compiler: clang version 16.0.4 (https://github.com/llvm/llvm-project.git 
ae42196bc493ffe877a7e3dff8be32035dea4d07)
reproduce (this is a W=1 build): 
(https://download.01.org/0day-ci/archive/20231124/202311241442.f0s4bazk-...@intel.com/reproduce)

If you fix the issue in a separate patch/commit (i.e. not just a new version of
the same patch/commit), kindly add following tags
| Reported-by: kernel test robot 
| Closes: 
https://lore.kernel.org/oe-kbuild-all/202311241442.f0s4bazk-...@intel.com/

All errors (new ones prefixed by >>):

   warning: unknown warning option '-Wstringop-truncation'; did you mean 
'-Wstring-concatenation'? [-Wunknown-warning-option]
   warning: unknown warning option '-Wpacked-not-aligned'; did you mean 
'-Wpacked-non-pod'? [-Wunknown-warning-option]
>> drivers/gpu/drm/amd/amdgpu/amdgpu_debugfs.c:642:55: error: use of undeclared 
>> identifier '__FUNC__'
   dev_err(adev->dev, "%s adev->didt_rreg is null!\n", 
__FUNC__);
   ^
   drivers/gpu/drm/amd/amdgpu/amdgpu_debugfs.c:703:55: error: use of undeclared 
identifier '__FUNC__'
   dev_err(adev->dev, "%s adev->didt_wreg is null!\n", 
__FUNC__);
   ^
   2 warnings and 2 errors generated.


vim +/__FUNC__ +642 drivers/gpu/drm/amd/amdgpu/amdgpu_debugfs.c

   618  
   619  /**
   620   * amdgpu_debugfs_regs_didt_read - Read from a DIDT register
   621   *
   622   * @f: open file handle
   623   * @buf: User buffer to store read data in
   624   * @size: Number of bytes to read
   625   * @pos:  Offset to seek to
   626   *
   627   * The lower bits are the BYTE offset of the register to read.  This
   628   * allows reading multiple registers in a single call and having
   629   * the returned size reflect that.
   630   */
   631  static ssize_t amdgpu_debugfs_regs_didt_read(struct file *f, char 
__user *buf,
   632  size_t size, loff_t *pos)
   633  {
   634  struct amdgpu_device *adev = file_inode(f)->i_private;
   635  ssize_t result = 0;
   636  int r;
   637  
   638  if (size & 0x3 || *pos & 0x3)
   639  return -EINVAL;
   640  
   641  if (adev->didt_rreg == NULL) {
 > 642  dev_err(adev->dev, "%s adev->didt_rreg is null!\n", 
 > __FUNC__);
   643  return -EPERM;
   644  }
   645  
   646  r = pm_runtime_get_sync(adev_to_drm(adev)->dev);
   647  if (r < 0) {
   648  pm_runtime_put_autosuspend(adev_to_drm(adev)->dev);
   649  return r;
   650  }
   651  
   652  r = amdgpu_virt_enable_access_debugfs(adev);
   653  if (r < 0) {
   654  pm_runtime_put_autosuspend(adev_to_drm(adev)->dev);
   655  return r;
   656  }
   657  
   658  while (size) {
   659  uint32_t value;
   660  
   661  value = RREG32_DIDT(*pos >> 2);
   662  r = put_user(value, (uint32_t *)buf);
   663  if (r)
   664  goto out;
   665  
   666  result += 4;
   667  buf += 4;
   668  *pos += 4;
   669  size -= 4;
   670  }
   671  
   672  r = result;
   673  out:
   674  pm_runtime_mark_last_busy(adev_to_drm(adev)->dev);
   675  pm_runtime_put_autosuspend(adev_to_drm(adev)->dev);
   676  amdgpu_virt_disable_access_debugfs(adev);
   677  return r;
   678  }
   679  

-- 
0-DAY CI Kernel Test Service
https://github.com/intel/lkp-tests/wiki

Patch "drm/mediatek/dp: fix memory leak on ->get_edid callback audio detection" has been added to the 6.5-stable tree



This is a note to let you know that I've just added the patch titled

drm/mediatek/dp: fix memory leak on ->get_edid callback audio detection

to the 6.5-stable tree which can be found at:

http://www.kernel.org/git/?p=linux/kernel/git/stable/stable-queue.git;a=summary

The filename of the patch is:
 drm-mediatek-dp-fix-memory-leak-on-get_edid-callback-audio-detection.patch
and it can be found in the queue-6.5 subdirectory.

If you, or anyone else, feels it should not be added to the stable tree,
please let  know about it.


>From dab12fa8d2bd3868cf2de485ed15a3feef28a13d Mon Sep 17 00:00:00 2001
From: Jani Nikula 
Date: Thu, 14 Sep 2023 18:53:17 +0300
Subject: drm/mediatek/dp: fix memory leak on ->get_edid callback audio detection

From: Jani Nikula 

commit dab12fa8d2bd3868cf2de485ed15a3feef28a13d upstream.

The sads returned by drm_edid_to_sad() needs to be freed.

Fixes: e71a8ebbe086 ("drm/mediatek: dp: Audio support for MT8195")
Cc: Guillaume Ranquet 
Cc: Bo-Chen Chen 
Cc: AngeloGioacchino Del Regno 
Cc: Dmitry Osipenko 
Cc: Chun-Kuang Hu 
Cc: Philipp Zabel 
Cc: Matthias Brugger 
Cc: dri-devel@lists.freedesktop.org
Cc: linux-media...@lists.infradead.org
Cc: linux-ker...@vger.kernel.org
Cc: linux-arm-ker...@lists.infradead.org
Cc:  # v6.1+
Signed-off-by: Jani Nikula 
Reviewed-by: Chen-Yu Tsai 
Link: 
https://patchwork.kernel.org/project/dri-devel/patch/20230914155317.2511876-1-jani.nik...@intel.com/
Signed-off-by: Chun-Kuang Hu 
Signed-off-by: Greg Kroah-Hartman 
---
 drivers/gpu/drm/mediatek/mtk_dp.c |5 -
 1 file changed, 4 insertions(+), 1 deletion(-)

--- a/drivers/gpu/drm/mediatek/mtk_dp.c
+++ b/drivers/gpu/drm/mediatek/mtk_dp.c
@@ -1983,7 +1983,6 @@ static struct edid *mtk_dp_get_edid(stru
bool enabled = mtk_dp->enabled;
struct edid *new_edid = NULL;
struct mtk_dp_audio_cfg *audio_caps = &mtk_dp->info.audio_cur_cfg;
-   struct cea_sad *sads;
 
if (!enabled) {
drm_atomic_bridge_chain_pre_enable(bridge, 
connector->state->state);
@@ -2010,7 +2009,11 @@ static struct edid *mtk_dp_get_edid(stru
}
 
if (new_edid) {
+   struct cea_sad *sads;
+
audio_caps->sad_count = drm_edid_to_sad(new_edid, &sads);
+   kfree(sads);
+
audio_caps->detect_monitor = drm_detect_monitor_audio(new_edid);
}
 


Patches currently in stable-queue which might be from jani.nik...@intel.com are

queue-6.5/drm-msm-dp-skip-validity-check-for-dp-cts-edid-check.patch
queue-6.5/i915-perf-fix-null-deref-bugs-with-drm_dbg-calls.patch
queue-6.5/drm-mediatek-dp-fix-memory-leak-on-get_edid-callback-error-path.patch
queue-6.5/drm-edid-fixup-h-vsync_end-instead-of-h-vtotal.patch
queue-6.5/drm-mediatek-dp-fix-memory-leak-on-get_edid-callback-audio-detection.patch
queue-6.5/drm-i915-mtl-avoid-stringop-overflow-warning.patch
queue-6.5/drm-i915-tc-fix-wformat-truncation-in-intel_tc_port_.patch

Patch "drm/mediatek/dp: fix memory leak on ->get_edid callback error path" has been added to the 6.5-stable tree



This is a note to let you know that I've just added the patch titled

drm/mediatek/dp: fix memory leak on ->get_edid callback error path

to the 6.5-stable tree which can be found at:

http://www.kernel.org/git/?p=linux/kernel/git/stable/stable-queue.git;a=summary

The filename of the patch is:
 drm-mediatek-dp-fix-memory-leak-on-get_edid-callback-error-path.patch
and it can be found in the queue-6.5 subdirectory.

If you, or anyone else, feels it should not be added to the stable tree,
please let  know about it.


>From fcaf9761fd5884a64eaac48536f8c27ecfd2e6bc Mon Sep 17 00:00:00 2001
From: Jani Nikula 
Date: Thu, 14 Sep 2023 16:10:58 +0300
Subject: drm/mediatek/dp: fix memory leak on ->get_edid callback error path

From: Jani Nikula 

commit fcaf9761fd5884a64eaac48536f8c27ecfd2e6bc upstream.

Setting new_edid to NULL leaks the buffer.

Fixes: f70ac097a2cf ("drm/mediatek: Add MT8195 Embedded DisplayPort driver")
Cc: Markus Schneider-Pargmann 
Cc: Guillaume Ranquet 
Cc: Bo-Chen Chen 
Cc: CK Hu 
Cc: AngeloGioacchino Del Regno 
Cc: Dmitry Osipenko 
Cc: Chun-Kuang Hu 
Cc: Philipp Zabel 
Cc: Matthias Brugger 
Cc: dri-devel@lists.freedesktop.org
Cc: linux-media...@lists.infradead.org
Cc: linux-ker...@vger.kernel.org
Cc: linux-arm-ker...@lists.infradead.org
Cc:  # v6.1+
Signed-off-by: Jani Nikula 
Reviewed-by: Guillaume Ranquet 
Link: 
https://patchwork.kernel.org/project/dri-devel/patch/20230914131058.2472260-1-jani.nik...@intel.com/
Signed-off-by: Chun-Kuang Hu 
Signed-off-by: Greg Kroah-Hartman 
---
 drivers/gpu/drm/mediatek/mtk_dp.c |1 +
 1 file changed, 1 insertion(+)

--- a/drivers/gpu/drm/mediatek/mtk_dp.c
+++ b/drivers/gpu/drm/mediatek/mtk_dp.c
@@ -2005,6 +2005,7 @@ static struct edid *mtk_dp_get_edid(stru
 */
if (mtk_dp_parse_capabilities(mtk_dp)) {
drm_err(mtk_dp->drm_dev, "Can't parse capabilities\n");
+   kfree(new_edid);
new_edid = NULL;
}
 


Patches currently in stable-queue which might be from jani.nik...@intel.com are

queue-6.5/drm-msm-dp-skip-validity-check-for-dp-cts-edid-check.patch
queue-6.5/i915-perf-fix-null-deref-bugs-with-drm_dbg-calls.patch
queue-6.5/drm-mediatek-dp-fix-memory-leak-on-get_edid-callback-error-path.patch
queue-6.5/drm-edid-fixup-h-vsync_end-instead-of-h-vtotal.patch
queue-6.5/drm-mediatek-dp-fix-memory-leak-on-get_edid-callback-audio-detection.patch
queue-6.5/drm-i915-mtl-avoid-stringop-overflow-warning.patch
queue-6.5/drm-i915-tc-fix-wformat-truncation-in-intel_tc_port_.patch

Patch "drm/mediatek/dp: fix memory leak on ->get_edid callback error path" has been added to the 6.1-stable tree



This is a note to let you know that I've just added the patch titled

drm/mediatek/dp: fix memory leak on ->get_edid callback error path

to the 6.1-stable tree which can be found at:

http://www.kernel.org/git/?p=linux/kernel/git/stable/stable-queue.git;a=summary

The filename of the patch is:
 drm-mediatek-dp-fix-memory-leak-on-get_edid-callback-error-path.patch
and it can be found in the queue-6.1 subdirectory.

If you, or anyone else, feels it should not be added to the stable tree,
please let  know about it.


>From fcaf9761fd5884a64eaac48536f8c27ecfd2e6bc Mon Sep 17 00:00:00 2001
From: Jani Nikula 
Date: Thu, 14 Sep 2023 16:10:58 +0300
Subject: drm/mediatek/dp: fix memory leak on ->get_edid callback error path

From: Jani Nikula 

commit fcaf9761fd5884a64eaac48536f8c27ecfd2e6bc upstream.

Setting new_edid to NULL leaks the buffer.

Fixes: f70ac097a2cf ("drm/mediatek: Add MT8195 Embedded DisplayPort driver")
Cc: Markus Schneider-Pargmann 
Cc: Guillaume Ranquet 
Cc: Bo-Chen Chen 
Cc: CK Hu 
Cc: AngeloGioacchino Del Regno 
Cc: Dmitry Osipenko 
Cc: Chun-Kuang Hu 
Cc: Philipp Zabel 
Cc: Matthias Brugger 
Cc: dri-devel@lists.freedesktop.org
Cc: linux-media...@lists.infradead.org
Cc: linux-ker...@vger.kernel.org
Cc: linux-arm-ker...@lists.infradead.org
Cc:  # v6.1+
Signed-off-by: Jani Nikula 
Reviewed-by: Guillaume Ranquet 
Link: 
https://patchwork.kernel.org/project/dri-devel/patch/20230914131058.2472260-1-jani.nik...@intel.com/
Signed-off-by: Chun-Kuang Hu 
Signed-off-by: Greg Kroah-Hartman 
---
 drivers/gpu/drm/mediatek/mtk_dp.c |1 +
 1 file changed, 1 insertion(+)

--- a/drivers/gpu/drm/mediatek/mtk_dp.c
+++ b/drivers/gpu/drm/mediatek/mtk_dp.c
@@ -2005,6 +2005,7 @@ static struct edid *mtk_dp_get_edid(stru
 */
if (mtk_dp_parse_capabilities(mtk_dp)) {
drm_err(mtk_dp->drm_dev, "Can't parse capabilities\n");
+   kfree(new_edid);
new_edid = NULL;
}
 


Patches currently in stable-queue which might be from jani.nik...@intel.com are

queue-6.1/drm-msm-dp-skip-validity-check-for-dp-cts-edid-check.patch
queue-6.1/i915-perf-fix-null-deref-bugs-with-drm_dbg-calls.patch
queue-6.1/drm-mediatek-dp-fix-memory-leak-on-get_edid-callback-error-path.patch
queue-6.1/drm-mediatek-dp-fix-memory-leak-on-get_edid-callback-audio-detection.patch

Patch "drm/mediatek/dp: fix memory leak on ->get_edid callback audio detection" has been added to the 6.1-stable tree



This is a note to let you know that I've just added the patch titled

drm/mediatek/dp: fix memory leak on ->get_edid callback audio detection

to the 6.1-stable tree which can be found at:

http://www.kernel.org/git/?p=linux/kernel/git/stable/stable-queue.git;a=summary

The filename of the patch is:
 drm-mediatek-dp-fix-memory-leak-on-get_edid-callback-audio-detection.patch
and it can be found in the queue-6.1 subdirectory.

If you, or anyone else, feels it should not be added to the stable tree,
please let  know about it.


>From dab12fa8d2bd3868cf2de485ed15a3feef28a13d Mon Sep 17 00:00:00 2001
From: Jani Nikula 
Date: Thu, 14 Sep 2023 18:53:17 +0300
Subject: drm/mediatek/dp: fix memory leak on ->get_edid callback audio detection

From: Jani Nikula 

commit dab12fa8d2bd3868cf2de485ed15a3feef28a13d upstream.

The sads returned by drm_edid_to_sad() needs to be freed.

Fixes: e71a8ebbe086 ("drm/mediatek: dp: Audio support for MT8195")
Cc: Guillaume Ranquet 
Cc: Bo-Chen Chen 
Cc: AngeloGioacchino Del Regno 
Cc: Dmitry Osipenko 
Cc: Chun-Kuang Hu 
Cc: Philipp Zabel 
Cc: Matthias Brugger 
Cc: dri-devel@lists.freedesktop.org
Cc: linux-media...@lists.infradead.org
Cc: linux-ker...@vger.kernel.org
Cc: linux-arm-ker...@lists.infradead.org
Cc:  # v6.1+
Signed-off-by: Jani Nikula 
Reviewed-by: Chen-Yu Tsai 
Link: 
https://patchwork.kernel.org/project/dri-devel/patch/20230914155317.2511876-1-jani.nik...@intel.com/
Signed-off-by: Chun-Kuang Hu 
Signed-off-by: Greg Kroah-Hartman 
---
 drivers/gpu/drm/mediatek/mtk_dp.c |5 -
 1 file changed, 4 insertions(+), 1 deletion(-)

--- a/drivers/gpu/drm/mediatek/mtk_dp.c
+++ b/drivers/gpu/drm/mediatek/mtk_dp.c
@@ -1983,7 +1983,6 @@ static struct edid *mtk_dp_get_edid(stru
bool enabled = mtk_dp->enabled;
struct edid *new_edid = NULL;
struct mtk_dp_audio_cfg *audio_caps = &mtk_dp->info.audio_cur_cfg;
-   struct cea_sad *sads;
 
if (!enabled) {
drm_bridge_chain_pre_enable(bridge);
@@ -2010,7 +2009,11 @@ static struct edid *mtk_dp_get_edid(stru
}
 
if (new_edid) {
+   struct cea_sad *sads;
+
audio_caps->sad_count = drm_edid_to_sad(new_edid, &sads);
+   kfree(sads);
+
audio_caps->detect_monitor = drm_detect_monitor_audio(new_edid);
}
 


Patches currently in stable-queue which might be from jani.nik...@intel.com are

queue-6.1/drm-msm-dp-skip-validity-check-for-dp-cts-edid-check.patch
queue-6.1/i915-perf-fix-null-deref-bugs-with-drm_dbg-calls.patch
queue-6.1/drm-mediatek-dp-fix-memory-leak-on-get_edid-callback-error-path.patch
queue-6.1/drm-mediatek-dp-fix-memory-leak-on-get_edid-callback-audio-detection.patch

Re: [PATCH] drm/panfrost: Really power off GPU cores in panfrost_gpu_power_off()

2023-11-24 Thread Marek Szyprowski

On 22.11.2023 10:29, Krzysztof Kozlowski wrote:
> On 22/11/2023 10:06, AngeloGioacchino Del Regno wrote:
> Hey Krzysztof,
>
> This is interesting. It might be about the cores that are missing from 
> the partial
> core_mask raising interrupts, but an external abort on non-linefetch is 
> strange to
> see here.
 I've seen such external aborts in the past, and the fault type has
 often been misleading. It's unlikely to have anything to do with a
>>> Yeah, often accessing device with power or clocks gated.
>>>
>> Except my commit does *not* gate SoC power, nor SoC clocks 🙂
> It could be that something (like clocks or power supplies) was missing
> on this board/SoC, which was not critical till your patch came.
>
>> What the "Really power off ..." commit does is to ask the GPU to internally 
>> power
>> off the shaders, tilers and L2, that's why I say that it is strange to see 
>> that
>> kind of abort.
>>
>> The GPU_INT_CLEAR GPU_INT_STAT, GPU_FAULT_STATUS and 
>> GPU_FAULT_ADDRESS_{HI/LO}
>> registers should still be accessible even with shaders, tilers and cache OFF.
>>
>> Anyway, yes, synchronizing IRQs before calling the poweroff sequence would 
>> also
>> work, but that'd add up quite a bit of latency on the runtime_suspend() 
>> call, so
>> in this case I'd be more for avoiding to execute any register r/w in the 
>> handler
>> by either checking if the GPU is supposed to be OFF, or clearing interrupts, 
>> which
>> may not work if those are generated after the execution of the poweroff 
>> function.
>> Or we could simply disable the irq after power_off, but that'd be hacky (as 
>> well).
>>
>>
>> Let's see if asking to poweroff *everything* works:
> Worked.

Yes, I also got into this issue some time ago, but I didn't report it 
because I also had some power supply related problems on my test farm 
and everything was a bit unstable. I wasn't 100% sure that the $subject 
patch is responsible for the observed issues. Now, after fixing power 
supply, I confirm that the issue was revealed by the $subject patch and 
above mentioned change fixes the problem. Feel free to add:

Tested-by: Marek Szyprowski 

Best regards
-- 
Marek Szyprowski, PhD
Samsung R&D Institute Poland

[PATCH 8/8] drm/i915: use drm_printf() with the drm_err_printer intead of pr_err()

There's already a related drm_printer. Use it to preserve the context
instead of a separate pr_err().

Signed-off-by: Jani Nikula 
---
 drivers/gpu/drm/i915/gt/selftest_engine_heartbeat.c | 6 +++---
 drivers/gpu/drm/i915/selftests/i915_active.c| 4 ++--
 2 files changed, 5 insertions(+), 5 deletions(-)

diff --git a/drivers/gpu/drm/i915/gt/selftest_engine_heartbeat.c 
b/drivers/gpu/drm/i915/gt/selftest_engine_heartbeat.c
index b4970c1ed572..88fd2ab65f3b 100644
--- a/drivers/gpu/drm/i915/gt/selftest_engine_heartbeat.c
+++ b/drivers/gpu/drm/i915/gt/selftest_engine_heartbeat.c
@@ -124,7 +124,7 @@ static int __live_idle_pulse(struct intel_engine_cs *engine,
if (engine_sync_barrier(engine)) {
struct drm_printer m = drm_err_printer(&engine->i915->drm, 
"pulse");
 
-   pr_err("%s: no heartbeat pulse?\n", engine->name);
+   drm_printf(&m, "%s: no heartbeat pulse?\n", engine->name);
intel_engine_dump(engine, &m, "%s", engine->name);
 
err = -ETIME;
@@ -138,8 +138,8 @@ static int __live_idle_pulse(struct intel_engine_cs *engine,
if (!i915_active_is_idle(&p->active)) {
struct drm_printer m = drm_err_printer(&engine->i915->drm, 
"pulse");
 
-   pr_err("%s: heartbeat pulse did not flush idle tasks\n",
-  engine->name);
+   drm_printf(&m, "%s: heartbeat pulse did not flush idle tasks\n",
+  engine->name);
i915_active_print(&p->active, &m);
 
err = -EINVAL;
diff --git a/drivers/gpu/drm/i915/selftests/i915_active.c 
b/drivers/gpu/drm/i915/selftests/i915_active.c
index 8886752ade63..0d89d70b9c36 100644
--- a/drivers/gpu/drm/i915/selftests/i915_active.c
+++ b/drivers/gpu/drm/i915/selftests/i915_active.c
@@ -158,7 +158,7 @@ static int live_active_wait(void *arg)
if (!READ_ONCE(active->retired)) {
struct drm_printer p = drm_err_printer(&i915->drm, __func__);
 
-   pr_err("i915_active not retired after waiting!\n");
+   drm_printf(&p, "i915_active not retired after waiting!\n");
i915_active_print(&active->base, &p);
 
err = -EINVAL;
@@ -191,7 +191,7 @@ static int live_active_retire(void *arg)
if (!READ_ONCE(active->retired)) {
struct drm_printer p = drm_err_printer(&i915->drm, __func__);
 
-   pr_err("i915_active not retired after flushing!\n");
+   drm_printf(&p, "i915_active not retired after flushing!\n");
i915_active_print(&active->base, &p);
 
err = -EINVAL;
-- 
2.39.2

[PATCH 7/8] drm/i915: switch from drm_debug_printer() to device specific drm_dbg_printer()

Prefer the device specific debug printer.

Signed-off-by: Jani Nikula 
---
 drivers/gpu/drm/i915/display/intel_display_driver.c | 3 ++-
 drivers/gpu/drm/i915/gt/intel_engine_heartbeat.c| 3 ++-
 drivers/gpu/drm/i915/gt/intel_reset.c   | 3 ++-
 drivers/gpu/drm/i915/gt/intel_workarounds.c | 3 ++-
 drivers/gpu/drm/i915/gt/selftest_context.c  | 3 ++-
 drivers/gpu/drm/i915/i915_driver.c  | 3 ++-
 6 files changed, 12 insertions(+), 6 deletions(-)

diff --git a/drivers/gpu/drm/i915/display/intel_display_driver.c 
b/drivers/gpu/drm/i915/display/intel_display_driver.c
index 62f7b10484be..b6e7d66f895d 100644
--- a/drivers/gpu/drm/i915/display/intel_display_driver.c
+++ b/drivers/gpu/drm/i915/display/intel_display_driver.c
@@ -387,7 +387,8 @@ int intel_display_driver_probe(struct drm_i915_private 
*i915)
 
 void intel_display_driver_register(struct drm_i915_private *i915)
 {
-   struct drm_printer p = drm_debug_printer("i915 display info:");
+   struct drm_printer p = drm_dbg_printer(&i915->drm, DRM_UT_KMS,
+  "i915 display info:");
 
if (!HAS_DISPLAY(i915))
return;
diff --git a/drivers/gpu/drm/i915/gt/intel_engine_heartbeat.c 
b/drivers/gpu/drm/i915/gt/intel_engine_heartbeat.c
index 1a8e2b7db013..0f6406f0cca0 100644
--- a/drivers/gpu/drm/i915/gt/intel_engine_heartbeat.c
+++ b/drivers/gpu/drm/i915/gt/intel_engine_heartbeat.c
@@ -96,7 +96,8 @@ static void heartbeat_commit(struct i915_request *rq,
 static void show_heartbeat(const struct i915_request *rq,
   struct intel_engine_cs *engine)
 {
-   struct drm_printer p = drm_debug_printer("heartbeat");
+   struct drm_printer p = drm_dbg_printer(&rq->i915->drm, DRM_UT_DRIVER,
+  "heartbeat");
 
if (!rq) {
intel_engine_dump(engine, &p,
diff --git a/drivers/gpu/drm/i915/gt/intel_reset.c 
b/drivers/gpu/drm/i915/gt/intel_reset.c
index d5ed904f355d..5909d8115eb6 100644
--- a/drivers/gpu/drm/i915/gt/intel_reset.c
+++ b/drivers/gpu/drm/i915/gt/intel_reset.c
@@ -1015,7 +1015,8 @@ void intel_gt_set_wedged(struct intel_gt *gt)
mutex_lock(>->reset.mutex);
 
if (GEM_SHOW_DEBUG()) {
-   struct drm_printer p = drm_debug_printer(__func__);
+   struct drm_printer p = drm_dbg_printer(>->i915->drm,
+  DRM_UT_DRIVER, __func__);
struct intel_engine_cs *engine;
enum intel_engine_id id;
 
diff --git a/drivers/gpu/drm/i915/gt/intel_workarounds.c 
b/drivers/gpu/drm/i915/gt/intel_workarounds.c
index 9bc0654efdc0..66aea33ed894 100644
--- a/drivers/gpu/drm/i915/gt/intel_workarounds.c
+++ b/drivers/gpu/drm/i915/gt/intel_workarounds.c
@@ -1230,7 +1230,8 @@ static void __set_mcr_steering(struct i915_wa_list *wal,
 
 static void debug_dump_steering(struct intel_gt *gt)
 {
-   struct drm_printer p = drm_debug_printer("MCR Steering:");
+   struct drm_printer p = drm_dbg_printer(>->i915->drm, DRM_UT_DRIVER,
+  "MCR Steering:");
 
if (drm_debug_enabled(DRM_UT_DRIVER))
intel_gt_mcr_report_steering(&p, gt, false);
diff --git a/drivers/gpu/drm/i915/gt/selftest_context.c 
b/drivers/gpu/drm/i915/gt/selftest_context.c
index 47070cba7eb1..12eca750f7d0 100644
--- a/drivers/gpu/drm/i915/gt/selftest_context.c
+++ b/drivers/gpu/drm/i915/gt/selftest_context.c
@@ -285,7 +285,8 @@ static int __live_active_context(struct intel_engine_cs 
*engine)
intel_engine_pm_flush(engine);
 
if (intel_engine_pm_is_awake(engine)) {
-   struct drm_printer p = drm_debug_printer(__func__);
+   struct drm_printer p = drm_dbg_printer(&engine->i915->drm,
+  DRM_UT_DRIVER, __func__);
 
intel_engine_dump(engine, &p,
  "%s is still awake:%d after idle-barriers\n",
diff --git a/drivers/gpu/drm/i915/i915_driver.c 
b/drivers/gpu/drm/i915/i915_driver.c
index 15e58b8ef027..1d99a1fb4093 100644
--- a/drivers/gpu/drm/i915/i915_driver.c
+++ b/drivers/gpu/drm/i915/i915_driver.c
@@ -681,7 +681,8 @@ i915_print_iommu_status(struct drm_i915_private *i915, 
struct drm_printer *p)
 static void i915_welcome_messages(struct drm_i915_private *dev_priv)
 {
if (drm_debug_enabled(DRM_UT_DRIVER)) {
-   struct drm_printer p = drm_debug_printer("i915 device info:");
+   struct drm_printer p = drm_dbg_printer(&dev_priv->drm, 
DRM_UT_DRIVER,
+  "device info:");
struct intel_gt *gt;
unsigned int i;
 
-- 
2.39.2

[PATCH 6/8] drm/dp: switch drm_dp_vsc_sdp_log() to struct drm_printer

Use the existing drm printer infrastructure instead of local macros.

Signed-off-by: Jani Nikula 
---
 drivers/gpu/drm/display/drm_dp_helper.c   | 17 +---
 .../drm/i915/display/intel_crtc_state_dump.c  |  5 ++--
 drivers/gpu/drm/i915/display/intel_display.c  | 27 +--
 include/drm/display/drm_dp_helper.h   |  3 +--
 4 files changed, 23 insertions(+), 29 deletions(-)

diff --git a/drivers/gpu/drm/display/drm_dp_helper.c 
b/drivers/gpu/drm/display/drm_dp_helper.c
index d72b6f9a352c..1cf51a748022 100644
--- a/drivers/gpu/drm/display/drm_dp_helper.c
+++ b/drivers/gpu/drm/display/drm_dp_helper.c
@@ -2898,22 +2898,19 @@ static const char *dp_content_type_get_name(enum 
dp_content_type content_type)
}
 }
 
-void drm_dp_vsc_sdp_log(const char *level, struct device *dev,
-   const struct drm_dp_vsc_sdp *vsc)
+void drm_dp_vsc_sdp_log(struct drm_printer *p, const struct drm_dp_vsc_sdp 
*vsc)
 {
-#define DP_SDP_LOG(fmt, ...) dev_printk(level, dev, fmt, ##__VA_ARGS__)
-   DP_SDP_LOG("DP SDP: %s, revision %u, length %u\n", "VSC",
+   drm_printf(p, "DP SDP: VSC, revision %u, length %u\n",
   vsc->revision, vsc->length);
-   DP_SDP_LOG("pixelformat: %s\n",
+   drm_printf(p, "pixelformat: %s\n",
   dp_pixelformat_get_name(vsc->pixelformat));
-   DP_SDP_LOG("colorimetry: %s\n",
+   drm_printf(p, "colorimetry: %s\n",
   dp_colorimetry_get_name(vsc->pixelformat, vsc->colorimetry));
-   DP_SDP_LOG("bpc: %u\n", vsc->bpc);
-   DP_SDP_LOG("dynamic range: %s\n",
+   drm_printf(p, "bpc: %u\n", vsc->bpc);
+   drm_printf(p, "dynamic range: %s\n",
   dp_dynamic_range_get_name(vsc->dynamic_range));
-   DP_SDP_LOG("content type: %s\n",
+   drm_printf(p, "content type: %s\n",
   dp_content_type_get_name(vsc->content_type));
-#undef DP_SDP_LOG
 }
 EXPORT_SYMBOL(drm_dp_vsc_sdp_log);
 
diff --git a/drivers/gpu/drm/i915/display/intel_crtc_state_dump.c 
b/drivers/gpu/drm/i915/display/intel_crtc_state_dump.c
index fbe89b6f038a..a6c55a357b13 100644
--- a/drivers/gpu/drm/i915/display/intel_crtc_state_dump.c
+++ b/drivers/gpu/drm/i915/display/intel_crtc_state_dump.c
@@ -55,10 +55,9 @@ static void
 intel_dump_dp_vsc_sdp(struct drm_i915_private *i915,
  const struct drm_dp_vsc_sdp *vsc)
 {
-   if (!drm_debug_enabled(DRM_UT_KMS))
-   return;
+   struct drm_printer p = drm_dbg_printer(&i915->drm, DRM_UT_KMS, NULL);
 
-   drm_dp_vsc_sdp_log(KERN_DEBUG, i915->drm.dev, vsc);
+   drm_dp_vsc_sdp_log(&p, vsc);
 }
 
 static void
diff --git a/drivers/gpu/drm/i915/display/intel_display.c 
b/drivers/gpu/drm/i915/display/intel_display.c
index 5cf162628b95..5f05017570da 100644
--- a/drivers/gpu/drm/i915/display/intel_display.c
+++ b/drivers/gpu/drm/i915/display/intel_display.c
@@ -4799,28 +4799,27 @@ pipe_config_infoframe_mismatch(struct drm_i915_private 
*dev_priv,
 }
 
 static void
-pipe_config_dp_vsc_sdp_mismatch(struct drm_i915_private *dev_priv,
+pipe_config_dp_vsc_sdp_mismatch(struct drm_i915_private *i915,
bool fastset, const char *name,
const struct drm_dp_vsc_sdp *a,
const struct drm_dp_vsc_sdp *b)
 {
+   struct drm_printer p;
+
if (fastset) {
-   if (!drm_debug_enabled(DRM_UT_KMS))
-   return;
+   p = drm_dbg_printer(&i915->drm, DRM_UT_KMS, NULL);
 
-   drm_dbg_kms(&dev_priv->drm,
-   "fastset requirement not met in %s dp sdp\n", name);
-   drm_dbg_kms(&dev_priv->drm, "expected:\n");
-   drm_dp_vsc_sdp_log(KERN_DEBUG, dev_priv->drm.dev, a);
-   drm_dbg_kms(&dev_priv->drm, "found:\n");
-   drm_dp_vsc_sdp_log(KERN_DEBUG, dev_priv->drm.dev, b);
+   drm_printf(&p, "fastset requirement not met in %s dp sdp\n", 
name);
} else {
-   drm_err(&dev_priv->drm, "mismatch in %s dp sdp\n", name);
-   drm_err(&dev_priv->drm, "expected:\n");
-   drm_dp_vsc_sdp_log(KERN_ERR, dev_priv->drm.dev, a);
-   drm_err(&dev_priv->drm, "found:\n");
-   drm_dp_vsc_sdp_log(KERN_ERR, dev_priv->drm.dev, b);
+   p = drm_err_printer(&i915->drm, NULL);
+
+   drm_printf(&p, "mismatch in %s dp sdp\n", name);
}
+
+   drm_printf(&p, "expected:\n");
+   drm_dp_vsc_sdp_log(&p, a);
+   drm_printf(&p, "found:\n");
+   drm_dp_vsc_sdp_log(&p, b);
 }
 
 /* Returns the length up to and including the last differing byte */
diff --git a/include/drm/display/drm_dp_helper.h 
b/include/drm/display/drm_dp_helper.h
index 863b2e7add29..d02014a87f12 100644
--- a/include/drm/display/drm_dp_helper.h
+++ b/include/drm/display/drm_dp_helper.h
@@ -98,8 +98,7 @@ struct drm_dp_vsc_sd

[PATCH 5/8] drm/mode: switch from drm_debug_printer() to device specific drm_dbg_printer()

Prefer the device specific debug printer.

Signed-off-by: Jani Nikula 
---
 drivers/gpu/drm/drm_mode_config.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/gpu/drm/drm_mode_config.c 
b/drivers/gpu/drm/drm_mode_config.c
index 8525ef851540..48fd2d67f352 100644
--- a/drivers/gpu/drm/drm_mode_config.c
+++ b/drivers/gpu/drm/drm_mode_config.c
@@ -544,7 +544,7 @@ void drm_mode_config_cleanup(struct drm_device *dev)
 */
WARN_ON(!list_empty(&dev->mode_config.fb_list));
list_for_each_entry_safe(fb, fbt, &dev->mode_config.fb_list, head) {
-   struct drm_printer p = drm_debug_printer("[leaked fb]");
+   struct drm_printer p = drm_dbg_printer(dev, DRM_UT_KMS, 
"[leaked fb]");
 
drm_printf(&p, "framebuffer[%u]:\n", fb->base.id);
drm_framebuffer_print_info(&p, 1, fb);
-- 
2.39.2

[PATCH 4/8] drm/dp_mst: switch from drm_debug_printer() to device specific drm_dbg_printer()

Prefer the device specific debug printer.

Signed-off-by: Jani Nikula 
---
 drivers/gpu/drm/display/drm_dp_mst_topology.c | 23 +++
 1 file changed, 14 insertions(+), 9 deletions(-)

diff --git a/drivers/gpu/drm/display/drm_dp_mst_topology.c 
b/drivers/gpu/drm/display/drm_dp_mst_topology.c
index 8ca01a6bf645..fba6e37b051b 100644
--- a/drivers/gpu/drm/display/drm_dp_mst_topology.c
+++ b/drivers/gpu/drm/display/drm_dp_mst_topology.c
@@ -1306,7 +1306,8 @@ static int drm_dp_mst_wait_tx_reply(struct 
drm_dp_mst_branch *mstb,
}
 out:
if (unlikely(ret == -EIO) && drm_debug_enabled(DRM_UT_DP)) {
-   struct drm_printer p = drm_debug_printer(DBG_PREFIX);
+   struct drm_printer p = drm_dbg_printer(mgr->dev, DRM_UT_DP,
+  DBG_PREFIX);
 
drm_dp_mst_dump_sideband_msg_tx(&p, txmsg);
}
@@ -1593,10 +1594,11 @@ topology_ref_type_to_str(enum 
drm_dp_mst_topology_ref_type type)
 }
 
 static void
-__dump_topology_ref_history(struct drm_dp_mst_topology_ref_history *history,
+__dump_topology_ref_history(struct drm_device *drm,
+   struct drm_dp_mst_topology_ref_history *history,
void *ptr, const char *type_str)
 {
-   struct drm_printer p = drm_debug_printer(DBG_PREFIX);
+   struct drm_printer p = drm_dbg_printer(drm, DRM_UT_DP, DBG_PREFIX);
char *buf = kzalloc(PAGE_SIZE, GFP_KERNEL);
int i;
 
@@ -1638,15 +1640,15 @@ __dump_topology_ref_history(struct 
drm_dp_mst_topology_ref_history *history,
 static __always_inline void
 drm_dp_mst_dump_mstb_topology_history(struct drm_dp_mst_branch *mstb)
 {
-   __dump_topology_ref_history(&mstb->topology_ref_history, mstb,
-   "MSTB");
+   __dump_topology_ref_history(mstb->mgr->dev, &mstb->topology_ref_history,
+   mstb, "MSTB");
 }
 
 static __always_inline void
 drm_dp_mst_dump_port_topology_history(struct drm_dp_mst_port *port)
 {
-   __dump_topology_ref_history(&port->topology_ref_history, port,
-   "Port");
+   __dump_topology_ref_history(port->mgr->dev, &port->topology_ref_history,
+   port, "Port");
 }
 
 static __always_inline void
@@ -2824,7 +2826,9 @@ static int process_single_tx_qlock(struct 
drm_dp_mst_topology_mgr *mgr,
ret = drm_dp_send_sideband_msg(mgr, up, chunk, idx);
if (ret) {
if (drm_debug_enabled(DRM_UT_DP)) {
-   struct drm_printer p = drm_debug_printer(DBG_PREFIX);
+   struct drm_printer p = drm_dbg_printer(mgr->dev,
+  DRM_UT_DP,
+  DBG_PREFIX);
 
drm_printf(&p, "sideband msg failed to send\n");
drm_dp_mst_dump_sideband_msg_tx(&p, txmsg);
@@ -2869,7 +2873,8 @@ static void drm_dp_queue_down_tx(struct 
drm_dp_mst_topology_mgr *mgr,
list_add_tail(&txmsg->next, &mgr->tx_msg_downq);
 
if (drm_debug_enabled(DRM_UT_DP)) {
-   struct drm_printer p = drm_debug_printer(DBG_PREFIX);
+   struct drm_printer p = drm_dbg_printer(mgr->dev, DRM_UT_DP,
+  DBG_PREFIX);
 
drm_dp_mst_dump_sideband_msg_tx(&p, txmsg);
}
-- 
2.39.2

[PATCH 3/8] drm/print: add drm_dbg_printer() for drm device specific printer

We've lacked a device specific debug printer. Add one. Take category
into account too.

__builtin_return_address(0) is inaccurate here, so don't use it. If
necessary, we can later pass __func__ to drm_dbg_printer() by wrapping
it inside a macro.

Signed-off-by: Jani Nikula 
---
 drivers/gpu/drm/drm_print.c | 21 +
 include/drm/drm_print.h | 24 
 2 files changed, 45 insertions(+)

diff --git a/drivers/gpu/drm/drm_print.c b/drivers/gpu/drm/drm_print.c
index 91dbcdeaad3f..673b29c732ea 100644
--- a/drivers/gpu/drm/drm_print.c
+++ b/drivers/gpu/drm/drm_print.c
@@ -189,6 +189,27 @@ void __drm_printfn_debug(struct drm_printer *p, struct 
va_format *vaf)
 }
 EXPORT_SYMBOL(__drm_printfn_debug);
 
+void __drm_printfn_dbg(struct drm_printer *p, struct va_format *vaf)
+{
+   const struct drm_device *drm = p->arg;
+   const struct device *dev = drm ? drm->dev : NULL;
+   enum drm_debug_category category = p->category;
+   const char *prefix = p->prefix ?: "";
+   const char *prefix_pad = p->prefix ? " " : "";
+
+   if (!__drm_debug_enabled(category))
+   return;
+
+   /* Note: __builtin_return_address(0) is useless here. */
+   if (dev)
+   dev_printk(KERN_DEBUG, dev, "[" DRM_NAME "]%s%s %pV",
+  prefix_pad, prefix, vaf);
+   else
+   printk(KERN_DEBUG "[" DRM_NAME "]%s%s %pV",
+  prefix_pad, prefix, vaf);
+}
+EXPORT_SYMBOL(__drm_printfn_dbg);
+
 void __drm_printfn_err(struct drm_printer *p, struct va_format *vaf)
 {
struct drm_device *drm = p->arg;
diff --git a/include/drm/drm_print.h b/include/drm/drm_print.h
index 2d57939429a9..c6a7a7fe 100644
--- a/include/drm/drm_print.h
+++ b/include/drm/drm_print.h
@@ -176,6 +176,7 @@ struct drm_printer {
void (*puts)(struct drm_printer *p, const char *str);
void *arg;
const char *prefix;
+   enum drm_debug_category category;
 };
 
 void __drm_printfn_coredump(struct drm_printer *p, struct va_format *vaf);
@@ -184,6 +185,7 @@ void __drm_printfn_seq_file(struct drm_printer *p, struct 
va_format *vaf);
 void __drm_puts_seq_file(struct drm_printer *p, const char *str);
 void __drm_printfn_info(struct drm_printer *p, struct va_format *vaf);
 void __drm_printfn_debug(struct drm_printer *p, struct va_format *vaf);
+void __drm_printfn_dbg(struct drm_printer *p, struct va_format *vaf);
 void __drm_printfn_err(struct drm_printer *p, struct va_format *vaf);
 
 __printf(2, 3)
@@ -331,6 +333,28 @@ static inline struct drm_printer drm_debug_printer(const 
char *prefix)
return p;
 }
 
+/**
+ * drm_dbg_printer - construct a &drm_printer for drm device specific output
+ * @drm: the &struct drm_device pointer, or NULL
+ * @category: the debug category to use
+ * @prefix: debug output prefix, or NULL for no prefix
+ *
+ * RETURNS:
+ * The &drm_printer object
+ */
+static inline struct drm_printer drm_dbg_printer(struct drm_device *drm,
+enum drm_debug_category 
category,
+const char *prefix)
+{
+   struct drm_printer p = {
+   .printfn = __drm_printfn_dbg,
+   .arg = drm,
+   .prefix = prefix,
+   .category = category,
+   };
+   return p;
+}
+
 /**
  * drm_err_printer - construct a &drm_printer that outputs to drm_err()
  * @drm: the &struct drm_device pointer
-- 
2.39.2

[PATCH 2/8] drm/print: move enum drm_debug_category etc. earlier in drm_print.h

Avoid forward declarations in subsequent changes, but separate this
movement to an independent change.

Signed-off-by: Jani Nikula 
---
 include/drm/drm_print.h | 190 
 1 file changed, 95 insertions(+), 95 deletions(-)

diff --git a/include/drm/drm_print.h b/include/drm/drm_print.h
index 3d899fb0793c..2d57939429a9 100644
--- a/include/drm/drm_print.h
+++ b/include/drm/drm_print.h
@@ -69,6 +69,101 @@ extern unsigned long __drm_debug;
  * }
  */
 
+/**
+ * enum drm_debug_category - The DRM debug categories
+ *
+ * Each of the DRM debug logging macros use a specific category, and the 
logging
+ * is filtered by the drm.debug module parameter. This enum specifies the 
values
+ * for the interface.
+ *
+ * Each DRM_DEBUG_ macro logs to DRM_UT_ category, except
+ * DRM_DEBUG() logs to DRM_UT_CORE.
+ *
+ * Enabling verbose debug messages is done through the drm.debug parameter, 
each
+ * category being enabled by a bit:
+ *
+ *  - drm.debug=0x1 will enable CORE messages
+ *  - drm.debug=0x2 will enable DRIVER messages
+ *  - drm.debug=0x3 will enable CORE and DRIVER messages
+ *  - ...
+ *  - drm.debug=0x1ff will enable all messages
+ *
+ * An interesting feature is that it's possible to enable verbose logging at
+ * run-time by echoing the debug value in its sysfs node::
+ *
+ *   # echo 0xf > /sys/module/drm/parameters/debug
+ *
+ */
+enum drm_debug_category {
+   /* These names must match those in DYNAMIC_DEBUG_CLASSBITS */
+   /**
+* @DRM_UT_CORE: Used in the generic drm code: drm_ioctl.c, drm_mm.c,
+* drm_memory.c, ...
+*/
+   DRM_UT_CORE,
+   /**
+* @DRM_UT_DRIVER: Used in the vendor specific part of the driver: i915,
+* radeon, ... macro.
+*/
+   DRM_UT_DRIVER,
+   /**
+* @DRM_UT_KMS: Used in the modesetting code.
+*/
+   DRM_UT_KMS,
+   /**
+* @DRM_UT_PRIME: Used in the prime code.
+*/
+   DRM_UT_PRIME,
+   /**
+* @DRM_UT_ATOMIC: Used in the atomic code.
+*/
+   DRM_UT_ATOMIC,
+   /**
+* @DRM_UT_VBL: Used for verbose debug message in the vblank code.
+*/
+   DRM_UT_VBL,
+   /**
+* @DRM_UT_STATE: Used for verbose atomic state debugging.
+*/
+   DRM_UT_STATE,
+   /**
+* @DRM_UT_LEASE: Used in the lease code.
+*/
+   DRM_UT_LEASE,
+   /**
+* @DRM_UT_DP: Used in the DP code.
+*/
+   DRM_UT_DP,
+   /**
+* @DRM_UT_DRMRES: Used in the drm managed resources code.
+*/
+   DRM_UT_DRMRES
+};
+
+static inline bool drm_debug_enabled_raw(enum drm_debug_category category)
+{
+   return unlikely(__drm_debug & BIT(category));
+}
+
+#define drm_debug_enabled_instrumented(category)   \
+   ({  \
+   pr_debug("todo: is this frequent enough to optimize ?\n"); \
+   drm_debug_enabled_raw(category);\
+   })
+
+#if defined(CONFIG_DRM_USE_DYNAMIC_DEBUG)
+/*
+ * the drm.debug API uses dyndbg, so each drm_*dbg macro/callsite gets
+ * a descriptor, and only enabled callsites are reachable.  They use
+ * the private macro to avoid re-testing the enable-bit.
+ */
+#define __drm_debug_enabled(category)  true
+#define drm_debug_enabled(category)drm_debug_enabled_instrumented(category)
+#else
+#define __drm_debug_enabled(category)  drm_debug_enabled_raw(category)
+#define drm_debug_enabled(category)drm_debug_enabled_raw(category)
+#endif
+
 /**
  * struct drm_printer - drm output "stream"
  *
@@ -255,101 +350,6 @@ static inline struct drm_printer drm_err_printer(struct 
drm_device *drm,
return p;
 }
 
-/**
- * enum drm_debug_category - The DRM debug categories
- *
- * Each of the DRM debug logging macros use a specific category, and the 
logging
- * is filtered by the drm.debug module parameter. This enum specifies the 
values
- * for the interface.
- *
- * Each DRM_DEBUG_ macro logs to DRM_UT_ category, except
- * DRM_DEBUG() logs to DRM_UT_CORE.
- *
- * Enabling verbose debug messages is done through the drm.debug parameter, 
each
- * category being enabled by a bit:
- *
- *  - drm.debug=0x1 will enable CORE messages
- *  - drm.debug=0x2 will enable DRIVER messages
- *  - drm.debug=0x3 will enable CORE and DRIVER messages
- *  - ...
- *  - drm.debug=0x1ff will enable all messages
- *
- * An interesting feature is that it's possible to enable verbose logging at
- * run-time by echoing the debug value in its sysfs node::
- *
- *   # echo 0xf > /sys/module/drm/parameters/debug
- *
- */
-enum drm_debug_category {
-   /* These names must match those in DYNAMIC_DEBUG_CLASSBITS */
-   /**
-* @DRM_UT_CORE: Used in the generic drm code: drm_ioctl.c, drm_mm.c,
-* drm_memory.c, ...
-*/
-   DRM_UT_CORE,
-   /**
-* @DRM_UT_DRIVER: Used in the vendor

[PATCH 1/8] drm/print: make drm_err_printer() device specific by using drm_err()

With few users for drm_err_printer(), it's still feasible to convert it
to be device specific. Use drm_err() under the hood.

While at it, make the prefix optional.

Signed-off-by: Jani Nikula 
---
 drivers/gpu/drm/drm_print.c |  7 ++-
 drivers/gpu/drm/i915/gt/selftest_engine_heartbeat.c |  4 ++--
 drivers/gpu/drm/i915/selftests/i915_active.c|  4 ++--
 include/drm/drm_print.h | 11 ---
 4 files changed, 18 insertions(+), 8 deletions(-)

diff --git a/drivers/gpu/drm/drm_print.c b/drivers/gpu/drm/drm_print.c
index 5b93c11895bb..91dbcdeaad3f 100644
--- a/drivers/gpu/drm/drm_print.c
+++ b/drivers/gpu/drm/drm_print.c
@@ -191,7 +191,12 @@ EXPORT_SYMBOL(__drm_printfn_debug);
 
 void __drm_printfn_err(struct drm_printer *p, struct va_format *vaf)
 {
-   pr_err("*ERROR* %s %pV", p->prefix, vaf);
+   struct drm_device *drm = p->arg;
+
+   if (p->prefix)
+   drm_err(drm, "%s %pV", p->prefix, vaf);
+   else
+   drm_err(drm, "%pV", vaf);
 }
 EXPORT_SYMBOL(__drm_printfn_err);
 
diff --git a/drivers/gpu/drm/i915/gt/selftest_engine_heartbeat.c 
b/drivers/gpu/drm/i915/gt/selftest_engine_heartbeat.c
index 273d440a53e3..b4970c1ed572 100644
--- a/drivers/gpu/drm/i915/gt/selftest_engine_heartbeat.c
+++ b/drivers/gpu/drm/i915/gt/selftest_engine_heartbeat.c
@@ -122,7 +122,7 @@ static int __live_idle_pulse(struct intel_engine_cs *engine,
GEM_BUG_ON(!llist_empty(&engine->barrier_tasks));
 
if (engine_sync_barrier(engine)) {
-   struct drm_printer m = drm_err_printer("pulse");
+   struct drm_printer m = drm_err_printer(&engine->i915->drm, 
"pulse");
 
pr_err("%s: no heartbeat pulse?\n", engine->name);
intel_engine_dump(engine, &m, "%s", engine->name);
@@ -136,7 +136,7 @@ static int __live_idle_pulse(struct intel_engine_cs *engine,
pulse_unlock_wait(p); /* synchronize with the retirement callback */
 
if (!i915_active_is_idle(&p->active)) {
-   struct drm_printer m = drm_err_printer("pulse");
+   struct drm_printer m = drm_err_printer(&engine->i915->drm, 
"pulse");
 
pr_err("%s: heartbeat pulse did not flush idle tasks\n",
   engine->name);
diff --git a/drivers/gpu/drm/i915/selftests/i915_active.c 
b/drivers/gpu/drm/i915/selftests/i915_active.c
index b61fe850e924..8886752ade63 100644
--- a/drivers/gpu/drm/i915/selftests/i915_active.c
+++ b/drivers/gpu/drm/i915/selftests/i915_active.c
@@ -156,7 +156,7 @@ static int live_active_wait(void *arg)
 
__i915_active_wait(&active->base, TASK_UNINTERRUPTIBLE);
if (!READ_ONCE(active->retired)) {
-   struct drm_printer p = drm_err_printer(__func__);
+   struct drm_printer p = drm_err_printer(&i915->drm, __func__);
 
pr_err("i915_active not retired after waiting!\n");
i915_active_print(&active->base, &p);
@@ -189,7 +189,7 @@ static int live_active_retire(void *arg)
err = -EIO;
 
if (!READ_ONCE(active->retired)) {
-   struct drm_printer p = drm_err_printer(__func__);
+   struct drm_printer p = drm_err_printer(&i915->drm, __func__);
 
pr_err("i915_active not retired after flushing!\n");
i915_active_print(&active->base, &p);
diff --git a/include/drm/drm_print.h b/include/drm/drm_print.h
index dd4883df876a..3d899fb0793c 100644
--- a/include/drm/drm_print.h
+++ b/include/drm/drm_print.h
@@ -35,6 +35,8 @@
 
 #include 
 
+struct drm_device;
+
 /* Do *not* use outside of drm_print.[ch]! */
 extern unsigned long __drm_debug;
 
@@ -235,16 +237,19 @@ static inline struct drm_printer drm_debug_printer(const 
char *prefix)
 }
 
 /**
- * drm_err_printer - construct a &drm_printer that outputs to pr_err()
- * @prefix: debug output prefix
+ * drm_err_printer - construct a &drm_printer that outputs to drm_err()
+ * @drm: the &struct drm_device pointer
+ * @prefix: debug output prefix, or NULL for no prefix
  *
  * RETURNS:
  * The &drm_printer object
  */
-static inline struct drm_printer drm_err_printer(const char *prefix)
+static inline struct drm_printer drm_err_printer(struct drm_device *drm,
+const char *prefix)
 {
struct drm_printer p = {
.printfn = __drm_printfn_err,
+   .arg = drm,
.prefix = prefix
};
return p;
-- 
2.39.2

[PATCH 0/8] drm: drm debug and error logging improvements

Make drm_err_printer() drm device specific, and add drm device specific
drm_dbg_printer(). Switch code over to use them.

Jani Nikula (8):
  drm/print: make drm_err_printer() device specific by using drm_err()
  drm/print: move enum drm_debug_category etc. earlier in drm_print.h
  drm/print: add drm_dbg_printer() for drm device specific printer
  drm/dp_mst: switch from drm_debug_printer() to device specific
drm_dbg_printer()
  drm/mode: switch from drm_debug_printer() to device specific
drm_dbg_printer()
  drm/dp: switch drm_dp_vsc_sdp_log() to struct drm_printer
  drm/i915: switch from drm_debug_printer() to device specific
drm_dbg_printer()
  drm/i915: use drm_printf() with the drm_err_printer intead of pr_err()

 drivers/gpu/drm/display/drm_dp_helper.c   |  17 +-
 drivers/gpu/drm/display/drm_dp_mst_topology.c |  23 +-
 drivers/gpu/drm/drm_mode_config.c |   2 +-
 drivers/gpu/drm/drm_print.c   |  28 ++-
 .../drm/i915/display/intel_crtc_state_dump.c  |   5 +-
 drivers/gpu/drm/i915/display/intel_display.c  |  27 ++-
 .../drm/i915/display/intel_display_driver.c   |   3 +-
 .../gpu/drm/i915/gt/intel_engine_heartbeat.c  |   3 +-
 drivers/gpu/drm/i915/gt/intel_reset.c |   3 +-
 drivers/gpu/drm/i915/gt/intel_workarounds.c   |   3 +-
 drivers/gpu/drm/i915/gt/selftest_context.c|   3 +-
 .../drm/i915/gt/selftest_engine_heartbeat.c   |  10 +-
 drivers/gpu/drm/i915/i915_driver.c|   3 +-
 drivers/gpu/drm/i915/selftests/i915_active.c  |   8 +-
 include/drm/display/drm_dp_helper.h   |   3 +-
 include/drm/drm_print.h   | 217 ++
 16 files changed, 209 insertions(+), 149 deletions(-)

-- 
2.39.2

Re: [PATCH] drm/panfrost: Really power off GPU cores in panfrost_gpu_power_off()

2023-11-24 Thread Marek Szyprowski


On 22.11.2023 10:29, Krzysztof Kozlowski wrote:
> On 22/11/2023 10:06, AngeloGioacchino Del Regno wrote:
> Hey Krzysztof,
>
> This is interesting. It might be about the cores that are missing from 
> the partial
> core_mask raising interrupts, but an external abort on non-linefetch is 
> strange to
> see here.
 I've seen such external aborts in the past, and the fault type has
 often been misleading. It's unlikely to have anything to do with a
>>> Yeah, often accessing device with power or clocks gated.
>>>
>> Except my commit does *not* gate SoC power, nor SoC clocks 🙂
> It could be that something (like clocks or power supplies) was missing
> on this board/SoC, which was not critical till your patch came.
>
>> What the "Really power off ..." commit does is to ask the GPU to internally 
>> power
>> off the shaders, tilers and L2, that's why I say that it is strange to see 
>> that
>> kind of abort.
>>
>> The GPU_INT_CLEAR GPU_INT_STAT, GPU_FAULT_STATUS and 
>> GPU_FAULT_ADDRESS_{HI/LO}
>> registers should still be accessible even with shaders, tilers and cache OFF.
>>
>> Anyway, yes, synchronizing IRQs before calling the poweroff sequence would 
>> also
>> work, but that'd add up quite a bit of latency on the runtime_suspend() 
>> call, so
>> in this case I'd be more for avoiding to execute any register r/w in the 
>> handler
>> by either checking if the GPU is supposed to be OFF, or clearing interrupts, 
>> which
>> may not work if those are generated after the execution of the poweroff 
>> function.
>> Or we could simply disable the irq after power_off, but that'd be hacky (as 
>> well).
>>
>>
>> Let's see if asking to poweroff *everything* works:
> Worked.

Yes, I also got into this issue some time ago, but I didn't report it 
because I also had some power supply related problems on my test farm 
and everything was a bit unstable. I wasn't 100% sure that the $subject 
patch is responsible for the observed issues. Now, after fixing power 
supply, I confirm that the issue was revealed by the $subject patch and 
above mentioned change fixes the problem. Feel free to add:

Tested-by: Marek Szyprowski 

Best regards
-- 
Marek Szyprowski, PhD
Samsung R&D Institute Poland

Re: [PATCH v9 04/12] dt-bindings: phy: amlogic,g12a-mipi-dphy-analog: drop unneeded reg property and example

2023-11-24 Thread Conor Dooley

On Fri, Nov 24, 2023 at 09:41:15AM +0100, Neil Armstrong wrote:
> The amlogic,g12a-mipi-dphy-analog is a feature of the simple-mfd
> amlogic,meson-axg-hhi-sysctrl system control register zone which is an
> intermixed registers zone, thus it's very hard to define clear ranges for
> each SoC controlled features even if possible.
> 
> The amlogic,g12a-mipi-dphy-analog was wrongly documented as an independent
> register range, which is not the reality, thus fix the bindings by dropping
> the reg property now it's referred from amlogic,meson-gx-hhi-sysctrl.yaml
> and documented as a subnode of amlogic,meson-axg-hhi-sysctrl.
> 
> Also drop the unnecessary example, the top level bindings example should
> be enough.
> 
> Fixes: 76ab79f9726c ("dt-bindings: phy: add Amlogic G12A Analog MIPI D-PHY 
> bindings")
> Signed-off-by: Neil Armstrong 

I feel like I left a tag on this one before, but I can't remember.
Perhaps I missed the conclusion to the discussion to the discussion with
Rob about whether having "reg" was desirable that lead to a tag being
dropped?

> ---
>  .../bindings/phy/amlogic,g12a-mipi-dphy-analog.yaml  | 12 
> 
>  1 file changed, 12 deletions(-)
> 
> diff --git 
> a/Documentation/devicetree/bindings/phy/amlogic,g12a-mipi-dphy-analog.yaml 
> b/Documentation/devicetree/bindings/phy/amlogic,g12a-mipi-dphy-analog.yaml
> index c8c83acfb871..81c2654b7e57 100644
> --- a/Documentation/devicetree/bindings/phy/amlogic,g12a-mipi-dphy-analog.yaml
> +++ b/Documentation/devicetree/bindings/phy/amlogic,g12a-mipi-dphy-analog.yaml
> @@ -16,20 +16,8 @@ properties:
>"#phy-cells":
>  const: 0
>  
> -  reg:
> -maxItems: 1
> -
>  required:
>- compatible
> -  - reg
>- "#phy-cells"
>  
>  additionalProperties: false
> -
> -examples:
> -  - |
> -phy@0 {
> -  compatible = "amlogic,g12a-mipi-dphy-analog";
> -  reg = <0x0 0xc>;
> -  #phy-cells = <0>;
> -};
> 
> -- 
> 2.34.1
> 


signature.asc
Description: PGP signature

Re: linux-next: build warning after merge of the drm tree

2023-11-24 Thread Donald Robson

Hi Stephen,

Thanks for the report.  I've fixed these locally, along with a few other doc 
issues
I found.  I'll get the patch out as soon as I can.

Thanks,
Donald

On Fri, 2023-11-24 at 13:25 +1100, Stephen Rothwell wrote:
> *** CAUTION: This email originates from a source not known to Imagination 
> Technologies. Think before you click a link or open an attachment ***
> 
> Hi all,
> 
> After merging the drm tree, today's linux-next build (htmldocs) produced
> this warning:
> 
> include/uapi/drm/pvr_drm.h:1: warning: 'Flags for 
> DRM_PVR_DEV_QUERY_HEAP_INFO_GET.' not found
> 
> Introduced by commit
> 
>   1088d89e5515 ("drm/imagination/uapi: Add PowerVR driver UAPI")
>

Re: [PATCH v6 0/6] drm: simplify support for transparent DRM bridges

2023-11-24 Thread Dmitry Baryshkov

On Fri, 24 Nov 2023 at 14:23, Bryan O'Donoghue
 wrote:
>
> On 03/11/2023 23:03, Dmitry Baryshkov wrote:
> > Supporting DP/USB-C can result in a chain of several transparent
> > bridges (PHY, redrivers, mux, etc). All attempts to implement DP support
> > in a different way resulted either in series of hacks or in device tree
> > not reflecting the actual hardware design. This results in drivers
> > having similar boilerplate code for such bridges.
> >
> > Next, these drivers are susceptible to -EPROBE_DEFER loops: the next
> > bridge can either be probed from the bridge->attach callback, when it is
> > too late to return -EPROBE_DEFER, or from the probe() callback, when the
> > next bridge might not yet be available, because it depends on the
> > resources provided by the probing device. Device links can not fully
> > solve this problem since there are mutual dependencies between adjancent
> > devices.
> >
> > Last, but not least, this results in the the internal knowledge of DRM
> > subsystem slowly diffusing into other subsystems, like PHY or USB/TYPEC.
> >
> > To solve all these issues, define a separate DRM helper, which creates
> > separate aux device just for the bridge. During probe such aux device
> > doesn't result in the EPROBE_DEFER loops. Instead it allows the device
> > drivers to probe properly, according to the actual resource
> > dependencies. The bridge auxdevs are then probed when the next bridge
> > becomes available, sparing drivers from drm_bridge_attach() returning
> > -EPROBE_DEFER.
>
> Dmitry,
>
> Looking to give you a Tested-by: here but got the below splat.

This should be fixed by
https://gitlab.freedesktop.org/drm/msm/-/tags/drm-msm-fixes-2023-11-21

>
> https://git.codelinaro.org/bryan.odonoghue/kernel/-/tree/next-20231123-tcpm-fix?ref_type=heads
>
> - Boot via fastboot
> - Remove USB cable
> - Attach DisplayPort cable
> - Get some activity on the DP
> - Then this
>
> root@linaro-gnome:~# [  376.861822] xhci-hcd xhci-hcd.4.auto: xHCI Host
> Controller
> [  376.867584] xhci-hcd xhci-hcd.4.auto: new USB bus registered,
> assigned bus number 3
> [  376.875775] xhci-hcd xhci-hcd.4.auto: hcc params 0x0230ffe5 hci
> version 0x110 quirks 0x0010
> [  376.885666] xhci-hcd xhci-hcd.4.auto: irq 229, io mem 0x0a60
> [  376.892140] xhci-hcd xhci-hcd.4.auto: xHCI Host Controller
> [  376.897951] xhci-hcd xhci-hcd.4.auto: new USB bus registered,
> assigned bus number 4
> [  376.905869] xhci-hcd xhci-hcd.4.auto: Host supports USB 3.1 Enhanced
> SuperSpeed
> [  376.914130] hub 3-0:1.0: USB hub found
> [  376.918030] hub 3-0:1.0: 1 port detected
> [  376.922417] usb usb4: We don't know the algorithms for LPM for this
> host, disabling LPM.
> [  376.931540] hub 4-0:1.0: USB hub found
> [  376.935439] hub 4-0:1.0: 1 port detected
> [  377.885638] Unable to handle kernel NULL pointer dereference at
> virtual address 0060
> [  377.892927] msm_dpu ae01000.display-controller: [drm] Cannot find any
> crtc or sizes
> [  377.894724] Mem abort info:
> [  377.905504]   ESR = 0x9604
> [  377.909375]   EC = 0x25: DABT (current EL), IL = 32 bits
> [  377.914852]   SET = 0, FnV = 0
> [  377.918005]   EA = 0, S1PTW = 0
> [  377.921250]   FSC = 0x04: level 0 translation fault
> [  377.926269] Data abort info:
> [  377.929239]   ISV = 0, ISS = 0x0004, ISS2 = 0x
> [  377.934881]   CM = 0, WnR = 0, TnD = 0, TagAccess = 0
> [  377.940095]   GCS = 0, Overlay = 0, DirtyBit = 0, Xs = 0
> [  377.945563] user pgtable: 4k pages, 48-bit VAs, pgdp=000101992000
> [  377.952441] [0060] pgd=, p4d=
> [  377.959448] Internal error: Oops: 9604 [#1] PREEMPT SMP
> [  377.965882] Modules linked in: typec_displayport nf_tables libcrc32c
> nfnetlink q6asm_dai q6routing q6afe_clocks q6afe_dai q6asm q6adm
> snd_q6dsp_common q6afe q6core apr pdr_interfacer
> [  377.965984]  aux_bridge crct10dif_ce snd_soc_lpass_macro_common
> drm_kms_helper qnoc_sm8250 qcom_wdt icc_osm_l3 fuse drm backlight dm_mod
> ip_tables x_tables
> [  378.072201] CPU: 5 PID: 379 Comm: dp_hpd_handler Not tainted
> 6.7.0-rc2-next-20231123-8-g812004aeedc0-dirty #30
> [  378.082817] Hardware name: Qualcomm Technologies, Inc. Robotics RB5 (DT)
> [  378.04] msm_dpu ae01000.display-controller: [drm] Cannot find any
> crtc or sizes
> [  378.089697] pstate: 6045 (nZCv daif +PAN -UAO -TCO -DIT -SSBS
> BTYPE=--)
> [  378.089700] pc : drm_object_property_set_value+0x0/0x88 [drm]
> [  378.110607] lr : drm_dp_set_subconnector_property+0x58/0x64
> [drm_display_helper]
> [  378.118205] sp : 800081fbbda0
> [  378.121616] x29: 800081fbbda0 x28:  x27:
> 
> [  378.128940] x26:  x25:  x24:
> 38d1ccef2880
> [  378.136264] x23: 38d1ccef2a10 x22: 38d1ccef2880 x21:
> 38d1ccef29f0
> [  378.143587] x20:  x19: 38d1ccef2880 x18:
> 
> [  378.150911] x17

Re: [PATCH v1 1/2] dt-bindings: display: panel: raspberrypi: Add compatible property for waveshare 7inch touchscreen panel

2023-11-24 Thread Conor Dooley

On Fri, Nov 24, 2023 at 06:44:50PM +0800, Shengyang Chen wrote:
> The waveshare 7inch touchscreen panel is a kind of raspberrypi pi
> panel

Can you be more specific about what "is a kind of rpi panel" means?
Are they using identical chips as controllers or something like that?

> and it can be drived by panel-raspberrypi-touchscreen.c.
> Add compatible property for it.
> 
> Signed-off-by: Keith Zhao 
> Signed-off-by: Shengyang Chen 
> ---
>  .../bindings/display/panel/raspberrypi,7inch-touchscreen.yaml | 4 +++-
>  1 file changed, 3 insertions(+), 1 deletion(-)
> 
> diff --git 
> a/Documentation/devicetree/bindings/display/panel/raspberrypi,7inch-touchscreen.yaml
>  
> b/Documentation/devicetree/bindings/display/panel/raspberrypi,7inch-touchscreen.yaml
> index 22a083f7bc8e..e4e6cb4d4e5b 100644
> --- 
> a/Documentation/devicetree/bindings/display/panel/raspberrypi,7inch-touchscreen.yaml
> +++ 
> b/Documentation/devicetree/bindings/display/panel/raspberrypi,7inch-touchscreen.yaml
> @@ -22,7 +22,9 @@ description: |+
>  
>  properties:
>compatible:
> -const: raspberrypi,7inch-touchscreen-panel
> +enum:
> +  - raspberrypi,7inch-touchscreen-panel
> +  - waveshare,7inch-touchscreen-panel
>  
>reg:
>  const: 0x45
> -- 
> 2.17.1
> 


signature.asc
Description: PGP signature

Re: [PATCH v6 0/6] drm: simplify support for transparent DRM bridges

2023-11-24 Thread Bryan O'Donoghue


On 03/11/2023 23:03, Dmitry Baryshkov wrote:

Supporting DP/USB-C can result in a chain of several transparent
bridges (PHY, redrivers, mux, etc). All attempts to implement DP support
in a different way resulted either in series of hacks or in device tree
not reflecting the actual hardware design. This results in drivers
having similar boilerplate code for such bridges.

Next, these drivers are susceptible to -EPROBE_DEFER loops: the next
bridge can either be probed from the bridge->attach callback, when it is
too late to return -EPROBE_DEFER, or from the probe() callback, when the
next bridge might not yet be available, because it depends on the
resources provided by the probing device. Device links can not fully
solve this problem since there are mutual dependencies between adjancent
devices.

Last, but not least, this results in the the internal knowledge of DRM
subsystem slowly diffusing into other subsystems, like PHY or USB/TYPEC.

To solve all these issues, define a separate DRM helper, which creates
separate aux device just for the bridge. During probe such aux device
doesn't result in the EPROBE_DEFER loops. Instead it allows the device
drivers to probe properly, according to the actual resource
dependencies. The bridge auxdevs are then probed when the next bridge
becomes available, sparing drivers from drm_bridge_attach() returning
-EPROBE_DEFER.


Dmitry,

Looking to give you a Tested-by: here but got the below splat.

https://git.codelinaro.org/bryan.odonoghue/kernel/-/tree/next-20231123-tcpm-fix?ref_type=heads

- Boot via fastboot
- Remove USB cable
- Attach DisplayPort cable
- Get some activity on the DP
- Then this

root@linaro-gnome:~# [  376.861822] xhci-hcd xhci-hcd.4.auto: xHCI Host 
Controller
[  376.867584] xhci-hcd xhci-hcd.4.auto: new USB bus registered, 
assigned bus number 3
[  376.875775] xhci-hcd xhci-hcd.4.auto: hcc params 0x0230ffe5 hci 
version 0x110 quirks 0x0010

[  376.885666] xhci-hcd xhci-hcd.4.auto: irq 229, io mem 0x0a60
[  376.892140] xhci-hcd xhci-hcd.4.auto: xHCI Host Controller
[  376.897951] xhci-hcd xhci-hcd.4.auto: new USB bus registered, 
assigned bus number 4
[  376.905869] xhci-hcd xhci-hcd.4.auto: Host supports USB 3.1 Enhanced 
SuperSpeed

[  376.914130] hub 3-0:1.0: USB hub found
[  376.918030] hub 3-0:1.0: 1 port detected
[  376.922417] usb usb4: We don't know the algorithms for LPM for this 
host, disabling LPM.

[  376.931540] hub 4-0:1.0: USB hub found
[  376.935439] hub 4-0:1.0: 1 port detected
[  377.885638] Unable to handle kernel NULL pointer dereference at 
virtual address 0060
[  377.892927] msm_dpu ae01000.display-controller: [drm] Cannot find any 
crtc or sizes

[  377.894724] Mem abort info:
[  377.905504]   ESR = 0x9604
[  377.909375]   EC = 0x25: DABT (current EL), IL = 32 bits
[  377.914852]   SET = 0, FnV = 0
[  377.918005]   EA = 0, S1PTW = 0
[  377.921250]   FSC = 0x04: level 0 translation fault
[  377.926269] Data abort info:
[  377.929239]   ISV = 0, ISS = 0x0004, ISS2 = 0x
[  377.934881]   CM = 0, WnR = 0, TnD = 0, TagAccess = 0
[  377.940095]   GCS = 0, Overlay = 0, DirtyBit = 0, Xs = 0
[  377.945563] user pgtable: 4k pages, 48-bit VAs, pgdp=000101992000
[  377.952441] [0060] pgd=, p4d=
[  377.959448] Internal error: Oops: 9604 [#1] PREEMPT SMP
[  377.965882] Modules linked in: typec_displayport nf_tables libcrc32c 
nfnetlink q6asm_dai q6routing q6afe_clocks q6afe_dai q6asm q6adm 
snd_q6dsp_common q6afe q6core apr pdr_interfacer
[  377.965984]  aux_bridge crct10dif_ce snd_soc_lpass_macro_common 
drm_kms_helper qnoc_sm8250 qcom_wdt icc_osm_l3 fuse drm backlight dm_mod 
ip_tables x_tables
[  378.072201] CPU: 5 PID: 379 Comm: dp_hpd_handler Not tainted 
6.7.0-rc2-next-20231123-8-g812004aeedc0-dirty #30

[  378.082817] Hardware name: Qualcomm Technologies, Inc. Robotics RB5 (DT)
[  378.04] msm_dpu ae01000.display-controller: [drm] Cannot find any 
crtc or sizes
[  378.089697] pstate: 6045 (nZCv daif +PAN -UAO -TCO -DIT -SSBS 
BTYPE=--)

[  378.089700] pc : drm_object_property_set_value+0x0/0x88 [drm]
[  378.110607] lr : drm_dp_set_subconnector_property+0x58/0x64 
[drm_display_helper]

[  378.118205] sp : 800081fbbda0
[  378.121616] x29: 800081fbbda0 x28:  x27: 

[  378.128940] x26:  x25:  x24: 
38d1ccef2880
[  378.136264] x23: 38d1ccef2a10 x22: 38d1ccef2880 x21: 
38d1ccef29f0
[  378.143587] x20:  x19: 38d1ccef2880 x18: 

[  378.150911] x17: 00040044 x16: a79c03e1fe34 x15: 

[  378.158235] x14: 38d1c5861000 x13: 03ec x12: 
0001
[  378.165560] x11: 071c71c71c71c71c x10: 0b00 x9 : 
800081fbb9d0
[  378.172884] x8 : a79b9b4d9000 x7 : 0001 x6 : 
a79b9b6d74b0
[  378.180207] x5 :  x4 : 38d1cb

Re: [PATCH 15/22] arch: vdso: consolidate gettime prototypes

2023-11-24 Thread Mark Brown

On Wed, Nov 08, 2023 at 01:58:36PM +0100, Arnd Bergmann wrote:
> From: Arnd Bergmann 
> 
> The VDSO functions are defined as globals in the kernel sources but intended
> to be called from userspace, so there is no need to declare them in a kernel
> side header.

This is in -next as commit 42874e4eb35bdfc54f8514685e50434098ba4f6c and
breaks an arm64 defconfig build, the 32 bit vDSO build is broken:

/build/stage/linux/arch/arm64/kernel/vdso32/vgettimeofday.c:10:5: error: conflic
ting types for ‘__vdso_clock_gettime’; have ‘int(clockid_t,  struct old_timespec
32 *)’ {aka ‘int(int,  struct old_timespec32 *)’}
   10 | int __vdso_clock_gettime(clockid_t clock,
  | ^~~~
In file included from /build/stage/linux/arch/arm64/kernel/vdso32/vgettimeofday.
c:8:
/build/stage/linux/include/vdso/gettime.h:16:5: note: previous declaration of 
‘__vdso_clock_gettime’ with type ‘int(clockid_t,  struct __kernel_timespec *)’ 
{aka ‘int(int,  struct __kernel_timespec *)’}
   16 | int __vdso_clock_gettime(clockid_t clock, struct __kernel_timespec *ts);
  | ^~~~
/build/stage/linux/arch/arm64/kernel/vdso32/vgettimeofday.c:28:5: error: 
conflicting types for ‘__vdso_clock_getres’; have ‘int(clockid_t,  struct 
old_timespec32 *)’ {aka ‘int(int,  struct old_timespec32 *)’}
   28 | int __vdso_clock_getres(clockid_t clock_id,
  | ^~~
/build/stage/linux/include/vdso/gettime.h:15:5: note: previous declaration of 
‘__vdso_clock_getres’ with type ‘int(clockid_t,  struct __kernel_timespec *)’ 
{aka ‘int(int,  struct __kernel_timespec *)’}
   15 | int __vdso_clock_getres(clockid_t clock, struct __kernel_timespec *res);
  | ^~~

signature.asc
Description: PGP signature

[PATCH] drm/atomic-helpers: Invoke end_fb_access while owning plane state

2023-11-24 Thread Thomas Zimmermann

Invoke drm_plane_helper_funcs.end_fb_access before
drm_atomic_helper_commit_hw_done(). The latter function hands over
ownership of the plane state to the following commit, which might
free it. Releasing resources in end_fb_access then operates on undefined
state. This bug has been observed with non-blocking commits when they
are being queued up quickly.

Here is an example stack trace from the bug report. The plane state has
been free'd already, so the pages for drm_gem_fb_vunmap() are gone.

Unable to handle kernel paging request at virtual address 00010049
[...]
 drm_gem_fb_vunmap+0x18/0x74
 drm_gem_end_shadow_fb_access+0x1c/0x2c
 drm_atomic_helper_cleanup_planes+0x58/0xd8
 drm_atomic_helper_commit_tail+0x90/0xa0
 commit_tail+0x15c/0x188
 commit_work+0x14/0x20

For aborted commits, it is still ok to run end_fb_access as part of the
plane's cleanup. Add a test to drm_atomic_helper_cleanup_planes().

Reported-by: Alyssa Ross 
Closes: https://lore.kernel.org/dri-devel/87leazm0ya@alyssa.is/
Suggested-by: Daniel Vetter 
Fixes: 94d879eaf7fb ("drm/atomic-helper: Add {begin,end}_fb_access to plane 
helpers")
Signed-off-by: Thomas Zimmermann 
Cc:  # v6.2+
---
 drivers/gpu/drm/drm_atomic_helper.c | 17 +
 1 file changed, 17 insertions(+)

diff --git a/drivers/gpu/drm/drm_atomic_helper.c 
b/drivers/gpu/drm/drm_atomic_helper.c
index c3f677130def0..08d0511405e90 100644
--- a/drivers/gpu/drm/drm_atomic_helper.c
+++ b/drivers/gpu/drm/drm_atomic_helper.c
@@ -2784,6 +2784,17 @@ void drm_atomic_helper_commit_planes(struct drm_device 
*dev,
 
funcs->atomic_flush(crtc, old_state);
}
+
+   /*
+* Signal end of framebuffer access here before hw_done. After hw_done,
+* a later commit might have already released the plane state.
+*/
+   for_each_oldnew_plane_in_state(old_state, plane, old_plane_state, 
new_plane_state, i) {
+   const struct drm_plane_helper_funcs *funcs = 
plane->helper_private;
+
+   if (funcs->end_fb_access)
+   funcs->end_fb_access(plane, new_plane_state);
+   }
 }
 EXPORT_SYMBOL(drm_atomic_helper_commit_planes);
 
@@ -2924,6 +2935,12 @@ void drm_atomic_helper_cleanup_planes(struct drm_device 
*dev,
for_each_oldnew_plane_in_state(old_state, plane, old_plane_state, 
new_plane_state, i) {
const struct drm_plane_helper_funcs *funcs = 
plane->helper_private;
 
+   /*
+* Only clean up here if we're aborting the commit.
+*/
+   if (new_plane_state == plane->state)
+   continue;
+
if (funcs->end_fb_access)
funcs->end_fb_access(plane, new_plane_state);
}
-- 
2.42.1

Re: [PATCH v18 08/26] drm/shmem-helper: Add and use lockless drm_gem_shmem_get_pages()

On Fri, 24 Nov 2023 11:47:57 +0100
Maxime Ripard  wrote:

> On Mon, Oct 30, 2023 at 02:01:47AM +0300, Dmitry Osipenko wrote:
> > Add lockless drm_gem_shmem_get_pages() helper that skips taking reservation
> > lock if pages_use_count is non-zero, leveraging from atomicity of the
> > refcount_t. Make drm_gem_shmem_mmap() to utilize the new helper.
> > 
> > Reviewed-by: Boris Brezillon 
> > Suggested-by: Boris Brezillon 
> > Signed-off-by: Dmitry Osipenko 
> > ---
> >  drivers/gpu/drm/drm_gem_shmem_helper.c | 19 +++
> >  1 file changed, 15 insertions(+), 4 deletions(-)
> > 
> > diff --git a/drivers/gpu/drm/drm_gem_shmem_helper.c 
> > b/drivers/gpu/drm/drm_gem_shmem_helper.c
> > index 6e02643ed87e..41b749bedb11 100644
> > --- a/drivers/gpu/drm/drm_gem_shmem_helper.c
> > +++ b/drivers/gpu/drm/drm_gem_shmem_helper.c
> > @@ -226,6 +226,20 @@ void drm_gem_shmem_put_pages_locked(struct 
> > drm_gem_shmem_object *shmem)
> >  }
> >  EXPORT_SYMBOL_GPL(drm_gem_shmem_put_pages_locked);
> >  
> > +static int drm_gem_shmem_get_pages(struct drm_gem_shmem_object *shmem)
> > +{
> > +   int ret;
> > +
> > +   if (refcount_inc_not_zero(&shmem->pages_use_count))
> > +   return 0;
> > +
> > +   dma_resv_lock(shmem->base.resv, NULL);
> > +   ret = drm_gem_shmem_get_pages_locked(shmem);
> > +   dma_resv_unlock(shmem->base.resv);
> > +
> > +   return ret;
> > +}
> > +  
> 
> Wait, so the locked suffix is to indicate that we need to take the lock
> before we call it? I think that's the opposite to all(?) the naming
> convention we have

If you grep for "_locked(" and "_unlocked(" in the DRM sub-tree, you'll
see it's actually mixed, with maybe a few more helpers suffixed
_locked() than we have suffixed with _unlocked().

> 
> Especially since the function name doesn't describe what the function
> does anymore, but the context in which to call it.

Well, that's the same for "_unlocked", and we do have to pick one of
the _locked/_unlocked pattern if we want to expose both flavors.

> I'm sure if I was to
> use it, I would have gotten it wrong, or at the very least been very
> confused about it.

I personally find both equally confusing tbh, but we do have cases
where we need to expose the exact same functionality without the extra
locking. I do have a slight preference for _locked though, because it's
two characters shorter ;-).

Re: [PATCH v18 04/26] drm/shmem-helper: Refactor locked/unlocked functions

On Fri, 24 Nov 2023 11:40:06 +0100
Maxime Ripard  wrote:

> On Mon, Oct 30, 2023 at 02:01:43AM +0300, Dmitry Osipenko wrote:
> > Add locked and remove unlocked postfixes from drm-shmem function names,
> > making names consistent with the drm/gem core code.
> > 
> > Reviewed-by: Boris Brezillon 
> > Suggested-by: Boris Brezillon 
> > Signed-off-by: Dmitry Osipenko   
> 
> This contradicts my earlier ack on a patch but...
> 
> > ---
> >  drivers/gpu/drm/drm_gem_shmem_helper.c| 64 +--
> >  drivers/gpu/drm/lima/lima_gem.c   |  8 +--
> >  drivers/gpu/drm/panfrost/panfrost_drv.c   |  2 +-
> >  drivers/gpu/drm/panfrost/panfrost_gem.c   |  6 +-
> >  .../gpu/drm/panfrost/panfrost_gem_shrinker.c  |  2 +-
> >  drivers/gpu/drm/panfrost/panfrost_mmu.c   |  2 +-
> >  drivers/gpu/drm/v3d/v3d_bo.c  |  4 +-
> >  drivers/gpu/drm/virtio/virtgpu_object.c   |  4 +-
> >  include/drm/drm_gem_shmem_helper.h| 36 +--
> >  9 files changed, 64 insertions(+), 64 deletions(-)
> > 
> > diff --git a/drivers/gpu/drm/drm_gem_shmem_helper.c 
> > b/drivers/gpu/drm/drm_gem_shmem_helper.c
> > index 0d61f2b3e213..154585ddae08 100644
> > --- a/drivers/gpu/drm/drm_gem_shmem_helper.c
> > +++ b/drivers/gpu/drm/drm_gem_shmem_helper.c
> > @@ -43,8 +43,8 @@ static const struct drm_gem_object_funcs 
> > drm_gem_shmem_funcs = {
> > .pin = drm_gem_shmem_object_pin,
> > .unpin = drm_gem_shmem_object_unpin,
> > .get_sg_table = drm_gem_shmem_object_get_sg_table,
> > -   .vmap = drm_gem_shmem_object_vmap,
> > -   .vunmap = drm_gem_shmem_object_vunmap,
> > +   .vmap = drm_gem_shmem_object_vmap_locked,
> > +   .vunmap = drm_gem_shmem_object_vunmap_locked,  
> 
> While I think we should indeed be consistent with the names, I would
> also expect helpers to get the locking right by default.

Wait, actually I think this patch does what you suggest already. The
_locked() prefix tells the caller: "you should take care of the locking,
I expect the lock to be held when this is hook/function is called". So
helpers without the _locked() prefix take care of the locking (which I
guess matches your 'helpers get the locking right' expectation), and
those with the _locked() prefix don't.

> 
> I'm not sure how reasonable it is, but I think I'd prefer to turn this
> around and keep the drm_gem_shmem_object_vmap/unmap helpers name, and
> convert whatever function needs to be converted to the unlock suffix so
> we get a consistent naming.

That would be an _unlocked() prefix if we do it the other way around. I
think the main confusion comes from the names of the hooks in
drm_gem_shmem_funcs. Some of them, like drm_gem_shmem_funcs::v[un]map()
are called with the GEM resv lock held, and locking is handled by the
core, others, like drm_gem_shmem_funcs::[un]pin() are called
without the GEM resv lock held, and locking is deferred to the
implementation. As I said, I don't mind prefixing hooks/helpers with
_unlocked() for those that take care of the locking, and no prefix for
those that expects locks to be held, as long as it's consistent, but I
just wanted to make sure we're on the same page :-).

Re: [PATCH v6 0/9] Fix cursor planes with virtualized drivers

2023-11-24 Thread Javier Martinez Canillas

Simon Ser  writes:

Hello Simon,

> On Wednesday, November 22nd, 2023 at 13:49, Javier Martinez Canillas 
>  wrote:
>
>> Any objections to merge the series ?
>
> No objections from me :)
>

Perfect, I'll merge this series then to unblock the mutter MR. Thanks again!

-- 
Best regards,

Javier Martinez Canillas
Core Platforms
Red Hat

Re: [PATCH v9 11/12] DONOTMERGE: arm64: meson: khadas-vim3l: add DSI panel

Hi,

On Fri, Nov 24, 2023 at 09:41:22AM +0100, Neil Armstrong wrote:
> This add nodes to support the Khadas TS050 panel on the
> Khadas VIM3 & VIM3L boards.
> 
> Signed-off-by: Neil Armstrong 
> ---
>  .../boot/dts/amlogic/meson-g12b-khadas-vim3.dtsi   |  2 +-
>  arch/arm64/boot/dts/amlogic/meson-khadas-vim3.dtsi | 74 
> ++
>  .../boot/dts/amlogic/meson-sm1-khadas-vim3l.dts|  2 +-
>  3 files changed, 76 insertions(+), 2 deletions(-)

Generally, those kind of patches still have value. Now that we are
accepting overlays, could this be converted to one and merged maybe?

Maxime


signature.asc
Description: PGP signature

Re: [PATCH v18 10/26] drm/shmem-helper: Use refcount_t for vmap_use_count

On Mon, 30 Oct 2023 02:01:49 +0300, Dmitry Osipenko wrote:
> Use refcount_t helper for vmap_use_count to make refcounting consistent
> with pages_use_count and pages_pin_count that use refcount_t. This also
> makes vmapping to benefit from the refcount_t's overflow checks.
> 
> Reviewed-by: Boris Brezillon 
> 
> [ ... ]

Acked-by: Maxime Ripard 

Thanks!
Maxime

Re: [PATCH v18 09/26] drm/shmem-helper: Switch drm_gem_shmem_vmap/vunmap to use pin/unpin

On Mon, Oct 30, 2023 at 02:01:48AM +0300, Dmitry Osipenko wrote:
> The vmapped pages shall be pinned in memory and previously get/put_pages()
> were implicitly hard-pinning/unpinning the pages. This will no longer be
> the case with addition of memory shrinker because pages_use_count > 0 won't
> determine anymore whether pages are hard-pinned (they will be soft-pinned),
> while the new pages_pin_count will do the hard-pinning. Switch the
> vmap/vunmap() to use pin/unpin() functions in a preparation of addition
> of the memory shrinker support to drm-shmem.
> 
> Reviewed-by: Boris Brezillon 
> Signed-off-by: Dmitry Osipenko 

The naming convention discussion aside, and once it's settled and fixed:
Acked-by: Maxime Ripard 

Maxime


signature.asc
Description: PGP signature

Re: [PATCH] drm/panfrost: Ignore core_mask for poweroff and sync interrupts

2023-11-24 Thread AngeloGioacchino Del Regno


Il 24/11/23 11:21, Boris Brezillon ha scritto:

On Fri, 24 Nov 2023 11:12:57 +0100
AngeloGioacchino Del Regno 
wrote:


Il 24/11/23 10:17, AngeloGioacchino Del Regno ha scritto:

Il 23/11/23 16:40, Boris Brezillon ha scritto:

On Thu, 23 Nov 2023 16:14:12 +0100
AngeloGioacchino Del Regno 
wrote:
  

Il 23/11/23 14:51, Boris Brezillon ha scritto:

On Thu, 23 Nov 2023 14:24:57 +0100
AngeloGioacchino Del Regno 
wrote:


So, while I agree that it'd be slightly more readable as a diff if those
were two different commits I do have reasons against splitting.


If we just need a quick fix to avoid PWRTRANS interrupts from kicking
in when we power-off the cores, I think we'd be better off dropping
GPU_IRQ_POWER_CHANGED[_ALL] from the value we write to GPU_INT_MASK
at [re]initialization time, and then have a separate series that fixes
the problem more generically.


But that didn't work:
https://lore.kernel.org/all/d95259b8-10cf-4ded-866c-47cbd2a44...@linaro.org/


I meant, your 'ignore-core_mask' fix + the
'drop GPU_IRQ_POWER_CHANGED[_ALL] in GPU_INT_MASK' one.

So,

https://lore.kernel.org/all/4c73f67e-174c-497e-85a5-cb053ce65...@collabora.com/
+
https://lore.kernel.org/all/d95259b8-10cf-4ded-866c-47cbd2a44...@linaro.org/



...while this "full" solution worked:
https://lore.kernel.org/all/39e9514b-087c-42eb-8d0e-f75dc620e...@linaro.org/

https://lore.kernel.org/all/5b24cc73-23aa-4837-abb9-b6d138b46...@linaro.org/


...so this *is* a "quick fix" already... :-)


It's a half-baked solution for the missing irq-synchronization-on-suspend
issue IMHO. I understand why you want it all in one patch that can serve
as a fix for 123b431f8a5c ("drm/panfrost: Really power off GPU cores in
panfrost_gpu_power_off()"), which is why I'm suggesting to go for an
even simpler diff (see below), and then fully address the
irq-synhronization-on-suspend issue in a follow-up patchset.
--->8---
diff --git a/drivers/gpu/drm/panfrost/panfrost_gpu.c
b/drivers/gpu/drm/panfrost/panfrost_gpu.c
index 09f5e1563ebd..6e2d7650cc2b 100644
--- a/drivers/gpu/drm/panfrost/panfrost_gpu.c
+++ b/drivers/gpu/drm/panfrost/panfrost_gpu.c
@@ -78,7 +78,10 @@ int panfrost_gpu_soft_reset(struct panfrost_device *pfdev)
   }
   gpu_write(pfdev, GPU_INT_CLEAR, GPU_IRQ_MASK_ALL);
-   gpu_write(pfdev, GPU_INT_MASK, GPU_IRQ_MASK_ALL);


We probably want a comment here:

 /* Only enable the interrupts we care about. */
  

+   gpu_write(pfdev, GPU_INT_MASK,
+ GPU_IRQ_MASK_ERROR |
+ GPU_IRQ_PERFCNT_SAMPLE_COMPLETED |
+ GPU_IRQ_CLEAN_CACHES_COMPLETED);


...but if we do that, the next patch(es) will contain a partial revert of this
commit, putting back this to gpu_write(pfdev, GPU_INT_MASK, GPU_IRQ_MASK_ALL)...


Why should we revert it? We're not processing the PWRTRANS interrupts
in the interrupt handler, those should never have been enabled in the
first place. The only reason we'd want to revert that change is if we
decide to do have interrupt-based waits in the poweron/off
implementation, which, as far as I'm aware, is not something we intend
to do any time soon.
  


You're right, yes. Okay, I'll push the new code soon.

Cheers!
   


Update: I was running some (rather fast) tests here because I ... felt like 
playing
with it, basically :-)

So, I had an issue with MediaTek platforms being unable to cut power to the GPU 
or
disable clocks aggressively... and after trying "this and that" I couldn't get 
it
working (in runtime suspend).

Long story short - after implementing `panfrost_{job,mmu,gpu}_suspend_irq()` 
(only
gpu irq, as you said, is a half solution), I can not only turn off clocks, but 
even
turn off GPU power supplies entirely, bringing the power consumption of the GPU
itself during *runtime* suspend to ... zero.


Very nice!



The result of this test makes me truly happy, even though complete powercut 
during
runtime suspend may not be feasible for other reasons (takes ~20ns on AVG,
MIN ~16ns, but the MAX is ~475000ns - and beware that I haven't run that for
long, I'd suspect to get up to 1-1.5ms as max time, so that's a big no).


Do you know what's taking so long? I'm disabling clks + the main power
domain in panthor (I leave the regulators enabled), but I didn't get to
measure the time it takes to enter/exit suspend. I might have to do
what you did in panfrost and have different paths for system and RPM
suspend.



That's SoC dependant... there's only one way to get runtime suspend right in 
terms
of timing, and that is to select what to do there on a per-SoC basis: this is 
why
some of them will take lots of time to turn off (or on!) clocks, because clock
controllers are not all equal: this is not only in relation to different vendors
(as in, rockchip vs nxp vs mediatek vs qcom vs...) but also for different parts
from the same vendor (as in, MSM8953 uses different clock controllers compared 
to
SM8350 and MT6795 different compared to MT6985 and MT8195).

Some of

Re: [PATCH v18 08/26] drm/shmem-helper: Add and use lockless drm_gem_shmem_get_pages()

On Mon, Oct 30, 2023 at 02:01:47AM +0300, Dmitry Osipenko wrote:
> Add lockless drm_gem_shmem_get_pages() helper that skips taking reservation
> lock if pages_use_count is non-zero, leveraging from atomicity of the
> refcount_t. Make drm_gem_shmem_mmap() to utilize the new helper.
> 
> Reviewed-by: Boris Brezillon 
> Suggested-by: Boris Brezillon 
> Signed-off-by: Dmitry Osipenko 
> ---
>  drivers/gpu/drm/drm_gem_shmem_helper.c | 19 +++
>  1 file changed, 15 insertions(+), 4 deletions(-)
> 
> diff --git a/drivers/gpu/drm/drm_gem_shmem_helper.c 
> b/drivers/gpu/drm/drm_gem_shmem_helper.c
> index 6e02643ed87e..41b749bedb11 100644
> --- a/drivers/gpu/drm/drm_gem_shmem_helper.c
> +++ b/drivers/gpu/drm/drm_gem_shmem_helper.c
> @@ -226,6 +226,20 @@ void drm_gem_shmem_put_pages_locked(struct 
> drm_gem_shmem_object *shmem)
>  }
>  EXPORT_SYMBOL_GPL(drm_gem_shmem_put_pages_locked);
>  
> +static int drm_gem_shmem_get_pages(struct drm_gem_shmem_object *shmem)
> +{
> + int ret;
> +
> + if (refcount_inc_not_zero(&shmem->pages_use_count))
> + return 0;
> +
> + dma_resv_lock(shmem->base.resv, NULL);
> + ret = drm_gem_shmem_get_pages_locked(shmem);
> + dma_resv_unlock(shmem->base.resv);
> +
> + return ret;
> +}
> +

Wait, so the locked suffix is to indicate that we need to take the lock
before we call it? I think that's the opposite to all(?) the naming
convention we have

Especially since the function name doesn't describe what the function
does anymore, but the context in which to call it. I'm sure if I was to
use it, I would have gotten it wrong, or at the very least been very
confused about it.

Maxime


signature.asc
Description: PGP signature

Re: [PATCH v18 04/26] drm/shmem-helper: Refactor locked/unlocked functions

On Fri, 24 Nov 2023 11:40:06 +0100
Maxime Ripard  wrote:

> On Mon, Oct 30, 2023 at 02:01:43AM +0300, Dmitry Osipenko wrote:
> > Add locked and remove unlocked postfixes from drm-shmem function names,
> > making names consistent with the drm/gem core code.
> > 
> > Reviewed-by: Boris Brezillon 
> > Suggested-by: Boris Brezillon 
> > Signed-off-by: Dmitry Osipenko   
> 
> This contradicts my earlier ack on a patch but...
> 
> > ---
> >  drivers/gpu/drm/drm_gem_shmem_helper.c| 64 +--
> >  drivers/gpu/drm/lima/lima_gem.c   |  8 +--
> >  drivers/gpu/drm/panfrost/panfrost_drv.c   |  2 +-
> >  drivers/gpu/drm/panfrost/panfrost_gem.c   |  6 +-
> >  .../gpu/drm/panfrost/panfrost_gem_shrinker.c  |  2 +-
> >  drivers/gpu/drm/panfrost/panfrost_mmu.c   |  2 +-
> >  drivers/gpu/drm/v3d/v3d_bo.c  |  4 +-
> >  drivers/gpu/drm/virtio/virtgpu_object.c   |  4 +-
> >  include/drm/drm_gem_shmem_helper.h| 36 +--
> >  9 files changed, 64 insertions(+), 64 deletions(-)
> > 
> > diff --git a/drivers/gpu/drm/drm_gem_shmem_helper.c 
> > b/drivers/gpu/drm/drm_gem_shmem_helper.c
> > index 0d61f2b3e213..154585ddae08 100644
> > --- a/drivers/gpu/drm/drm_gem_shmem_helper.c
> > +++ b/drivers/gpu/drm/drm_gem_shmem_helper.c
> > @@ -43,8 +43,8 @@ static const struct drm_gem_object_funcs 
> > drm_gem_shmem_funcs = {
> > .pin = drm_gem_shmem_object_pin,
> > .unpin = drm_gem_shmem_object_unpin,
> > .get_sg_table = drm_gem_shmem_object_get_sg_table,
> > -   .vmap = drm_gem_shmem_object_vmap,
> > -   .vunmap = drm_gem_shmem_object_vunmap,
> > +   .vmap = drm_gem_shmem_object_vmap_locked,
> > +   .vunmap = drm_gem_shmem_object_vunmap_locked,  
> 
> While I think we should indeed be consistent with the names, I would
> also expect helpers to get the locking right by default.
> 
> I'm not sure how reasonable it is, but I think I'd prefer to turn this
> around and keep the drm_gem_shmem_object_vmap/unmap helpers name, and
> convert whatever function needs to be converted to the unlock suffix so
> we get a consistent naming.
> 
> Does that make sense?

I don't mind, as long as it's consistent, it's just that that there's
probably more to patch if we do it the other way around.

Re: [PATCH v18 07/26] drm/shmem-helper: Use refcount_t for pages_use_count

On Mon, 30 Oct 2023 02:01:46 +0300, Dmitry Osipenko wrote:
> Use atomic refcount_t helper for pages_use_count to optimize pin/unpin
> functions by skipping reservation locking while GEM's pin refcount > 1.
> 
> Reviewed-by: Boris Brezillon 
> Suggested-by: Boris Brezillon 
> 
> [ ... ]

Acked-by: Maxime Ripard 

Thanks!
Maxime

Re: [PATCH v18 06/26] drm/shmem-helper: Add and use pages_pin_count

On Mon, 30 Oct 2023 02:01:45 +0300, Dmitry Osipenko wrote:
> Add separate pages_pin_count for tracking of whether drm-shmem pages are
> moveable or not. With the addition of memory shrinker support to drm-shmem,
> the pages_use_count will no longer determine whether pages are hard-pinned
> in memory, but whether pages exist and are soft-pinned (and could be swapped
> out). The pages_pin_count > 1 will hard-pin pages in memory.
> 
> [ ... ]

Acked-by: Maxime Ripard 

Thanks!
Maxime

Re: [PATCH v18 05/26] drm/shmem-helper: Remove obsoleted is_iomem test

On Mon, 30 Oct 2023 02:01:44 +0300, Dmitry Osipenko wrote:
> Everything that uses the mapped buffer should be agnostic to is_iomem.
> The only reason for the is_iomem test is that we're setting shmem->vaddr
> to the returned map->vaddr. Now that the shmem->vaddr code is gone, remove
> the obsoleted is_iomem test to clean up the code.
> 
> 
> [ ... ]

Acked-by: Maxime Ripard 

Thanks!
Maxime

Re: [PATCH v18 04/26] drm/shmem-helper: Refactor locked/unlocked functions

On Mon, Oct 30, 2023 at 02:01:43AM +0300, Dmitry Osipenko wrote:
> Add locked and remove unlocked postfixes from drm-shmem function names,
> making names consistent with the drm/gem core code.
> 
> Reviewed-by: Boris Brezillon 
> Suggested-by: Boris Brezillon 
> Signed-off-by: Dmitry Osipenko 

This contradicts my earlier ack on a patch but...

> ---
>  drivers/gpu/drm/drm_gem_shmem_helper.c| 64 +--
>  drivers/gpu/drm/lima/lima_gem.c   |  8 +--
>  drivers/gpu/drm/panfrost/panfrost_drv.c   |  2 +-
>  drivers/gpu/drm/panfrost/panfrost_gem.c   |  6 +-
>  .../gpu/drm/panfrost/panfrost_gem_shrinker.c  |  2 +-
>  drivers/gpu/drm/panfrost/panfrost_mmu.c   |  2 +-
>  drivers/gpu/drm/v3d/v3d_bo.c  |  4 +-
>  drivers/gpu/drm/virtio/virtgpu_object.c   |  4 +-
>  include/drm/drm_gem_shmem_helper.h| 36 +--
>  9 files changed, 64 insertions(+), 64 deletions(-)
> 
> diff --git a/drivers/gpu/drm/drm_gem_shmem_helper.c 
> b/drivers/gpu/drm/drm_gem_shmem_helper.c
> index 0d61f2b3e213..154585ddae08 100644
> --- a/drivers/gpu/drm/drm_gem_shmem_helper.c
> +++ b/drivers/gpu/drm/drm_gem_shmem_helper.c
> @@ -43,8 +43,8 @@ static const struct drm_gem_object_funcs 
> drm_gem_shmem_funcs = {
>   .pin = drm_gem_shmem_object_pin,
>   .unpin = drm_gem_shmem_object_unpin,
>   .get_sg_table = drm_gem_shmem_object_get_sg_table,
> - .vmap = drm_gem_shmem_object_vmap,
> - .vunmap = drm_gem_shmem_object_vunmap,
> + .vmap = drm_gem_shmem_object_vmap_locked,
> + .vunmap = drm_gem_shmem_object_vunmap_locked,

While I think we should indeed be consistent with the names, I would
also expect helpers to get the locking right by default.

I'm not sure how reasonable it is, but I think I'd prefer to turn this
around and keep the drm_gem_shmem_object_vmap/unmap helpers name, and
convert whatever function needs to be converted to the unlock suffix so
we get a consistent naming.

Does that make sense?
Maxime


signature.asc
Description: PGP signature

Re: [PATCH v18 03/26] drm/shmem-helper: Make all exported symbols GPL

On Mon, 30 Oct 2023 02:01:42 +0300, Dmitry Osipenko wrote:
> Make all drm-shmem exported symbols GPL to make them consistent with
> the rest of drm-shmem symbols.
> 
> Reviewed-by: Boris Brezillon 
> Signed-off-by: Dmitry Osipenko 
> 
> [ ... ]

Acked-by: Maxime Ripard 

Thanks!
Maxime

Re: [PATCH v18 02/26] drm/gem: Add _locked postfix to functions that have unlocked counterpart

On Mon, 30 Oct 2023 02:01:41 +0300, Dmitry Osipenko wrote:
> Add _locked postfix to drm_gem functions that have unlocked counterpart
> functions to make GEM functions naming more consistent and intuitive in
> regards to the locking requirements.
> 
> Reviewed-by: Boris Brezillon 
> 
> [ ... ]

Acked-by: Maxime Ripard 

Thanks!
Maxime

Re: [PATCH v18 01/26] drm/gem: Change locked/unlocked postfix of drm_gem_v/unmap() function names

On Mon, 30 Oct 2023 02:01:40 +0300, Dmitry Osipenko wrote:
> Make drm/gem API function names consistent by having locked function
> use the _locked postfix in the name, while the unlocked variants don't
> use the _unlocked postfix. Rename drm_gem_v/unmap() function names to
> make them consistent with the rest of the API functions.
> 
> 
> [ ... ]

Acked-by: Maxime Ripard 

Thanks!
Maxime

Re: [PATCH] drm/panfrost: Ignore core_mask for poweroff and sync interrupts

On Fri, 24 Nov 2023 11:12:57 +0100
AngeloGioacchino Del Regno 
wrote:

> Il 24/11/23 10:17, AngeloGioacchino Del Regno ha scritto:
> > Il 23/11/23 16:40, Boris Brezillon ha scritto:  
> >> On Thu, 23 Nov 2023 16:14:12 +0100
> >> AngeloGioacchino Del Regno 
> >> wrote:
> >>  
> >>> Il 23/11/23 14:51, Boris Brezillon ha scritto:  
>  On Thu, 23 Nov 2023 14:24:57 +0100
>  AngeloGioacchino Del Regno 
>  wrote:  
> >>>
> >>> So, while I agree that it'd be slightly more readable as a diff if 
> >>> those
> >>> were two different commits I do have reasons against splitting.  
> >>
> >> If we just need a quick fix to avoid PWRTRANS interrupts from kicking
> >> in when we power-off the cores, I think we'd be better off dropping
> >> GPU_IRQ_POWER_CHANGED[_ALL] from the value we write to GPU_INT_MASK
> >> at [re]initialization time, and then have a separate series that fixes
> >> the problem more generically.  
> >
> > But that didn't work:
> > https://lore.kernel.org/all/d95259b8-10cf-4ded-866c-47cbd2a44...@linaro.org/
> >   
> 
>  I meant, your 'ignore-core_mask' fix + the
>  'drop GPU_IRQ_POWER_CHANGED[_ALL] in GPU_INT_MASK' one.
> 
>  So,
> 
>  https://lore.kernel.org/all/4c73f67e-174c-497e-85a5-cb053ce65...@collabora.com/
>  +
>  https://lore.kernel.org/all/d95259b8-10cf-4ded-866c-47cbd2a44...@linaro.org/
>    
> >
> >
> > ...while this "full" solution worked:
> > https://lore.kernel.org/all/39e9514b-087c-42eb-8d0e-f75dc620e...@linaro.org/
> >
> > https://lore.kernel.org/all/5b24cc73-23aa-4837-abb9-b6d138b46...@linaro.org/
> >
> >
> > ...so this *is* a "quick fix" already... :-)  
> 
>  It's a half-baked solution for the missing irq-synchronization-on-suspend
>  issue IMHO. I understand why you want it all in one patch that can serve
>  as a fix for 123b431f8a5c ("drm/panfrost: Really power off GPU cores in
>  panfrost_gpu_power_off()"), which is why I'm suggesting to go for an
>  even simpler diff (see below), and then fully address the
>  irq-synhronization-on-suspend issue in a follow-up patchset.  
>  --->8---  
>  diff --git a/drivers/gpu/drm/panfrost/panfrost_gpu.c 
>  b/drivers/gpu/drm/panfrost/panfrost_gpu.c
>  index 09f5e1563ebd..6e2d7650cc2b 100644
>  --- a/drivers/gpu/drm/panfrost/panfrost_gpu.c
>  +++ b/drivers/gpu/drm/panfrost/panfrost_gpu.c
>  @@ -78,7 +78,10 @@ int panfrost_gpu_soft_reset(struct panfrost_device 
>  *pfdev)
>    }
>    gpu_write(pfdev, GPU_INT_CLEAR, GPU_IRQ_MASK_ALL);
>  -   gpu_write(pfdev, GPU_INT_MASK, GPU_IRQ_MASK_ALL);  
> >>
> >> We probably want a comment here:
> >>
> >> /* Only enable the interrupts we care about. */
> >>  
>  +   gpu_write(pfdev, GPU_INT_MASK,
>  + GPU_IRQ_MASK_ERROR |
>  + GPU_IRQ_PERFCNT_SAMPLE_COMPLETED |
>  + GPU_IRQ_CLEAN_CACHES_COMPLETED);  
> >>>
> >>> ...but if we do that, the next patch(es) will contain a partial revert of 
> >>> this
> >>> commit, putting back this to gpu_write(pfdev, GPU_INT_MASK, 
> >>> GPU_IRQ_MASK_ALL)...  
> >>
> >> Why should we revert it? We're not processing the PWRTRANS interrupts
> >> in the interrupt handler, those should never have been enabled in the
> >> first place. The only reason we'd want to revert that change is if we
> >> decide to do have interrupt-based waits in the poweron/off
> >> implementation, which, as far as I'm aware, is not something we intend
> >> to do any time soon.
> >>  
> > 
> > You're right, yes. Okay, I'll push the new code soon.
> > 
> > Cheers!
> >   
> 
> Update: I was running some (rather fast) tests here because I ... felt like 
> playing
> with it, basically :-)
> 
> So, I had an issue with MediaTek platforms being unable to cut power to the 
> GPU or
> disable clocks aggressively... and after trying "this and that" I couldn't 
> get it
> working (in runtime suspend).
> 
> Long story short - after implementing `panfrost_{job,mmu,gpu}_suspend_irq()` 
> (only
> gpu irq, as you said, is a half solution), I can not only turn off clocks, 
> but even
> turn off GPU power supplies entirely, bringing the power consumption of the 
> GPU
> itself during *runtime* suspend to ... zero.

Very nice!

> 
> The result of this test makes me truly happy, even though complete powercut 
> during
> runtime suspend may not be feasible for other reasons (takes ~20ns on AVG,
> MIN ~16ns, but the MAX is ~475000ns - and beware that I haven't run that 
> for
> long, I'd suspect to get up to 1-1.5ms as max time, so that's a big no).

Do you know what's taking so long? I'm disabling clks + the main power
domain in panthor (I leave the regulators enabled), but I didn't get to
measure the time it takes to enter/exit suspend. I might have to do
what you did in panfrost and have different paths for sys

Re: [PATCH v4] drm/test: add a test suite for GEM objects backed by shmem

2023-11-24 Thread Marco Pagani




On 2023-11-24 09:49, Maxime Ripard wrote:
> Hi,
> 
> On Thu, Nov 23, 2023 at 11:01:46AM +0100, Marco Pagani wrote:
>> +static int drm_gem_shmem_test_init(struct kunit *test)
>> +{
>> +struct device *dev;
>> +struct fake_dev {
>> +struct drm_device drm_dev;
>> +} *fdev;
>> +
> 
> [...]
> 
>> +
>> +/*
>> + * The DRM core will automatically initialize the GEM core and create
>> + * a DRM Memory Manager object which provides an address space pool
>> + * for GEM objects allocation.
>> + */
>> +fdev = drm_kunit_helper_alloc_drm_device(test, dev, struct fake_dev,
>> + drm_dev, DRIVER_GEM);
>> +KUNIT_ASSERT_NOT_ERR_OR_NULL(test, fdev);
> 
> Sorry I missed it earlier, but you don't need the intermediate structure
> if you use
> 
> struct drm_device *drm;
> 
> drm = __drm_kunit_helper_alloc_drm_device(test, dev, sizeof(*drm), 0, 
> DRIVER_GEM);
> KUNIT_ASSERT_NOT_ERR_OR_NULL(test, drm);
>

I prefer to use drm_kunit_helper_alloc_drm_device() with the intermediate
structure. It makes the code clearer, in my opinion. Initially, when
developing the suite, I was using __drm_kunit_helper_alloc_drm_device()
as most test suites do, but I feel the list of arguments including
"sizeof(*drm), 0," is less straightforward to understand.

Thanks,
Marco

Re: [PATCH] drm/panfrost: Ignore core_mask for poweroff and sync interrupts

2023-11-24 Thread AngeloGioacchino Del Regno


Il 24/11/23 10:17, AngeloGioacchino Del Regno ha scritto:

Il 23/11/23 16:40, Boris Brezillon ha scritto:

On Thu, 23 Nov 2023 16:14:12 +0100
AngeloGioacchino Del Regno 
wrote:


Il 23/11/23 14:51, Boris Brezillon ha scritto:

On Thu, 23 Nov 2023 14:24:57 +0100
AngeloGioacchino Del Regno 
wrote:


So, while I agree that it'd be slightly more readable as a diff if those
were two different commits I do have reasons against splitting.


If we just need a quick fix to avoid PWRTRANS interrupts from kicking
in when we power-off the cores, I think we'd be better off dropping
GPU_IRQ_POWER_CHANGED[_ALL] from the value we write to GPU_INT_MASK
at [re]initialization time, and then have a separate series that fixes
the problem more generically.


But that didn't work:
https://lore.kernel.org/all/d95259b8-10cf-4ded-866c-47cbd2a44...@linaro.org/


I meant, your 'ignore-core_mask' fix + the
'drop GPU_IRQ_POWER_CHANGED[_ALL] in GPU_INT_MASK' one.

So,

https://lore.kernel.org/all/4c73f67e-174c-497e-85a5-cb053ce65...@collabora.com/
+
https://lore.kernel.org/all/d95259b8-10cf-4ded-866c-47cbd2a44...@linaro.org/



...while this "full" solution worked:
https://lore.kernel.org/all/39e9514b-087c-42eb-8d0e-f75dc620e...@linaro.org/

https://lore.kernel.org/all/5b24cc73-23aa-4837-abb9-b6d138b46...@linaro.org/


...so this *is* a "quick fix" already... :-)


It's a half-baked solution for the missing irq-synchronization-on-suspend
issue IMHO. I understand why you want it all in one patch that can serve
as a fix for 123b431f8a5c ("drm/panfrost: Really power off GPU cores in
panfrost_gpu_power_off()"), which is why I'm suggesting to go for an
even simpler diff (see below), and then fully address the
irq-synhronization-on-suspend issue in a follow-up patchset.
--->8---
diff --git a/drivers/gpu/drm/panfrost/panfrost_gpu.c 
b/drivers/gpu/drm/panfrost/panfrost_gpu.c

index 09f5e1563ebd..6e2d7650cc2b 100644
--- a/drivers/gpu/drm/panfrost/panfrost_gpu.c
+++ b/drivers/gpu/drm/panfrost/panfrost_gpu.c
@@ -78,7 +78,10 @@ int panfrost_gpu_soft_reset(struct panfrost_device *pfdev)
  }
  gpu_write(pfdev, GPU_INT_CLEAR, GPU_IRQ_MASK_ALL);
-   gpu_write(pfdev, GPU_INT_MASK, GPU_IRQ_MASK_ALL);


We probably want a comment here:

/* Only enable the interrupts we care about. */


+   gpu_write(pfdev, GPU_INT_MASK,
+ GPU_IRQ_MASK_ERROR |
+ GPU_IRQ_PERFCNT_SAMPLE_COMPLETED |
+ GPU_IRQ_CLEAN_CACHES_COMPLETED);


...but if we do that, the next patch(es) will contain a partial revert of this
commit, putting back this to gpu_write(pfdev, GPU_INT_MASK, GPU_IRQ_MASK_ALL)...


Why should we revert it? We're not processing the PWRTRANS interrupts
in the interrupt handler, those should never have been enabled in the
first place. The only reason we'd want to revert that change is if we
decide to do have interrupt-based waits in the poweron/off
implementation, which, as far as I'm aware, is not something we intend
to do any time soon.



You're right, yes. Okay, I'll push the new code soon.

Cheers!



Update: I was running some (rather fast) tests here because I ... felt like 
playing
with it, basically :-)

So, I had an issue with MediaTek platforms being unable to cut power to the GPU 
or
disable clocks aggressively... and after trying "this and that" I couldn't get 
it
working (in runtime suspend).

Long story short - after implementing `panfrost_{job,mmu,gpu}_suspend_irq()` 
(only
gpu irq, as you said, is a half solution), I can not only turn off clocks, but 
even
turn off GPU power supplies entirely, bringing the power consumption of the GPU
itself during *runtime* suspend to ... zero.

The result of this test makes me truly happy, even though complete powercut 
during
runtime suspend may not be feasible for other reasons (takes ~20ns on AVG,
MIN ~16ns, but the MAX is ~475000ns - and beware that I haven't run that for
long, I'd suspect to get up to 1-1.5ms as max time, so that's a big no).

This means that I will take a day or two and I'll push both the "simple" fix for
the Really-power-off and also some more commits to add the full irq sync.

Cheers!
Angelo



I'm not sure that it's worth changing this like that, then changing it back 
right
after :-\

Anyway, if anyone else agrees with doing it and then partially revert, I have no
issues going with this one instead; what I care about ultimately is resolving 
the
regression ASAP :-)

Cheers,
Angelo


  /*
   * All in-flight jobs should have released their cycle
@@ -425,11 +428,10 @@ void panfrost_gpu_power_on(struct panfrost_device *pfdev)
   void panfrost_gpu_power_off(struct panfrost_device *pfdev)
   {
-   u64 core_mask = panfrost_get_core_mask(pfdev);
  int ret;
  u32 val;
-   gpu_write(pfdev, SHADER_PWROFF_LO, pfdev->features.shader_present & 
core_mask);

+   gpu_write(pfdev, SHADER_PWROFF_LO, pfdev->features.shader_present);
  ret = readl_relaxed_poll_timeout(

Re: [PATCH v18 26/26] drm/panfrost: Switch to generic memory shrinker