Re: [PATCH v3 07/12] drm/sched: Prevent any job recoveries after device is unplugged.

[Christian König]

Am 24.11.20 um 02:12 schrieb Luben Tuikov:

On 2020-11-23 3:06 a.m., Christian König wrote:

Am 23.11.20 um 06:37 schrieb Andrey Grodzovsky:

On 11/22/20 6:57 AM, Christian König wrote:

Am 21.11.20 um 06:21 schrieb Andrey Grodzovsky:

No point to try recovery if device is gone, it's meaningless.

I think that this should go into the device specific recovery
function and not in the scheduler.


The timeout timer is rearmed here, so this prevents any new recovery
work to restart from here
after drm_dev_unplug was executed from amdgpu_pci_remove.It will not
cover other places like
job cleanup or starting new job but those should stop once the
scheduler thread is stopped later.

Yeah, but this is rather unclean. We should probably return an error
code instead if the timer should be rearmed or not.

Christian, this is exactly my work I told you about
last week on Wednesday in our weekly meeting. And
which I wrote to you in an email last year about this
time.


Yeah, that's why I'm suggesting it here as well.


So what do we do now?


Split your patches into smaller parts and submit them chunk by chunk.

E.g. renames first and then functional changes grouped by area they change.

Regards,
Christian.



I can submit those changes without the last part,
which builds on this change.

I'm still testing the last part and was hoping
to submit it all in one sequence of patches,
after my testing.

Regards,
Luben


Christian.


Andrey



Christian.


Signed-off-by: Andrey Grodzovsky 
---
   drivers/gpu/drm/amd/amdgpu/amdgpu_fence.c |  2 +-
   drivers/gpu/drm/etnaviv/etnaviv_sched.c   |  3 ++-
   drivers/gpu/drm/lima/lima_sched.c |  3 ++-
   drivers/gpu/drm/panfrost/panfrost_job.c   |  2 +-
   drivers/gpu/drm/scheduler/sched_main.c    | 15 ++-
   drivers/gpu/drm/v3d/v3d_sched.c   | 15 ++-
   include/drm/gpu_scheduler.h   |  6 +-
   7 files changed, 35 insertions(+), 11 deletions(-)

diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_fence.c
b/drivers/gpu/drm/amd/amdgpu/amdgpu_fence.c
index d56f402..d0b0021 100644
--- a/drivers/gpu/drm/amd/amdgpu/amdgpu_fence.c
+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_fence.c
@@ -487,7 +487,7 @@ int amdgpu_fence_driver_init_ring(struct
amdgpu_ring *ring,
     r = drm_sched_init(>sched, _sched_ops,
  num_hw_submission, amdgpu_job_hang_limit,
-   timeout, ring->name);
+   timeout, ring->name, >ddev);
   if (r) {
   DRM_ERROR("Failed to create scheduler on ring %s.\n",
     ring->name);
diff --git a/drivers/gpu/drm/etnaviv/etnaviv_sched.c
b/drivers/gpu/drm/etnaviv/etnaviv_sched.c
index cd46c88..7678287 100644
--- a/drivers/gpu/drm/etnaviv/etnaviv_sched.c
+++ b/drivers/gpu/drm/etnaviv/etnaviv_sched.c
@@ -185,7 +185,8 @@ int etnaviv_sched_init(struct etnaviv_gpu *gpu)
     ret = drm_sched_init(>sched, _sched_ops,
    etnaviv_hw_jobs_limit, etnaviv_job_hang_limit,
- msecs_to_jiffies(500), dev_name(gpu->dev));
+ msecs_to_jiffies(500), dev_name(gpu->dev),
+ gpu->drm);
   if (ret)
   return ret;
   diff --git a/drivers/gpu/drm/lima/lima_sched.c
b/drivers/gpu/drm/lima/lima_sched.c
index dc6df9e..8a7e5d7ca 100644
--- a/drivers/gpu/drm/lima/lima_sched.c
+++ b/drivers/gpu/drm/lima/lima_sched.c
@@ -505,7 +505,8 @@ int lima_sched_pipe_init(struct lima_sched_pipe
*pipe, const char *name)
     return drm_sched_init(>base, _sched_ops, 1,
     lima_job_hang_limit, msecs_to_jiffies(timeout),
-  name);
+  name,
+  pipe->ldev->ddev);
   }
     void lima_sched_pipe_fini(struct lima_sched_pipe *pipe)
diff --git a/drivers/gpu/drm/panfrost/panfrost_job.c
b/drivers/gpu/drm/panfrost/panfrost_job.c
index 30e7b71..37b03b01 100644
--- a/drivers/gpu/drm/panfrost/panfrost_job.c
+++ b/drivers/gpu/drm/panfrost/panfrost_job.c
@@ -520,7 +520,7 @@ int panfrost_job_init(struct panfrost_device
*pfdev)
   ret = drm_sched_init(>queue[j].sched,
    _sched_ops,
    1, 0, msecs_to_jiffies(500),
- "pan_js");
+ "pan_js", pfdev->ddev);
   if (ret) {
   dev_err(pfdev->dev, "Failed to create scheduler: %d.",
ret);
   goto err_sched;
diff --git a/drivers/gpu/drm/scheduler/sched_main.c
b/drivers/gpu/drm/scheduler/sched_main.c
index c3f0bd0..95db8c6 100644
--- a/drivers/gpu/drm/scheduler/sched_main.c
+++ b/drivers/gpu/drm/scheduler/sched_main.c
@@ -53,6 +53,7 @@
   #include 
   #include 
   #include 
+#include 
     #define CREATE_TRACE_POINTS
   #include "gpu_scheduler_trace.h"
@@ -283,8 +284,16 @@ static void drm_sched_job_timedout(struct
work_struct *work)
   struct drm_gpu_scheduler *sched;
   struct drm_sched_job *job;
   +    int idx;
+
   sched = container_of(work, struct drm_gpu_scheduler,
work_tdr.work);
   +   

Re: [drm/fb] 6a1b34c0a3: WARNING:at_drivers/gpu/drm/drm_fb_helper.c:#drm_fb_helper_damage_work

[Thomas Zimmermann]

Hi

Am 24.11.20 um 02:58 schrieb Xing Zhengjun:



On 11/23/2020 4:04 PM, Thomas Zimmermann wrote:

Hi

Am 22.11.20 um 15:18 schrieb kernel test robot:


Greeting,

FYI, we noticed the following commit (built with gcc-9):

commit: 6a1b34c0a339fdc75d7932ad5702f2177c9d7a1c ("drm/fb-helper: 
Move damage blit code and its setup into separate routine")
url: 
https://github.com/0day-ci/linux/commits/Thomas-Zimmermann/drm-fb-helper-Various-fixes-and-cleanups/20201120-182750 




in testcase: trinity
version: trinity-static-i386-x86_64-f93256fb_2019-08-28
with following parameters:

runtime: 300s

test-description: Trinity is a linux system call fuzz tester.
test-url: http://codemonkey.org.uk/projects/trinity/


on test machine: qemu-system-i386 -enable-kvm -cpu SandyBridge -smp 2 
-m 8G


caused below changes (please refer to attached dmesg/kmsg for entire 
log/backtrace):


That dmesg is full of messages like

[  696.323556] alloc_vmap_area: 24 callbacks suppressed
[  696.323562] vmap allocation for size 3149824 failed: use 
vmalloc= to increase size


I think the test system needs to be reconfigured first.



We have tried "vmalloc=256M" and "vmalloc=512M", the same warning still 
happened.


OK, then I don't know. Thanks for testing.

Best regards
Thomas





Best regards
Thomas




+---+++ 

|   
| 154f2d1afd | 6a1b34c0a3 |
+---+++ 

| 
WARNING:at_drivers/gpu/drm/drm_fb_helper.c:#drm_fb_helper_damage_work 
| 0  | 36 |

| EIP:drm_fb_helper_damage_work | 0  | 36 |
+---+++ 




If you fix the issue, kindly add following tag
Reported-by: kernel test robot 


[  106.616652] WARNING: CPU: 1 PID: 173 at 
drivers/gpu/drm/drm_fb_helper.c:434 
drm_fb_helper_damage_work+0x371/0x390

[  106.627732] Modules linked in:
[  106.632419] CPU: 1 PID: 173 Comm: kworker/1:2 Not tainted 
5.10.0-rc4-next-20201120-7-g6a1b34c0a339 #3
[  106.637806] Hardware name: QEMU Standard PC (i440FX + PIIX, 1996), 
BIOS 1.12.0-1 04/01/2014

[  106.642853] Workqueue: events drm_fb_helper_damage_work
[  106.647664] EIP: drm_fb_helper_damage_work+0x371/0x390
[  106.652305] Code: b1 17 c7 01 68 bd 5b 2d c5 53 50 68 55 21 2d c5 
83 15 44 b1 17 c7 00 e8 ae bc b1 01 83 05 48 b1 17 c7 01 83 15 4c b1 
17 c7 00 <0f> 0b 83 05 50 b1 17 c7 01 83 15 54 b1 17 c7 00 83 c4 10 
e9 78 fd

[  106.663517] EAX: 002d EBX: c8730520 ECX: 0847 EDX: 
[  106.668423] ESI: ca987000 EDI: cab274d8 EBP: f62f5f20 ESP: f62f5ee8
[  106.673214] DS: 007b ES: 007b FS: 00d8 GS:  SS: 0068 EFLAGS: 
00010246

[  106.678295] CR0: 80050033 CR2:  CR3: 063a7000 CR4: 000406d0
[  106.683160] DR0:  DR1:  DR2:  DR3: 
[  106.687967] DR6: fffe0ff0 DR7: 0400
[  106.690763] Call Trace:
[  106.693394]  process_one_work+0x3ea/0xaa0
[  106.693501] ixgbevf: Intel(R) 10 Gigabit PCI Express Virtual 
Function Network Driver

[  106.695300]  worker_thread+0x330/0x900
[  106.697406] ixgbevf: Copyright (c) 2009 - 2018 Intel Corporation.
[  106.702963]  kthread+0x190/0x210
[  106.705709]  ? rescuer_thread+0x650/0x650
[  106.708379]  ? kthread_insert_work_sanity_check+0x120/0x120
[  106.711271]  ret_from_fork+0x1c/0x30
[  106.713973] ---[ end trace dd528799d3369ac1 ]---


To reproduce:

 # build kernel
cd linux
cp config-5.10.0-rc4-next-20201120-7-g6a1b34c0a339 .config
make HOSTCC=gcc-9 CC=gcc-9 ARCH=i386 olddefconfig prepare 
modules_prepare bzImage


 git clone https://github.com/intel/lkp-tests.git
 cd lkp-tests
 bin/lkp qemu -k  job-script # job-script is 
attached in this email




Thanks,
Oliver Sang




___
LKP mailing list -- l...@lists.01.org
To unsubscribe send an email to lkp-le...@lists.01.org





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


OpenPGP_0x680DC11D530B7A23.asc
Description: application/pgp-keys


OpenPGP_signature
Description: OpenPGP digital signature
___
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

Re: [PATCH v3 05/12] drm/ttm: Expose ttm_tt_unpopulate for driver use

[Christian König]

Am 23.11.20 um 22:08 schrieb Andrey Grodzovsky:


On 11/23/20 3:41 PM, Christian König wrote:

Am 23.11.20 um 21:38 schrieb Andrey Grodzovsky:


On 11/23/20 3:20 PM, Christian König wrote:

Am 23.11.20 um 21:05 schrieb Andrey Grodzovsky:


On 11/25/20 5:42 AM, Christian König wrote:

Am 21.11.20 um 06:21 schrieb Andrey Grodzovsky:

It's needed to drop iommu backed pages on device unplug
before device's IOMMU group is released.


It would be cleaner if we could do the whole handling in TTM. I 
also need to double check what you are doing with this function.


Christian.



Check patch "drm/amdgpu: Register IOMMU topology notifier per 
device." to see
how i use it. I don't see why this should go into TTM mid-layer - 
the stuff I do inside
is vendor specific and also I don't think TTM is explicitly aware 
of IOMMU ?
Do you mean you prefer the IOMMU notifier to be registered from 
within TTM

and then use a hook to call into vendor specific handler ?


No, that is really vendor specific.

What I meant is to have a function like 
ttm_resource_manager_evict_all() which you only need to call and 
all tt objects are unpopulated.



So instead of this BO list i create and later iterate in amdgpu from 
the IOMMU patch you just want to do it within

TTM with a single function ? Makes much more sense.


Yes, exactly.

The list_empty() checks we have in TTM for the LRU are actually not 
the best idea, we should now check the pin_count instead. This way we 
could also have a list of the pinned BOs in TTM.



So from my IOMMU topology handler I will iterate the TTM LRU for the 
unpinned BOs and this new function for the pinned ones  ?
It's probably a good idea to combine both iterations into this new 
function to cover all the BOs allocated on the device.


Yes, that's what I had in my mind as well.






BTW: Have you thought about what happens when we unpopulate a BO 
while we still try to use a kernel mapping for it? That could have 
unforeseen consequences.



Are you asking what happens to kmap or vmap style mapped CPU accesses 
once we drop all the DMA backing pages for a particular BO ? Because 
for user mappings
(mmap) we took care of this with dummy page reroute but indeed nothing 
was done for in kernel CPU mappings.


Yes exactly that.

In other words what happens if we free the ring buffer while the kernel 
still writes to it?


Christian.



Andrey




Christian.



Andrey




Give me a day or two to look into this.

Christian.



Andrey






Signed-off-by: Andrey Grodzovsky 
---
  drivers/gpu/drm/ttm/ttm_tt.c | 1 +
  1 file changed, 1 insertion(+)

diff --git a/drivers/gpu/drm/ttm/ttm_tt.c 
b/drivers/gpu/drm/ttm/ttm_tt.c

index 1ccf1ef..29248a5 100644
--- a/drivers/gpu/drm/ttm/ttm_tt.c
+++ b/drivers/gpu/drm/ttm/ttm_tt.c
@@ -495,3 +495,4 @@ void ttm_tt_unpopulate(struct ttm_tt *ttm)
  else
  ttm_pool_unpopulate(ttm);
  }
+EXPORT_SYMBOL(ttm_tt_unpopulate);



___
amd-gfx mailing list
amd-...@lists.freedesktop.org
https://nam11.safelinks.protection.outlook.com/?url=https%3A%2F%2Flists.freedesktop.org%2Fmailman%2Flistinfo%2Famd-gfxdata=04%7C01%7CAndrey.Grodzovsky%40amd.com%7C9be029f26a4746347a6108d88fed299b%7C3dd8961fe4884e608e11a82d994e183d%7C0%7C0%7C637417596065559955%7CUnknown%7CTWFpbGZsb3d8eyJWIjoiMC4wLjAwMDAiLCJQIjoiV2luMzIiLCJBTiI6Ik1haWwiLCJXVCI6Mn0%3D%7C1000sdata=tZ3do%2FeKzBtRlNaFbBjCtRvUHKdvwDZ7SoYhEBu4%2BT8%3Dreserved=0 







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

[PATCH 3/3] drm/virtio: consider dma-fence context when signaling

[Gurchetan Singh]

This an incremental refactor towards multiple dma-fence contexts
in virtio-gpu.  Since all fences are still allocated using
_gpu_fence_driver.context, nothing should break and every
processed fence will be signaled.

The overall idea is every 3D context can allocate a number of
dma-fence contexts.  Each dma-fence context refers to it's own
timeline.

For example, consider the following case where virgl submits
commands to the GPU (fence ids 1, 3) and does a metadata query with
the CPU (fence id 5).  In a different process, gfxstream submits
commands to the GPU (fence ids 2, 4).

fence_id (_fence.seqno)   | 1 2 3 4 5
--|---
fence_ctx 0 (virgl gpu)   | 1   3
fence_ctx 1 (virgl metadata query)| 5
fence_ctx 2 (gfxstream gpu)   |   2   4

With multiple fence contexts, we can wait for the metadata query
to finish without waiting for the virgl gpu to finish.  virgl gpu
does not have to wait for gfxstream gpu.  The fence id still is the
monotonically increasing sequence number, but it's only revelant to
the specific dma-fence context.

To fully enable this feature, we'll need to:
  - have each 3d context allocate a number of fence contexts. Not
too hard with explicit context initialization on the horizon.
  - have guest userspace specify fence context when performing
ioctls.
  - tag each fence emitted to the host with the fence context
information.  virtio_gpu_ctrl_hdr has padding + flags available,
so that should be easy.

This change goes in the direction specified above, by:
  - looking up the virtgpu_fence given a fence_id
  - signalling all prior fences in a given context
  - signalling current fence

v2: fix grammar in comment

Signed-off-by: Gurchetan Singh 
---
 drivers/gpu/drm/virtio/virtgpu_drv.h   |  1 +
 drivers/gpu/drm/virtio/virtgpu_fence.c | 39 --
 2 files changed, 31 insertions(+), 9 deletions(-)

diff --git a/drivers/gpu/drm/virtio/virtgpu_drv.h 
b/drivers/gpu/drm/virtio/virtgpu_drv.h
index 6a232553c99b..d9dbc4f258f3 100644
--- a/drivers/gpu/drm/virtio/virtgpu_drv.h
+++ b/drivers/gpu/drm/virtio/virtgpu_drv.h
@@ -136,6 +136,7 @@ struct virtio_gpu_fence_driver {
 
 struct virtio_gpu_fence {
struct dma_fence f;
+   uint64_t fence_id;
struct virtio_gpu_fence_driver *drv;
struct list_head node;
 };
diff --git a/drivers/gpu/drm/virtio/virtgpu_fence.c 
b/drivers/gpu/drm/virtio/virtgpu_fence.c
index b35fcd1d02d7..d28e25e8409b 100644
--- a/drivers/gpu/drm/virtio/virtgpu_fence.c
+++ b/drivers/gpu/drm/virtio/virtgpu_fence.c
@@ -51,7 +51,7 @@ static bool virtio_gpu_fence_signaled(struct dma_fence *f)
 
 static void virtio_gpu_fence_value_str(struct dma_fence *f, char *str, int 
size)
 {
-   snprintf(str, size, "%llu", f->seqno);
+   snprintf(str, size, "[%llu, %llu]", f->context, f->seqno);
 }
 
 static void virtio_gpu_timeline_value_str(struct dma_fence *f, char *str,
@@ -99,7 +99,7 @@ void virtio_gpu_fence_emit(struct virtio_gpu_device *vgdev,
unsigned long irq_flags;
 
spin_lock_irqsave(>lock, irq_flags);
-   fence->f.seqno = ++drv->current_fence_id;
+   fence->fence_id = fence->f.seqno = ++drv->current_fence_id;
dma_fence_get(>f);
list_add_tail(>node, >fences);
spin_unlock_irqrestore(>lock, irq_flags);
@@ -107,24 +107,45 @@ void virtio_gpu_fence_emit(struct virtio_gpu_device 
*vgdev,
trace_dma_fence_emit(>f);
 
cmd_hdr->flags |= cpu_to_le32(VIRTIO_GPU_FLAG_FENCE);
-   cmd_hdr->fence_id = cpu_to_le64(fence->f.seqno);
+   cmd_hdr->fence_id = cpu_to_le64(fence->fence_id);
 }
 
 void virtio_gpu_fence_event_process(struct virtio_gpu_device *vgdev,
u64 fence_id)
 {
struct virtio_gpu_fence_driver *drv = >fence_drv;
-   struct virtio_gpu_fence *fence, *tmp;
+   struct virtio_gpu_fence *signaled, *curr, *tmp;
unsigned long irq_flags;
 
spin_lock_irqsave(>lock, irq_flags);
atomic64_set(>fence_drv.last_fence_id, fence_id);
-   list_for_each_entry_safe(fence, tmp, >fences, node) {
-   if (fence_id < fence->f.seqno)
+   list_for_each_entry_safe(curr, tmp, >fences, node) {
+   if (fence_id != curr->fence_id)
continue;
-   dma_fence_signal_locked(>f);
-   list_del(>node);
-   dma_fence_put(>f);
+
+   signaled = curr;
+
+   /*
+* Signal any fences with a strictly smaller sequence number
+* than the current signaled fence.
+*/
+   list_for_each_entry_safe(curr, tmp, >fences, node) {
+   /* dma-fence contexts must match */
+   if (signaled->f.context != curr->f.context)
+   continue;
+
+   if (!dma_fence_is_later(>f, >f))
+   continue;
+
+   

[PATCH 2/3] drm/virtio: rework virtio_fence_signaled

[Gurchetan Singh]

virtio_gpu_fence_event_process sets the last_fence_id and
subsequently calls dma_fence_signal_locked(..).

dma_fence_signal_locked(..) sets DMA_FENCE_FLAG_SIGNALED_BIT,
which is actually checked before _fence_ops.(*signaled) is
called.

The check for last_fence_id is therefore a bit redundant, and
it will not be sufficient to check the last_fence_id for multiple
synchronization timelines.  Remove it.

Signed-off-by: Gurchetan Singh 
---
 drivers/gpu/drm/virtio/virtgpu_fence.c | 12 
 1 file changed, 4 insertions(+), 8 deletions(-)

diff --git a/drivers/gpu/drm/virtio/virtgpu_fence.c 
b/drivers/gpu/drm/virtio/virtgpu_fence.c
index 586034c90587..b35fcd1d02d7 100644
--- a/drivers/gpu/drm/virtio/virtgpu_fence.c
+++ b/drivers/gpu/drm/virtio/virtgpu_fence.c
@@ -42,14 +42,10 @@ static const char *virtio_gpu_get_timeline_name(struct 
dma_fence *f)
 
 static bool virtio_gpu_fence_signaled(struct dma_fence *f)
 {
-   struct virtio_gpu_fence *fence = to_virtio_gpu_fence(f);
-
-   if (WARN_ON_ONCE(fence->f.seqno == 0))
-   /* leaked fence outside driver before completing
-* initialization with virtio_gpu_fence_emit */
-   return false;
-   if (atomic64_read(>drv->last_fence_id) >= fence->f.seqno)
-   return true;
+   /* leaked fence outside driver before completing
+* initialization with virtio_gpu_fence_emit.
+*/
+   WARN_ON_ONCE(f->seqno == 0);
return false;
 }
 
-- 
2.29.2.454.gaff20da3a2-goog

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

[PATCH 3/3] drm/virtio: consider dma-fence context when signaling

[Gurchetan Singh]

This an incremental refactor towards multiple dma-fence contexts
in virtio-gpu.  Since all fences are still allocated using
_gpu_fence_driver.context, nothing should break and every
processed fence will be signaled.

The overall idea is every 3D context can allocate a number of
dma-fence contexts.  Each dma-fence context refers to it's own
timeline.

For example, consider the following case where virgl submits
commands to the GPU (fence ids 1, 3) and does a metadata query with
the CPU (fence id 5).  In a different process, gfxstream submits
commands to the GPU (fence ids 2, 4).

fence_id (_fence.seqno)   | 1 2 3 4 5
--|---
fence_ctx 0 (virgl gpu)   | 1   3
fence_ctx 1 (virgl metadata query)| 5
fence_ctx 2 (gfxstream gpu)   |   2   4

With multiple fence contexts, we can wait for the metadata query
to finish without waiting for the virgl gpu to finish.  virgl gpu
does not have to wait for gfxstream gpu.  The fence id still is the
monotonically increasing sequence number, but it's only revelant to
the specific dma-fence context.

To fully enable this feature, we'll need to:
  - have each 3d context allocate a number of fence contexts. Not
too hard with explicit context initialization on the horizon.
  - have guest userspace specify fence context when performing
ioctls.
  - tag each fence emitted to the host with the fence context
information.  virtio_gpu_ctrl_hdr has padding + flags available,
so that should be easy.

This change goes in the direction specified above, by:
  - looking up the virtgpu_fence given a fence_id
  - signalling all prior fences in a given context
  - signalling current fence

Signed-off-by: Gurchetan Singh 
---
 drivers/gpu/drm/virtio/virtgpu_drv.h   |  1 +
 drivers/gpu/drm/virtio/virtgpu_fence.c | 39 --
 2 files changed, 31 insertions(+), 9 deletions(-)

diff --git a/drivers/gpu/drm/virtio/virtgpu_drv.h 
b/drivers/gpu/drm/virtio/virtgpu_drv.h
index 6a232553c99b..d9dbc4f258f3 100644
--- a/drivers/gpu/drm/virtio/virtgpu_drv.h
+++ b/drivers/gpu/drm/virtio/virtgpu_drv.h
@@ -136,6 +136,7 @@ struct virtio_gpu_fence_driver {
 
 struct virtio_gpu_fence {
struct dma_fence f;
+   uint64_t fence_id;
struct virtio_gpu_fence_driver *drv;
struct list_head node;
 };
diff --git a/drivers/gpu/drm/virtio/virtgpu_fence.c 
b/drivers/gpu/drm/virtio/virtgpu_fence.c
index b35fcd1d02d7..3de09d78dada 100644
--- a/drivers/gpu/drm/virtio/virtgpu_fence.c
+++ b/drivers/gpu/drm/virtio/virtgpu_fence.c
@@ -51,7 +51,7 @@ static bool virtio_gpu_fence_signaled(struct dma_fence *f)
 
 static void virtio_gpu_fence_value_str(struct dma_fence *f, char *str, int 
size)
 {
-   snprintf(str, size, "%llu", f->seqno);
+   snprintf(str, size, "[%llu, %llu]", f->context, f->seqno);
 }
 
 static void virtio_gpu_timeline_value_str(struct dma_fence *f, char *str,
@@ -99,7 +99,7 @@ void virtio_gpu_fence_emit(struct virtio_gpu_device *vgdev,
unsigned long irq_flags;
 
spin_lock_irqsave(>lock, irq_flags);
-   fence->f.seqno = ++drv->current_fence_id;
+   fence->fence_id = fence->f.seqno = ++drv->current_fence_id;
dma_fence_get(>f);
list_add_tail(>node, >fences);
spin_unlock_irqrestore(>lock, irq_flags);
@@ -107,24 +107,45 @@ void virtio_gpu_fence_emit(struct virtio_gpu_device 
*vgdev,
trace_dma_fence_emit(>f);
 
cmd_hdr->flags |= cpu_to_le32(VIRTIO_GPU_FLAG_FENCE);
-   cmd_hdr->fence_id = cpu_to_le64(fence->f.seqno);
+   cmd_hdr->fence_id = cpu_to_le64(fence->fence_id);
 }
 
 void virtio_gpu_fence_event_process(struct virtio_gpu_device *vgdev,
u64 fence_id)
 {
struct virtio_gpu_fence_driver *drv = >fence_drv;
-   struct virtio_gpu_fence *fence, *tmp;
+   struct virtio_gpu_fence *signaled, *curr, *tmp;
unsigned long irq_flags;
 
spin_lock_irqsave(>lock, irq_flags);
atomic64_set(>fence_drv.last_fence_id, fence_id);
-   list_for_each_entry_safe(fence, tmp, >fences, node) {
-   if (fence_id < fence->f.seqno)
+   list_for_each_entry_safe(curr, tmp, >fences, node) {
+   if (fence_id != curr->fence_id)
continue;
-   dma_fence_signal_locked(>f);
-   list_del(>node);
-   dma_fence_put(>f);
+
+   signaled = curr;
+
+   /*
+* Signal any fences with a strictly smaller sequence number
+* the current signaled fence.
+*/
+   list_for_each_entry_safe(curr, tmp, >fences, node) {
+   /* dma-fence contexts must match */
+   if (signaled->f.context != curr->f.context)
+   continue;
+
+   if (!dma_fence_is_later(>f, >f))
+   continue;
+
+   

[PATCH 1/3] drm/virtio: virtio_{blah} --> virtio_gpu_{blah}

[Gurchetan Singh]

virtio_gpu typically uses the prefix virtio_gpu, but there are
a few places where the virtio prefix is used.  Modify this for
consistency.

Signed-off-by: Gurchetan Singh 
---
 drivers/gpu/drm/virtio/virtgpu_debugfs.c | 24 ++
 drivers/gpu/drm/virtio/virtgpu_fence.c   | 32 +---
 2 files changed, 30 insertions(+), 26 deletions(-)

diff --git a/drivers/gpu/drm/virtio/virtgpu_debugfs.c 
b/drivers/gpu/drm/virtio/virtgpu_debugfs.c
index 5fefc88d47e4..c2b20e0ee030 100644
--- a/drivers/gpu/drm/virtio/virtgpu_debugfs.c
+++ b/drivers/gpu/drm/virtio/virtgpu_debugfs.c
@@ -28,14 +28,13 @@
 
 #include "virtgpu_drv.h"
 
-static void virtio_add_bool(struct seq_file *m, const char *name,
-   bool value)
+static void virtio_gpu_add_bool(struct seq_file *m, const char *name,
+   bool value)
 {
seq_printf(m, "%-16s : %s\n", name, value ? "yes" : "no");
 }
 
-static void virtio_add_int(struct seq_file *m, const char *name,
-  int value)
+static void virtio_gpu_add_int(struct seq_file *m, const char *name, int value)
 {
seq_printf(m, "%-16s : %d\n", name, value);
 }
@@ -45,13 +44,16 @@ static int virtio_gpu_features(struct seq_file *m, void 
*data)
struct drm_info_node *node = (struct drm_info_node *)m->private;
struct virtio_gpu_device *vgdev = node->minor->dev->dev_private;
 
-   virtio_add_bool(m, "virgl", vgdev->has_virgl_3d);
-   virtio_add_bool(m, "edid", vgdev->has_edid);
-   virtio_add_bool(m, "indirect", vgdev->has_indirect);
-   virtio_add_bool(m, "resource uuid", vgdev->has_resource_assign_uuid);
-   virtio_add_bool(m, "blob resources", vgdev->has_resource_blob);
-   virtio_add_int(m, "cap sets", vgdev->num_capsets);
-   virtio_add_int(m, "scanouts", vgdev->num_scanouts);
+   virtio_gpu_add_bool(m, "virgl", vgdev->has_virgl_3d);
+   virtio_gpu_add_bool(m, "edid", vgdev->has_edid);
+   virtio_gpu_add_bool(m, "indirect", vgdev->has_indirect);
+
+   virtio_gpu_add_bool(m, "resource uuid",
+   vgdev->has_resource_assign_uuid);
+
+   virtio_gpu_add_bool(m, "blob resources", vgdev->has_resource_blob);
+   virtio_gpu_add_int(m, "cap sets", vgdev->num_capsets);
+   virtio_gpu_add_int(m, "scanouts", vgdev->num_scanouts);
if (vgdev->host_visible_region.len) {
seq_printf(m, "%-16s : 0x%lx +0x%lx\n", "host visible region",
   (unsigned long)vgdev->host_visible_region.addr,
diff --git a/drivers/gpu/drm/virtio/virtgpu_fence.c 
b/drivers/gpu/drm/virtio/virtgpu_fence.c
index 728ca36f6327..586034c90587 100644
--- a/drivers/gpu/drm/virtio/virtgpu_fence.c
+++ b/drivers/gpu/drm/virtio/virtgpu_fence.c
@@ -27,22 +27,22 @@
 
 #include "virtgpu_drv.h"
 
-#define to_virtio_fence(x) \
+#define to_virtio_gpu_fence(x) \
container_of(x, struct virtio_gpu_fence, f)
 
-static const char *virtio_get_driver_name(struct dma_fence *f)
+static const char *virtio_gpu_get_driver_name(struct dma_fence *f)
 {
return "virtio_gpu";
 }
 
-static const char *virtio_get_timeline_name(struct dma_fence *f)
+static const char *virtio_gpu_get_timeline_name(struct dma_fence *f)
 {
return "controlq";
 }
 
-static bool virtio_fence_signaled(struct dma_fence *f)
+static bool virtio_gpu_fence_signaled(struct dma_fence *f)
 {
-   struct virtio_gpu_fence *fence = to_virtio_fence(f);
+   struct virtio_gpu_fence *fence = to_virtio_gpu_fence(f);
 
if (WARN_ON_ONCE(fence->f.seqno == 0))
/* leaked fence outside driver before completing
@@ -53,25 +53,26 @@ static bool virtio_fence_signaled(struct dma_fence *f)
return false;
 }
 
-static void virtio_fence_value_str(struct dma_fence *f, char *str, int size)
+static void virtio_gpu_fence_value_str(struct dma_fence *f, char *str, int 
size)
 {
snprintf(str, size, "%llu", f->seqno);
 }
 
-static void virtio_timeline_value_str(struct dma_fence *f, char *str, int size)
+static void virtio_gpu_timeline_value_str(struct dma_fence *f, char *str,
+ int size)
 {
-   struct virtio_gpu_fence *fence = to_virtio_fence(f);
+   struct virtio_gpu_fence *fence = to_virtio_gpu_fence(f);
 
snprintf(str, size, "%llu",
 (u64)atomic64_read(>drv->last_fence_id));
 }
 
-static const struct dma_fence_ops virtio_fence_ops = {
-   .get_driver_name = virtio_get_driver_name,
-   .get_timeline_name   = virtio_get_timeline_name,
-   .signaled= virtio_fence_signaled,
-   .fence_value_str = virtio_fence_value_str,
-   .timeline_value_str  = virtio_timeline_value_str,
+static const struct dma_fence_ops virtio_gpu_fence_ops = {
+   .get_driver_name = virtio_gpu_get_driver_name,
+   .get_timeline_name   = virtio_gpu_get_timeline_name,
+   .signaled= 

[GIT PULL] exynos-drm-fixes

[Inki Dae]

Hi Dave,

   Just one bug fix to a build error due to common framework dependency.

   Please kindly let me know if there is any problem.


Thanks,
Inki Dae


The following changes since commit 6600f9d52213b5c3455481b5c9e61cf5e305c0e6:

  Merge tag 'drm-intel-fixes-2020-11-19' of 
git://anongit.freedesktop.org/drm/drm-intel into drm-fixes (2020-11-20 11:21:54 
+1000)

are available in the git repository at:

  git://git.kernel.org/pub/scm/linux/kernel/git/daeinki/drm-exynos 
tags/exynos-drm-fixes-for-v5.10-rc6

for you to fetch changes up to e2d3d2e904ad3d381753798dcd5cae03e3c47242:

  drm/exynos: depend on COMMON_CLK to fix compile tests (2020-11-23 10:01:32 
+0900)


One bug fix
. Add COMMON_CLK dependency to fix a build error below,
/usr/bin/mips-linux-gnu-ld: drivers/gpu/drm/exynos/exynos_mixer.o: in 
function `mixer_bind':
exynos_mixer.c:(.text+0x958): undefined reference to `clk_set_parent'


Krzysztof Kozlowski (1):
  drm/exynos: depend on COMMON_CLK to fix compile tests

 drivers/gpu/drm/exynos/Kconfig | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)
___
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

Re: [PATCH 000/141] Fix fall-through warnings for Clang

[Jakub Kicinski]

On Mon, 23 Nov 2020 17:32:51 -0800 Nick Desaulniers wrote:
> On Sun, Nov 22, 2020 at 8:17 AM Kees Cook  wrote:
> > On Fri, Nov 20, 2020 at 11:51:42AM -0800, Jakub Kicinski wrote:  
> > > If none of the 140 patches here fix a real bug, and there is no change
> > > to machine code then it sounds to me like a W=2 kind of a warning.  
> >
> > FWIW, this series has found at least one bug so far:
> > https://lore.kernel.org/lkml/CAFCwf11izHF=g1mGry1fE5kvFFFrxzhPSM6qKAO8gxSp=kr...@mail.gmail.com/
> >   
> 
> So looks like the bulk of these are:
> switch (x) {
>   case 0:
> ++x;
>   default:
> break;
> }
> 
> I have a patch that fixes those up for clang:
> https://reviews.llvm.org/D91895

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

Re: [PATCH v3 07/12] drm/sched: Prevent any job recoveries after device is unplugged.

[Luben Tuikov]

On 2020-11-23 3:06 a.m., Christian König wrote:
> Am 23.11.20 um 06:37 schrieb Andrey Grodzovsky:
>>
>> On 11/22/20 6:57 AM, Christian König wrote:
>>> Am 21.11.20 um 06:21 schrieb Andrey Grodzovsky:
 No point to try recovery if device is gone, it's meaningless.
>>>
>>> I think that this should go into the device specific recovery 
>>> function and not in the scheduler.
>>
>>
>> The timeout timer is rearmed here, so this prevents any new recovery 
>> work to restart from here
>> after drm_dev_unplug was executed from amdgpu_pci_remove.It will not 
>> cover other places like
>> job cleanup or starting new job but those should stop once the 
>> scheduler thread is stopped later.
> 
> Yeah, but this is rather unclean. We should probably return an error 
> code instead if the timer should be rearmed or not.

Christian, this is exactly my work I told you about
last week on Wednesday in our weekly meeting. And
which I wrote to you in an email last year about this
time.

So what do we do now?

I can submit those changes without the last part,
which builds on this change.

I'm still testing the last part and was hoping
to submit it all in one sequence of patches,
after my testing.

Regards,
Luben

> 
> Christian.
> 
>>
>> Andrey
>>
>>
>>>
>>> Christian.
>>>

 Signed-off-by: Andrey Grodzovsky 
 ---
   drivers/gpu/drm/amd/amdgpu/amdgpu_fence.c |  2 +-
   drivers/gpu/drm/etnaviv/etnaviv_sched.c   |  3 ++-
   drivers/gpu/drm/lima/lima_sched.c |  3 ++-
   drivers/gpu/drm/panfrost/panfrost_job.c   |  2 +-
   drivers/gpu/drm/scheduler/sched_main.c    | 15 ++-
   drivers/gpu/drm/v3d/v3d_sched.c   | 15 ++-
   include/drm/gpu_scheduler.h   |  6 +-
   7 files changed, 35 insertions(+), 11 deletions(-)

 diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_fence.c 
 b/drivers/gpu/drm/amd/amdgpu/amdgpu_fence.c
 index d56f402..d0b0021 100644
 --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_fence.c
 +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_fence.c
 @@ -487,7 +487,7 @@ int amdgpu_fence_driver_init_ring(struct 
 amdgpu_ring *ring,
     r = drm_sched_init(>sched, _sched_ops,
  num_hw_submission, amdgpu_job_hang_limit,
 -   timeout, ring->name);
 +   timeout, ring->name, >ddev);
   if (r) {
   DRM_ERROR("Failed to create scheduler on ring %s.\n",
     ring->name);
 diff --git a/drivers/gpu/drm/etnaviv/etnaviv_sched.c 
 b/drivers/gpu/drm/etnaviv/etnaviv_sched.c
 index cd46c88..7678287 100644
 --- a/drivers/gpu/drm/etnaviv/etnaviv_sched.c
 +++ b/drivers/gpu/drm/etnaviv/etnaviv_sched.c
 @@ -185,7 +185,8 @@ int etnaviv_sched_init(struct etnaviv_gpu *gpu)
     ret = drm_sched_init(>sched, _sched_ops,
    etnaviv_hw_jobs_limit, etnaviv_job_hang_limit,
 - msecs_to_jiffies(500), dev_name(gpu->dev));
 + msecs_to_jiffies(500), dev_name(gpu->dev),
 + gpu->drm);
   if (ret)
   return ret;
   diff --git a/drivers/gpu/drm/lima/lima_sched.c 
 b/drivers/gpu/drm/lima/lima_sched.c
 index dc6df9e..8a7e5d7ca 100644
 --- a/drivers/gpu/drm/lima/lima_sched.c
 +++ b/drivers/gpu/drm/lima/lima_sched.c
 @@ -505,7 +505,8 @@ int lima_sched_pipe_init(struct lima_sched_pipe 
 *pipe, const char *name)
     return drm_sched_init(>base, _sched_ops, 1,
     lima_job_hang_limit, msecs_to_jiffies(timeout),
 -  name);
 +  name,
 +  pipe->ldev->ddev);
   }
     void lima_sched_pipe_fini(struct lima_sched_pipe *pipe)
 diff --git a/drivers/gpu/drm/panfrost/panfrost_job.c 
 b/drivers/gpu/drm/panfrost/panfrost_job.c
 index 30e7b71..37b03b01 100644
 --- a/drivers/gpu/drm/panfrost/panfrost_job.c
 +++ b/drivers/gpu/drm/panfrost/panfrost_job.c
 @@ -520,7 +520,7 @@ int panfrost_job_init(struct panfrost_device 
 *pfdev)
   ret = drm_sched_init(>queue[j].sched,
    _sched_ops,
    1, 0, msecs_to_jiffies(500),
 - "pan_js");
 + "pan_js", pfdev->ddev);
   if (ret) {
   dev_err(pfdev->dev, "Failed to create scheduler: %d.", 
 ret);
   goto err_sched;
 diff --git a/drivers/gpu/drm/scheduler/sched_main.c 
 b/drivers/gpu/drm/scheduler/sched_main.c
 index c3f0bd0..95db8c6 100644
 --- a/drivers/gpu/drm/scheduler/sched_main.c
 +++ b/drivers/gpu/drm/scheduler/sched_main.c
 @@ -53,6 +53,7 @@
   #include 
   #include 
   #include 
 +#include 
     #define CREATE_TRACE_POINTS
   #include "gpu_scheduler_trace.h"
 @@ -283,8 +284,16 @@ static void 

Re: [PATCH 000/141] Fix fall-through warnings for Clang

[Joe Perches]

On Tue, 2020-11-24 at 11:58 +1100, Finn Thain wrote:
> it's not for me to prove that such patches don't affect code 
> generation. That's for the patch author and (unfortunately) for reviewers.

Ideally, that proof would be provided by the compilation system itself
and not patch authors nor reviewers nor maintainers.

Unfortunately gcc does not guarantee repeatability or deterministic output.
To my knowledge, neither does clang.


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

Re: [PATCH] drm/msm/dpu: update the qos remap only if the client type changes

[Rob Clark]

On Mon, Nov 23, 2020 at 4:38 PM  wrote:
>
> Hi Rob
>
> On 2020-11-23 15:18, Rob Clark wrote:
> > On Thu, Nov 19, 2020 at 1:41 PM Abhinav Kumar 
> > wrote:
> >>
> >> Update the qos remap only if the client type changes for the plane.
> >> This will avoid unnecessary register programming and also avoid log
> >> spam from the dpu_vbif_set_qos_remap() function.
> >>
> >> Signed-off-by: Abhinav Kumar 
> >> ---
> >>  drivers/gpu/drm/msm/disp/dpu1/dpu_crtc.c| 17 +
> >>  drivers/gpu/drm/msm/disp/dpu1/dpu_crtc.h|  7 +++
> >>  drivers/gpu/drm/msm/disp/dpu1/dpu_encoder.c |  2 ++
> >>  drivers/gpu/drm/msm/disp/dpu1/dpu_plane.c   | 12 ++--
> >>  drivers/gpu/drm/msm/disp/dpu1/dpu_plane.h   |  5 +
> >>  5 files changed, 41 insertions(+), 2 deletions(-)
> >>
> >> diff --git a/drivers/gpu/drm/msm/disp/dpu1/dpu_crtc.c
> >> b/drivers/gpu/drm/msm/disp/dpu1/dpu_crtc.c
> >> index d4662e8184cc..3867da47c683 100644
> >> --- a/drivers/gpu/drm/msm/disp/dpu1/dpu_crtc.c
> >> +++ b/drivers/gpu/drm/msm/disp/dpu1/dpu_crtc.c
> >> @@ -1037,6 +1037,23 @@ static int dpu_crtc_atomic_check(struct
> >> drm_crtc *crtc,
> >> return rc;
> >>  }
> >>
> >> +void dpu_crtc_set_qos_dirty(struct drm_crtc *crtc)
> >> +{
> >> +   struct drm_plane *plane;
> >> +   struct drm_plane_state *state;
> >> +   struct dpu_plane_state *pstate;
> >> +
> >> +   drm_atomic_crtc_for_each_plane(plane, crtc) {
> >> +   state = plane->state;
> >> +   if (!state)
> >> +   continue;
> >> +
> >> +   pstate = to_dpu_plane_state(state);
> >> +
> >> +   pstate->dirty |= DPU_PLANE_DIRTY_QOS;
> >> +   }
> >> +}
> >> +
> >>  int dpu_crtc_vblank(struct drm_crtc *crtc, bool en)
> >>  {
> >> struct dpu_crtc *dpu_crtc = to_dpu_crtc(crtc);
> >> diff --git a/drivers/gpu/drm/msm/disp/dpu1/dpu_crtc.h
> >> b/drivers/gpu/drm/msm/disp/dpu1/dpu_crtc.h
> >> index cec3474340e8..8ba11de605bc 100644
> >> --- a/drivers/gpu/drm/msm/disp/dpu1/dpu_crtc.h
> >> +++ b/drivers/gpu/drm/msm/disp/dpu1/dpu_crtc.h
> >> @@ -231,6 +231,13 @@ static inline int dpu_crtc_frame_pending(struct
> >> drm_crtc *crtc)
> >>   */
> >>  int dpu_crtc_vblank(struct drm_crtc *crtc, bool en);
> >>
> >> +/**
> >> + * dpu_crtc_set_qos_dirty - update plane dirty flag to include
> >> + * QoS reprogramming
> >> + * @crtc: Pointer to drm crtc structure
> >> + */
> >> +void dpu_crtc_set_qos_dirty(struct drm_crtc *crtc);
> >> +
> >>  /**
> >>   * dpu_crtc_vblank_callback - called on vblank irq, issues completion
> >> events
> >>   * @crtc: Pointer to drm crtc object
> >> diff --git a/drivers/gpu/drm/msm/disp/dpu1/dpu_encoder.c
> >> b/drivers/gpu/drm/msm/disp/dpu1/dpu_encoder.c
> >> index f7f5c258b553..c2db9dd6ec67 100644
> >> --- a/drivers/gpu/drm/msm/disp/dpu1/dpu_encoder.c
> >> +++ b/drivers/gpu/drm/msm/disp/dpu1/dpu_encoder.c
> >> @@ -1001,6 +1001,8 @@ static void dpu_encoder_virt_mode_set(struct
> >> drm_encoder *drm_enc,
> >>
> >> trace_dpu_enc_mode_set(DRMID(drm_enc));
> >>
> >> +   dpu_crtc_set_qos_dirty(drm_enc->crtc);
> >> +
> >> if (drm_enc->encoder_type == DRM_MODE_ENCODER_TMDS &&
> >> priv->dp)
> >> msm_dp_display_mode_set(priv->dp, drm_enc, mode,
> >> adj_mode);
> >>
> >> diff --git a/drivers/gpu/drm/msm/disp/dpu1/dpu_plane.c
> >> b/drivers/gpu/drm/msm/disp/dpu1/dpu_plane.c
> >> index 7ea90d25a3b6..f91d31a31e14 100644
> >> --- a/drivers/gpu/drm/msm/disp/dpu1/dpu_plane.c
> >> +++ b/drivers/gpu/drm/msm/disp/dpu1/dpu_plane.c
> >> @@ -1066,6 +1066,7 @@ static void dpu_plane_sspp_atomic_update(struct
> >> drm_plane *plane)
> >> struct dpu_plane_state *pstate = to_dpu_plane_state(state);
> >> struct drm_crtc *crtc = state->crtc;
> >> struct drm_framebuffer *fb = state->fb;
> >> +   bool is_rt_pipe;
> >> const struct dpu_format *fmt =
> >> to_dpu_format(msm_framebuffer_format(fb));
> >>
> >> @@ -1075,7 +1076,7 @@ static void dpu_plane_sspp_atomic_update(struct
> >> drm_plane *plane)
> >>
> >> pstate->pending = true;
> >>
> >> -   pdpu->is_rt_pipe = (dpu_crtc_get_client_type(crtc) !=
> >> NRT_CLIENT);
> >> +   is_rt_pipe = (dpu_crtc_get_client_type(crtc) != NRT_CLIENT);
> >> _dpu_plane_set_qos_ctrl(plane, false,
> >> DPU_PLANE_QOS_PANIC_CTRL);
> >>
> >> DPU_DEBUG_PLANE(pdpu, "FB[%u] " DRM_RECT_FP_FMT "->crtc%u "
> >> DRM_RECT_FMT
> >> @@ -1181,8 +1182,15 @@ static void dpu_plane_sspp_atomic_update(struct
> >> drm_plane *plane)
> >> _dpu_plane_set_ot_limit(plane, crtc);
> >> }
> >>
> >> -   _dpu_plane_set_qos_remap(plane);
> >> +   if (is_rt_pipe != pdpu->is_rt_pipe) {
> >> +   pdpu->is_rt_pipe = is_rt_pipe;
> >> +   pstate->dirty |= DPU_PLANE_DIRTY_QOS;
> >> +   }
> >>
> >> +   if (pstate->dirty & DPU_PLANE_DIRTY_QOS) {
> >> +   _dpu_plane_set_qos_remap(plane);
> >> +   

RE: [PATCH] drm/amd/powerplay: fix spelling mistake "smu_state_memroy_block" -> "smu_state_memory_block"

[Quan, Evan]

[AMD Official Use Only - Internal Distribution Only]

Reviewed-by: Evan Quan 

-Original Message-
From: Colin King 
Sent: Monday, November 23, 2020 6:54 PM
To: Deucher, Alexander ; Koenig, Christian 
; David Airlie ; Daniel Vetter 
; Quan, Evan ; Wang, Kevin(Yang) 
; Gui, Jack ; 
amd-...@lists.freedesktop.org; dri-devel@lists.freedesktop.org
Cc: kernel-janit...@vger.kernel.org; linux-ker...@vger.kernel.org
Subject: [PATCH] drm/amd/powerplay: fix spelling mistake 
"smu_state_memroy_block" -> "smu_state_memory_block"

From: Colin Ian King 

The struct name smu_state_memroy_block contains a spelling mistake, rename it 
to smu_state_memory_block

Fixes: 8554e67d6e22 ("drm/amd/powerplay: implement power_dpm_state sys 
interface for SMU11")
Signed-off-by: Colin Ian King 
---
 drivers/gpu/drm/amd/pm/inc/amdgpu_smu.h | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/drivers/gpu/drm/amd/pm/inc/amdgpu_smu.h 
b/drivers/gpu/drm/amd/pm/inc/amdgpu_smu.h
index 7550757cc059..a559ea2204c1 100644
--- a/drivers/gpu/drm/amd/pm/inc/amdgpu_smu.h
+++ b/drivers/gpu/drm/amd/pm/inc/amdgpu_smu.h
@@ -99,7 +99,7 @@ struct smu_state_display_block {
 bool  enable_vari_bright;
 };

-struct smu_state_memroy_block {
+struct smu_state_memory_block {
 bool  dll_off;
 uint8_t m3arb;
 uint8_t unused[3];
@@ -146,7 +146,7 @@ struct smu_power_state {
 struct smu_state_validation_block validation;
 struct smu_state_pcie_block   pcie;
 struct smu_state_display_blockdisplay;
-struct smu_state_memroy_block memory;
+struct smu_state_memory_block memory;
 struct smu_state_software_algorithm_block software;
 struct smu_uvd_clocks uvd_clocks;
 struct smu_hw_power_state hardware;
--
2.28.0

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

Re: [PATCH] drm/msm/dpu: update the qos remap only if the client type changes

[abhinavk]

Hi Rob

On 2020-11-23 15:18, Rob Clark wrote:
On Thu, Nov 19, 2020 at 1:41 PM Abhinav Kumar  
wrote:


Update the qos remap only if the client type changes for the plane.
This will avoid unnecessary register programming and also avoid log
spam from the dpu_vbif_set_qos_remap() function.

Signed-off-by: Abhinav Kumar 
---
 drivers/gpu/drm/msm/disp/dpu1/dpu_crtc.c| 17 +
 drivers/gpu/drm/msm/disp/dpu1/dpu_crtc.h|  7 +++
 drivers/gpu/drm/msm/disp/dpu1/dpu_encoder.c |  2 ++
 drivers/gpu/drm/msm/disp/dpu1/dpu_plane.c   | 12 ++--
 drivers/gpu/drm/msm/disp/dpu1/dpu_plane.h   |  5 +
 5 files changed, 41 insertions(+), 2 deletions(-)

diff --git a/drivers/gpu/drm/msm/disp/dpu1/dpu_crtc.c 
b/drivers/gpu/drm/msm/disp/dpu1/dpu_crtc.c

index d4662e8184cc..3867da47c683 100644
--- a/drivers/gpu/drm/msm/disp/dpu1/dpu_crtc.c
+++ b/drivers/gpu/drm/msm/disp/dpu1/dpu_crtc.c
@@ -1037,6 +1037,23 @@ static int dpu_crtc_atomic_check(struct 
drm_crtc *crtc,

return rc;
 }

+void dpu_crtc_set_qos_dirty(struct drm_crtc *crtc)
+{
+   struct drm_plane *plane;
+   struct drm_plane_state *state;
+   struct dpu_plane_state *pstate;
+
+   drm_atomic_crtc_for_each_plane(plane, crtc) {
+   state = plane->state;
+   if (!state)
+   continue;
+
+   pstate = to_dpu_plane_state(state);
+
+   pstate->dirty |= DPU_PLANE_DIRTY_QOS;
+   }
+}
+
 int dpu_crtc_vblank(struct drm_crtc *crtc, bool en)
 {
struct dpu_crtc *dpu_crtc = to_dpu_crtc(crtc);
diff --git a/drivers/gpu/drm/msm/disp/dpu1/dpu_crtc.h 
b/drivers/gpu/drm/msm/disp/dpu1/dpu_crtc.h

index cec3474340e8..8ba11de605bc 100644
--- a/drivers/gpu/drm/msm/disp/dpu1/dpu_crtc.h
+++ b/drivers/gpu/drm/msm/disp/dpu1/dpu_crtc.h
@@ -231,6 +231,13 @@ static inline int dpu_crtc_frame_pending(struct 
drm_crtc *crtc)

  */
 int dpu_crtc_vblank(struct drm_crtc *crtc, bool en);

+/**
+ * dpu_crtc_set_qos_dirty - update plane dirty flag to include
+ * QoS reprogramming
+ * @crtc: Pointer to drm crtc structure
+ */
+void dpu_crtc_set_qos_dirty(struct drm_crtc *crtc);
+
 /**
  * dpu_crtc_vblank_callback - called on vblank irq, issues completion 
events

  * @crtc: Pointer to drm crtc object
diff --git a/drivers/gpu/drm/msm/disp/dpu1/dpu_encoder.c 
b/drivers/gpu/drm/msm/disp/dpu1/dpu_encoder.c

index f7f5c258b553..c2db9dd6ec67 100644
--- a/drivers/gpu/drm/msm/disp/dpu1/dpu_encoder.c
+++ b/drivers/gpu/drm/msm/disp/dpu1/dpu_encoder.c
@@ -1001,6 +1001,8 @@ static void dpu_encoder_virt_mode_set(struct 
drm_encoder *drm_enc,


trace_dpu_enc_mode_set(DRMID(drm_enc));

+   dpu_crtc_set_qos_dirty(drm_enc->crtc);
+
if (drm_enc->encoder_type == DRM_MODE_ENCODER_TMDS && 
priv->dp)
msm_dp_display_mode_set(priv->dp, drm_enc, mode, 
adj_mode);


diff --git a/drivers/gpu/drm/msm/disp/dpu1/dpu_plane.c 
b/drivers/gpu/drm/msm/disp/dpu1/dpu_plane.c

index 7ea90d25a3b6..f91d31a31e14 100644
--- a/drivers/gpu/drm/msm/disp/dpu1/dpu_plane.c
+++ b/drivers/gpu/drm/msm/disp/dpu1/dpu_plane.c
@@ -1066,6 +1066,7 @@ static void dpu_plane_sspp_atomic_update(struct 
drm_plane *plane)

struct dpu_plane_state *pstate = to_dpu_plane_state(state);
struct drm_crtc *crtc = state->crtc;
struct drm_framebuffer *fb = state->fb;
+   bool is_rt_pipe;
const struct dpu_format *fmt =
to_dpu_format(msm_framebuffer_format(fb));

@@ -1075,7 +1076,7 @@ static void dpu_plane_sspp_atomic_update(struct 
drm_plane *plane)


pstate->pending = true;

-   pdpu->is_rt_pipe = (dpu_crtc_get_client_type(crtc) != 
NRT_CLIENT);

+   is_rt_pipe = (dpu_crtc_get_client_type(crtc) != NRT_CLIENT);
_dpu_plane_set_qos_ctrl(plane, false, 
DPU_PLANE_QOS_PANIC_CTRL);


DPU_DEBUG_PLANE(pdpu, "FB[%u] " DRM_RECT_FP_FMT "->crtc%u " 
DRM_RECT_FMT
@@ -1181,8 +1182,15 @@ static void dpu_plane_sspp_atomic_update(struct 
drm_plane *plane)

_dpu_plane_set_ot_limit(plane, crtc);
}

-   _dpu_plane_set_qos_remap(plane);
+   if (is_rt_pipe != pdpu->is_rt_pipe) {
+   pdpu->is_rt_pipe = is_rt_pipe;
+   pstate->dirty |= DPU_PLANE_DIRTY_QOS;
+   }

+   if (pstate->dirty & DPU_PLANE_DIRTY_QOS) {
+   _dpu_plane_set_qos_remap(plane);
+   pstate->dirty = 0x0;
+   }


So in the end, this looks roughly like "set qos remap on modesets or
switching between right/left pipe"?  Couldn't this be simpler if in
plane->atomic_check() you do something like:

   dpu_plane_state->needs_qos_remap = atomic_state->allow_modeset;

and then in plane->atomic_update:

if (pstate->needs_qos_remap || (is_rt_pipe != pdpu->is_rt_pipe) {
 pdpu->is_rt_pipe = is_rt_pipe;
 _dpu_plane_set_qos_remap(plane)
}

?

BR,
-R
Thanks for the suggestion, Yes this will make it much simpler. Let me 
update it.


Just one clarification, I believe you meant that

[GIT PULL] mediatek drm fixes for 5.10

[Chun-Kuang Hu]

Hi, Dave & Daniel:

This includes:

1. Remove unused variable.
2. Modify horizontal front/back porch byte formula.

Regards,
Chun-Kuang.

The following changes since commit 3650b228f83adda7e5ee532e2b90429c03f7b9ec:

  Linux 5.10-rc1 (2020-10-25 15:14:11 -0700)

are available in the Git repository at:

  https://git.kernel.org/pub/scm/linux/kernel/git/chunkuang.hu/linux.git 
tags/mediatek-drm-fixes-5.10

for you to fetch changes up to 487778f8d22fcdebb6436f0a5f96484ffa237b0b:

  drm/mediatek: dsi: Modify horizontal front/back porch byte formula 
(2020-11-21 18:31:07 +0800)


Mediatek DRM Fixes for Linux 5.10

1. Remove unused variable.
2. Modify horizontal front/back porch byte formula.


CK Hu (1):
  drm/mediatek: dsi: Modify horizontal front/back porch byte formula

Enric Balletbo i Serra (1):
  drm/mediatek: mtk_dpi: Fix unused variable 'mtk_dpi_encoder_funcs'

 drivers/gpu/drm/mediatek/mtk_dpi.c |  9 --
 drivers/gpu/drm/mediatek/mtk_dsi.c | 61 ++
 2 files changed, 22 insertions(+), 48 deletions(-)
___
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

Re: [PATCH] drm/msm/dpu: update the qos remap only if the client type changes

[Rob Clark]

On Thu, Nov 19, 2020 at 1:41 PM Abhinav Kumar  wrote:
>
> Update the qos remap only if the client type changes for the plane.
> This will avoid unnecessary register programming and also avoid log
> spam from the dpu_vbif_set_qos_remap() function.
>
> Signed-off-by: Abhinav Kumar 
> ---
>  drivers/gpu/drm/msm/disp/dpu1/dpu_crtc.c| 17 +
>  drivers/gpu/drm/msm/disp/dpu1/dpu_crtc.h|  7 +++
>  drivers/gpu/drm/msm/disp/dpu1/dpu_encoder.c |  2 ++
>  drivers/gpu/drm/msm/disp/dpu1/dpu_plane.c   | 12 ++--
>  drivers/gpu/drm/msm/disp/dpu1/dpu_plane.h   |  5 +
>  5 files changed, 41 insertions(+), 2 deletions(-)
>
> diff --git a/drivers/gpu/drm/msm/disp/dpu1/dpu_crtc.c 
> b/drivers/gpu/drm/msm/disp/dpu1/dpu_crtc.c
> index d4662e8184cc..3867da47c683 100644
> --- a/drivers/gpu/drm/msm/disp/dpu1/dpu_crtc.c
> +++ b/drivers/gpu/drm/msm/disp/dpu1/dpu_crtc.c
> @@ -1037,6 +1037,23 @@ static int dpu_crtc_atomic_check(struct drm_crtc *crtc,
> return rc;
>  }
>
> +void dpu_crtc_set_qos_dirty(struct drm_crtc *crtc)
> +{
> +   struct drm_plane *plane;
> +   struct drm_plane_state *state;
> +   struct dpu_plane_state *pstate;
> +
> +   drm_atomic_crtc_for_each_plane(plane, crtc) {
> +   state = plane->state;
> +   if (!state)
> +   continue;
> +
> +   pstate = to_dpu_plane_state(state);
> +
> +   pstate->dirty |= DPU_PLANE_DIRTY_QOS;
> +   }
> +}
> +
>  int dpu_crtc_vblank(struct drm_crtc *crtc, bool en)
>  {
> struct dpu_crtc *dpu_crtc = to_dpu_crtc(crtc);
> diff --git a/drivers/gpu/drm/msm/disp/dpu1/dpu_crtc.h 
> b/drivers/gpu/drm/msm/disp/dpu1/dpu_crtc.h
> index cec3474340e8..8ba11de605bc 100644
> --- a/drivers/gpu/drm/msm/disp/dpu1/dpu_crtc.h
> +++ b/drivers/gpu/drm/msm/disp/dpu1/dpu_crtc.h
> @@ -231,6 +231,13 @@ static inline int dpu_crtc_frame_pending(struct drm_crtc 
> *crtc)
>   */
>  int dpu_crtc_vblank(struct drm_crtc *crtc, bool en);
>
> +/**
> + * dpu_crtc_set_qos_dirty - update plane dirty flag to include
> + * QoS reprogramming
> + * @crtc: Pointer to drm crtc structure
> + */
> +void dpu_crtc_set_qos_dirty(struct drm_crtc *crtc);
> +
>  /**
>   * dpu_crtc_vblank_callback - called on vblank irq, issues completion events
>   * @crtc: Pointer to drm crtc object
> diff --git a/drivers/gpu/drm/msm/disp/dpu1/dpu_encoder.c 
> b/drivers/gpu/drm/msm/disp/dpu1/dpu_encoder.c
> index f7f5c258b553..c2db9dd6ec67 100644
> --- a/drivers/gpu/drm/msm/disp/dpu1/dpu_encoder.c
> +++ b/drivers/gpu/drm/msm/disp/dpu1/dpu_encoder.c
> @@ -1001,6 +1001,8 @@ static void dpu_encoder_virt_mode_set(struct 
> drm_encoder *drm_enc,
>
> trace_dpu_enc_mode_set(DRMID(drm_enc));
>
> +   dpu_crtc_set_qos_dirty(drm_enc->crtc);
> +
> if (drm_enc->encoder_type == DRM_MODE_ENCODER_TMDS && priv->dp)
> msm_dp_display_mode_set(priv->dp, drm_enc, mode, adj_mode);
>
> diff --git a/drivers/gpu/drm/msm/disp/dpu1/dpu_plane.c 
> b/drivers/gpu/drm/msm/disp/dpu1/dpu_plane.c
> index 7ea90d25a3b6..f91d31a31e14 100644
> --- a/drivers/gpu/drm/msm/disp/dpu1/dpu_plane.c
> +++ b/drivers/gpu/drm/msm/disp/dpu1/dpu_plane.c
> @@ -1066,6 +1066,7 @@ static void dpu_plane_sspp_atomic_update(struct 
> drm_plane *plane)
> struct dpu_plane_state *pstate = to_dpu_plane_state(state);
> struct drm_crtc *crtc = state->crtc;
> struct drm_framebuffer *fb = state->fb;
> +   bool is_rt_pipe;
> const struct dpu_format *fmt =
> to_dpu_format(msm_framebuffer_format(fb));
>
> @@ -1075,7 +1076,7 @@ static void dpu_plane_sspp_atomic_update(struct 
> drm_plane *plane)
>
> pstate->pending = true;
>
> -   pdpu->is_rt_pipe = (dpu_crtc_get_client_type(crtc) != NRT_CLIENT);
> +   is_rt_pipe = (dpu_crtc_get_client_type(crtc) != NRT_CLIENT);
> _dpu_plane_set_qos_ctrl(plane, false, DPU_PLANE_QOS_PANIC_CTRL);
>
> DPU_DEBUG_PLANE(pdpu, "FB[%u] " DRM_RECT_FP_FMT "->crtc%u " 
> DRM_RECT_FMT
> @@ -1181,8 +1182,15 @@ static void dpu_plane_sspp_atomic_update(struct 
> drm_plane *plane)
> _dpu_plane_set_ot_limit(plane, crtc);
> }
>
> -   _dpu_plane_set_qos_remap(plane);
> +   if (is_rt_pipe != pdpu->is_rt_pipe) {
> +   pdpu->is_rt_pipe = is_rt_pipe;
> +   pstate->dirty |= DPU_PLANE_DIRTY_QOS;
> +   }
>
> +   if (pstate->dirty & DPU_PLANE_DIRTY_QOS) {
> +   _dpu_plane_set_qos_remap(plane);
> +   pstate->dirty = 0x0;
> +   }

So in the end, this looks roughly like "set qos remap on modesets or
switching between right/left pipe"?  Couldn't this be simpler if in
plane->atomic_check() you do something like:

   dpu_plane_state->needs_qos_remap = atomic_state->allow_modeset;

and then in plane->atomic_update:

if (pstate->needs_qos_remap || (is_rt_pipe != pdpu->is_rt_pipe) {
 pdpu->is_rt_pipe = is_rt_pipe;
 _dpu_plane_set_qos_remap(plane)
}

?

BR,

Re: [PATCH 079/141] drm: Fix fall-through warnings for Clang

[Gustavo A. R. Silva]

On Sun, Nov 22, 2020 at 11:03:22PM +0100, Sam Ravnborg wrote:
> Hi Gustavo,
> On Fri, Nov 20, 2020 at 12:35:17PM -0600, Gustavo A. R. Silva wrote:
> > In preparation to enable -Wimplicit-fallthrough for Clang, fix a warning
> > by explicitly adding a break statement instead of letting the code fall
> > through to the next case.
> > 
> > Link: https://github.com/KSPP/linux/issues/115
> > Signed-off-by: Gustavo A. R. Silva 
> 
> thanks, applied to drm-misc-next.

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

Re: [PATCH 028/141] drm/amd/display: Fix fall-through warnings for Clang

[Gustavo A. R. Silva]

On Fri, Nov 20, 2020 at 05:45:07PM -0500, Alex Deucher wrote:
> On Fri, Nov 20, 2020 at 1:28 PM Gustavo A. R. Silva
>  wrote:
> >
> > In preparation to enable -Wimplicit-fallthrough for Clang, fix multiple
> > warnings by explicitly adding multiple break statements instead of just
> > letting the code fall through to the next case.
> >
> > Link: https://github.com/KSPP/linux/issues/115
> > Signed-off-by: Gustavo A. R. Silva 
> 
> Applied.  Thanks!

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

Re: [PATCH 004/141] drm/amdgpu: Fix fall-through warnings for Clang

[Gustavo A. R. Silva]

On Fri, Nov 20, 2020 at 05:42:38PM -0500, Alex Deucher wrote:
> On Fri, Nov 20, 2020 at 1:24 PM Gustavo A. R. Silva
>  wrote:
> >
> > In preparation to enable -Wimplicit-fallthrough for Clang, fix multiple
> > warnings by explicitly adding multiple break statements instead of just
> > letting the code fall through to the next case.
> >
> > Link: https://github.com/KSPP/linux/issues/115
> > Signed-off-by: Gustavo A. R. Silva 
> 
> Applied.  Thanks!

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

Re: [PATCH 2/2] drm/mcde: Support DPI output

[Sam Ravnborg]

Hi Linus.

On Thu, Nov 12, 2020 at 03:29:25PM +0100, Linus Walleij wrote:
> This implements support for DPI output using the port node
> in the device tree to connect a DPI LCD display to the
> MCDE. The block also supports TV-out but we leave that
> for another day when we have a hardware using it.
> 
> We implement parsing and handling of the "port" node,
> and follow that to the DPI endpoint.
> 
> The clock divider used by the MCDE to divide down the
> "lcdclk" (this has been designed for TV-like frequencies)
> is represented by an ordinary clock provider internally
> in the MCDE. This idea was inspired by the PL111 solution
> by Eric Anholt: the divider also works very similar to
> the Pl111 clock divider.
> 
> We take care to clear up some errors regarding the number
> of available formatters and their type. We have 6 DSI
> formatters and 2 DPI formatters.
> 
> Tested on the Samsung GT-I9070 Janice mobile phone.
> 
> Cc: Stephan Gerhold 
> Cc: phone-de...@vger.kernel.org
> Cc: upstream...@lists.sr.ht
> Signed-off-by: Linus Walleij 
> ---
>  drivers/gpu/drm/mcde/Kconfig |   1 +
>  drivers/gpu/drm/mcde/Makefile|   2 +-
>  drivers/gpu/drm/mcde/mcde_clk_div.c  | 192 ++
>  drivers/gpu/drm/mcde/mcde_display.c  | 304 ---
>  drivers/gpu/drm/mcde/mcde_display_regs.h |  87 +++
>  drivers/gpu/drm/mcde/mcde_drm.h  |  10 +
>  drivers/gpu/drm/mcde/mcde_drv.c  |  46 +++-
>  7 files changed, 595 insertions(+), 47 deletions(-)
>  create mode 100644 drivers/gpu/drm/mcde/mcde_clk_div.c
> 
> diff --git a/drivers/gpu/drm/mcde/Kconfig b/drivers/gpu/drm/mcde/Kconfig
> index b3990126562c..71c689b573c9 100644
> --- a/drivers/gpu/drm/mcde/Kconfig
> +++ b/drivers/gpu/drm/mcde/Kconfig
> @@ -4,6 +4,7 @@ config DRM_MCDE
>   depends on CMA
>   depends on ARM || COMPILE_TEST
>   depends on OF
> + depends on COMMON_CLK
>   select MFD_SYSCON
>   select DRM_MIPI_DSI
>   select DRM_BRIDGE
> diff --git a/drivers/gpu/drm/mcde/Makefile b/drivers/gpu/drm/mcde/Makefile
> index fe28f4e0fe46..15d9c89a3273 100644
> --- a/drivers/gpu/drm/mcde/Makefile
> +++ b/drivers/gpu/drm/mcde/Makefile
> @@ -1,3 +1,3 @@
> -mcde_drm-y +=mcde_drv.o mcde_dsi.o mcde_display.o
> +mcde_drm-y +=mcde_drv.o mcde_dsi.o mcde_clk_div.o mcde_display.o
>  
>  obj-$(CONFIG_DRM_MCDE) += mcde_drm.o
> diff --git a/drivers/gpu/drm/mcde/mcde_clk_div.c 
> b/drivers/gpu/drm/mcde/mcde_clk_div.c
> new file mode 100644
> index ..038821d2ef80
> --- /dev/null
> +++ b/drivers/gpu/drm/mcde/mcde_clk_div.c
> @@ -0,0 +1,192 @@
> +// SPDX-License-Identifier: GPL-2.0
> +#include 
> +#include 
> +
> +#include "mcde_drm.h"
> +#include "mcde_display_regs.h"
> +
> +/* The MCDE internal clock dividers for FIFO A and B */
> +struct mcde_clk_div {
> + struct clk_hw hw;
> + struct mcde *mcde;
> + u32 cr;
> + u32 cr_div;
> +};
> +
> +static int mcde_clk_div_enable(struct clk_hw *hw)
> +{
> + struct mcde_clk_div *cdiv = container_of(hw, struct mcde_clk_div, hw);
> + struct mcde *mcde = cdiv->mcde;
> + u32 val;
> +
> + spin_lock(>fifo_crx1_lock);
> + val = readl(mcde->regs + cdiv->cr);
> + /*
> +  * Select the PLL72 (LCD) clock as parent
> +  * FIXME: implement other parents.
> +  */
> + val &= ~MCDE_CRX1_CLKSEL_MASK;
> + val |= MCDE_CRX1_CLKSEL_CLKPLL72 << MCDE_CRX1_CLKSEL_SHIFT;
> + /* Internal clock */
> + val |= MCDE_CRA1_CLKTYPE_TVXCLKSEL1;
> +
> + /* Clear then set the divider */
> + val &= ~(MCDE_CRX1_BCD | MCDE_CRX1_PCD_MASK);
> + val |= cdiv->cr_div;
> +
> + writel(val, mcde->regs + cdiv->cr);
> + spin_unlock(>fifo_crx1_lock);
> +
> + return 0;
> +}
> +
> +static int mcde_clk_div_choose_div(struct clk_hw *hw, unsigned long rate,
> +unsigned long *prate, bool set_parent)
> +{
> + int best_div = 1, div;
> + struct clk_hw *parent = clk_hw_get_parent(hw);
> + unsigned long best_prate = 0;
> + unsigned long best_diff = ~0ul;
> + int max_div = (1 << MCDE_CRX1_PCD_BITS) - 1;
> +
> + for (div = 1; div < max_div; div++) {
> + unsigned long this_prate, div_rate, diff;
> +
> + if (set_parent)
> + this_prate = clk_hw_round_rate(parent, rate * div);
> + else
> + this_prate = *prate;
> + div_rate = DIV_ROUND_UP_ULL(this_prate, div);
> + diff = abs(rate - div_rate);
> +
> + if (diff < best_diff) {
> + best_div = div;
> + best_diff = diff;
> + best_prate = this_prate;
> + }
> + }
> +
> + *prate = best_prate;
> + return best_div;
> +}
> +
> +static long mcde_clk_div_round_rate(struct clk_hw *hw, unsigned long rate,
> +  unsigned long *prate)
> +{
> + int div = mcde_clk_div_choose_div(hw, rate, prate, 

Re: [PATCH 1/2] drm/mcde: Break out DSI set-up routine

[Sam Ravnborg]

Hi Linus

On Thu, Nov 12, 2020 at 03:29:24PM +0100, Linus Walleij wrote:
> To be able to support DPI without messing things up we
> first break out the DSI set-up to a separate function.
> 
> Cc: Stephan Gerhold 
> Cc: phone-de...@vger.kernel.org
> Cc: upstream...@lists.sr.ht
> Signed-off-by: Linus Walleij 
> ---
>  drivers/gpu/drm/mcde/mcde_display.c | 135 +++-
>  1 file changed, 75 insertions(+), 60 deletions(-)
> 
> diff --git a/drivers/gpu/drm/mcde/mcde_display.c 
> b/drivers/gpu/drm/mcde/mcde_display.c
> index c271e5bf042e..66a07e340f8a 100644
> --- a/drivers/gpu/drm/mcde/mcde_display.c
> +++ b/drivers/gpu/drm/mcde/mcde_display.c
> @@ -860,74 +860,44 @@ static int mcde_dsi_get_pkt_div(int ppl, int fifo_size)
>   return 1;
>  }
>  
> -static void mcde_display_enable(struct drm_simple_display_pipe *pipe,
> - struct drm_crtc_state *cstate,
> - struct drm_plane_state *plane_state)
> +static void mcde_setup_dsi(struct mcde *mcde, const struct drm_display_mode 
> *mode,
> +int cpp, int *fifo_wtrmrk_lvl, int 
> *dsi_formatter_frame,
> +int *dsi_pkt_size)
>  {
> - struct drm_crtc *crtc = >crtc;
> - struct drm_plane *plane = >plane;
> - struct drm_device *drm = crtc->dev;
> - struct mcde *mcde = to_mcde(drm);
> - const struct drm_display_mode *mode = >mode;
> - struct drm_framebuffer *fb = plane->state->fb;
> - u32 format = fb->format->format;
>   u32 formatter_ppl = mode->hdisplay; /* pixels per line */
>   u32 formatter_lpf = mode->vdisplay; /* lines per frame */
> - int pkt_size, fifo_wtrmrk;
> - int cpp = fb->format->cpp[0];
> + int formatter_frame;
>   int formatter_cpp;
> - struct drm_format_name_buf tmp;
> - u32 formatter_frame;
> + int fifo_wtrmrk;
>   u32 pkt_div;
> + int pkt_size;
>   u32 val;
> - int ret;
>  
> - /* This powers up the entire MCDE block and the DSI hardware */
> - ret = regulator_enable(mcde->epod);
> - if (ret) {
> - dev_err(drm->dev, "can't re-enable EPOD regulator\n");
> - return;
> - }
> -
> - dev_info(drm->dev, "enable MCDE, %d x %d format %s\n",
> -  mode->hdisplay, mode->vdisplay,
> -  drm_get_format_name(format, ));
> - if (!mcde->mdsi) {
> - /* TODO: deal with this for non-DSI output */
> - dev_err(drm->dev, "no DSI master attached!\n");
> - return;
> - }
> + dev_info(mcde->dev, "output in %s mode, format %dbpp\n",
> +  (mcde->mdsi->mode_flags & MIPI_DSI_MODE_VIDEO) ?
> +  "VIDEO" : "CMD",
> +  mipi_dsi_pixel_format_to_bpp(mcde->mdsi->format));
> + formatter_cpp =
> + mipi_dsi_pixel_format_to_bpp(mcde->mdsi->format) / 8;
> + dev_info(mcde->dev, "Overlay CPP: %d bytes, DSI formatter CPP %d 
> bytes\n",
> +  cpp, formatter_cpp);
>  
>   /* Set up the main control, watermark level at 7 */
>   val = 7 << MCDE_CONF0_IFIFOCTRLWTRMRKLVL_SHIFT;
> - /* 24 bits DPI: connect LSB Ch B to D[0:7] */
> +
> + /*
> +  * This is the internal silicon muxing of the DPI
> +  * (parallell display) lines. Since we are not using
> +  * this at all (we are using DSI) these are just
> +  * dummy values from the vendor tree.
> +  */
>   val |= 3 << MCDE_CONF0_OUTMUX0_SHIFT;
> - /* TV out: connect LSB Ch B to D[8:15] */
>   val |= 3 << MCDE_CONF0_OUTMUX1_SHIFT;
> - /* Don't care about this muxing */
>   val |= 0 << MCDE_CONF0_OUTMUX2_SHIFT;
> - /* 24 bits DPI: connect MID Ch B to D[24:31] */
>   val |= 4 << MCDE_CONF0_OUTMUX3_SHIFT;
> - /* 5: 24 bits DPI: connect MSB Ch B to D[32:39] */
>   val |= 5 << MCDE_CONF0_OUTMUX4_SHIFT;
> - /* Syncmux bits zero: DPI channel A and B on output pins A and B resp */
>   writel(val, mcde->regs + MCDE_CONF0);
They are dummy values, but is still seems a shame to write the comments
about the bit interpretation in the registers. Sigh.


>  
> - /* Clear any pending interrupts */
> - mcde_display_disable_irqs(mcde);
> - writel(0, mcde->regs + MCDE_IMSCERR);
> - writel(0x, mcde->regs + MCDE_RISERR);
> -
> - dev_info(drm->dev, "output in %s mode, format %dbpp\n",
> -  (mcde->mdsi->mode_flags & MIPI_DSI_MODE_VIDEO) ?
> -  "VIDEO" : "CMD",
> -  mipi_dsi_pixel_format_to_bpp(mcde->mdsi->format));
> - formatter_cpp =
> - mipi_dsi_pixel_format_to_bpp(mcde->mdsi->format) / 8;
> - dev_info(drm->dev, "overlay CPP %d bytes, DSI CPP %d bytes\n",
> -  cpp,
> -  formatter_cpp);
> -
>   /* Calculations from mcde_fmtr_dsi.c, fmtr_dsi_enable_video() */
>  
>   /*
> @@ -948,9 +918,9 @@ static void mcde_display_enable(struct 
> drm_simple_display_pipe *pipe,
>   /* The FIFO is 640 entries deep on 

Re: [PATCH] drm/mcde: Fix RGB/BGR bug

[Sam Ravnborg]

Hi Linus.

On Tue, Nov 17, 2020 at 06:54:13PM +0100, Linus Walleij wrote:
> I was confused when the graphics came out with blue
> penguins on the DPI panel.
> 
> It turns out that the so-called "packed RGB666" mode
> on the DSI formatter is incorrect: this mode is the
> actual RGB888 mode, and the mode called RGB888 is
> BGR888.
> 
> The claims that the MCDE had inverse RGB/BGR buffer
> formats was wrong, so correct this and the buggy
> register and everything is much more consistent, and
> graphics look good on all targets, both DPI and
> DSI.
> 
> Cc: phone-de...@vger.kernel.org
> Cc: Stephan Gerhold 
> Signed-off-by: Linus Walleij 

Looks fine and seems to do what you write it should do.

I do not understand why this part:
>   case DRM_FORMAT_BGR888:
>   val |= MCDE_EXTSRCXCONF_BPP_RGB888 <<
>   MCDE_EXTSRCXCONF_BPP_SHIFT;
> + val |= MCDE_EXTSRCXCONF_BGR;
>   break;

does not use MCDE_EXTSRCXCONF_BPP_BGR888
But maybe there is no counterpart to MCDE_DSICONF0_PACKING_BGR888?

Assuming all is good despite my confusion:
Reviewed-by: Sam Ravnborg 
___
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

Re: [PATCH v2 1/6] drm/panel: st7703: Use dev_err_probe

[Sam Ravnborg]

On Wed, Nov 18, 2020 at 09:29:48AM +0100, Guido Günther wrote:
> Less code and easier probe deferral debugging.
> 
> Signed-off-by: Guido Günther 
> Reviewed-by: Linus Walleij 

Nice.

I hope someone comes around and update all panel drivers to use
dev_err_probe. It is simpler and better than the current code.
And it will fix a lot of drivers that are noisy during deferral.

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

Re: [PATCH v2 0/6] drm/panel: mantix and st7703 fixes and additions

[Sam Ravnborg]

Hi Guido,

On Wed, Nov 18, 2020 at 09:29:47AM +0100, Guido Günther wrote:
> This adds new panel type to the mantix driver as found on the Librem 5 and
> fixes a glitch in the init sequence (affecting both panels). The fix is at the
> start of the series to make backporting simpler.
> It also adds a patch to make st7703 use dev_err_probe().
> 
> changes from v1
> - as per review comments by Linus Walleij
>   - fix alphabetical ordering in 
> Documentation/devicetree/bindings/vendor-prefixes.yaml
> 
> https://lore.kernel.org/dri-devel/CACRpkdao_TMcpRsdK=7k5fnkjse0bqwk58iwu0xsxddndcf...@mail.gmail.com/
>   - add reviewed by to all except 5/6, thanks
> 
> Guido Günther (6):
>   drm/panel: st7703: Use dev_err_probe
>   drm/panel: mantix: Tweak init sequence
>   drm/panel: mantix: Allow to specify default mode for different panels
>   drm/panel: mantix: Support panel from Shenzhen Yashi Changhua
> Intelligent Technology Co
>   dt-bindings: vendor-prefixes: Add ys vendor prefix
The above are all:
Reviewed-by: Sam Ravnborg 

>   dt-binding: display: mantix: Add compatible for panel from YS
Please fix the subjects to read "dt-bindings" - just to be consistent.
With that:
Reviewed-by: Sam Ravnborg 


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

Re: [PATCH] via/via_irq: use __func__ to replace string function name

[Sam Ravnborg]

Hi Bernard.

On Wed, Nov 18, 2020 at 11:29:55PM -0800, Bernard Zhao wrote:
> This change also fix checkpatch.pl warning:
> WARNING: Prefer using '"%s...", __func__' to using
> 'via_driver_irq_postinstall', this function's name, in a string
> + DRM_DEBUG("via_driver_irq_postinstall\n");
> 
> Signed-off-by: Bernard Zhao 
> ---
>  drivers/gpu/drm/via/via_irq.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/drivers/gpu/drm/via/via_irq.c b/drivers/gpu/drm/via/via_irq.c
> index 24cc445169e2..56ce5d4ee24a 100644
> --- a/drivers/gpu/drm/via/via_irq.c
> +++ b/drivers/gpu/drm/via/via_irq.c
> @@ -308,7 +308,7 @@ int via_driver_irq_postinstall(struct drm_device *dev)
>   drm_via_private_t *dev_priv = (drm_via_private_t *) dev->dev_private;
>   u32 status;
>  
> - DRM_DEBUG("via_driver_irq_postinstall\n");
> + DRM_DEBUG("fun: %s\n", __func__);

DRM_DEBUG() will print the name of the calling function so the use of
__func__ here is not relevant.


For this old driver we should try to limit patches to bug fixing and
infrastructure updates.

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

Re: [PATCH] drm/mcde: Fix uninitialized value

[Sam Ravnborg]

On Thu, Nov 19, 2020 at 03:07:07PM +0100, Linus Walleij wrote:
> "val" isn't initialized on the default: errorpath.
> Just return from the function if this happens.
> 
> Reported-by: kernel test robot 
> Reported-by: Dan Carpenter 
> Signed-off-by: Linus Walleij 
Reviewed-by: Sam Ravnborg 

In the same function - it looks like a bug that the code
returns when it sees a MCDE_CHANNEL_3 - line 524.

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

Re: [PATCH 14/40] drm/msm/disp/dpu1/dpu_hw_catalog: Move definitions to the only place they are used

[abhinavk]

On 2020-11-23 03:18, Lee Jones wrote:

These tables are not large or overbearing, so moving them into the
source file seems like the right thing to do.  The alternative is to
use __maybe_unused, which is undesirable.

Fixes the following W=1 kernel build warning(s):

 In file included from 
drivers/gpu/drm/msm/disp/dpu1/dpu_hw_catalog.c:11:

 drivers/gpu/drm/msm/disp/dpu1/dpu_hw_catalog_format.h:7:23: warning:
‘qcom_compressed_supported_formats’ defined but not used
[-Wunused-const-variable=]
 7 | static const uint32_t qcom_compressed_supported_formats[] = {
 | ^
 drivers/gpu/drm/msm/disp/dpu1/dpu_hw_catalog_format.h:48:23: warning:
‘plane_formats_yuv’ defined but not used [-Wunused-const-variable=]
 48 | static const uint32_t plane_formats_yuv[] = {
 | ^
 drivers/gpu/drm/msm/disp/dpu1/dpu_hw_catalog_format.h:17:23: warning:
‘plane_formats’ defined but not used [-Wunused-const-variable=]
 17 | static const uint32_t plane_formats[] = {
 | ^

Cc: Rob Clark 
Cc: Sean Paul 
Cc: David Airlie 
Cc: Daniel Vetter 
Cc: linux-arm-...@vger.kernel.org
Cc: dri-devel@lists.freedesktop.org
Cc: freedr...@lists.freedesktop.org
Signed-off-by: Lee Jones 

I think this is reasonable,
Reviewed-by: Abhinav Kumar 

---
 .../gpu/drm/msm/disp/dpu1/dpu_hw_catalog.c| 74 +++-
 .../drm/msm/disp/dpu1/dpu_hw_catalog_format.h | 88 ---
 drivers/gpu/drm/msm/disp/dpu1/dpu_plane.c | 11 ++-
 3 files changed, 83 insertions(+), 90 deletions(-)
 delete mode 100644 
drivers/gpu/drm/msm/disp/dpu1/dpu_hw_catalog_format.h


diff --git a/drivers/gpu/drm/msm/disp/dpu1/dpu_hw_catalog.c
b/drivers/gpu/drm/msm/disp/dpu1/dpu_hw_catalog.c
index a7004f18523b0..9ed6d0c6cd9b2 100644
--- a/drivers/gpu/drm/msm/disp/dpu1/dpu_hw_catalog.c
+++ b/drivers/gpu/drm/msm/disp/dpu1/dpu_hw_catalog.c
@@ -8,7 +8,6 @@
 #include 
 #include "dpu_hw_mdss.h"
 #include "dpu_hw_catalog.h"
-#include "dpu_hw_catalog_format.h"
 #include "dpu_kms.h"

 #define VIG_MASK \
@@ -62,6 +61,79 @@

 #define STRCAT(X, Y) (X Y)

+static const uint32_t plane_formats[] = {
+   DRM_FORMAT_ARGB,
+   DRM_FORMAT_ABGR,
+   DRM_FORMAT_RGBA,
+   DRM_FORMAT_BGRA,
+   DRM_FORMAT_XRGB,
+   DRM_FORMAT_RGBX,
+   DRM_FORMAT_BGRX,
+   DRM_FORMAT_XBGR,
+   DRM_FORMAT_RGB888,
+   DRM_FORMAT_BGR888,
+   DRM_FORMAT_RGB565,
+   DRM_FORMAT_BGR565,
+   DRM_FORMAT_ARGB1555,
+   DRM_FORMAT_ABGR1555,
+   DRM_FORMAT_RGBA5551,
+   DRM_FORMAT_BGRA5551,
+   DRM_FORMAT_XRGB1555,
+   DRM_FORMAT_XBGR1555,
+   DRM_FORMAT_RGBX5551,
+   DRM_FORMAT_BGRX5551,
+   DRM_FORMAT_ARGB,
+   DRM_FORMAT_ABGR,
+   DRM_FORMAT_RGBA,
+   DRM_FORMAT_BGRA,
+   DRM_FORMAT_XRGB,
+   DRM_FORMAT_XBGR,
+   DRM_FORMAT_RGBX,
+   DRM_FORMAT_BGRX,
+};
+
+static const uint32_t plane_formats_yuv[] = {
+   DRM_FORMAT_ARGB,
+   DRM_FORMAT_ABGR,
+   DRM_FORMAT_RGBA,
+   DRM_FORMAT_BGRX,
+   DRM_FORMAT_BGRA,
+   DRM_FORMAT_XRGB,
+   DRM_FORMAT_XBGR,
+   DRM_FORMAT_RGBX,
+   DRM_FORMAT_RGB888,
+   DRM_FORMAT_BGR888,
+   DRM_FORMAT_RGB565,
+   DRM_FORMAT_BGR565,
+   DRM_FORMAT_ARGB1555,
+   DRM_FORMAT_ABGR1555,
+   DRM_FORMAT_RGBA5551,
+   DRM_FORMAT_BGRA5551,
+   DRM_FORMAT_XRGB1555,
+   DRM_FORMAT_XBGR1555,
+   DRM_FORMAT_RGBX5551,
+   DRM_FORMAT_BGRX5551,
+   DRM_FORMAT_ARGB,
+   DRM_FORMAT_ABGR,
+   DRM_FORMAT_RGBA,
+   DRM_FORMAT_BGRA,
+   DRM_FORMAT_XRGB,
+   DRM_FORMAT_XBGR,
+   DRM_FORMAT_RGBX,
+   DRM_FORMAT_BGRX,
+
+   DRM_FORMAT_NV12,
+   DRM_FORMAT_NV21,
+   DRM_FORMAT_NV16,
+   DRM_FORMAT_NV61,
+   DRM_FORMAT_VYUY,
+   DRM_FORMAT_UYVY,
+   DRM_FORMAT_YUYV,
+   DRM_FORMAT_YVYU,
+   DRM_FORMAT_YUV420,
+   DRM_FORMAT_YVU420,
+};
+
 /*
  * DPU sub blocks config
  */
diff --git a/drivers/gpu/drm/msm/disp/dpu1/dpu_hw_catalog_format.h
b/drivers/gpu/drm/msm/disp/dpu1/dpu_hw_catalog_format.h
deleted file mode 100644
index 3766f0fd0bf08..0
--- a/drivers/gpu/drm/msm/disp/dpu1/dpu_hw_catalog_format.h
+++ /dev/null
@@ -1,88 +0,0 @@
-/* SPDX-License-Identifier: GPL-2.0-only */
-/* Copyright (c) 2015-2018, The Linux Foundation. All rights reserved.
- */
-
-#include "dpu_hw_mdss.h"
-
-static const uint32_t qcom_compressed_supported_formats[] = {
-   DRM_FORMAT_ABGR,
-   DRM_FORMAT_ARGB,
-   DRM_FORMAT_XBGR,
-   DRM_FORMAT_XRGB,
-   DRM_FORMAT_BGR565,
-
-   DRM_FORMAT_NV12,
-};
-
-static const uint32_t plane_formats[] = {
-   DRM_FORMAT_ARGB,
-   DRM_FORMAT_ABGR,
-   DRM_FORMAT_RGBA,
-   

Re: [Freedreno] [PATCH 38/40] drm/msm/msm_drv: Make '_msm_ioremap()' static

[abhinavk]

On 2020-11-23 03:19, Lee Jones wrote:

Fixes the following W=1 kernel build warning(s):

 drivers/gpu/drm/msm/msm_drv.c:124:15: warning: no previous prototype
for ‘_msm_ioremap’ [-Wmissing-prototypes]

Cc: Rob Clark 
Cc: Sean Paul 
Cc: David Airlie 
Cc: Daniel Vetter 
Cc: linux-arm-...@vger.kernel.org
Cc: dri-devel@lists.freedesktop.org
Cc: freedr...@lists.freedesktop.org
Signed-off-by: Lee Jones 

Reviewed-by: Abhinav Kumar 

---
 drivers/gpu/drm/msm/msm_drv.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/drivers/gpu/drm/msm/msm_drv.c 
b/drivers/gpu/drm/msm/msm_drv.c

index 4d58668b80853..edc99e3b0eca7 100644
--- a/drivers/gpu/drm/msm/msm_drv.c
+++ b/drivers/gpu/drm/msm/msm_drv.c
@@ -121,8 +121,8 @@ struct clk *msm_clk_get(struct platform_device
*pdev, const char *name)
return clk;
 }

-void __iomem *_msm_ioremap(struct platform_device *pdev, const char 
*name,

-  const char *dbgname, bool quiet)
+static void __iomem *_msm_ioremap(struct platform_device *pdev, const
char *name,
+ const char *dbgname, bool quiet)
 {
struct resource *res;
unsigned long size;

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

Re: [Freedreno] [PATCH 31/40] drm/msm/disp/dpu1/dpu_rm: Fix formatting issues and supply 'global_state' description

[abhinavk]

On 2020-11-23 03:19, Lee Jones wrote:

Fixes the following W=1 kernel build warning(s):

 drivers/gpu/drm/msm/disp/dpu1/dpu_rm.c:247: warning: Excess function
parameter 'Return' description in '_dpu_rm_check_lm_peer'
 drivers/gpu/drm/msm/disp/dpu1/dpu_rm.c:283: warning: Function
parameter or member 'global_state' not described in
'_dpu_rm_check_lm_and_get_connected_blks'
 drivers/gpu/drm/msm/disp/dpu1/dpu_rm.c:283: warning: Excess function
parameter 'Return' description in
'_dpu_rm_check_lm_and_get_connected_blks'

Cc: Rob Clark 
Cc: Sean Paul 
Cc: David Airlie 
Cc: Daniel Vetter 
Cc: Drew Davenport 
Cc: linux-arm-...@vger.kernel.org
Cc: dri-devel@lists.freedesktop.org
Cc: freedr...@lists.freedesktop.org
Signed-off-by: Lee Jones 

Reviewed-by: Abhinav Kumar 

---
 drivers/gpu/drm/msm/disp/dpu1/dpu_rm.c | 5 +++--
 1 file changed, 3 insertions(+), 2 deletions(-)

diff --git a/drivers/gpu/drm/msm/disp/dpu1/dpu_rm.c
b/drivers/gpu/drm/msm/disp/dpu1/dpu_rm.c
index 0ae8a36ffcff3..fd2d104f0a91d 100644
--- a/drivers/gpu/drm/msm/disp/dpu1/dpu_rm.c
+++ b/drivers/gpu/drm/msm/disp/dpu1/dpu_rm.c
@@ -239,7 +239,7 @@ static bool _dpu_rm_needs_split_display(const
struct msm_display_topology *top)
  * @rm: dpu resource manager handle
  * @primary_idx: index of primary mixer in rm->mixer_blks[]
  * @peer_idx: index of other mixer in rm->mixer_blks[]
- * @Return: true if rm->mixer_blks[peer_idx] is a peer of
+ * Return: true if rm->mixer_blks[peer_idx] is a peer of
  *  rm->mixer_blks[primary_idx]
  */
 static bool _dpu_rm_check_lm_peer(struct dpu_rm *rm, int primary_idx,
@@ -264,6 +264,7 @@ static bool _dpu_rm_check_lm_peer(struct dpu_rm
*rm, int primary_idx,
  *	proposed use case requirements, incl. hardwired dependent blocks 
like

  * pingpong
  * @rm: dpu resource manager handle
+ * @global_state: resources shared across multiple kms objects
  * @enc_id: encoder id requesting for allocation
  * @lm_idx: index of proposed layer mixer in rm->mixer_blks[], 
function checks
  *  if lm, and all other hardwired blocks connected to the lm (pp) 
is

@@ -274,7 +275,7 @@ static bool _dpu_rm_check_lm_peer(struct dpu_rm
*rm, int primary_idx,
  *  mixer in rm->dspp_blks[].
  * @reqs: input parameter, rm requirements for HW blocks needed in the
  *  datapath.
- * @Return: true if lm matches all requirements, false otherwise
+ * Return: true if lm matches all requirements, false otherwise
  */
 static bool _dpu_rm_check_lm_and_get_connected_blks(struct dpu_rm *rm,
struct dpu_global_state *global_state,

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

Re: [Freedreno] [PATCH 35/40] drm/msm/disp/dpu1/dpu_plane: Fix some spelling and missing function param descriptions

[abhinavk]

On 2020-11-23 03:19, Lee Jones wrote:

Fixes the following W=1 kernel build warning(s):

 drivers/gpu/drm/msm/disp/dpu1/dpu_plane.c:152: warning: Function
parameter or member 'plane' not described in '_dpu_plane_calc_bw'
 drivers/gpu/drm/msm/disp/dpu1/dpu_plane.c:152: warning: Function
parameter or member 'fb' not described in '_dpu_plane_calc_bw'
 drivers/gpu/drm/msm/disp/dpu1/dpu_plane.c:152: warning: Excess
function parameter 'Plane' description in '_dpu_plane_calc_bw'
 drivers/gpu/drm/msm/disp/dpu1/dpu_plane.c:200: warning: Function
parameter or member 'plane' not described in '_dpu_plane_calc_clk'
 drivers/gpu/drm/msm/disp/dpu1/dpu_plane.c:200: warning: Excess
function parameter 'Plane' description in '_dpu_plane_calc_clk'
 drivers/gpu/drm/msm/disp/dpu1/dpu_plane.c:232: warning: Function
parameter or member 'src_width' not described in
'_dpu_plane_calc_fill_level'
 drivers/gpu/drm/msm/disp/dpu1/dpu_plane.c:232: warning: Excess
function parameter 'src_wdith' description in
'_dpu_plane_calc_fill_level'
 drivers/gpu/drm/msm/disp/dpu1/dpu_plane.c:1060: warning: Function
parameter or member 'error' not described in 'dpu_plane_set_error'

Cc: Rob Clark 
Cc: Sean Paul 
Cc: David Airlie 
Cc: Daniel Vetter 
Cc: Kalyan Thota 
Cc: linux-arm-...@vger.kernel.org
Cc: dri-devel@lists.freedesktop.org
Cc: freedr...@lists.freedesktop.org
Signed-off-by: Lee Jones 

Reviewed-by: Abhinav Kumar 

---
 drivers/gpu/drm/msm/disp/dpu1/dpu_plane.c | 8 +---
 1 file changed, 5 insertions(+), 3 deletions(-)

diff --git a/drivers/gpu/drm/msm/disp/dpu1/dpu_plane.c
b/drivers/gpu/drm/msm/disp/dpu1/dpu_plane.c
index c0b1d77369e53..cf0084d7cc7c9 100644
--- a/drivers/gpu/drm/msm/disp/dpu1/dpu_plane.c
+++ b/drivers/gpu/drm/msm/disp/dpu1/dpu_plane.c
@@ -142,7 +142,8 @@ static struct dpu_kms *_dpu_plane_get_kms(struct
drm_plane *plane)

 /**
  * _dpu_plane_calc_bw - calculate bandwidth required for a plane
- * @Plane: Pointer to drm plane.
+ * @plane: Pointer to drm plane.
+ * @fb:   Pointer to framebuffer associated with the given plane
  * Result: Updates calculated bandwidth in the plane state.
  * BW Equation: src_w * src_h * bpp * fps * (v_total / v_dest)
  * Prefill BW Equation: line src bytes * line_time
@@ -192,7 +193,7 @@ static void _dpu_plane_calc_bw(struct drm_plane 
*plane,


 /**
  * _dpu_plane_calc_clk - calculate clock required for a plane
- * @Plane: Pointer to drm plane.
+ * @plane: Pointer to drm plane.
  * Result: Updates calculated clock in the plane state.
  * Clock equation: dst_w * v_total * fps * (src_h / dst_h)
  */
@@ -224,7 +225,7 @@ static void _dpu_plane_calc_clk(struct drm_plane 
*plane)
  * _dpu_plane_calc_fill_level - calculate fill level of the given 
source format

  * @plane: Pointer to drm plane
  * @fmt:   Pointer to source buffer format
- * @src_wdith: width of source buffer
+ * @src_width: width of source buffer
  * Return: fill level corresponding to the source buffer/format or 0 
if error

  */
 static int _dpu_plane_calc_fill_level(struct drm_plane *plane,
@@ -1055,6 +1056,7 @@ void dpu_plane_flush(struct drm_plane *plane)
 /**
  * dpu_plane_set_error: enable/disable error condition
  * @plane: pointer to drm_plane structure
+ * @error: error value to set
  */
 void dpu_plane_set_error(struct drm_plane *plane, bool error)
 {

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

Re: [Freedreno] [PATCH 19/40] drm/msm/disp/dpu1/dpu_hw_lm: Fix misnaming of parameter 'ctx'

[abhinavk]

On 2020-11-23 03:18, Lee Jones wrote:

Fixes the following W=1 kernel build warning(s):

 drivers/gpu/drm/msm/disp/dpu1/dpu_hw_lm.c:55: warning: Function
parameter or member 'ctx' not described in '_stage_offset'
 drivers/gpu/drm/msm/disp/dpu1/dpu_hw_lm.c:55: warning: Excess
function parameter 'c' description in '_stage_offset'

Cc: Rob Clark 
Cc: Sean Paul 
Cc: David Airlie 
Cc: Daniel Vetter 
Cc: linux-arm-...@vger.kernel.org
Cc: dri-devel@lists.freedesktop.org
Cc: freedr...@lists.freedesktop.org
Signed-off-by: Lee Jones 

Reviewed-by: Abhinav Kumar 

---
 drivers/gpu/drm/msm/disp/dpu1/dpu_hw_lm.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/gpu/drm/msm/disp/dpu1/dpu_hw_lm.c
b/drivers/gpu/drm/msm/disp/dpu1/dpu_hw_lm.c
index 4b8baf71423f2..6ac0b5a0e0573 100644
--- a/drivers/gpu/drm/msm/disp/dpu1/dpu_hw_lm.c
+++ b/drivers/gpu/drm/msm/disp/dpu1/dpu_hw_lm.c
@@ -48,7 +48,7 @@ static const struct dpu_lm_cfg *_lm_offset(enum 
dpu_lm mixer,

 /**
  * _stage_offset(): returns the relative offset of the blend registers
  * for the stage to be setup
- * @c: mixer ctx contains the mixer to be programmed
+ * @ctx: mixer ctx contains the mixer to be programmed
  * @stage: stage index to setup
  */
 static inline int _stage_offset(struct dpu_hw_mixer *ctx, enum 
dpu_stage stage)

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

Re: [Freedreno] [PATCH 20/40] drm/msm/disp/dpu1/dpu_hw_sspp: Fix kernel-doc formatting abuse

[abhinavk]

On 2020-11-23 03:18, Lee Jones wrote:

Fixes the following W=1 kernel build warning(s):

 drivers/gpu/drm/msm/disp/dpu1/dpu_hw_sspp.c:240: warning: Function
parameter or member 'ctx' not described in 'dpu_hw_sspp_setup_format'
 drivers/gpu/drm/msm/disp/dpu1/dpu_hw_sspp.c:240: warning: Function
parameter or member 'fmt' not described in 'dpu_hw_sspp_setup_format'
 drivers/gpu/drm/msm/disp/dpu1/dpu_hw_sspp.c:240: warning: Function
parameter or member 'flags' not described in
'dpu_hw_sspp_setup_format'
 drivers/gpu/drm/msm/disp/dpu1/dpu_hw_sspp.c:240: warning: Function
parameter or member 'rect_mode' not described in
'dpu_hw_sspp_setup_format'
 drivers/gpu/drm/msm/disp/dpu1/dpu_hw_sspp.c:446: warning: Function
parameter or member 'ctx' not described in 'dpu_hw_sspp_setup_rects'
 drivers/gpu/drm/msm/disp/dpu1/dpu_hw_sspp.c:446: warning: Function
parameter or member 'cfg' not described in 'dpu_hw_sspp_setup_rects'
 drivers/gpu/drm/msm/disp/dpu1/dpu_hw_sspp.c:446: warning: Function
parameter or member 'rect_index' not described in
'dpu_hw_sspp_setup_rects'

Cc: Rob Clark 
Cc: Sean Paul 
Cc: David Airlie 
Cc: Daniel Vetter 
Cc: Jonathan Marek 
Cc: linux-arm-...@vger.kernel.org
Cc: dri-devel@lists.freedesktop.org
Cc: freedr...@lists.freedesktop.org
Signed-off-by: Lee Jones 

Reviewed-by: Abhinav Kumar 

---
 drivers/gpu/drm/msm/disp/dpu1/dpu_hw_sspp.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/drivers/gpu/drm/msm/disp/dpu1/dpu_hw_sspp.c
b/drivers/gpu/drm/msm/disp/dpu1/dpu_hw_sspp.c
index c940b69435e16..2c2ca5335aa8c 100644
--- a/drivers/gpu/drm/msm/disp/dpu1/dpu_hw_sspp.c
+++ b/drivers/gpu/drm/msm/disp/dpu1/dpu_hw_sspp.c
@@ -231,7 +231,7 @@ static void _sspp_setup_csc10_opmode(struct
dpu_hw_pipe *ctx,
DPU_REG_WRITE(>hw, SSPP_VIG_CSC_10_OP_MODE + idx, opmode);
 }

-/**
+/*
  * Setup source pixel format, flip,
  */
 static void dpu_hw_sspp_setup_format(struct dpu_hw_pipe *ctx,
@@ -437,7 +437,7 @@ static u32 _dpu_hw_sspp_get_scaler3_ver(struct
dpu_hw_pipe *ctx)
return dpu_hw_get_scaler3_ver(>hw, idx);
 }

-/**
+/*
  * dpu_hw_sspp_setup_rects()
  */
 static void dpu_hw_sspp_setup_rects(struct dpu_hw_pipe *ctx,

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

Re: [Freedreno] [PATCH 10/40] drm/msm/disp/dpu1/dpu_core_perf: Fix kernel-doc formatting issues

[abhinavk]

On 2020-11-23 03:18, Lee Jones wrote:

Fixes the following W=1 kernel build warning(s):

 drivers/gpu/drm/msm/disp/dpu1/dpu_core_perf.c:31: warning: Enum value
'DPU_PERF_MODE_MAX' not described in enum 'dpu_perf_mode'
 drivers/gpu/drm/msm/disp/dpu1/dpu_core_perf.c:34: warning: Cannot
understand  * @_dpu_core_perf_calc_bw() - to calculate BW per crtc
 drivers/gpu/drm/msm/disp/dpu1/dpu_core_perf.c:73: warning: Function
parameter or member 'kms' not described in '_dpu_core_perf_calc_clk'
 drivers/gpu/drm/msm/disp/dpu1/dpu_core_perf.c:73: warning: Function
parameter or member 'crtc' not described in '_dpu_core_perf_calc_clk'
 drivers/gpu/drm/msm/disp/dpu1/dpu_core_perf.c:73: warning: Function
parameter or member 'state' not described in '_dpu_core_perf_calc_clk'
 drivers/gpu/drm/msm/disp/dpu1/dpu_core_perf.c:249: warning: Cannot
understand  * @dpu_core_perf_crtc_release_bw() - request zero
bandwidth

Cc: Rob Clark 
Cc: Sean Paul 
Cc: David Airlie 
Cc: Daniel Vetter 
Cc: Kalyan Thota 
Cc: linux-arm-...@vger.kernel.org
Cc: dri-devel@lists.freedesktop.org
Cc: freedr...@lists.freedesktop.org
Signed-off-by: Lee Jones 

Reviewed-by: Abhinav Kumar 

---
 drivers/gpu/drm/msm/disp/dpu1/dpu_core_perf.c | 17 +
 1 file changed, 9 insertions(+), 8 deletions(-)

diff --git a/drivers/gpu/drm/msm/disp/dpu1/dpu_core_perf.c
b/drivers/gpu/drm/msm/disp/dpu1/dpu_core_perf.c
index 36927fc04a388..b6b3bbab03336 100644
--- a/drivers/gpu/drm/msm/disp/dpu1/dpu_core_perf.c
+++ b/drivers/gpu/drm/msm/disp/dpu1/dpu_core_perf.c
@@ -22,6 +22,7 @@
  * @DPU_PERF_MODE_NORMAL: performance controlled by user mode client
  * @DPU_PERF_MODE_MINIMUM: performance bounded by minimum setting
  * @DPU_PERF_MODE_FIXED: performance bounded by fixed setting
+ * @DPU_PERF_MODE_MAX: maximum value, used for error checking
  */
 enum dpu_perf_mode {
DPU_PERF_MODE_NORMAL,
@@ -31,9 +32,9 @@ enum dpu_perf_mode {
 };

 /**
- * @_dpu_core_perf_calc_bw() - to calculate BW per crtc
- * @kms -  pointer to the dpu_kms
- * @crtc - pointer to a crtc
+ * _dpu_core_perf_calc_bw() - to calculate BW per crtc
+ * @kms:  pointer to the dpu_kms
+ * @crtc: pointer to a crtc
  * Return: returns aggregated BW for all planes in crtc.
  */
 static u64 _dpu_core_perf_calc_bw(struct dpu_kms *kms,
@@ -63,9 +64,9 @@ static u64 _dpu_core_perf_calc_bw(struct dpu_kms 
*kms,


 /**
  * _dpu_core_perf_calc_clk() - to calculate clock per crtc
- * @kms -  pointer to the dpu_kms
- * @crtc - pointer to a crtc
- * @state - pointer to a crtc state
+ * @kms:  pointer to the dpu_kms
+ * @crtc: pointer to a crtc
+ * @state: pointer to a crtc state
  * Return: returns max clk for all planes in crtc.
  */
 static u64 _dpu_core_perf_calc_clk(struct dpu_kms *kms,
@@ -246,8 +247,8 @@ static int _dpu_core_perf_crtc_update_bus(struct
dpu_kms *kms,
 }

 /**
- * @dpu_core_perf_crtc_release_bw() - request zero bandwidth
- * @crtc - pointer to a crtc
+ * dpu_core_perf_crtc_release_bw() - request zero bandwidth
+ * @crtc: pointer to a crtc
  *
  * Function checks a state variable for the crtc, if all pending 
commit

  * requests are done, meaning no more bandwidth is needed, release

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

Re: [Freedreno] [PATCH 18/40] drm/msm/disp/dpu1/dpu_encoder: Fix a few parameter/member formatting issues

[abhinavk]

On 2020-11-23 03:18, Lee Jones wrote:

Fixes the following W=1 kernel build warning(s):

 drivers/gpu/drm/msm/disp/dpu1/dpu_encoder.c:207: warning: Function
parameter or member 'cur_slave' not described in 'dpu_encoder_virt'
 drivers/gpu/drm/msm/disp/dpu1/dpu_encoder.c:207: warning: Function
parameter or member 'hw_pp' not described in 'dpu_encoder_virt'
 drivers/gpu/drm/msm/disp/dpu1/dpu_encoder.c:207: warning: Function
parameter or member 'intfs_swapped' not described in
'dpu_encoder_virt'
 drivers/gpu/drm/msm/disp/dpu1/dpu_encoder.c:1445: warning: Function
parameter or member 'drm_enc' not described in
'_dpu_encoder_trigger_flush'
 drivers/gpu/drm/msm/disp/dpu1/dpu_encoder.c:1445: warning: Function
parameter or member 'phys' not described in
'_dpu_encoder_trigger_flush'
 drivers/gpu/drm/msm/disp/dpu1/dpu_encoder.c:1445: warning: Function
parameter or member 'extra_flush_bits' not described in
'_dpu_encoder_trigger_flush'
 drivers/gpu/drm/msm/disp/dpu1/dpu_encoder.c:1481: warning: Function
parameter or member 'phys' not described in
'_dpu_encoder_trigger_start'
 drivers/gpu/drm/msm/disp/dpu1/dpu_encoder.c:1564: warning: Function
parameter or member 'dpu_enc' not described in
'_dpu_encoder_kickoff_phys'

Cc: Rob Clark 
Cc: Sean Paul 
Cc: David Airlie 
Cc: Daniel Vetter 
Cc: linux-arm-...@vger.kernel.org
Cc: dri-devel@lists.freedesktop.org
Cc: freedr...@lists.freedesktop.org
Signed-off-by: Lee Jones 

Reviewed-by: Abhinav Kumar 

---
 drivers/gpu/drm/msm/disp/dpu1/dpu_encoder.c | 15 ---
 1 file changed, 8 insertions(+), 7 deletions(-)

diff --git a/drivers/gpu/drm/msm/disp/dpu1/dpu_encoder.c
b/drivers/gpu/drm/msm/disp/dpu1/dpu_encoder.c
index 289bfb6f1861c..288e95ee8e1d5 100644
--- a/drivers/gpu/drm/msm/disp/dpu1/dpu_encoder.c
+++ b/drivers/gpu/drm/msm/disp/dpu1/dpu_encoder.c
@@ -132,9 +132,10 @@ enum dpu_enc_rc_states {
  * @phys_encs: Container of physical encoders managed.
  * @cur_master:		Pointer to the current master in this mode. 
Optimization

  * Only valid after enable. Cleared as disable.
- * @hw_pp  Handle to the pingpong blocks used for the display. No.
+ * @cur_slave: As above but for the slave encoder.
+ * @hw_pp: Handle to the pingpong blocks used for the display. No.
  * pingpong blocks can be different than num_phys_encs.
- * @intfs_swapped	Whether or not the phys_enc interfaces have been 
swapped
+ * @intfs_swapped:	Whether or not the phys_enc interfaces have been 
swapped

  * for partial update right-only cases, such as pingpong
  * split where virtual pingpong does not generate IRQs
  * @crtc:  Pointer to the currently assigned crtc. Normally you
@@ -1436,9 +1437,9 @@ static void dpu_encoder_off_work(struct 
work_struct *work)


 /**
  * _dpu_encoder_trigger_flush - trigger flush for a physical encoder
- * drm_enc: Pointer to drm encoder structure
- * phys: Pointer to physical encoder structure
- * extra_flush_bits: Additional bit mask to include in flush trigger
+ * @drm_enc: Pointer to drm encoder structure
+ * @phys: Pointer to physical encoder structure
+ * @extra_flush_bits: Additional bit mask to include in flush trigger
  */
 static void _dpu_encoder_trigger_flush(struct drm_encoder *drm_enc,
struct dpu_encoder_phys *phys, uint32_t extra_flush_bits)
@@ -1475,7 +1476,7 @@ static void _dpu_encoder_trigger_flush(struct
drm_encoder *drm_enc,

 /**
  * _dpu_encoder_trigger_start - trigger start for a physical encoder
- * phys: Pointer to physical encoder structure
+ * @phys: Pointer to physical encoder structure
  */
 static void _dpu_encoder_trigger_start(struct dpu_encoder_phys *phys)
 {
@@ -1558,7 +1559,7 @@ static void dpu_encoder_helper_hw_reset(struct
dpu_encoder_phys *phys_enc)
  * encoder rather than the individual physical ones in order to handle
  *	use cases that require visibility into multiple physical encoders 
at

  * a time.
- * dpu_enc: Pointer to virtual encoder structure
+ * @dpu_enc: Pointer to virtual encoder structure
  */
 static void _dpu_encoder_kickoff_phys(struct dpu_encoder_virt 
*dpu_enc)

 {

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

Re: [PATCH v3 05/12] drm/ttm: Expose ttm_tt_unpopulate for driver use

[Andrey Grodzovsky]

On 11/23/20 3:41 PM, Christian König wrote:

Am 23.11.20 um 21:38 schrieb Andrey Grodzovsky:


On 11/23/20 3:20 PM, Christian König wrote:

Am 23.11.20 um 21:05 schrieb Andrey Grodzovsky:


On 11/25/20 5:42 AM, Christian König wrote:

Am 21.11.20 um 06:21 schrieb Andrey Grodzovsky:

It's needed to drop iommu backed pages on device unplug
before device's IOMMU group is released.


It would be cleaner if we could do the whole handling in TTM. I also need 
to double check what you are doing with this function.


Christian.



Check patch "drm/amdgpu: Register IOMMU topology notifier per device." to see
how i use it. I don't see why this should go into TTM mid-layer - the stuff 
I do inside

is vendor specific and also I don't think TTM is explicitly aware of IOMMU ?
Do you mean you prefer the IOMMU notifier to be registered from within TTM
and then use a hook to call into vendor specific handler ?


No, that is really vendor specific.

What I meant is to have a function like ttm_resource_manager_evict_all() 
which you only need to call and all tt objects are unpopulated.



So instead of this BO list i create and later iterate in amdgpu from the 
IOMMU patch you just want to do it within

TTM with a single function ? Makes much more sense.


Yes, exactly.

The list_empty() checks we have in TTM for the LRU are actually not the best 
idea, we should now check the pin_count instead. This way we could also have a 
list of the pinned BOs in TTM.



So from my IOMMU topology handler I will iterate the TTM LRU for the unpinned 
BOs and this new function for the pinned ones  ?
It's probably a good idea to combine both iterations into this new function to 
cover all the BOs allocated on the device.





BTW: Have you thought about what happens when we unpopulate a BO while we 
still try to use a kernel mapping for it? That could have unforeseen 
consequences.



Are you asking what happens to kmap or vmap style mapped CPU accesses once we 
drop all the DMA backing pages for a particular BO ? Because for user mappings
(mmap) we took care of this with dummy page reroute but indeed nothing was done 
for in kernel CPU mappings.


Andrey




Christian.



Andrey




Give me a day or two to look into this.

Christian.



Andrey






Signed-off-by: Andrey Grodzovsky 
---
  drivers/gpu/drm/ttm/ttm_tt.c | 1 +
  1 file changed, 1 insertion(+)

diff --git a/drivers/gpu/drm/ttm/ttm_tt.c b/drivers/gpu/drm/ttm/ttm_tt.c
index 1ccf1ef..29248a5 100644
--- a/drivers/gpu/drm/ttm/ttm_tt.c
+++ b/drivers/gpu/drm/ttm/ttm_tt.c
@@ -495,3 +495,4 @@ void ttm_tt_unpopulate(struct ttm_tt *ttm)
  else
  ttm_pool_unpopulate(ttm);
  }
+EXPORT_SYMBOL(ttm_tt_unpopulate);



___
amd-gfx mailing list
amd-...@lists.freedesktop.org
https://nam11.safelinks.protection.outlook.com/?url=https%3A%2F%2Flists.freedesktop.org%2Fmailman%2Flistinfo%2Famd-gfxdata=04%7C01%7CAndrey.Grodzovsky%40amd.com%7C9be029f26a4746347a6108d88fed299b%7C3dd8961fe4884e608e11a82d994e183d%7C0%7C0%7C637417596065559955%7CUnknown%7CTWFpbGZsb3d8eyJWIjoiMC4wLjAwMDAiLCJQIjoiV2luMzIiLCJBTiI6Ik1haWwiLCJXVCI6Mn0%3D%7C1000sdata=tZ3do%2FeKzBtRlNaFbBjCtRvUHKdvwDZ7SoYhEBu4%2BT8%3Dreserved=0 






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

Re: [Freedreno] [PATCH 12/40] drm/msm/disp/dpu1/dpu_formats: Demote non-conformant kernel-doc header

[abhinavk]

On 2020-11-23 03:18, Lee Jones wrote:

Fixes the following W=1 kernel build warning(s):

 drivers/gpu/drm/msm/disp/dpu1/dpu_formats.c:50: warning: Function
parameter or member 'fmt' not described in 'INTERLEAVED_RGB_FMT'
 drivers/gpu/drm/msm/disp/dpu1/dpu_formats.c:50: warning: Function
parameter or member 'a' not described in 'INTERLEAVED_RGB_FMT'
 drivers/gpu/drm/msm/disp/dpu1/dpu_formats.c:50: warning: Function
parameter or member 'r' not described in 'INTERLEAVED_RGB_FMT'
 drivers/gpu/drm/msm/disp/dpu1/dpu_formats.c:50: warning: Function
parameter or member 'g' not described in 'INTERLEAVED_RGB_FMT'
 drivers/gpu/drm/msm/disp/dpu1/dpu_formats.c:50: warning: Function
parameter or member 'b' not described in 'INTERLEAVED_RGB_FMT'
 drivers/gpu/drm/msm/disp/dpu1/dpu_formats.c:50: warning: Function
parameter or member 'e0' not described in 'INTERLEAVED_RGB_FMT'
 drivers/gpu/drm/msm/disp/dpu1/dpu_formats.c:50: warning: Function
parameter or member 'e1' not described in 'INTERLEAVED_RGB_FMT'
 drivers/gpu/drm/msm/disp/dpu1/dpu_formats.c:50: warning: Function
parameter or member 'e2' not described in 'INTERLEAVED_RGB_FMT'
 drivers/gpu/drm/msm/disp/dpu1/dpu_formats.c:50: warning: Function
parameter or member 'e3' not described in 'INTERLEAVED_RGB_FMT'
 drivers/gpu/drm/msm/disp/dpu1/dpu_formats.c:50: warning: Function
parameter or member 'uc' not described in 'INTERLEAVED_RGB_FMT'
 drivers/gpu/drm/msm/disp/dpu1/dpu_formats.c:50: warning: Function
parameter or member 'alpha' not described in 'INTERLEAVED_RGB_FMT'
 drivers/gpu/drm/msm/disp/dpu1/dpu_formats.c:50: warning: Function
parameter or member 'bp' not described in 'INTERLEAVED_RGB_FMT'
 drivers/gpu/drm/msm/disp/dpu1/dpu_formats.c:50: warning: Function
parameter or member 'flg' not described in 'INTERLEAVED_RGB_FMT'
 drivers/gpu/drm/msm/disp/dpu1/dpu_formats.c:50: warning: Function
parameter or member 'fm' not described in 'INTERLEAVED_RGB_FMT'
 drivers/gpu/drm/msm/disp/dpu1/dpu_formats.c:50: warning: Function
parameter or member 'np' not described in 'INTERLEAVED_RGB_FMT'

Cc: Rob Clark 
Cc: Sean Paul 
Cc: David Airlie 
Cc: Daniel Vetter 
Cc: Fritz Koenig 
Cc: linux-arm-...@vger.kernel.org
Cc: dri-devel@lists.freedesktop.org
Cc: freedr...@lists.freedesktop.org
Signed-off-by: Lee Jones 

Reviewed-by: Abhinav Kumar 

---
 drivers/gpu/drm/msm/disp/dpu1/dpu_formats.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/gpu/drm/msm/disp/dpu1/dpu_formats.c
b/drivers/gpu/drm/msm/disp/dpu1/dpu_formats.c
index a05282dede91b..21ff8f9e5dfd1 100644
--- a/drivers/gpu/drm/msm/disp/dpu1/dpu_formats.c
+++ b/drivers/gpu/drm/msm/disp/dpu1/dpu_formats.c
@@ -22,7 +22,7 @@
 #define DPU_MAX_IMG_WIDTH  0x3FFF
 #define DPU_MAX_IMG_HEIGHT 0x3FFF

-/**
+/*
  * DPU supported format packing, bpp, and other format
  * information.
  * DPU currently only supports interleaved RGB formats

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

Re: [Freedreno] [PATCH 13/40] drm/msm/disp/dpu1/dpu_hw_catalog: Remove duplicated initialisation of 'max_linewidth'

[abhinavk]

On 2020-11-23 03:18, Lee Jones wrote:

Fixes the following W=1 kernel build warning(s):

 drivers/gpu/drm/msm/disp/dpu1/dpu_hw_catalog.c:124:19: warning:
initialized field overwritten [-Woverride-init]
 drivers/gpu/drm/msm/disp/dpu1/dpu_hw_catalog.c:124:19: note: (near
initialization for ‘sm8250_dpu_caps.max_linewidth’)

Cc: Rob Clark 
Cc: Sean Paul 
Cc: David Airlie 
Cc: Daniel Vetter 
Cc: linux-arm-...@vger.kernel.org
Cc: dri-devel@lists.freedesktop.org
Cc: freedr...@lists.freedesktop.org
Signed-off-by: Lee Jones 

Reviewed-by: Abhinav Kumar 

---
 drivers/gpu/drm/msm/disp/dpu1/dpu_hw_catalog.c | 1 -
 1 file changed, 1 deletion(-)

diff --git a/drivers/gpu/drm/msm/disp/dpu1/dpu_hw_catalog.c
b/drivers/gpu/drm/msm/disp/dpu1/dpu_hw_catalog.c
index 6a6a7172e3beb..a7004f18523b0 100644
--- a/drivers/gpu/drm/msm/disp/dpu1/dpu_hw_catalog.c
+++ b/drivers/gpu/drm/msm/disp/dpu1/dpu_hw_catalog.c
@@ -113,7 +113,6 @@ static const struct dpu_caps sm8150_dpu_caps = {
 static const struct dpu_caps sm8250_dpu_caps = {
.max_mixer_width = DEFAULT_DPU_OUTPUT_LINE_WIDTH,
.max_mixer_blendstages = 0xb,
-   .max_linewidth = 4096,
.qseed_type = DPU_SSPP_SCALER_QSEED3, /* TODO: qseed3 lite */
.smart_dma_rev = DPU_SSPP_SMART_DMA_V2, /* TODO: v2.5 */
.ubwc_version = DPU_HW_UBWC_VER_40,

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

Re: [Freedreno] [PATCH 11/40] drm/msm/disp/dpu1/dpu_hw_blk: Add one missing and remove an extra param description

[abhinavk]

On 2020-11-23 03:18, Lee Jones wrote:

Fixes the following W=1 kernel build warning(s):

 drivers/gpu/drm/msm/disp/dpu1/dpu_hw_blk.c:28: warning: Function
parameter or member 'hw_blk' not described in 'dpu_hw_blk_init'
 drivers/gpu/drm/msm/disp/dpu1/dpu_hw_blk.c:120: warning: Excess
function parameter 'free_blk' description in 'dpu_hw_blk_put'

Cc: Rob Clark 
Cc: Sean Paul 
Cc: David Airlie 
Cc: Daniel Vetter 
Cc: linux-arm-...@vger.kernel.org
Cc: dri-devel@lists.freedesktop.org
Cc: freedr...@lists.freedesktop.org
Signed-off-by: Lee Jones 

Reviewed-by: Abhinav Kumar 

---
 drivers/gpu/drm/msm/disp/dpu1/dpu_hw_blk.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/gpu/drm/msm/disp/dpu1/dpu_hw_blk.c
b/drivers/gpu/drm/msm/disp/dpu1/dpu_hw_blk.c
index ca2d2af91..819b26e660b9c 100644
--- a/drivers/gpu/drm/msm/disp/dpu1/dpu_hw_blk.c
+++ b/drivers/gpu/drm/msm/disp/dpu1/dpu_hw_blk.c
@@ -19,6 +19,7 @@ static LIST_HEAD(dpu_hw_blk_list);

 /**
  * dpu_hw_blk_init - initialize hw block object
+ * @hw_blk: pointer to hw block object
  * @type: hw block type - enum dpu_hw_blk_type
  * @id: instance id of the hw block
  * @ops: Pointer to block operations
@@ -114,7 +115,6 @@ struct dpu_hw_blk *dpu_hw_blk_get(struct
dpu_hw_blk *hw_blk, u32 type, int id)
 /**
  * dpu_hw_blk_put - put hw_blk to free pool if decremented refcount is 
zero

  * @hw_blk: hw block to be freed
- * @free_blk: function to be called when reference count goes to zero
  */
 void dpu_hw_blk_put(struct dpu_hw_blk *hw_blk)
 {

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

Re: [PATCH 2/2] drm: panel: add Khadas TS050 panel driver

[Sam Ravnborg]

Hi Neil.

Looks good but a few comments in the following that needs some attention.

Sam

On Mon, Nov 23, 2020 at 03:33:54PM +0100, Neil Armstrong wrote:
> This add support for the Khadas TS050 1080x1920 5" LCD DSI panel designed to 
> work
> with the Khadas Edge-V, Captain, VIM3 and VIM3L Single Board Computers.
> It provides a MIPI DSI interface to the host, a built-in LED backlight
> and touch controller.
> 
> The init values was taken from the vendor source tree, comments were added to 
> the
> know values but most of the init table is undocumented.
> 
> Signed-off-by: Neil Armstrong 
> ---
>  drivers/gpu/drm/panel/Kconfig  |  11 +
>  drivers/gpu/drm/panel/Makefile |   1 +
>  drivers/gpu/drm/panel/panel-khadas-ts050.c | 876 +
>  3 files changed, 888 insertions(+)
>  create mode 100644 drivers/gpu/drm/panel/panel-khadas-ts050.c
> 
> diff --git a/drivers/gpu/drm/panel/Kconfig b/drivers/gpu/drm/panel/Kconfig
> index b4e021ea30f9..8fec45b2ce02 100644
> --- a/drivers/gpu/drm/panel/Kconfig
> +++ b/drivers/gpu/drm/panel/Kconfig
> @@ -145,6 +145,17 @@ config DRM_PANEL_JDI_LT070ME05000
> The panel has a 1200(RGB)×1920 (WUXGA) resolution and uses
> 24 bit per pixel.
>  
> +config DRM_PANEL_KHADAS_TS050
> + tristate "Khadas TS050 panel"
> + depends on OF
> + depends on DRM_MIPI_DSI
> + depends on BACKLIGHT_CLASS_DEVICE
> + help
> +   Say Y here if you want to enable support for Khadas TS050 TFT-LCD
> +   panel module. The panel has a 1080x1920 resolution and uses
> +   24 bit RGB per pixel. It provides a MIPI DSI interface to
> +   the host, a built-in LED backlight and touch controller.
> +
>  config DRM_PANEL_KINGDISPLAY_KD097D04
>   tristate "Kingdisplay kd097d04 panel"
>   depends on OF
> diff --git a/drivers/gpu/drm/panel/Makefile b/drivers/gpu/drm/panel/Makefile
> index ebbf488c7eac..03496695e03f 100644
> --- a/drivers/gpu/drm/panel/Makefile
> +++ b/drivers/gpu/drm/panel/Makefile
> @@ -13,6 +13,7 @@ obj-$(CONFIG_DRM_PANEL_ILITEK_IL9322) += 
> panel-ilitek-ili9322.o
>  obj-$(CONFIG_DRM_PANEL_ILITEK_ILI9881C) += panel-ilitek-ili9881c.o
>  obj-$(CONFIG_DRM_PANEL_INNOLUX_P079ZCA) += panel-innolux-p079zca.o
>  obj-$(CONFIG_DRM_PANEL_JDI_LT070ME05000) += panel-jdi-lt070me05000.o
> +obj-$(CONFIG_DRM_PANEL_KHADAS_TS050) += panel-khadas-ts050.o
>  obj-$(CONFIG_DRM_PANEL_KINGDISPLAY_KD097D04) += panel-kingdisplay-kd097d04.o
>  obj-$(CONFIG_DRM_PANEL_LEADTEK_LTK050H3146W) += panel-leadtek-ltk050h3146w.o
>  obj-$(CONFIG_DRM_PANEL_LEADTEK_LTK500HD1829) += panel-leadtek-ltk500hd1829.o
> diff --git a/drivers/gpu/drm/panel/panel-khadas-ts050.c 
> b/drivers/gpu/drm/panel/panel-khadas-ts050.c
> new file mode 100644
> index ..856fcad69306
> --- /dev/null
> +++ b/drivers/gpu/drm/panel/panel-khadas-ts050.c
> @@ -0,0 +1,876 @@
> +// SPDX-License-Identifier: GPL-2.0
> +/*
> + * Copyright (C) 2020 BayLibre, SAS
> + * Author: Neil Armstrong 
> + */
> +
> +#include 
> +#include 
> +#include 
> +#include 
> +#include 
> +
> +#include 
> +
> +#include 
> +#include 
> +#include 
> +#include 
> +#include 
> +#include 
Panels do not use DRM_ or drm_ for logging, so this include is not
needed.

> +
> +struct khadas_ts050_panel {
> + struct drm_panel base;
> + struct mipi_dsi_device *link;
> +
> + struct regulator *supply;
> + struct gpio_desc *reset_gpio;
> + struct gpio_desc *enable_gpio;
> +
> + bool prepared;
> + bool enabled;
> +};
> +
> +struct khadas_ts050_panel_cmd {
> + u8 cmd;
> + u8 data;
> +};
> +
> +/* Only the CMD1 User Command set is documented */
> +static const struct khadas_ts050_panel_cmd init_code[] = {
> + /* Select Unknown CMD Page (Undocumented) */
> + {0xff, 0xee},
...
> + {0xd4, 0x04}, /* RGBMIPICTRL: VSYNC front porch = 4 */
> +};
> +
> +static inline
> +struct khadas_ts050_panel *to_khadas_ts050_panel(struct drm_panel *panel)
> +{
> + return container_of(panel, struct khadas_ts050_panel, base);
> +}
> +
> +static int khadas_ts050_panel_prepare(struct drm_panel *panel)
> +{
> + struct khadas_ts050_panel *khadas_ts050 = to_khadas_ts050_panel(panel);
> + int err, regulator_err;
> + unsigned int i;
> +
> + if (khadas_ts050->prepared)
> + return 0;
> +
> + gpiod_set_value_cansleep(khadas_ts050->enable_gpio, 0);
> +
> + err = regulator_enable(khadas_ts050->supply);
> + if (err < 0)
> + return err;
> +
> + gpiod_set_value_cansleep(khadas_ts050->enable_gpio, 1);
> +
> + msleep(60);
> +
> + gpiod_set_value_cansleep(khadas_ts050->reset_gpio, 1);
> +
> + usleep_range(1, 11000);
> +
> + gpiod_set_value_cansleep(khadas_ts050->reset_gpio, 0);
> +
> + /* Select CMD2 page 4 (Undocumented) */
> + mipi_dsi_dcs_write(khadas_ts050->link, 0xff, (u8[]){ 0x05 }, 1);
> +
> + /* Reload CMD1: Don't reload default value to register */
> + 

[Bug 203905] amdgpu:actual_brightness has unreal/wrong value

[bugzilla-daemon]

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

Germán Ríos González (german.rios.gonza...@gmail.com) changed:

   What|Removed |Added

 CC||german.rios.gonzalez@gmail.
   ||com

--- Comment #18 from Germán Ríos González (german.rios.gonza...@gmail.com) ---
(In reply to Xia Mu from comment #14)
> The bug should be fixed in 5.9.0-0.rc3. I tested it on my laptop with AMD
> Ryzen 7 PRO 4750U CPU with Renoir GPU.

I'm actually using 5.9.9-200.fc33.x86_64 and the problem it's still there

-- 
You are receiving this mail because:
You are watching the assignee of the bug.
___
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

Re: [PATCH 1/2] dt-bindings: panel-simple-dsi: add Khadas TS050 panel bindings

[Sam Ravnborg]

On Mon, Nov 23, 2020 at 03:33:53PM +0100, Neil Armstrong wrote:
> This add the bindings for the Khadas TS050 1080x1920 5" LCD DSI panel 
> designed to work
> with the Khadas Edge-V, Captain, VIM3 and VIM3L Single Board Computers.
> 
> Signed-off-by: Neil Armstrong 
Reviewed-by: Sam Ravnborg 
___
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

Re: [PATCH v3 05/12] drm/ttm: Expose ttm_tt_unpopulate for driver use

[Christian König]

Am 23.11.20 um 21:38 schrieb Andrey Grodzovsky:


On 11/23/20 3:20 PM, Christian König wrote:

Am 23.11.20 um 21:05 schrieb Andrey Grodzovsky:


On 11/25/20 5:42 AM, Christian König wrote:

Am 21.11.20 um 06:21 schrieb Andrey Grodzovsky:

It's needed to drop iommu backed pages on device unplug
before device's IOMMU group is released.


It would be cleaner if we could do the whole handling in TTM. I 
also need to double check what you are doing with this function.


Christian.



Check patch "drm/amdgpu: Register IOMMU topology notifier per 
device." to see
how i use it. I don't see why this should go into TTM mid-layer - 
the stuff I do inside
is vendor specific and also I don't think TTM is explicitly aware of 
IOMMU ?
Do you mean you prefer the IOMMU notifier to be registered from 
within TTM

and then use a hook to call into vendor specific handler ?


No, that is really vendor specific.

What I meant is to have a function like 
ttm_resource_manager_evict_all() which you only need to call and all 
tt objects are unpopulated.



So instead of this BO list i create and later iterate in amdgpu from 
the IOMMU patch you just want to do it within

TTM with a single function ? Makes much more sense.


Yes, exactly.

The list_empty() checks we have in TTM for the LRU are actually not the 
best idea, we should now check the pin_count instead. This way we could 
also have a list of the pinned BOs in TTM.


BTW: Have you thought about what happens when we unpopulate a BO while 
we still try to use a kernel mapping for it? That could have unforeseen 
consequences.


Christian.



Andrey




Give me a day or two to look into this.

Christian.



Andrey






Signed-off-by: Andrey Grodzovsky 
---
  drivers/gpu/drm/ttm/ttm_tt.c | 1 +
  1 file changed, 1 insertion(+)

diff --git a/drivers/gpu/drm/ttm/ttm_tt.c 
b/drivers/gpu/drm/ttm/ttm_tt.c

index 1ccf1ef..29248a5 100644
--- a/drivers/gpu/drm/ttm/ttm_tt.c
+++ b/drivers/gpu/drm/ttm/ttm_tt.c
@@ -495,3 +495,4 @@ void ttm_tt_unpopulate(struct ttm_tt *ttm)
  else
  ttm_pool_unpopulate(ttm);
  }
+EXPORT_SYMBOL(ttm_tt_unpopulate);



___
amd-gfx mailing list
amd-...@lists.freedesktop.org
https://nam11.safelinks.protection.outlook.com/?url=https%3A%2F%2Flists.freedesktop.org%2Fmailman%2Flistinfo%2Famd-gfxdata=04%7C01%7CAndrey.Grodzovsky%40amd.com%7C9be029f26a4746347a6108d88fed299b%7C3dd8961fe4884e608e11a82d994e183d%7C0%7C0%7C637417596065559955%7CUnknown%7CTWFpbGZsb3d8eyJWIjoiMC4wLjAwMDAiLCJQIjoiV2luMzIiLCJBTiI6Ik1haWwiLCJXVCI6Mn0%3D%7C1000sdata=tZ3do%2FeKzBtRlNaFbBjCtRvUHKdvwDZ7SoYhEBu4%2BT8%3Dreserved=0 





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

Re: [PATCH 000/141] Fix fall-through warnings for Clang

[Mark Brown]

On Fri, 20 Nov 2020 12:21:39 -0600, Gustavo A. R. Silva wrote:
> This series aims to fix almost all remaining fall-through warnings in
> order to enable -Wimplicit-fallthrough for Clang.
> 
> In preparation to enable -Wimplicit-fallthrough for Clang, explicitly
> add multiple break/goto/return/fallthrough statements instead of just
> letting the code fall through to the next case.
> 
> [...]

Applied to

   https://git.kernel.org/pub/scm/linux/kernel/git/broonie/regulator.git 
for-next

Thanks!

[1/1] regulator: as3722: Fix fall-through warnings for Clang
  commit: b52b417ccac4fae5b1f2ec4f1d46eb91e4493dc5

All being well this means that it will be integrated into the linux-next
tree (usually sometime in the next 24 hours) and sent to Linus during
the next merge window (or sooner if it is a bug fix), however if
problems are discovered then the patch may be dropped or reverted.

You may get further e-mails resulting from automated or manual testing
and review of the tree, please engage with people reporting problems and
send followup patches addressing any issues that are reported if needed.

If any updates are required or you are submitting further changes they
should be sent as incremental updates against current git, existing
patches will not be replaced.

Please add any relevant lists and maintainers to the CCs when replying
to this mail.

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

Re: [PATCH v3 05/12] drm/ttm: Expose ttm_tt_unpopulate for driver use

[Andrey Grodzovsky]

On 11/23/20 3:20 PM, Christian König wrote:

Am 23.11.20 um 21:05 schrieb Andrey Grodzovsky:


On 11/25/20 5:42 AM, Christian König wrote:

Am 21.11.20 um 06:21 schrieb Andrey Grodzovsky:

It's needed to drop iommu backed pages on device unplug
before device's IOMMU group is released.


It would be cleaner if we could do the whole handling in TTM. I also need to 
double check what you are doing with this function.


Christian.



Check patch "drm/amdgpu: Register IOMMU topology notifier per device." to see
how i use it. I don't see why this should go into TTM mid-layer - the stuff I 
do inside

is vendor specific and also I don't think TTM is explicitly aware of IOMMU ?
Do you mean you prefer the IOMMU notifier to be registered from within TTM
and then use a hook to call into vendor specific handler ?


No, that is really vendor specific.

What I meant is to have a function like ttm_resource_manager_evict_all() which 
you only need to call and all tt objects are unpopulated.



So instead of this BO list i create and later iterate in amdgpu from the IOMMU 
patch you just want to do it within

TTM with a single function ? Makes much more sense.

Andrey




Give me a day or two to look into this.

Christian.



Andrey






Signed-off-by: Andrey Grodzovsky 
---
  drivers/gpu/drm/ttm/ttm_tt.c | 1 +
  1 file changed, 1 insertion(+)

diff --git a/drivers/gpu/drm/ttm/ttm_tt.c b/drivers/gpu/drm/ttm/ttm_tt.c
index 1ccf1ef..29248a5 100644
--- a/drivers/gpu/drm/ttm/ttm_tt.c
+++ b/drivers/gpu/drm/ttm/ttm_tt.c
@@ -495,3 +495,4 @@ void ttm_tt_unpopulate(struct ttm_tt *ttm)
  else
  ttm_pool_unpopulate(ttm);
  }
+EXPORT_SYMBOL(ttm_tt_unpopulate);



___
amd-gfx mailing list
amd-...@lists.freedesktop.org
https://nam11.safelinks.protection.outlook.com/?url=https%3A%2F%2Flists.freedesktop.org%2Fmailman%2Flistinfo%2Famd-gfxdata=04%7C01%7CAndrey.Grodzovsky%40amd.com%7C9be029f26a4746347a6108d88fed299b%7C3dd8961fe4884e608e11a82d994e183d%7C0%7C0%7C637417596065559955%7CUnknown%7CTWFpbGZsb3d8eyJWIjoiMC4wLjAwMDAiLCJQIjoiV2luMzIiLCJBTiI6Ik1haWwiLCJXVCI6Mn0%3D%7C1000sdata=tZ3do%2FeKzBtRlNaFbBjCtRvUHKdvwDZ7SoYhEBu4%2BT8%3Dreserved=0 




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

Re: [PATCH 000/141] Fix fall-through warnings for Clang

[James Bottomley]

On Mon, 2020-11-23 at 19:56 +0100, Miguel Ojeda wrote:
> On Mon, Nov 23, 2020 at 4:58 PM James Bottomley
>  wrote:
> > Well, I used git.  It says that as of today in Linus' tree we have
> > 889 patches related to fall throughs and the first series went in
> > in october 2017 ... ignoring a couple of outliers back to February.
> 
> I can see ~10k insertions over ~1k commits and 15 years that mention
> a fallthrough in the entire repo. That is including some commits
> (like the biggest one, 960 insertions) that have nothing to do with C
> fallthrough. A single kernel release has an order of magnitude more
> changes than this...
> 
> But if we do the math, for an author, at even 1 minute per line
> change and assuming nothing can be automated at all, it would take 1
> month of work. For maintainers, a couple of trivial lines is noise
> compared to many other patches.

So you think a one line patch should take one minute to produce ... I
really don't think that's grounded in reality.  I suppose a one line
patch only takes a minute to merge with b4 if no-one reviews or tests
it, but that's not really desirable.

> In fact, this discussion probably took more time than the time it
> would take to review the 200 lines. :-)

I'm framing the discussion in terms of the whole series of changes we
have done for fall through, both what's in the tree currently (889
patches) both in terms of the produce and the consumer.  That's what I
used for my figures for cost.

> > We're also complaining about the inability to recruit maintainers:
> > 
> > https://www.theregister.com/2020/06/30/hard_to_find_linux_maintainers_says_torvalds/
> > 
> > And burn out:
> > 
> > http://antirez.com/news/129
> 
> Accepting trivial and useful 1-line patches

Part of what I'm trying to measure is the "and useful" bit because
that's not a given.

> is not what makes a voluntary maintainer quit...

so the proverb "straw which broke the camel's back" uniquely doesn't
apply to maintainers

>  Thankless work with demanding deadlines is.

That's another potential reason, but it doesn't may other reasons less
valid.

> > The whole crux of your argument seems to be maintainers' time isn't
> > important so we should accept all trivial patches
> 
> I have not said that, at all. In fact, I am a voluntary one and I
> welcome patches like this. It takes very little effort on my side to
> review and it helps the kernel overall.

Well, you know, subsystems are very different in terms of the amount of
patches a maintainer has to process per release cycle of the kernel. 
If a maintainer is close to capacity, additional patches, however
trivial, become a problem.  If a maintainer has spare cycles, trivial
patches may look easy.

> Paid maintainers are the ones that can take care of big
> features/reviews.
> 
> > What I'm actually trying to articulate is a way of measuring value
> > of the patch vs cost ... it has nothing really to do with who foots
> > the actual bill.
> 
> I understand your point, but you were the one putting it in terms of
> a junior FTE.

No, I evaluated the producer side in terms of an FTE.  What we're
mostly arguing about here is the consumer side: the maintainers and
people who have to rework their patch sets. I estimated that at 100h.

>  In my view, 1 month-work (worst case) is very much worth
> removing a class of errors from a critical codebase.
> 
> > One thesis I'm actually starting to formulate is that this
> > continual devaluing of maintainers is why we have so much
> > difficulty keeping and recruiting them.
> 
> That may very well be true, but I don't feel anybody has devalued
> maintainers in this discussion.

You seem to be saying that because you find it easy to merge trivial
patches, everyone should.  I'm reminded of a friend long ago who
thought being a Tees River Pilot was a sinecure because he could
navigate the Tees blindfold.  What he forgot, of course, is that just
because it's easy with a trawler doesn't mean it's easy with an oil
tanker.  In fact it takes longer to qualify as a Tees River Pilot than
it does to get a PhD.

James


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

Re: [PATCH v3 05/12] drm/ttm: Expose ttm_tt_unpopulate for driver use

[Christian König]

Am 23.11.20 um 21:05 schrieb Andrey Grodzovsky:


On 11/25/20 5:42 AM, Christian König wrote:

Am 21.11.20 um 06:21 schrieb Andrey Grodzovsky:

It's needed to drop iommu backed pages on device unplug
before device's IOMMU group is released.


It would be cleaner if we could do the whole handling in TTM. I also 
need to double check what you are doing with this function.


Christian.



Check patch "drm/amdgpu: Register IOMMU topology notifier per device." 
to see
how i use it. I don't see why this should go into TTM mid-layer - the 
stuff I do inside
is vendor specific and also I don't think TTM is explicitly aware of 
IOMMU ?
Do you mean you prefer the IOMMU notifier to be registered from within 
TTM

and then use a hook to call into vendor specific handler ?


No, that is really vendor specific.

What I meant is to have a function like ttm_resource_manager_evict_all() 
which you only need to call and all tt objects are unpopulated.


Give me a day or two to look into this.

Christian.



Andrey






Signed-off-by: Andrey Grodzovsky 
---
  drivers/gpu/drm/ttm/ttm_tt.c | 1 +
  1 file changed, 1 insertion(+)

diff --git a/drivers/gpu/drm/ttm/ttm_tt.c 
b/drivers/gpu/drm/ttm/ttm_tt.c

index 1ccf1ef..29248a5 100644
--- a/drivers/gpu/drm/ttm/ttm_tt.c
+++ b/drivers/gpu/drm/ttm/ttm_tt.c
@@ -495,3 +495,4 @@ void ttm_tt_unpopulate(struct ttm_tt *ttm)
  else
  ttm_pool_unpopulate(ttm);
  }
+EXPORT_SYMBOL(ttm_tt_unpopulate);



___
amd-gfx mailing list
amd-...@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/amd-gfx


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

Re: [PATCH v3 05/12] drm/ttm: Expose ttm_tt_unpopulate for driver use

[Andrey Grodzovsky]

On 11/25/20 5:42 AM, Christian König wrote:

Am 21.11.20 um 06:21 schrieb Andrey Grodzovsky:

It's needed to drop iommu backed pages on device unplug
before device's IOMMU group is released.


It would be cleaner if we could do the whole handling in TTM. I also need to 
double check what you are doing with this function.


Christian.



Check patch "drm/amdgpu: Register IOMMU topology notifier per device." to see
how i use it. I don't see why this should go into TTM mid-layer - the stuff I do 
inside

is vendor specific and also I don't think TTM is explicitly aware of IOMMU ?
Do you mean you prefer the IOMMU notifier to be registered from within TTM
and then use a hook to call into vendor specific handler ?

Andrey






Signed-off-by: Andrey Grodzovsky 
---
  drivers/gpu/drm/ttm/ttm_tt.c | 1 +
  1 file changed, 1 insertion(+)

diff --git a/drivers/gpu/drm/ttm/ttm_tt.c b/drivers/gpu/drm/ttm/ttm_tt.c
index 1ccf1ef..29248a5 100644
--- a/drivers/gpu/drm/ttm/ttm_tt.c
+++ b/drivers/gpu/drm/ttm/ttm_tt.c
@@ -495,3 +495,4 @@ void ttm_tt_unpopulate(struct ttm_tt *ttm)
  else
  ttm_pool_unpopulate(ttm);
  }
+EXPORT_SYMBOL(ttm_tt_unpopulate);



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

RE: [PATCH rdma-core 3/5] pyverbs: Add dma-buf based MR support

[Xiong, Jianxin]

> -Original Message-
> From: Jason Gunthorpe 
> Sent: Monday, November 23, 2020 10:05 AM
> To: Xiong, Jianxin 
> Cc: linux-r...@vger.kernel.org; dri-devel@lists.freedesktop.org; Doug Ledford 
> ; Leon Romanovsky
> ; Sumit Semwal ; Christian Koenig 
> ; Vetter, Daniel
> 
> Subject: Re: [PATCH rdma-core 3/5] pyverbs: Add dma-buf based MR support
> 
> On Mon, Nov 23, 2020 at 09:53:02AM -0800, Jianxin Xiong wrote:
> 
> > +cdef class DmaBuf:
> > +def __init__(self, size, unit=0):
> > +"""
> > +Allocate DmaBuf object from a GPU device. This is done through the
> > +DRI device interface (/dev/dri/card*). Usually this requires the
> > +effective user id being root or being a member of the 'video' 
> > group.
> > +:param size: The size (in number of bytes) of the buffer.
> > +:param unit: The unit number of the GPU to allocate the buffer 
> > from.
> > +:return: The newly created DmaBuf object on success.
> > +"""
> > +self.dmabuf_mrs = weakref.WeakSet()
> > +self.dri_fd = open('/dev/dri/card'+str(unit), O_RDWR)
> > +
> > +args = bytearray(32)
> > +pack_into('=iiq', args, 0, 1, size, 8, 0, 0, 0, 0)
> > +ioctl(self.dri_fd, DRM_IOCTL_MODE_CREATE_DUMB, args)
> > +a, b, c, d, self.handle, e, self.size = unpack('=iiq',
> > + args)
> > +
> > +args = bytearray(12)
> > +pack_into('=iii', args, 0, self.handle, O_RDWR, 0)
> > +ioctl(self.dri_fd, DRM_IOCTL_PRIME_HANDLE_TO_FD, args)
> > +a, b, self.fd = unpack('=iii', args)
> > +
> > +args = bytearray(16)
> > +pack_into('=iiq', args, 0, self.handle, 0, 0)
> > +ioctl(self.dri_fd, DRM_IOCTL_MODE_MAP_DUMB, args);
> > +a, b, self.map_offset = unpack('=iiq', args);
> 
> Wow, OK
> 
> Is it worth using ctypes here instead? Can you at least add a comment before 
> each pack specifying the 'struct XXX' this is following?
> 

The ioctl call only accept a bytearray, not sure how to use ctypes here. I will 
add 
comments with the actual layout of the parameter structure.

> Does this work with normal Intel GPUs, like in a Laptop? AMD too?
> 

Yes, the interface is generic and works with most GPUs. Works with AMD, too.

> Christian, I would be very happy to hear from you that this entire work is 
> good for AMD as well
> 
> Edward should look through this, but I'm glad to see something like this
> 
> Thanks,
> Jason
___
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

RE: [PATCH rdma-core 2/5] mlx5: Support dma-buf based memory region

[Xiong, Jianxin]

> -Original Message-
> From: Jason Gunthorpe 
> Sent: Monday, November 23, 2020 10:02 AM
> To: Xiong, Jianxin 
> Cc: linux-r...@vger.kernel.org; dri-devel@lists.freedesktop.org; Doug Ledford 
> ; Leon Romanovsky
> ; Sumit Semwal ; Christian Koenig 
> ; Vetter, Daniel
> 
> Subject: Re: [PATCH rdma-core 2/5] mlx5: Support dma-buf based memory region
> 
> On Mon, Nov 23, 2020 at 09:53:01AM -0800, Jianxin Xiong wrote:
> 
> > +struct ibv_mr *mlx5_reg_dmabuf_mr(struct ibv_pd *pd, uint64_t offset, 
> > size_t length,
> > + uint64_t iova, int fd, int acc) {
> > +   struct mlx5_mr *mr;
> > +   int ret;
> > +   enum ibv_access_flags access = (enum ibv_access_flags)acc;
> 
> Why?

It's copied from mlx5_reg_mr(). Didn't pay attention to this but looks 
unnecessary now.

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

Re: [PATCH v2 10/10] drm/fb-helper: Acquire modeset lock around shadow-buffer flushing

[Sam Ravnborg]

On Fri, Nov 20, 2020 at 11:25:45AM +0100, Thomas Zimmermann wrote:
> Flushing the fbdev's shadow buffer requires vmap'ing the BO memory, which
> in turn requires pinning the BO. While being pinned, the BO cannot be moved
> into VRAM for scanout. Consequently, a concurrent modeset operation that
> involves the fbdev framebuffer would likely fail.
> 
> Resolve this problem be acquiring the modeset lock of the planes that use
> the fbdev framebuffer. On non-atomic drivers, also acquire the mode-config
> lock. This serializes the flushing of the framebuffer with concurrent
> modeset operations.
> 
> v2:
>   * only acquire struct drm_fb_helper.lock in damage blitter (Daniel,
> Christian)
> 
> Signed-off-by: Thomas Zimmermann 
Acked-by: Sam Ravnborg 
___
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

Re: [PATCH v2 09/10] drm/fb-helper: Copy dma-buf map before flushing shadow fb

[Sam Ravnborg]

On Fri, Nov 20, 2020 at 11:25:44AM +0100, Thomas Zimmermann wrote:
> Copy the vmap()'ed instance of struct dma_buf_map before modifying it,
> in case the implementation of vunmap() depends on the exact address.
> 
> Signed-off-by: Thomas Zimmermann 
Acked-by: Sam Ravnborg 
___
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

Re: [PATCH v2 08/10] drm/fb-helper: Restore damage area upon errors

[Sam Ravnborg]

On Fri, Nov 20, 2020 at 11:25:43AM +0100, Thomas Zimmermann wrote:
> If the damage handling fails, restore the damage area. The next invocation
> of the damage worker will then perform the update.
> 
> v2:
>   * print a single warning if dirty callback fails (Daniel, Sebastian)
>   * update comment
> 
> Signed-off-by: Thomas Zimmermann 
Acked-by: Sam Ravnborg 
___
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

Re: [PATCH v2 07/10] drm/fb-helper: Move damage blit code and its setup into separate routine

[Sam Ravnborg]

On Fri, Nov 20, 2020 at 11:25:42AM +0100, Thomas Zimmermann wrote:
> Introduce a separate function for the blit code and its vmap setup. Done
> in preparation of additional changes. No functional changes are made.
> 
> v2:
>   * print a single warning if damage blitter fails
> 
> Signed-off-by: Thomas Zimmermann 
Reviewed-by: Sam Ravnborg 
___
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

Re: [PATCH v2 06/10] drm/fb-helper: Separate shadow-buffer flushing and calling dirty callback

[Sam Ravnborg]

On Fri, Nov 20, 2020 at 11:25:41AM +0100, Thomas Zimmermann wrote:
> Flushing the shadow framebuffer and invoking the dirty callback are two
> separate operations, so do them seprately. The flush operation is paired
> with calls to vmap and vunmap. They are not needed for the dirty callback,
> which performs its own invocations if necessary.
> 
> Signed-off-by: Thomas Zimmermann 
Reviewed-by: Sam Ravnborg 
___
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

Re: [PATCH v2 05/10] drm/fb-helper: Return early in dirty worker

[Sam Ravnborg]

On Fri, Nov 20, 2020 at 11:25:40AM +0100, Thomas Zimmermann wrote:
> Returning early in the dirty worker if no update is required makes the
> code more readable. No functional changes are made.
> 
> Signed-off-by: Thomas Zimmermann 

It is a damage worker (both subject and changelog).

Reviewed-by: Sam Ravnborg 

> ---
>  drivers/gpu/drm/drm_fb_helper.c | 31 +++
>  1 file changed, 15 insertions(+), 16 deletions(-)
> 
> diff --git a/drivers/gpu/drm/drm_fb_helper.c b/drivers/gpu/drm/drm_fb_helper.c
> index 87d4759de04a..c90185f9 100644
> --- a/drivers/gpu/drm/drm_fb_helper.c
> +++ b/drivers/gpu/drm/drm_fb_helper.c
> @@ -407,24 +407,23 @@ static void drm_fb_helper_damage_work(struct 
> work_struct *work)
>   clip->x2 = clip->y2 = 0;
>   spin_unlock_irqrestore(>damage_lock, flags);
>  
> - /* call dirty callback only when it has been really touched */
> - if (clip_copy.x1 < clip_copy.x2 && clip_copy.y1 < clip_copy.y2) {
> -
> - /* Generic fbdev uses a shadow buffer */
> - if (helper->buffer) {
> - ret = drm_client_buffer_vmap(helper->buffer, );
> - if (ret)
> - return;
> - drm_fb_helper_damage_blit_real(helper, _copy, 
> );
> - }
> -
> - if (helper->fb->funcs->dirty)
> - helper->fb->funcs->dirty(helper->fb, NULL, 0, 0,
> -  _copy, 1);
> + /* Call damage handlers only if necessary */
> + if (!(clip_copy.x1 < clip_copy.x2 && clip_copy.y1 < clip_copy.y2))
> + return;
>  
> - if (helper->buffer)
> - drm_client_buffer_vunmap(helper->buffer);
> + /* Generic fbdev uses a shadow buffer */
> + if (helper->buffer) {
> + ret = drm_client_buffer_vmap(helper->buffer, );
> + if (ret)
> + return;
> + drm_fb_helper_damage_blit_real(helper, _copy, );
>   }
> +
> + if (helper->fb->funcs->dirty)
> + helper->fb->funcs->dirty(helper->fb, NULL, 0, 0, _copy, 1);
> +
> + if (helper->buffer)
> + drm_client_buffer_vunmap(helper->buffer);
>  }
>  
>  /**
> -- 
> 2.29.2
> 
> ___
> dri-devel mailing list
> dri-devel@lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/dri-devel
___
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

Re: [PATCH v2 04/10] drm/fb-helper: Rename dirty worker to damage worker

[Sam Ravnborg]

On Fri, Nov 20, 2020 at 11:25:39AM +0100, Thomas Zimmermann wrote:
> The dirty worker handles all damage updates, instead of just calling
> the framebuffer's dirty callback. Rename it to damage worker. Also
> rename related variables accordingly. No functional changes are made.
> 
> Signed-off-by: Thomas Zimmermann 
Reviewed-by: Sam Ravnborg 

It had been nice to move to inline comments in struct drm_fb_helper_funcs
but that can be another day.

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

Re: [PATCHv8 0/8] System Cache support for GPU and required SMMU support

[Rob Clark]

On Mon, Nov 23, 2020 at 9:01 AM Sai Prakash Ranjan
 wrote:
>
> On 2020-11-23 20:51, Will Deacon wrote:
> > On Tue, Nov 17, 2020 at 08:00:39PM +0530, Sai Prakash Ranjan wrote:
> >> Some hardware variants contain a system cache or the last level
> >> cache(llc). This cache is typically a large block which is shared
> >> by multiple clients on the SOC. GPU uses the system cache to cache
> >> both the GPU data buffers(like textures) as well the SMMU pagetables.
> >> This helps with improved render performance as well as lower power
> >> consumption by reducing the bus traffic to the system memory.
> >>
> >> The system cache architecture allows the cache to be split into slices
> >> which then be used by multiple SOC clients. This patch series is an
> >> effort to enable and use two of those slices preallocated for the GPU,
> >> one for the GPU data buffers and another for the GPU SMMU hardware
> >> pagetables.
> >>
> >> Patch 1 - Patch 6 adds system cache support in SMMU and GPU driver.
> >> Patch 7 and 8 are minor cleanups for arm-smmu impl.
> >>
> >> Changes in v8:
> >>  * Introduce a generic domain attribute for pagetable config (Will)
> >>  * Rename quirk to more generic IO_PGTABLE_QUIRK_ARM_OUTER_WBWA (Will)
> >>  * Move non-strict mode to use new struct domain_attr_io_pgtbl_config
> >> (Will)
> >
> > Modulo some minor comments I've made, this looks good to me. What is
> > the
> > plan for merging it? I can take the IOMMU parts, but patches 4-6 touch
> > the
> > MSM GPU driver and I'd like to avoid conflicts with that.
> >
>
> SMMU bits are pretty much independent and GPU relies on the domain
> attribute
> and the quirk exposed, so as long as SMMU changes go in first it should
> be good.
> Rob?

I suppose one option would be to split out the patch that adds the
attribute into it's own patch, and merge that both thru drm and iommu?

If Will/Robin dislike that approach, I'll pick up the parts of the drm
patches which don't depend on the new attribute for v5.11 and the rest
for v5.12.. or possibly a second late v5.11 pull req if airlied
doesn't hate me too much for it.

Going forward, I think we will have one or two more co-dependent
series, like the smmu iova fault handler improvements that Jordan
posted.  So I would like to hear how Will and Robin prefer to handle
those.

BR,
-R


> Thanks,
> Sai
>
> --
> QUALCOMM INDIA, on behalf of Qualcomm Innovation Center, Inc. is a
> member
> of Code Aurora Forum, hosted by The Linux Foundation
___
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

Re: [PATCH v2 02/10] drm/fb-helper: Unmap client buffer during shutdown

[Sam Ravnborg]

On Fri, Nov 20, 2020 at 11:25:37AM +0100, Thomas Zimmermann wrote:
> The fbdev helper's generic probe function establishes a mapping for
> framebuffers without shadow buffer. The clean-up function did not unmap
> the buffer object. Add the unmap operation.
> 
> As fbdev devices are usally released during system shutdown, this has
> not been a problem in practice.
> 
> Signed-off-by: Thomas Zimmermann 
Reviewed-by: Sam Ravnborg 
___
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

Re: [PATCH v2 01/10] drm/fb-helper: Call dirty helper after writing to fbdev

[Sam Ravnborg]

On Fri, Nov 20, 2020 at 11:25:36AM +0100, Thomas Zimmermann wrote:
> If fbdev uses a shadow framebuffer, call the damage handler. Otherwise
> the update might not make it to the screen.
> 
> v2:
>   * mark virtual screen as dirty (Ville)
> 
> Signed-off-by: Thomas Zimmermann 
> Fixes: 222ec45f4c69 ("drm/fb_helper: Support framebuffers in I/O memory")
> Cc: Thomas Zimmermann 
> Cc: Daniel Vetter 
> Cc: Sam Ravnborg 
> Cc: Maarten Lankhorst 
> Cc: Maxime Ripard 
> Cc: David Airlie 
> Cc: Daniel Vetter 
> Cc: Gerd Hoffmann 
> Cc: dri-devel@lists.freedesktop.org
> Cc: virtualizat...@lists.linux-foundation.org
Acked-by: Sam Ravnborg 
___
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

Re: [PATCH 0/2] drm/cma-helper: Move mmap to object functions

[Thomas Zimmermann]

Am 23.11.20 um 13:32 schrieb Maxime Ripard:

On Mon, Nov 23, 2020 at 12:56:44PM +0100, Thomas Zimmermann wrote:

This patchset moves CMA's mmap into a GEM object function. This change
allows for removing CMA's mmap and gem_prime_mmap callbacks in favor of
the default implementation.

Tested with vc4 on RPi3.


For both patches,
Acked-by: Maxime Ripard 


Thanks, Maxime!



Maxime



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


OpenPGP_0x680DC11D530B7A23.asc
Description: application/pgp-keys


OpenPGP_signature
Description: OpenPGP digital signature
___
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

Re: [PATCH v3 6/6] drm/bridge: cdns-mhdp8546: Fix the interrupt enable/disable

[Tomi Valkeinen]

On 19/11/2020 18:01, Nikhil Devshatwar wrote:
> When removing the tidss driver, there is a warning reported by
> kernel about an unhandled interrupt for mhdp driver.
> 
> [   43.238895] irq 31: nobody cared (try booting with the "irqpoll" option)
> ... [snipped backtrace]
> [   43.330735] handlers:
> [   43.333020] [<5367c4f9>] irq_default_primary_handler threaded 
> [<7e02b601>]
> cdns_mhdp_irq_handler [cdns_mhdp8546]
> [   43.344607] Disabling IRQ #31
> 
> This happens because as part of cdns_mhdp_bridge_hpd_disable, driver tries
> to disable the interrupts. While disabling the SW_EVENT interrupts,
> it accidentally enables the MBOX interrupts, which are not handled by
> the driver.
> 
> Fix this with a read-modify-write to update only required bits.
> Use the enable / disable function as required in other places.
> 
> Signed-off-by: Nikhil Devshatwar 
> Reviewed-by: Swapnil Jakhade 

Reviewed-by: Tomi Valkeinen 

I think this is fine as a fix for this issue, but at some point the irq 
management needs some work.
E.g. we call cdns_mhdp_bridge_hpd_enable when attaching/enabling the hw, but 
also via
drm_bridge_funcs->hpd_enable. This doesn't make sense, as one of those calls 
doesn't achieve
anything, as cdns_mhdp_bridge_hpd_enable has already been called.

 Tomi

-- 
Texas Instruments Finland Oy, Porkkalankatu 22, 00180 Helsinki.
Y-tunnus/Business ID: 0615521-4. Kotipaikka/Domicile: Helsinki
___
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

[PATCH rdma-core 5/5] tests: Bug fix for get_access_flags()

[Jianxin Xiong]

The filter defintion is wrong and causes get_access_flags() always
returning empty list. As the result the MR tests using this function
are effectively skipped (but report success).

Also fix a typo in the comments.

Signed-off-by: Jianxin Xiong 
---
 tests/utils.py | 6 +++---
 1 file changed, 3 insertions(+), 3 deletions(-)

diff --git a/tests/utils.py b/tests/utils.py
index 0ad7110..eee44b4 100644
--- a/tests/utils.py
+++ b/tests/utils.py
@@ -55,8 +55,8 @@ def filter_illegal_access_flags(element):
 :param element: A list of access flags to check
 :return: True if this list is legal, else False
 """
-if e.IBV_ACCESS_REMOTE_ATOMIC in element or e.IBV_ACCESS_REMOTE_WRITE:
-if e.IBV_ACCESS_LOCAL_WRITE:
+if e.IBV_ACCESS_REMOTE_ATOMIC in element or e.IBV_ACCESS_REMOTE_WRITE in 
element:
+if not e.IBV_ACCESS_LOCAL_WRITE in element:
 return False
 return True
 
@@ -69,7 +69,7 @@ def get_access_flags(ctx):
 added as well.
 After verifying that the flags selection is legal, it is appended to an
 array, assuming it wasn't previously appended.
-:param ctx: Device Context to check capabilities
+:param ctx: Device Coyyntext to check capabilities
 :param num: Size of initial collection
 :return: A random legal value for MR flags
 """
-- 
1.8.3.1

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

[PATCH rdma-core 3/5] pyverbs: Add dma-buf based MR support

[Jianxin Xiong]

Define a new sub-class of 'MR' that uses dma-buf object for the memory
region. Define a new class 'DmaBuf' for dma-buf object allocation.

Signed-off-by: Jianxin Xiong 
---
 pyverbs/CMakeLists.txt |  2 ++
 pyverbs/dmabuf.pxd | 13 +
 pyverbs/dmabuf.pyx | 58 +
 pyverbs/libibverbs.pxd |  2 ++
 pyverbs/mr.pxd |  5 
 pyverbs/mr.pyx | 77 --
 6 files changed, 155 insertions(+), 2 deletions(-)
 create mode 100644 pyverbs/dmabuf.pxd
 create mode 100644 pyverbs/dmabuf.pyx

diff --git a/pyverbs/CMakeLists.txt b/pyverbs/CMakeLists.txt
index 9542c4b..5aee02b 100644
--- a/pyverbs/CMakeLists.txt
+++ b/pyverbs/CMakeLists.txt
@@ -1,5 +1,6 @@
 # SPDX-License-Identifier: (GPL-2.0 OR Linux-OpenIB)
 # Copyright (c) 2019, Mellanox Technologies. All rights reserved. See COPYING 
file
+# Copyright (c) 2020, Intel Corporation. All rights reserved.
 
 rdma_cython_module(pyverbs ""
   addr.pyx
@@ -16,6 +17,7 @@ rdma_cython_module(pyverbs ""
   wr.pyx
   xrcd.pyx
   srq.pyx
+  dmabuf.pyx
   )
 
 rdma_python_module(pyverbs
diff --git a/pyverbs/dmabuf.pxd b/pyverbs/dmabuf.pxd
new file mode 100644
index 000..040db4b
--- /dev/null
+++ b/pyverbs/dmabuf.pxd
@@ -0,0 +1,13 @@
+# SPDX-License-Identifier: (GPL-2.0 OR Linux-OpenIB)
+# Copyright (c) 2020, Intel Corporation. All rights reserved.
+
+#cython: language_level=3
+
+cdef class DmaBuf:
+cdef int dri_fd
+cdef int handle
+cdef int fd
+cdef unsigned long size
+cdef unsigned long map_offset
+cdef object dmabuf_mrs
+cdef add_ref(self, obj)
diff --git a/pyverbs/dmabuf.pyx b/pyverbs/dmabuf.pyx
new file mode 100644
index 000..6c7622d
--- /dev/null
+++ b/pyverbs/dmabuf.pyx
@@ -0,0 +1,58 @@
+# SPDX-License-Identifier: (GPL-2.0 OR Linux-OpenIB)
+# Copyright (c) 2020, Intel Corporation. All rights reserved.
+
+#cython: language_level=3
+
+import weakref
+
+from os import open, close, O_RDWR
+from fcntl import ioctl
+from struct import pack_into, unpack
+from pyverbs.base cimport close_weakrefs
+from pyverbs.mr cimport DmaBufMR
+
+cdef extern from "drm/drm.h":
+cdef int DRM_IOCTL_MODE_CREATE_DUMB
+cdef int DRM_IOCTL_MODE_MAP_DUMB
+cdef int DRM_IOCTL_MODE_DESTROY_DUMB
+cdef int DRM_IOCTL_PRIME_HANDLE_TO_FD
+
+cdef class DmaBuf:
+def __init__(self, size, unit=0):
+"""
+Allocate DmaBuf object from a GPU device. This is done through the
+DRI device interface (/dev/dri/card*). Usually this requires the
+effective user id being root or being a member of the 'video' group.
+:param size: The size (in number of bytes) of the buffer.
+:param unit: The unit number of the GPU to allocate the buffer from.
+:return: The newly created DmaBuf object on success.
+"""
+self.dmabuf_mrs = weakref.WeakSet()
+self.dri_fd = open('/dev/dri/card'+str(unit), O_RDWR)
+
+args = bytearray(32)
+pack_into('=iiq', args, 0, 1, size, 8, 0, 0, 0, 0)
+ioctl(self.dri_fd, DRM_IOCTL_MODE_CREATE_DUMB, args)
+a, b, c, d, self.handle, e, self.size = unpack('=iiq', args)
+
+args = bytearray(12)
+pack_into('=iii', args, 0, self.handle, O_RDWR, 0)
+ioctl(self.dri_fd, DRM_IOCTL_PRIME_HANDLE_TO_FD, args)
+a, b, self.fd = unpack('=iii', args)
+
+args = bytearray(16)
+pack_into('=iiq', args, 0, self.handle, 0, 0)
+ioctl(self.dri_fd, DRM_IOCTL_MODE_MAP_DUMB, args);
+a, b, self.map_offset = unpack('=iiq', args);
+
+def __dealloc__(self):
+close_weakrefs([self.dmabuf_mrs])
+args = bytearray(4)
+pack_into('=i', args, 0, self.handle)
+ioctl(self.dri_fd, DRM_IOCTL_MODE_DESTROY_DUMB, args)
+close(self.dri_fd)
+
+cdef add_ref(self, obj):
+if isinstance(obj, DmaBufMR):
+self.dmabuf_mrs.add(obj)
+
diff --git a/pyverbs/libibverbs.pxd b/pyverbs/libibverbs.pxd
index 6fbba54..95e51e1 100644
--- a/pyverbs/libibverbs.pxd
+++ b/pyverbs/libibverbs.pxd
@@ -507,6 +507,8 @@ cdef extern from 'infiniband/verbs.h':
 ibv_pd *ibv_alloc_pd(ibv_context *context)
 int ibv_dealloc_pd(ibv_pd *pd)
 ibv_mr *ibv_reg_mr(ibv_pd *pd, void *addr, size_t length, int access)
+ibv_mr *ibv_reg_dmabuf_mr(ibv_pd *pd, uint64_t offset, size_t length,
+ int fd, int access)
 int ibv_dereg_mr(ibv_mr *mr)
 int ibv_advise_mr(ibv_pd *pd, uint32_t advice, uint32_t flags,
   ibv_sge *sg_list, uint32_t num_sge)
diff --git a/pyverbs/mr.pxd b/pyverbs/mr.pxd
index ebe8ada..b89cf02 100644
--- a/pyverbs/mr.pxd
+++ b/pyverbs/mr.pxd
@@ -1,5 +1,6 @@
 # SPDX-License-Identifier: (GPL-2.0 OR Linux-OpenIB)
 # Copyright (c) 2019, Mellanox Technologies. All rights reserved. See COPYING 
file
+# Copyright (c) 2020, Intel Corporation. All rights reserved.
 
 #cython: language_level=3
 
@@ -33,3 +34,7 @@ cdef class 

[PATCH rdma-core 4/5] tests: Add tests for dma-buf based memory regions

[Jianxin Xiong]

Define a full set of tests similar to regular MR tests. Add a utility
function to generate access flags for dma-buf based MRs because the
set of supported flags is smaller.

Signed-off-by: Jianxin Xiong 
---
 tests/test_mr.py | 130 ++-
 tests/utils.py   |  25 +++
 2 files changed, 154 insertions(+), 1 deletion(-)

diff --git a/tests/test_mr.py b/tests/test_mr.py
index adc649c..8d7f002 100644
--- a/tests/test_mr.py
+++ b/tests/test_mr.py
@@ -9,9 +9,10 @@ import errno
 
 from tests.base import PyverbsAPITestCase, RCResources, RDMATestCase
 from pyverbs.pyverbs_error import PyverbsRDMAError, PyverbsError
-from pyverbs.mr import MR, MW, DMMR, MWBindInfo, MWBind
+from pyverbs.mr import MR, MW, DMMR, DmaBufMR, MWBindInfo, MWBind
 from pyverbs.qp import QPCap, QPInitAttr, QPAttr, QP
 from pyverbs.wr import SendWR
+from pyverbs.dmabuf import DmaBuf
 import pyverbs.device as d
 from pyverbs.pd import PD
 import pyverbs.enums as e
@@ -366,3 +367,130 @@ class DMMRTest(PyverbsAPITestCase):
 dm_mr = DMMR(pd, dm_mr_len, e.IBV_ACCESS_ZERO_BASED,
  dm=dm, offset=dm_mr_offset)
 dm_mr.close()
+
+class DmaBufMRTest(PyverbsAPITestCase):
+"""
+Test various functionalities of the DmaBufMR class.
+"""
+def test_dmabuf_reg_mr(self):
+"""
+Test ibv_reg_dmabuf_mr()
+"""
+for ctx, attr, attr_ex in self.devices:
+with PD(ctx) as pd:
+flags = u.get_dmabuf_access_flags(ctx)
+for f in flags:
+len = u.get_mr_length()
+off = random.randint(0, len//2)
+with DmaBufMR(pd, len, f, offset=off) as mr:
+pass
+
+def test_dmabuf_dereg_mr(self):
+"""
+Test ibv_dereg_mr() with DmaBufMR
+"""
+for ctx, attr, attr_ex in self.devices:
+with PD(ctx) as pd:
+flags = u.get_dmabuf_access_flags(ctx)
+for f in flags:
+len = u.get_mr_length()
+off = random.randint(0, len//2)
+with DmaBufMR(pd, len, f, offset=off) as mr:
+mr.close()
+
+def test_dmabuf_dereg_mr_twice(self):
+"""
+Verify that explicit call to DmaBufMR's close() doesn't fail
+"""
+for ctx, attr, attr_ex in self.devices:
+with PD(ctx) as pd:
+flags = u.get_dmabuf_access_flags(ctx)
+for f in flags:
+len = u.get_mr_length()
+off = random.randint(0, len//2)
+with DmaBufMR(pd, len, f, offset=off) as mr:
+# Pyverbs supports multiple destruction of objects,
+# we are not expecting an exception here.
+mr.close()
+mr.close()
+
+def test_dmabuf_reg_mr_bad_flags(self):
+"""
+Verify that illegal flags combination fails as expected
+"""
+for ctx, attr, attr_ex in self.devices:
+with PD(ctx) as pd:
+for i in range(5):
+flags = random.sample([e.IBV_ACCESS_REMOTE_WRITE,
+   e.IBV_ACCESS_REMOTE_ATOMIC],
+  random.randint(1, 2))
+mr_flags = 0
+for i in flags:
+mr_flags += i.value
+try:
+DmaBufMR(pd, u.get_mr_length(), mr_flags)
+except PyverbsRDMAError as err:
+assert 'Failed to register a dma-buf MR' in err.args[0]
+else:
+raise PyverbsRDMAError('Registered a dma-buf MR with 
illegal falgs')
+
+def test_dmabuf_write(self):
+"""
+Test writing to DmaBufMR's buffer
+"""
+for ctx, attr, attr_ex in self.devices:
+with PD(ctx) as pd:
+for i in range(10):
+mr_len = u.get_mr_length()
+mr_off = random.randint(0, mr_len//2)
+flags = u.get_dmabuf_access_flags(ctx)
+for f in flags:
+with DmaBufMR(pd, mr_len, f, offset=mr_off) as mr:
+write_len = min(random.randint(1, MAX_IO_LEN),
+mr_len)
+mr.write('a' * write_len, write_len)
+
+def test_dmabuf_read(self):
+"""
+Test reading from DmaBufMR's buffer
+"""
+for ctx, attr, attr_ex in self.devices:
+with PD(ctx) as pd:
+for i in range(10):
+mr_len = u.get_mr_length()
+mr_off = random.randint(0, mr_len//2)
+flags = 

[PATCH rdma-core 2/5] mlx5: Support dma-buf based memory region

[Jianxin Xiong]

Implement the new provider method for registering dma-buf based memory
regions.

Signed-off-by: Jianxin Xiong 
---
 providers/mlx5/mlx5.c  |  2 ++
 providers/mlx5/mlx5.h  |  3 +++
 providers/mlx5/verbs.c | 23 +++
 3 files changed, 28 insertions(+)

diff --git a/providers/mlx5/mlx5.c b/providers/mlx5/mlx5.c
index 1378acf..b3e2d57 100644
--- a/providers/mlx5/mlx5.c
+++ b/providers/mlx5/mlx5.c
@@ -1,5 +1,6 @@
 /*
  * Copyright (c) 2012 Mellanox Technologies, Inc.  All rights reserved.
+ * Copyright (c) 2020 Intel Corporation.  All rights reserved.
  *
  * This software is available to you under a choice of one of two
  * licenses.  You may choose to be licensed under the terms of the GNU
@@ -96,6 +97,7 @@ static const struct verbs_context_ops mlx5_ctx_common_ops = {
.async_event   = mlx5_async_event,
.dealloc_pd= mlx5_free_pd,
.reg_mr= mlx5_reg_mr,
+   .reg_dmabuf_mr = mlx5_reg_dmabuf_mr,
.rereg_mr  = mlx5_rereg_mr,
.dereg_mr  = mlx5_dereg_mr,
.alloc_mw  = mlx5_alloc_mw,
diff --git a/providers/mlx5/mlx5.h b/providers/mlx5/mlx5.h
index 8c94f72..17a2470 100644
--- a/providers/mlx5/mlx5.h
+++ b/providers/mlx5/mlx5.h
@@ -1,5 +1,6 @@
 /*
  * Copyright (c) 2012 Mellanox Technologies, Inc.  All rights reserved.
+ * Copyright (c) 2020 Intel Corporation.  All rights reserved.
  *
  * This software is available to you under a choice of one of two
  * licenses.  You may choose to be licensed under the terms of the GNU
@@ -903,6 +904,8 @@ void mlx5_async_event(struct ibv_context *context,
 struct ibv_mr *mlx5_alloc_null_mr(struct ibv_pd *pd);
 struct ibv_mr *mlx5_reg_mr(struct ibv_pd *pd, void *addr, size_t length,
   uint64_t hca_va, int access);
+struct ibv_mr *mlx5_reg_dmabuf_mr(struct ibv_pd *pd, uint64_t offset, size_t 
length,
+ uint64_t iova, int fd, int access);
 int mlx5_rereg_mr(struct verbs_mr *mr, int flags, struct ibv_pd *pd, void 
*addr,
  size_t length, int access);
 int mlx5_dereg_mr(struct verbs_mr *mr);
diff --git a/providers/mlx5/verbs.c b/providers/mlx5/verbs.c
index b956156..6279993 100644
--- a/providers/mlx5/verbs.c
+++ b/providers/mlx5/verbs.c
@@ -1,5 +1,6 @@
 /*
  * Copyright (c) 2012 Mellanox Technologies, Inc.  All rights reserved.
+ * Copyright (c) 2020 Intel Corporation.  All rights reserved.
  *
  * This software is available to you under a choice of one of two
  * licenses.  You may choose to be licensed under the terms of the GNU
@@ -647,6 +648,28 @@ struct ibv_mr *mlx5_reg_mr(struct ibv_pd *pd, void *addr, 
size_t length,
return >vmr.ibv_mr;
 }
 
+struct ibv_mr *mlx5_reg_dmabuf_mr(struct ibv_pd *pd, uint64_t offset, size_t 
length,
+ uint64_t iova, int fd, int acc)
+{
+   struct mlx5_mr *mr;
+   int ret;
+   enum ibv_access_flags access = (enum ibv_access_flags)acc;
+
+   mr = calloc(1, sizeof(*mr));
+   if (!mr)
+   return NULL;
+
+   ret = ibv_cmd_reg_dmabuf_mr(pd, offset, length, iova, fd, access,
+   >vmr);
+   if (ret) {
+   free(mr);
+   return NULL;
+   }
+   mr->alloc_flags = acc;
+
+   return >vmr.ibv_mr;
+}
+
 struct ibv_mr *mlx5_alloc_null_mr(struct ibv_pd *pd)
 {
struct mlx5_mr *mr;
-- 
1.8.3.1

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

[PATCH rdma-core 1/5] verbs: Support dma-buf based memory region

[Jianxin Xiong]

Add new API function and new provider method for registering dma-buf
based memory region. Update the man page and bump the API version.

Signed-off-by: Jianxin Xiong 
---
 kernel-headers/rdma/ib_user_ioctl_cmds.h | 14 
 libibverbs/cmd_mr.c  | 38 
 libibverbs/driver.h  |  7 ++
 libibverbs/dummy_ops.c   | 11 +
 libibverbs/libibverbs.map.in |  6 +
 libibverbs/man/ibv_reg_mr.3  | 21 --
 libibverbs/verbs.c   | 19 
 libibverbs/verbs.h   | 10 +
 8 files changed, 124 insertions(+), 2 deletions(-)

diff --git a/kernel-headers/rdma/ib_user_ioctl_cmds.h 
b/kernel-headers/rdma/ib_user_ioctl_cmds.h
index 7968a18..dafc7eb 100644
--- a/kernel-headers/rdma/ib_user_ioctl_cmds.h
+++ b/kernel-headers/rdma/ib_user_ioctl_cmds.h
@@ -1,5 +1,6 @@
 /*
  * Copyright (c) 2018, Mellanox Technologies inc.  All rights reserved.
+ * Copyright (c) 2020, Intel Corporation. All rights reserved.
  *
  * This software is available to you under a choice of one of two
  * licenses.  You may choose to be licensed under the terms of the GNU
@@ -251,6 +252,7 @@ enum uverbs_methods_mr {
UVERBS_METHOD_MR_DESTROY,
UVERBS_METHOD_ADVISE_MR,
UVERBS_METHOD_QUERY_MR,
+   UVERBS_METHOD_REG_DMABUF_MR,
 };
 
 enum uverbs_attrs_mr_destroy_ids {
@@ -272,6 +274,18 @@ enum uverbs_attrs_query_mr_cmd_attr_ids {
UVERBS_ATTR_QUERY_MR_RESP_IOVA,
 };
 
+enum uverbs_attrs_reg_dmabuf_mr_cmd_attr_ids {
+   UVERBS_ATTR_REG_DMABUF_MR_HANDLE,
+   UVERBS_ATTR_REG_DMABUF_MR_PD_HANDLE,
+   UVERBS_ATTR_REG_DMABUF_MR_OFFSET,
+   UVERBS_ATTR_REG_DMABUF_MR_LENGTH,
+   UVERBS_ATTR_REG_DMABUF_MR_IOVA,
+   UVERBS_ATTR_REG_DMABUF_MR_FD,
+   UVERBS_ATTR_REG_DMABUF_MR_ACCESS_FLAGS,
+   UVERBS_ATTR_REG_DMABUF_MR_RESP_LKEY,
+   UVERBS_ATTR_REG_DMABUF_MR_RESP_RKEY,
+};
+
 enum uverbs_attrs_create_counters_cmd_attr_ids {
UVERBS_ATTR_CREATE_COUNTERS_HANDLE,
 };
diff --git a/libibverbs/cmd_mr.c b/libibverbs/cmd_mr.c
index 42dbe42..91ce2ef 100644
--- a/libibverbs/cmd_mr.c
+++ b/libibverbs/cmd_mr.c
@@ -1,5 +1,6 @@
 /*
  * Copyright (c) 2018 Mellanox Technologies, Ltd.  All rights reserved.
+ * Copyright (c) 2020 Intel Corporation.  All rights reserved.
  *
  * This software is available to you under a choice of one of two
  * licenses.  You may choose to be licensed under the terms of the GNU
@@ -116,3 +117,40 @@ int ibv_cmd_query_mr(struct ibv_pd *pd, struct verbs_mr 
*vmr,
return 0;
 }
 
+int ibv_cmd_reg_dmabuf_mr(struct ibv_pd *pd, uint64_t offset, size_t length,
+ uint64_t iova, int fd, int access,
+ struct verbs_mr *vmr)
+{
+   DECLARE_COMMAND_BUFFER(cmdb, UVERBS_OBJECT_MR,
+  UVERBS_METHOD_REG_DMABUF_MR,
+  9);
+   struct ib_uverbs_attr *handle;
+   uint32_t lkey, rkey;
+   int ret;
+
+   handle = fill_attr_out_obj(cmdb, UVERBS_ATTR_REG_DMABUF_MR_HANDLE);
+   fill_attr_out_ptr(cmdb, UVERBS_ATTR_REG_DMABUF_MR_RESP_LKEY, );
+   fill_attr_out_ptr(cmdb, UVERBS_ATTR_REG_DMABUF_MR_RESP_RKEY, );
+
+   fill_attr_in_obj(cmdb, UVERBS_ATTR_REG_DMABUF_MR_PD_HANDLE, pd->handle);
+   fill_attr_in_uint64(cmdb, UVERBS_ATTR_REG_DMABUF_MR_OFFSET, offset);
+   fill_attr_in_uint64(cmdb, UVERBS_ATTR_REG_DMABUF_MR_LENGTH, length);
+   fill_attr_in_uint64(cmdb, UVERBS_ATTR_REG_DMABUF_MR_IOVA, iova);
+   fill_attr_in_uint32(cmdb, UVERBS_ATTR_REG_DMABUF_MR_FD, fd);
+   fill_attr_in_uint32(cmdb, UVERBS_ATTR_REG_DMABUF_MR_ACCESS_FLAGS, 
access);
+
+   ret = execute_ioctl(pd->context, cmdb);
+   if (ret)
+   return errno;
+
+   vmr->ibv_mr.handle =
+   read_attr_obj(UVERBS_ATTR_REG_DMABUF_MR_HANDLE, handle);
+   vmr->ibv_mr.context = pd->context;
+   vmr->ibv_mr.lkey= lkey;
+   vmr->ibv_mr.rkey= rkey;
+   vmr->ibv_mr.pd  = pd;
+   vmr->ibv_mr.addr= (void *)offset;
+   vmr->ibv_mr.length  = length;
+   vmr->mr_type= IBV_MR_TYPE_MR;
+   return 0;
+}
diff --git a/libibverbs/driver.h b/libibverbs/driver.h
index ab80f4b..d6a9d0a 100644
--- a/libibverbs/driver.h
+++ b/libibverbs/driver.h
@@ -2,6 +2,7 @@
  * Copyright (c) 2004, 2005 Topspin Communications.  All rights reserved.
  * Copyright (c) 2005, 2006 Cisco Systems, Inc.  All rights reserved.
  * Copyright (c) 2005 PathScale, Inc.  All rights reserved.
+ * Copyright (c) 2020 Intel Corporation. All rights reserved.
  *
  * This software is available to you under a choice of one of two
  * licenses.  You may choose to be licensed under the terms of the GNU
@@ -373,6 +374,9 @@ struct verbs_context_ops {
struct ibv_mr *(*reg_dm_mr)(struct ibv_pd *pd, struct ibv_dm *dm,
uint64_t 

[PATCH rdma-core 0/5] Add user space dma-buf support

[Jianxin Xiong]

This is the user space counter-part of the kernel patch set to add
dma-buf support to the RDMA subsystem. This patch series adds user
space API for registering dma-buf based memory regions, updates
pyverbs with the new API, and adds new tests.

This series consists of five patches. The first patch adds the new API
function and updates the man pages. Patch 2 implements the new API in
the mlx5 provider. Patch 3 adds new class definitions to pyverbs for
the new API. Patch 4 adds new tests for the new API. Patch 5 fixes bug
in the utility code of the tests.

Jianxin Xiong (5):
  verbs: Support dma-buf based memory region
  mlx5: Support dma-buf based memory region
  pyverbs: Add dma-buf based MR support
  tests: Add tests for dma-buf based memory regions
  tests: Bug fix for get_access_flags()

 kernel-headers/rdma/ib_user_ioctl_cmds.h |  14 
 libibverbs/cmd_mr.c  |  38 +
 libibverbs/driver.h  |   7 ++
 libibverbs/dummy_ops.c   |  11 +++
 libibverbs/libibverbs.map.in |   6 ++
 libibverbs/man/ibv_reg_mr.3  |  21 -
 libibverbs/verbs.c   |  19 +
 libibverbs/verbs.h   |  10 +++
 providers/mlx5/mlx5.c|   2 +
 providers/mlx5/mlx5.h|   3 +
 providers/mlx5/verbs.c   |  23 ++
 pyverbs/CMakeLists.txt   |   2 +
 pyverbs/dmabuf.pxd   |  13 
 pyverbs/dmabuf.pyx   |  58 ++
 pyverbs/libibverbs.pxd   |   2 +
 pyverbs/mr.pxd   |   5 ++
 pyverbs/mr.pyx   |  77 +-
 tests/test_mr.py | 130 ++-
 tests/utils.py   |  31 +++-
 19 files changed, 464 insertions(+), 8 deletions(-)
 create mode 100644 pyverbs/dmabuf.pxd
 create mode 100644 pyverbs/dmabuf.pyx

-- 
1.8.3.1

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

[PATCH v11 4/4] RDMA/mlx5: Support dma-buf based userspace memory region

[Jianxin Xiong]

Implement the new driver method 'reg_user_mr_dmabuf'.  Utilize the core
functions to import dma-buf based memory region and update the mappings.

Add code to handle dma-buf related page fault.

Signed-off-by: Jianxin Xiong 
Reviewed-by: Sean Hefty 
Acked-by: Michael J. Ruhl 
Acked-by: Christian Koenig 
Acked-by: Daniel Vetter 
---
 drivers/infiniband/hw/mlx5/main.c|   2 +
 drivers/infiniband/hw/mlx5/mlx5_ib.h |  18 +
 drivers/infiniband/hw/mlx5/mr.c  | 127 +--
 drivers/infiniband/hw/mlx5/odp.c |  86 ++--
 4 files changed, 224 insertions(+), 9 deletions(-)

diff --git a/drivers/infiniband/hw/mlx5/main.c 
b/drivers/infiniband/hw/mlx5/main.c
index 36b15a0..e647ea4 100644
--- a/drivers/infiniband/hw/mlx5/main.c
+++ b/drivers/infiniband/hw/mlx5/main.c
@@ -1,6 +1,7 @@
 // SPDX-License-Identifier: GPL-2.0 OR Linux-OpenIB
 /*
  * Copyright (c) 2013-2020, Mellanox Technologies inc. All rights reserved.
+ * Copyright (c) 2020, Intel Corporation. All rights reserved.
  */
 
 #include 
@@ -4055,6 +4056,7 @@ static int mlx5_ib_enable_driver(struct ib_device *dev)
.query_srq = mlx5_ib_query_srq,
.query_ucontext = mlx5_ib_query_ucontext,
.reg_user_mr = mlx5_ib_reg_user_mr,
+   .reg_user_mr_dmabuf = mlx5_ib_reg_user_mr_dmabuf,
.req_notify_cq = mlx5_ib_arm_cq,
.rereg_user_mr = mlx5_ib_rereg_user_mr,
.resize_cq = mlx5_ib_resize_cq,
diff --git a/drivers/infiniband/hw/mlx5/mlx5_ib.h 
b/drivers/infiniband/hw/mlx5/mlx5_ib.h
index bb44080..3ef6872 100644
--- a/drivers/infiniband/hw/mlx5/mlx5_ib.h
+++ b/drivers/infiniband/hw/mlx5/mlx5_ib.h
@@ -1,6 +1,7 @@
 /* SPDX-License-Identifier: GPL-2.0 OR Linux-OpenIB */
 /*
  * Copyright (c) 2013-2020, Mellanox Technologies inc. All rights reserved.
+ * Copyright (c) 2020, Intel Corporation. All rights reserved.
  */
 
 #ifndef MLX5_IB_H
@@ -665,6 +666,12 @@ static inline bool is_odp_mr(struct mlx5_ib_mr *mr)
   mr->umem->is_odp;
 }
 
+static inline bool is_dmabuf_mr(struct mlx5_ib_mr *mr)
+{
+   return IS_ENABLED(CONFIG_INFINIBAND_ON_DEMAND_PAGING) && mr->umem &&
+  mr->umem->is_dmabuf;
+}
+
 struct mlx5_ib_mw {
struct ib_mwibmw;
struct mlx5_core_mkey   mmkey;
@@ -1200,6 +1207,10 @@ int mlx5_ib_create_cq(struct ib_cq *ibcq, const struct 
ib_cq_init_attr *attr,
 struct ib_mr *mlx5_ib_reg_user_mr(struct ib_pd *pd, u64 start, u64 length,
  u64 virt_addr, int access_flags,
  struct ib_udata *udata);
+struct ib_mr *mlx5_ib_reg_user_mr_dmabuf(struct ib_pd *pd, u64 start,
+u64 length, u64 virt_addr,
+int fd, int access_flags,
+struct ib_udata *udata);
 int mlx5_ib_advise_mr(struct ib_pd *pd,
  enum ib_uverbs_advise_mr_advice advice,
  u32 flags,
@@ -1210,11 +1221,13 @@ int mlx5_ib_advise_mr(struct ib_pd *pd,
 int mlx5_ib_dealloc_mw(struct ib_mw *mw);
 int mlx5_ib_update_xlt(struct mlx5_ib_mr *mr, u64 idx, int npages,
   int page_shift, int flags);
+int mlx5_ib_update_mr_pas(struct mlx5_ib_mr *mr, unsigned int flags);
 struct mlx5_ib_mr *mlx5_ib_alloc_implicit_mr(struct mlx5_ib_pd *pd,
 struct ib_udata *udata,
 int access_flags);
 void mlx5_ib_free_implicit_mr(struct mlx5_ib_mr *mr);
 void mlx5_ib_fence_odp_mr(struct mlx5_ib_mr *mr);
+void mlx5_ib_fence_dmabuf_mr(struct mlx5_ib_mr *mr);
 int mlx5_ib_rereg_user_mr(struct ib_mr *ib_mr, int flags, u64 start,
  u64 length, u64 virt_addr, int access_flags,
  struct ib_pd *pd, struct ib_udata *udata);
@@ -1306,6 +1319,7 @@ int mlx5_ib_advise_mr_prefetch(struct ib_pd *pd,
   enum ib_uverbs_advise_mr_advice advice,
   u32 flags, struct ib_sge *sg_list, u32 num_sge);
 int mlx5_ib_init_odp_mr(struct mlx5_ib_mr *mr, bool enable);
+int mlx5_ib_init_dmabuf_mr(struct mlx5_ib_mr *mr);
 #else /* CONFIG_INFINIBAND_ON_DEMAND_PAGING */
 static inline void mlx5_ib_internal_fill_odp_caps(struct mlx5_ib_dev *dev)
 {
@@ -1331,6 +1345,10 @@ static inline int mlx5_ib_init_odp_mr(struct mlx5_ib_mr 
*mr, bool enable)
 {
return -EOPNOTSUPP;
 }
+static inline int mlx5_ib_init_dmabuf_mr(struct mlx5_ib_mr *mr)
+{
+   return -EOPNOTSUPP;
+}
 #endif /* CONFIG_INFINIBAND_ON_DEMAND_PAGING */
 
 extern const struct mmu_interval_notifier_ops mlx5_mn_ops;
diff --git a/drivers/infiniband/hw/mlx5/mr.c b/drivers/infiniband/hw/mlx5/mr.c
index 9f653b4..b7c869c 100644
--- a/drivers/infiniband/hw/mlx5/mr.c
+++ b/drivers/infiniband/hw/mlx5/mr.c
@@ -1,5 +1,6 @@
 /*
  * Copyright (c) 2013-2015, Mellanox Technologies. All rights reserved.
+ * Copyright (c) 2020, Intel Corporation. 

[PATCH v11 3/4] RDMA/uverbs: Add uverbs command for dma-buf based MR registration

[Jianxin Xiong]

Implement a new uverbs ioctl method for memory registration with file
descriptor as an extra parameter.

Signed-off-by: Jianxin Xiong 
Reviewed-by: Sean Hefty 
Acked-by: Michael J. Ruhl 
Acked-by: Christian Koenig 
Acked-by: Daniel Vetter 
---
 drivers/infiniband/core/uverbs_std_types_mr.c | 114 +-
 include/uapi/rdma/ib_user_ioctl_cmds.h|  14 
 2 files changed, 126 insertions(+), 2 deletions(-)

diff --git a/drivers/infiniband/core/uverbs_std_types_mr.c 
b/drivers/infiniband/core/uverbs_std_types_mr.c
index 9b22bb5..e9028ea 100644
--- a/drivers/infiniband/core/uverbs_std_types_mr.c
+++ b/drivers/infiniband/core/uverbs_std_types_mr.c
@@ -1,5 +1,6 @@
 /*
  * Copyright (c) 2018, Mellanox Technologies inc.  All rights reserved.
+ * Copyright (c) 2020, Intel Corporation.  All rights reserved.
  *
  * This software is available to you under a choice of one of two
  * licenses.  You may choose to be licensed under the terms of the GNU
@@ -178,6 +179,83 @@ static int UVERBS_HANDLER(UVERBS_METHOD_QUERY_MR)(
return IS_UVERBS_COPY_ERR(ret) ? ret : 0;
 }
 
+static int UVERBS_HANDLER(UVERBS_METHOD_REG_DMABUF_MR)(
+   struct uverbs_attr_bundle *attrs)
+{
+   struct ib_uobject *uobj =
+   uverbs_attr_get_uobject(attrs, 
UVERBS_ATTR_REG_DMABUF_MR_HANDLE);
+   struct ib_pd *pd =
+   uverbs_attr_get_obj(attrs, UVERBS_ATTR_REG_DMABUF_MR_PD_HANDLE);
+   struct ib_device *ib_dev = pd->device;
+
+   u64 offset, length, virt_addr;
+   u32 fd, access_flags;
+   struct ib_mr *mr;
+   int ret;
+
+   if (!ib_dev->ops.reg_user_mr_dmabuf)
+   return -EOPNOTSUPP;
+
+   ret = uverbs_copy_from(, attrs,
+  UVERBS_ATTR_REG_DMABUF_MR_OFFSET);
+   if (ret)
+   return ret;
+
+   ret = uverbs_copy_from(, attrs,
+  UVERBS_ATTR_REG_DMABUF_MR_LENGTH);
+   if (ret)
+   return ret;
+
+   ret = uverbs_copy_from(_addr, attrs,
+  UVERBS_ATTR_REG_DMABUF_MR_IOVA);
+   if (ret)
+   return ret;
+
+   ret = uverbs_copy_from(, attrs,
+  UVERBS_ATTR_REG_DMABUF_MR_FD);
+   if (ret)
+   return ret;
+
+   ret = uverbs_get_flags32(_flags, attrs,
+UVERBS_ATTR_REG_DMABUF_MR_ACCESS_FLAGS,
+IB_ACCESS_LOCAL_WRITE |
+IB_ACCESS_REMOTE_READ |
+IB_ACCESS_REMOTE_WRITE |
+IB_ACCESS_REMOTE_ATOMIC |
+IB_ACCESS_RELAXED_ORDERING);
+   if (ret)
+   return ret;
+
+   ret = ib_check_mr_access(access_flags);
+   if (ret)
+   return ret;
+
+   mr = pd->device->ops.reg_user_mr_dmabuf(pd, offset, length, virt_addr,
+   fd, access_flags,
+   >driver_udata);
+   if (IS_ERR(mr))
+   return PTR_ERR(mr);
+
+   mr->device = pd->device;
+   mr->pd = pd;
+   mr->type = IB_MR_TYPE_USER;
+   mr->uobject = uobj;
+   atomic_inc(>usecnt);
+
+   uobj->object = mr;
+
+   uverbs_finalize_uobj_create(attrs, UVERBS_ATTR_REG_DMABUF_MR_HANDLE);
+
+   ret = uverbs_copy_to(attrs, UVERBS_ATTR_REG_DMABUF_MR_RESP_LKEY,
+>lkey, sizeof(mr->lkey));
+   if (ret)
+   return ret;
+
+   ret = uverbs_copy_to(attrs, UVERBS_ATTR_REG_DMABUF_MR_RESP_RKEY,
+>rkey, sizeof(mr->rkey));
+   return ret;
+}
+
 DECLARE_UVERBS_NAMED_METHOD(
UVERBS_METHOD_ADVISE_MR,
UVERBS_ATTR_IDR(UVERBS_ATTR_ADVISE_MR_PD_HANDLE,
@@ -243,6 +321,37 @@ static int UVERBS_HANDLER(UVERBS_METHOD_QUERY_MR)(
UVERBS_ATTR_TYPE(u32),
UA_MANDATORY));
 
+DECLARE_UVERBS_NAMED_METHOD(
+   UVERBS_METHOD_REG_DMABUF_MR,
+   UVERBS_ATTR_IDR(UVERBS_ATTR_REG_DMABUF_MR_HANDLE,
+   UVERBS_OBJECT_MR,
+   UVERBS_ACCESS_NEW,
+   UA_MANDATORY),
+   UVERBS_ATTR_IDR(UVERBS_ATTR_REG_DMABUF_MR_PD_HANDLE,
+   UVERBS_OBJECT_PD,
+   UVERBS_ACCESS_READ,
+   UA_MANDATORY),
+   UVERBS_ATTR_PTR_IN(UVERBS_ATTR_REG_DMABUF_MR_OFFSET,
+  UVERBS_ATTR_TYPE(u64),
+  UA_MANDATORY),
+   UVERBS_ATTR_PTR_IN(UVERBS_ATTR_REG_DMABUF_MR_LENGTH,
+  UVERBS_ATTR_TYPE(u64),
+  UA_MANDATORY),
+   UVERBS_ATTR_PTR_IN(UVERBS_ATTR_REG_DMABUF_MR_IOVA,
+  UVERBS_ATTR_TYPE(u64),
+  UA_MANDATORY),
+   UVERBS_ATTR_PTR_IN(UVERBS_ATTR_REG_DMABUF_MR_FD,
+  UVERBS_ATTR_TYPE(u32),
+

[PATCH v11 1/4] RDMA/umem: Support importing dma-buf as user memory region

[Jianxin Xiong]

Dma-buf is a standard cross-driver buffer sharing mechanism that can be
used to support peer-to-peer access from RDMA devices.

Device memory exported via dma-buf is associated with a file descriptor.
This is passed to the user space as a property associated with the
buffer allocation. When the buffer is registered as a memory region,
the file descriptor is passed to the RDMA driver along with other
parameters.

Implement the common code for importing dma-buf object and mapping
dma-buf pages.

Signed-off-by: Jianxin Xiong 
Reviewed-by: Sean Hefty 
Acked-by: Michael J. Ruhl 
Acked-by: Christian Koenig 
Acked-by: Daniel Vetter 
---
 drivers/infiniband/core/Makefile  |   2 +-
 drivers/infiniband/core/umem.c|   4 +
 drivers/infiniband/core/umem_dmabuf.c | 174 ++
 drivers/infiniband/core/umem_dmabuf.h |  11 +++
 include/rdma/ib_umem.h|  39 +++-
 5 files changed, 228 insertions(+), 2 deletions(-)
 create mode 100644 drivers/infiniband/core/umem_dmabuf.c
 create mode 100644 drivers/infiniband/core/umem_dmabuf.h

diff --git a/drivers/infiniband/core/Makefile b/drivers/infiniband/core/Makefile
index ccf2670..8ab4eea 100644
--- a/drivers/infiniband/core/Makefile
+++ b/drivers/infiniband/core/Makefile
@@ -40,5 +40,5 @@ ib_uverbs-y :=uverbs_main.o 
uverbs_cmd.o uverbs_marshall.o \
uverbs_std_types_srq.o \
uverbs_std_types_wq.o \
uverbs_std_types_qp.o
-ib_uverbs-$(CONFIG_INFINIBAND_USER_MEM) += umem.o
+ib_uverbs-$(CONFIG_INFINIBAND_USER_MEM) += umem.o umem_dmabuf.o
 ib_uverbs-$(CONFIG_INFINIBAND_ON_DEMAND_PAGING) += umem_odp.o
diff --git a/drivers/infiniband/core/umem.c b/drivers/infiniband/core/umem.c
index f1fc7e3..4bc455f 100644
--- a/drivers/infiniband/core/umem.c
+++ b/drivers/infiniband/core/umem.c
@@ -2,6 +2,7 @@
  * Copyright (c) 2005 Topspin Communications.  All rights reserved.
  * Copyright (c) 2005 Cisco Systems.  All rights reserved.
  * Copyright (c) 2005 Mellanox Technologies. All rights reserved.
+ * Copyright (c) 2020 Intel Corporation. All rights reserved.
  *
  * This software is available to you under a choice of one of two
  * licenses.  You may choose to be licensed under the terms of the GNU
@@ -43,6 +44,7 @@
 #include 
 
 #include "uverbs.h"
+#include "umem_dmabuf.h"
 
 static void __ib_umem_release(struct ib_device *dev, struct ib_umem *umem, int 
dirty)
 {
@@ -278,6 +280,8 @@ void ib_umem_release(struct ib_umem *umem)
 {
if (!umem)
return;
+   if (umem->is_dmabuf)
+   return ib_umem_dmabuf_release(to_ib_umem_dmabuf(umem));
if (umem->is_odp)
return ib_umem_odp_release(to_ib_umem_odp(umem));
 
diff --git a/drivers/infiniband/core/umem_dmabuf.c 
b/drivers/infiniband/core/umem_dmabuf.c
new file mode 100644
index 000..2fcd072
--- /dev/null
+++ b/drivers/infiniband/core/umem_dmabuf.c
@@ -0,0 +1,174 @@
+// SPDX-License-Identifier: (GPL-2.0 OR BSD-3-Clause)
+/*
+ * Copyright (c) 2020 Intel Corporation. All rights reserved.
+ */
+
+#include 
+#include 
+#include 
+
+#include "uverbs.h"
+#include "umem_dmabuf.h"
+
+int ib_umem_dmabuf_map_pages(struct ib_umem_dmabuf *umem_dmabuf)
+{
+   struct sg_table *sgt;
+   struct scatterlist *sg;
+   struct dma_fence *fence;
+   unsigned long start, end, cur;
+   unsigned int nmap;
+   int i;
+
+   dma_resv_assert_held(umem_dmabuf->attach->dmabuf->resv);
+
+   if (umem_dmabuf->sgt)
+   return 0;
+
+   sgt = dma_buf_map_attachment(umem_dmabuf->attach, DMA_BIDIRECTIONAL);
+   if (IS_ERR(sgt))
+   return PTR_ERR(sgt);
+
+   /* modify the sg list in-place to match umem address and length */
+
+   start = ALIGN_DOWN(umem_dmabuf->umem.address, PAGE_SIZE);
+   end = ALIGN(umem_dmabuf->umem.address + umem_dmabuf->umem.length,
+   PAGE_SIZE);
+   cur = 0;
+   nmap = 0;
+   for_each_sgtable_dma_sg(sgt, sg, i) {
+   if (start < cur + sg_dma_len(sg) && cur < end)
+   nmap++;
+   if (cur <= start && start < cur + sg_dma_len(sg)) {
+   unsigned long offset = start - cur;
+
+   umem_dmabuf->first_sg = sg;
+   umem_dmabuf->first_sg_offset = offset;
+   sg_dma_address(sg) += offset;
+   sg_dma_len(sg) -= offset;
+   cur += offset;
+   }
+   if (cur < end && end <= cur + sg_dma_len(sg)) {
+   unsigned long trim = cur + sg_dma_len(sg) - end;
+
+   umem_dmabuf->last_sg = sg;
+   umem_dmabuf->last_sg_trim = trim;
+   sg_dma_len(sg) -= trim;
+   break;
+   }
+   cur += sg_dma_len(sg);
+   }
+
+   

[PATCH v11 2/4] RDMA/core: Add device method for registering dma-buf based memory region

[Jianxin Xiong]

Dma-buf based memory region requires one extra parameter and is processed
quite differently. Adding a separate method allows clean separation from
regular memory regions.

Signed-off-by: Jianxin Xiong 
Reviewed-by: Sean Hefty 
Acked-by: Michael J. Ruhl 
Acked-by: Christian Koenig 
Acked-by: Daniel Vetter 
---
 drivers/infiniband/core/device.c | 1 +
 include/rdma/ib_verbs.h  | 6 +-
 2 files changed, 6 insertions(+), 1 deletion(-)

diff --git a/drivers/infiniband/core/device.c b/drivers/infiniband/core/device.c
index ce26564..6768a19 100644
--- a/drivers/infiniband/core/device.c
+++ b/drivers/infiniband/core/device.c
@@ -2689,6 +2689,7 @@ void ib_set_device_ops(struct ib_device *dev, const 
struct ib_device_ops *ops)
SET_DEVICE_OP(dev_ops, read_counters);
SET_DEVICE_OP(dev_ops, reg_dm_mr);
SET_DEVICE_OP(dev_ops, reg_user_mr);
+   SET_DEVICE_OP(dev_ops, reg_user_mr_dmabuf);
SET_DEVICE_OP(dev_ops, req_ncomp_notif);
SET_DEVICE_OP(dev_ops, req_notify_cq);
SET_DEVICE_OP(dev_ops, rereg_user_mr);
diff --git a/include/rdma/ib_verbs.h b/include/rdma/ib_verbs.h
index 9420827..3d1d098 100644
--- a/include/rdma/ib_verbs.h
+++ b/include/rdma/ib_verbs.h
@@ -2,7 +2,7 @@
 /*
  * Copyright (c) 2004 Mellanox Technologies Ltd.  All rights reserved.
  * Copyright (c) 2004 Infinicon Corporation.  All rights reserved.
- * Copyright (c) 2004 Intel Corporation.  All rights reserved.
+ * Copyright (c) 2004, 2020 Intel Corporation.  All rights reserved.
  * Copyright (c) 2004 Topspin Corporation.  All rights reserved.
  * Copyright (c) 2004 Voltaire Corporation.  All rights reserved.
  * Copyright (c) 2005 Sun Microsystems, Inc. All rights reserved.
@@ -2433,6 +2433,10 @@ struct ib_device_ops {
struct ib_mr *(*reg_user_mr)(struct ib_pd *pd, u64 start, u64 length,
 u64 virt_addr, int mr_access_flags,
 struct ib_udata *udata);
+   struct ib_mr *(*reg_user_mr_dmabuf)(struct ib_pd *pd, u64 offset,
+u64 length, u64 virt_addr, int fd,
+int mr_access_flags,
+struct ib_udata *udata);
int (*rereg_user_mr)(struct ib_mr *mr, int flags, u64 start, u64 length,
 u64 virt_addr, int mr_access_flags,
 struct ib_pd *pd, struct ib_udata *udata);
-- 
1.8.3.1

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

[PATCH v11 0/4] RDMA: Add dma-buf support

[Jianxin Xiong]

This is the eleventh version of the patch set. Changelog:

v11:
* Rework the parameter checking code inside ib_umem_dmabuf_get() 
* Fix incorrect error handling in the new verbs command handler
* Put a duplicated code sequence for checking iova and setting page size
  into a function
* In the invalidation callback, check for if the buffer has been mapped
  and thus the presence of a valid driver mr is ensured
* The patch that checks for dma_virt_ops is dropped because it is no
  longer needed
* The patch that documents that dma-buf size is fixed has landed at:
  https://cgit.freedesktop.org/drm/drm-misc/commit/?id=476b485be03c
  and thus is no longer included here
* The matching user space patch set is sent separately

v10: https://www.spinics.net/lists/linux-rdma/msg97483.html
* Don't map the pages in ib_umem_dmabuf_get(); use the size information
  of the dma-buf object to validate the umem size instead
* Use PAGE_SIZE directly instead of use ib_umem_find_best_pgsz() when
  the MR is created since the pages have not been mapped yet and dma-buf
  requires PAGE_SIZE anyway
* Always call mlx5_umem_find_best_pgsz() after mapping the pages to
  verify that the page size requirement is satisfied
* Add a patch to document that dma-buf size is fixed

v9: https://www.spinics.net/lists/linux-rdma/msg97432.html
* Clean up the code for sg list in-place modification
* Prevent dma-buf pages from being mapped multiple times
* Map the pages in ib_umem_dmabuf_get() so that inproper values of
  address/length/iova can be caught early
* Check for unsupported flags in the new uverbs command
* Add missing uverbs_finalize_uobj_create()
* Sort uverbs objects by name
* Fix formating issue -- unnecessary alignment of '='
* Unmap pages in mlx5_ib_fence_dmabuf_mr()
* Remove address range checking from pagefault_dmabuf_mr()

v8: https://www.spinics.net/lists/linux-rdma/msg97370.html
* Modify the dma-buf sg list in place to get a proper umem sg list and
  restore it before calling dma_buf_unmap_attachment()
* Validate the umem sg list with ib_umem_find_best_pgsz()
* Remove the logic for slicing the sg list at runtime

v7: https://www.spinics.net/lists/linux-rdma/msg97297.html
* Rebase on top of latest mlx5 MR patch series
* Slice dma-buf sg list at runtime instead of creating a new list
* Preload the buffer page mapping when the MR is created
* Move the 'dma_virt_ops' check into dma_buf_dynamic_attach()

v6: https://www.spinics.net/lists/linux-rdma/msg96923.html
* Move the dma-buf invalidation callback from the core to the device
  driver
* Move mapping update from work queue to pagefault handler
* Add dma-buf based MRs to the xarray of mmkeys so that the pagefault
  handler can be reached
* Update the new driver method and uverbs command signature by changing
  the paramter 'addr' to 'offset'
* Modify the sg list returned from dma_buf_map_attachment() based on
  the parameters 'offset' and 'length'
* Don't import dma-buf if 'dma_virt_ops' is used by the dma device
* The patch that clarifies dma-buf sg lists alignment has landed at
  https://cgit.freedesktop.org/drm/drm-misc/commit/?id=ac80cd17a615
  and thus is no longer included with this set

v5: https://www.spinics.net/lists/linux-rdma/msg96786.html
* Fix a few warnings reported by kernel test robot:
- no previous prototype for function 'ib_umem_dmabuf_release' 
- no previous prototype for function 'ib_umem_dmabuf_map_pages'
- comparison of distinct pointer types in 'check_add_overflow'
* Add comment for the wait between getting the dma-buf sg tagle and
  updating the NIC page table

v4: https://www.spinics.net/lists/linux-rdma/msg96767.html
* Add a new ib_device method reg_user_mr_dmabuf() instead of expanding
  the existing method reg_user_mr()
* Use a separate code flow for dma-buf instead of adding special cases
  to the ODP memory region code path
* In invalidation callback, new mapping is updated as whole using work
  queue instead of being updated in page granularity in the page fault
  handler
* Use dma_resv_get_excl() and dma_fence_wait() to ensure the content of
  the pages have been moved to the new location before the new mapping
  is programmed into the NIC
* Add code to the ODP page fault handler to check the mapping status
* The new access flag added in v3 is removed.
* The checking for on-demand paging support in the new uverbs command
  is removed because it is implied by implementing the new ib_device
  method
* Clarify that dma-buf sg lists are page aligned

v3: https://www.spinics.net/lists/linux-rdma/msg96330.html
* Use dma_buf_dynamic_attach() instead of dma_buf_attach()
* Use on-demand paging mechanism to avoid pinning the GPU memory
* Instead of adding a new parameter to the device method for memory
  registration, pass all the attributes including the file descriptor
  as a structure
* Define a new access flag for dma-buf based memory region
* Check for on-demand paging support in the new uverbs command

v2: 

Re: [PATCH v4 1/5] drm: panel: simple: Fixup the struct panel_desc kernel doc

[Doug Anderson]

Hi,

On Mon, Nov 9, 2020 at 5:01 PM Douglas Anderson  wrote:
>
> When I run:
>   scripts/kernel-doc -rst drivers/gpu/drm/panel/panel-simple.c
>
> I see that several of the kernel-doc entries aren't showing up because
> they don't specify the full path down the hierarchy.  Let's fix that
> and also move to inline kernel docs.
>
> Signed-off-by: Douglas Anderson 
> ---
>
> Changes in v4:
> - ("drm: panel: simple: Fixup the struct panel_desc kernel doc") new for v4.
>
>  drivers/gpu/drm/panel/panel-simple.c | 59 
>  1 file changed, 42 insertions(+), 17 deletions(-)

I'm curious if there are any outstanding actions I need to take for
this series.  I know I found that on patch #4 I should have added the
sync flags [1] but I was waiting to see if there was any other
feedback before sending a v5.  If there's nothing else I'm happy with
any of:

a) I can send a v5 with that small fix.

b) v4 can land an the maintainer landing can add the ".flags =
DRM_MODE_FLAG_PHSYNC | DRM_MODE_FLAG_NVSYNC," into the patch when
landing.

c) v4 can land and I can send a follow-up patch with the sync flags fix.

I know it's the holidays right now and folks are busy, but I just
wanted to see if there were any actions I needed to take.  Personally
I'm on vacation the last 3 days this week and I imagine I'll come back
to work Monday swamped with email, so today/tomorrow would be an ideal
time to send a spin if one is needed.  Thanks!

-Doug

[1] 
https://lore.kernel.org/r/CAD=FV=UPkuJ5E2sCQeozNR3CO+LZW=dw9h77vfooeismqgn...@mail.gmail.com
___
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

Re: [RFC] MAINTAINERS tag for cleanup robot

[Tom Rix]

On 11/22/20 10:22 AM, Joe Perches wrote:
> On Sun, 2020-11-22 at 08:33 -0800, Tom Rix wrote:
>> On 11/21/20 9:10 AM, Joe Perches wrote:
>>> On Sat, 2020-11-21 at 08:50 -0800, t...@redhat.com wrote:
 A difficult part of automating commits is composing the subsystem
 preamble in the commit log.  For the ongoing effort of a fixer producing
 one or two fixes a release the use of 'treewide:' does not seem 
 appropriate.

 It would be better if the normal prefix was used.  Unfortunately normal is
 not consistent across the tree.

 So I am looking for comments for adding a new tag to the MAINTAINERS file

D: Commit subsystem prefix

 ex/ for FPGA DFL DRIVERS

D: fpga: dfl:
>>> I'm all for it.  Good luck with the effort.  It's not completely trivial.
>>>
>>> From a decade ago:
>>>
>>> https://lore.kernel.org/lkml/1289919077.28741.50.camel@Joe-Laptop/
>>>
>>> (and that thread started with extra semicolon patches too)
>> Reading the history, how about this.
>>
>> get_maintainer.pl outputs a single prefix, if multiple files have the
>> same prefix it works, if they don't its an error.
>>
>> Another script 'commit_one_file.sh' does the call to get_mainainter.pl
>> to get the prefix and be called by run-clang-tools.py to get the fixer
>> specific message.
> It's not whether the script used is get_maintainer or any other script,
> the question is really if the MAINTAINERS file is the appropriate place
> to store per-subsystem patch specific prefixes.
>
> It is.
>
> Then the question should be how are the forms described and what is the
> inheritance priority.  My preference would be to have a default of
> inherit the parent base and add basename(subsystem dirname).
>
> Commit history seems to have standardized on using colons as the separator
> between the commit prefix and the subject.
>
> A good mechanism to explore how various subsystems have uses prefixes in
> the past might be something like:
>
> $ git log --no-merges --pretty='%s' -  | \
>   perl -n -e 'print substr($_, 0, rindex($_, ":") + 1) . "\n";' | \
>   sort | uniq -c | sort -rn

Thanks, I have shamelessly stolen this line and limited the commits to the 
maintainer.

I will post something once the generation of the prefixes is done.

Tom

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

[Bug 210321] New: /display/dc/dcn20/dcn20_resource.c:3240 dcn20_validate_bandwidth_fp+0x8b/0xd0 [amdgpu]

[bugzilla-daemon]

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

Bug ID: 210321
   Summary: /display/dc/dcn20/dcn20_resource.c:3240
dcn20_validate_bandwidth_fp+0x8b/0xd0 [amdgpu]
   Product: Drivers
   Version: 2.5
Kernel Version: 5.10.0-rc5
  Hardware: All
OS: Linux
  Tree: Mainline
Status: NEW
  Severity: normal
  Priority: P1
 Component: Video(DRI - non Intel)
  Assignee: drivers_video-...@kernel-bugs.osdl.org
  Reporter: vl...@aresgate.net
Regression: No

Been happening since 5.9



[Mon Nov 23 08:42:42 2020] [ cut here ]
[Mon Nov 23 08:42:42 2020] WARNING: CPU: 15 PID: 155 at
drivers/gpu/drm/amd/amdgpu/../display/dc/dcn20/dcn20_resource.c:3240
dcn20_validate_bandwidth_fp+0x8b/0xd0 [amdgpu]
[Mon Nov 23 08:42:42 2020] Modules linked in: autofs4 ipv6 tcp_bic binfmt_misc
btusb btrtl btbcm btintel bluetooth snd_hda_codec_realtek snd_hda_codec_generic
ledtrig_audio snd_hda_intel snd_intel_dspcfg snd_usb_audio snd_hda_codec
snd_hwdep snd_usbmidi_lib snd_hda_core rfkill amdgpu ecdh_generic snd_pcm ecc
snd_seq_midi mousedev snd_seq_midi_event snd_rawmidi cdc_acm input_leds
led_class iommu_v2 evdev gpu_sched snd_seq ttm i2c_algo_bit snd_seq_device
drm_kms_helper snd_timer syscopyarea sysfillrect sysimgblt fb_sys_fops snd drm
k10temp ccp soundcore sha1_generic button efivarfs dm_crypt encrypted_keys
dm_mod r8169 realtek mdio_devres libphy usbhid
[Mon Nov 23 08:42:42 2020] CPU: 15 PID: 155 Comm: kworker/15:1 Not tainted
5.10.0-rc5 #50
[Mon Nov 23 08:42:42 2020] Hardware name: Micro-Star International Co., Ltd.
MS-7B89/B450M MORTAR (MS-7B89), BIOS 1.D0 01/06/2020
[Mon Nov 23 08:42:42 2020] Workqueue: events drm_mode_rmfb_work_fn [drm]
[Mon Nov 23 08:42:42 2020] RIP: 0010:dcn20_validate_bandwidth_fp+0x8b/0xd0
[amdgpu]
[Mon Nov 23 08:42:42 2020] Code: 00 7b 2b 22 85 14 1f 00 00 75 26 f2 0f 11 85
58 26 00 00 31 d2 48 89 ee 4c 89 e7 e8 8f f6 ff ff 89 c2 22 95 14 1f 00 00 75
33 <0f> 0b eb 03 90 75 d2 66 0f 12 14 24 f2 0f 11 95 58 26 00 00 48 83
[Mon Nov 23 08:42:42 2020] RSP: 0018:a93000607c28 EFLAGS: 00010246
[Mon Nov 23 08:42:42 2020] RAX: 0001 RBX:  RCX:
0001876f
[Mon Nov 23 08:42:42 2020] RDX:  RSI: 211d5948abbbab22 RDI:
000261a0
[Mon Nov 23 08:42:42 2020] RBP: 8d0cab2a R08: 8d0cab2a1ec8 R09:
8d0c8e44
[Mon Nov 23 08:42:42 2020] R10: 8d0c86dde000 R11: 00010001 R12:
8d0c8e44
[Mon Nov 23 08:42:42 2020] R13:  R14: 8d0cab2a R15:
8d0c85b20308
[Mon Nov 23 08:42:42 2020] FS:  ()
GS:8d0f8efc() knlGS:
[Mon Nov 23 08:42:42 2020] CS:  0010 DS:  ES:  CR0: 80050033
[Mon Nov 23 08:42:42 2020] CR2: 56169e9dd940 CR3: 0001114fa000 CR4:
00350ee0
[Mon Nov 23 08:42:42 2020] Call Trace:
[Mon Nov 23 08:42:42 2020]  dcn20_validate_bandwidth+0x2d/0x50 [amdgpu]
[Mon Nov 23 08:42:42 2020]  dc_validate_global_state+0x279/0x300 [amdgpu]
[Mon Nov 23 08:42:42 2020]  amdgpu_dm_atomic_check+0xb39/0xc30 [amdgpu]
[Mon Nov 23 08:42:42 2020]  drm_atomic_check_only+0x557/0x7e0 [drm]
[Mon Nov 23 08:42:42 2020]  drm_atomic_commit+0x19/0x70 [drm]
[Mon Nov 23 08:42:42 2020]  drm_framebuffer_remove+0x395/0x430 [drm]
[Mon Nov 23 08:42:42 2020]  drm_mode_rmfb_work_fn+0x37/0x50 [drm]
[Mon Nov 23 08:42:42 2020]  process_one_work+0x1ae/0x300
[Mon Nov 23 08:42:42 2020]  worker_thread+0x1f6/0x3e0
[Mon Nov 23 08:42:42 2020]  ? rescuer_thread+0x410/0x410
[Mon Nov 23 08:42:42 2020]  kthread+0x12a/0x150
[Mon Nov 23 08:42:42 2020]  ? kthread_create_worker_on_cpu+0x70/0x70
[Mon Nov 23 08:42:42 2020]  ret_from_fork+0x22/0x30
[Mon Nov 23 08:42:42 2020] ---[ end trace cb43a970abd417bb ]---

❯ emerge --info
Portage 3.0.9 (python 3.7.9-final-0, default/linux/amd64/17.1/desktop/plasma,
gcc-10.2.0, glibc-2.32-r2, 5.10.0-rc5 x86_64)
=
System uname:
Linux-5.10.0-rc5-x86_64-AMD_Ryzen_7_3700X_8-Core_Processor-with-gentoo-2.7
KiB Mem:16419668 total,  12449312 free
KiB Swap:   16137212 total,  16137212 free
Timestamp of repository gentoo: Mon, 23 Nov 2020 15:05:42 +
Head commit of repository gentoo: b98ac916976c6de90fd633bcb2b579f57762a23c

Head commit of repository lto-overlay: 3c0f2f8521d5d8e5180e742bee80f93e64575b38

Head commit of repository mv: 45f322abb49bf38da282afe2aa8c287d99c9e630

Head commit of repository steam-overlay:
6ed5630096d43208168572264f204d40e6b2a5c0

Head commit of repository eroen: 168c481db277e12e30ba6dea71a774337512ce32

Head commit of repository mva: 88a3abdfc840fc7d5645a0570fc0c6b03a45218d

sh bash 5.0_p18
ld GNU ld (Gentoo 2.34 p6) 2.34.0
distcc 3.3.3 x86_64-pc-linux-gnu [disabled]
ccache version 3.7.12 [disabled]
app-shells/bash:  5.0_p18::gentoo
dev-java/java-config: 2.3.1::gentoo
dev-lang/perl:

Re: [PATCH 000/141] Fix fall-through warnings for Clang

[Joe Perches]

On Mon, 2020-11-23 at 07:58 -0800, James Bottomley wrote:
> We're also complaining about the inability to recruit maintainers:
> 
> https://www.theregister.com/2020/06/30/hard_to_find_linux_maintainers_says_torvalds/
> 
> And burn out:
> 
> http://antirez.com/news/129

https://www.wired.com/story/open-source-coders-few-tired/

> What I'm actually trying to articulate is a way of measuring value of
> the patch vs cost ... it has nothing really to do with who foots the
> actual bill.

It's unclear how to measure value in consistency.

But one way that costs can be reduced is by automation and _not_
involving maintainers when the patch itself is provably correct.

> One thesis I'm actually starting to formulate is that this continual
> devaluing of maintainers is why we have so much difficulty keeping and
> recruiting them.

The linux kernel has something like 1500 different maintainers listed
in the MAINTAINERS file.  That's not a trivial number.

$ git grep '^M:' MAINTAINERS | sort | uniq -c | wc -l
1543
$ git grep '^M:' MAINTAINERS| cut -f1 -d'<' | sort | uniq -c | wc -l
1446

I think the question you are asking is about trust and how it
effects development.

And back to that wired story, the actual number of what you might
be considering to be maintainers is likely less than 10% of the
listed numbers above.


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

Re: [Intel-wired-lan] [PATCH 000/141] Fix fall-through warnings for Clang

[James Bottomley]

On Mon, 2020-11-23 at 07:03 -0600, Gustavo A. R. Silva wrote:
> On Sun, Nov 22, 2020 at 11:53:55AM -0800, James Bottomley wrote:
> > On Sun, 2020-11-22 at 11:22 -0800, Joe Perches wrote:
> > > On Sun, 2020-11-22 at 11:12 -0800, James Bottomley wrote:
> > > > On Sun, 2020-11-22 at 10:25 -0800, Joe Perches wrote:
> > > > > On Sun, 2020-11-22 at 10:21 -0800, James Bottomley wrote:
> > > > > > Please tell me our reward for all this effort isn't a
> > > > > > single missing error print.
> > > > > 
> > > > > There were quite literally dozens of logical defects found
> > > > > by the fallthrough additions.  Very few were logging only.
> > > > 
> > > > So can you give us the best examples (or indeed all of them if
> > > > someone is keeping score)?  hopefully this isn't a US election
> > > > situation ...
> > > 
> > > Gustavo?  Are you running for congress now?
> > > 
> > > https://lwn.net/Articles/794944/
> > 
> > That's 21 reported fixes of which about 50% seem to produce no
> > change in code behaviour at all, a quarter seem to have no user
> > visible effect with the remaining quarter producing unexpected
> > errors on obscure configuration parameters, which is why no-one
> > really noticed them before.
> 
> The really important point here is the number of bugs this has
> prevented and will prevent in the future. See an example of this,
> below:
> 
> https://lore.kernel.org/linux-iio/20190813135802.gb27...@kroah.com/

I think this falls into the same category as the other six bugs: it
changes the output/input for parameters but no-one has really noticed,
usually because the command is obscure or the bias effect is minor.

> This work is still relevant, even if the total number of issues/bugs
> we find in the process is zero (which is not the case).

Really, no ... something which produces no improvement has no value at
all ... we really shouldn't be wasting maintainer time with it because
it has a cost to merge.  I'm not sure we understand where the balance
lies in value vs cost to merge but I am confident in the zero value
case.

> "The sucky thing about doing hard work to deploy hardening is that
> the result is totally invisible by definition (things not happening)
> [..]"
> - Dmitry Vyukov

Really, no.  Something that can't be measured at all doesn't exist.

And actually hardening is one of those things you can measure (which I
do have to admit isn't true for everything in the security space) ...
it's number of exploitable bugs found before you did it vs number of
exploitable bugs found after you did it.  Usually hardening eliminates
a class of bug, so the way I've measured hardening before is to go
through the CVE list for the last couple of years for product X, find
all the bugs that are of the class we're looking to eliminate and say
if we had hardened X against this class of bug we'd have eliminated Y%
of the exploits.  It can be quite impressive if Y is a suitably big
number.

James


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

[Bug 201957] amdgpu: ring gfx timeout

[bugzilla-daemon]

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

majorgo...@juno.com changed:

   What|Removed |Added

 CC||majorgo...@juno.com

--- Comment #38 from majorgo...@juno.com ---
I have a similar problem, a cascade of errors that typically starts with one of
these:
[drm:amdgpu_job_timedout [amdgpu]] *ERROR* ring gfx timeout, signaled
seq=1093546, emitted seq=1093548

This used to occur only when playing Dauntless, and only after my MSI Radeon
RX580 ran hot for a while.  Warframe never crashed.  Totally different methods
of running the games (Dauntless=Lutris and Epic Games Store, Warframe = Steam
and Proton).  Something then changed after one of the updates within the last
month, and now it crashes on both Warframe and Dauntless well before the card
is at a high temp.  Basically can't run more than about 5 minutes.  

I was running Ubuntu 18.04, so I figured maybe a newer kernel would fix this,
but updating to 20.10 did nothing but waste a couple of days of reloading
everything.

System:  Ryzen 5 3600 on Gigabyte x570 UD with a MSI Radeon RX580 8GB

I'm willing to work with whoever sending whatever info/logs are necessary to
get this fixed.

-- 
You are receiving this mail because:
You are watching the assignee of the bug.
___
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

Re: [GIT PULL] Allwinner MBUS and DMA-ops rework

[Arnd Bergmann]

On Wed, Nov 18, 2020 at 10:13 AM Maxime Ripard  wrote:
>
> Hi Arnd, Olof,
>
> Here's the PR for the MBUS rework we discussed in the last couple of
> weeks, for what will become 5.11.
>
> As Arnd suggested, this is based on a PR sent to drm-misc-fixes to merge
> the initial fix for a probe error in drm/sun4i due to
> dma_direct_set_offset.
>
>   ssh://g...@gitolite.kernel.org/pub/scm/linux/kernel/git/mripard/linux.git 
> tags/sunxi-rework-mbus

Looks good, nice cleanup!

I've applied it to the arm/drivers branch now, but I almost missed it, as
you sent it to the old a...@kernel.org address instead of s...@kernel.org
that is connected to the patchwork.

Please also use http:// or git:// addresses in pull requests instead of ssh://

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

Re: [PATCH 000/141] Fix fall-through warnings for Clang

[Rafael J. Wysocki]

On Mon, Nov 23, 2020 at 4:58 PM James Bottomley
 wrote:
>
> On Mon, 2020-11-23 at 15:19 +0100, Miguel Ojeda wrote:
> > On Sun, Nov 22, 2020 at 11:36 PM James Bottomley
> >  wrote:

[cut]

> >
> > Maintainers routinely review 1-line trivial patches, not to mention
> > internal API changes, etc.
>
> We're also complaining about the inability to recruit maintainers:
>
> https://www.theregister.com/2020/06/30/hard_to_find_linux_maintainers_says_torvalds/
>
> And burn out:
>
> http://antirez.com/news/129

Right.

> The whole crux of your argument seems to be maintainers' time isn't
> important so we should accept all trivial patches ... I'm pushing back
> on that assumption in two places, firstly the valulessness of the time
> and secondly that all trivial patches are valuable.
>
> > If some company does not want to pay for that, that's fine, but they
> > don't get to be maintainers and claim `Supported`.
>
> What I'm actually trying to articulate is a way of measuring value of
> the patch vs cost ... it has nothing really to do with who foots the
> actual bill.
>
> One thesis I'm actually starting to formulate is that this continual
> devaluing of maintainers is why we have so much difficulty keeping and
> recruiting them.

Absolutely.

This is just one of the factors involved, but a significant one IMV.
___
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

Re: [PATCH v1] drm/bridge: lt9611: Fix handling of 4k panels

[Vinod Koul]

On 23-11-20, 11:46, Robert Foss wrote:
> 4k requires two dsi pipes, so don't report MODE_OK when only a
> single pipe is configured. But rather report MODE_PANEL to
> signal that requirements of the panel are not being met.

Acked-By: Vinod Koul 

> Reported-by: Peter Collingbourne 
> Suggested-by: Peter Collingbourne 
> Signed-off-by: Robert Foss 
> Tested-by: John Stultz 
> ---
>  drivers/gpu/drm/bridge/lontium-lt9611.c | 8 +++-
>  1 file changed, 7 insertions(+), 1 deletion(-)
> 
> diff --git a/drivers/gpu/drm/bridge/lontium-lt9611.c 
> b/drivers/gpu/drm/bridge/lontium-lt9611.c
> index d734d9402c35..e8eb8deb444b 100644
> --- a/drivers/gpu/drm/bridge/lontium-lt9611.c
> +++ b/drivers/gpu/drm/bridge/lontium-lt9611.c
> @@ -867,8 +867,14 @@ static enum drm_mode_status 
> lt9611_bridge_mode_valid(struct drm_bridge *bridge,
>const struct 
> drm_display_mode *mode)
>  {
>   struct lt9611_mode *lt9611_mode = lt9611_find_mode(mode);
> + struct lt9611 *lt9611 = bridge_to_lt9611(bridge);
>  
> - return lt9611_mode ? MODE_OK : MODE_BAD;
> + if (!lt9611_mode)
> + return MODE_BAD;
> + else if (lt9611_mode->intfs > 1 && !lt9611->dsi1)
> + return MODE_PANEL;
> + else
> + return MODE_OK;
>  }
>  
>  static void lt9611_bridge_pre_enable(struct drm_bridge *bridge)
> -- 
> 2.27.0

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

Re: [PATCH 000/141] Fix fall-through warnings for Clang

[James Bottomley]

On Mon, 2020-11-23 at 15:19 +0100, Miguel Ojeda wrote:
> On Sun, Nov 22, 2020 at 11:36 PM James Bottomley
>  wrote:
> > Well, it seems to be three years of someone's time plus the
> > maintainer review time and series disruption of nearly a thousand
> > patches.  Let's be conservative and assume the producer worked
> > about 30% on the series and it takes about 5-10 minutes per patch
> > to review, merge and for others to rework existing series.  So
> > let's say it's cost a person year of a relatively junior engineer
> > producing the patches and say 100h of review and application
> > time.  The latter is likely the big ticket item because it's what
> > we have in least supply in the kernel (even though it's 20x vs the
> > producer time).
> 
> How are you arriving at such numbers? It is a total of ~200 trivial
> lines.

Well, I used git.  It says that as of today in Linus' tree we have 889
patches related to fall throughs and the first series went in in
october 2017 ... ignoring a couple of outliers back to February.

> > It's not about the risk of the changes it's about the cost of
> > implementing them.  Even if you discount the producer time (which
> > someone gets to pay for, and if I were the engineering manager, I'd
> > be unhappy about), the review/merge/rework time is pretty
> > significant in exchange for six minor bug fixes.  Fine, when a new
> > compiler warning comes along it's certainly reasonable to see if we
> > can benefit from it and the fact that the compiler people think
> > it's worthwhile is enough evidence to assume this initially.  But
> > at some point you have to ask whether that assumption is supported
> > by the evidence we've accumulated over the time we've been using
> > it.  And if the evidence doesn't support it perhaps it is time to
> > stop the experiment.
> 
> Maintainers routinely review 1-line trivial patches, not to mention
> internal API changes, etc.

We're also complaining about the inability to recruit maintainers:

https://www.theregister.com/2020/06/30/hard_to_find_linux_maintainers_says_torvalds/

And burn out:

http://antirez.com/news/129

The whole crux of your argument seems to be maintainers' time isn't
important so we should accept all trivial patches ... I'm pushing back
on that assumption in two places, firstly the valulessness of the time
and secondly that all trivial patches are valuable.

> If some company does not want to pay for that, that's fine, but they
> don't get to be maintainers and claim `Supported`.

What I'm actually trying to articulate is a way of measuring value of
the patch vs cost ... it has nothing really to do with who foots the
actual bill.

One thesis I'm actually starting to formulate is that this continual
devaluing of maintainers is why we have so much difficulty keeping and
recruiting them.

James



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

Re: [RFC] MAINTAINERS tag for cleanup robot

[Jani Nikula]

On Sat, 21 Nov 2020, James Bottomley  
wrote:
> On Sat, 2020-11-21 at 08:50 -0800, t...@redhat.com wrote:
>> A difficult part of automating commits is composing the subsystem
>> preamble in the commit log.  For the ongoing effort of a fixer
>> producing
>> one or two fixes a release the use of 'treewide:' does not seem
>> appropriate.
>> 
>> It would be better if the normal prefix was used.  Unfortunately
>> normal is
>> not consistent across the tree.
>> 
>> 
>>  D: Commit subsystem prefix
>> 
>> ex/ for FPGA DFL DRIVERS
>> 
>>  D: fpga: dfl:
>> 
>
> I've got to bet this is going to cause more issues than it solves.

Agreed.

> SCSI uses scsi: : for drivers but not every driver has a
> MAINTAINERS entry.  We use either scsi: or scsi: core: for mid layer
> things, but we're not consistent.  Block uses blk-: for all
> of it's stuff but almost no s have a MAINTAINERS entry.  So
> the next thing you're going to cause is an explosion of suggested
> MAINTAINERs entries.

On the one hand, adoption of new MAINTAINERS entries has been really
slow. Look at B, C, or P, for instance. On the other hand, if this were
to get adopted, you'll potentially get conflicting prefixes for patches
touching multiple files. Then what?

I'm guessing a script looking at git log could come up with better
suggestions for prefixes via popularity contest than manually maintained
MAINTAINERS entries. It might not always get it right, but then human
outsiders aren't going to always get it right either.

Now you'll only need Someone(tm) to write the script. ;)

Something quick like this:

git log --since={1year} --pretty=format:%s --  |\
grep -v "^\(Merge\|Revert\)" |\
sed 's/:[^:]*$//' |\
sort | uniq -c | sort -rn | head -5

already gives me results that really aren't worse than some of the
prefixes invented by drive-by contributors.

> Has anyone actually complained about treewide:?

As Joe said, I'd feel silly applying patches to drivers with that
prefix. If it gets applied by someone else higher up, literally
treewide, then no complaints.

BR,
Jani.


-- 
Jani Nikula, Intel Open Source Graphics Center
___
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

[Bug 210317] Sleep mode not working in ryzen 4000 laptop

[bugzilla-daemon]

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

--- Comment #1 from Sujay1844 (sujay1...@protonmail.com) ---
btw the same issue persists for 5.6, 5.7, 5.8 and 5.9 kernels.

-- 
You are receiving this mail because:
You are watching the assignee of the bug.
___
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

[Bug 210317] New: Sleep mode not working in ryzen 4000 laptop

[bugzilla-daemon]

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

Bug ID: 210317
   Summary: Sleep mode not working in ryzen 4000 laptop
   Product: Drivers
   Version: 2.5
Kernel Version: 5.10.rc4
  Hardware: All
OS: Linux
  Tree: Mainline
Status: NEW
  Severity: high
  Priority: P1
 Component: Video(DRI - non Intel)
  Assignee: drivers_video-...@kernel-bugs.osdl.org
  Reporter: sujay1...@protonmail.com
Regression: No

Created attachment 293787
  --> https://bugzilla.kernel.org/attachment.cgi?id=293787=edit
Output of inxi -Fxz

I have an Acer Swift 3 with AMD Ryzen 5 4500U running Manjaro. When I close and
open the laptop lid, the screen doesn't turn back on but the keyboard backlight
does. If I remember correctly, when I installed 5.10.rc2, sleep worked normally
for exactly twice and then never again till now. I have the output of inxi -Fxz
in a text file attached. This is my first ever bug report so hopefully I have
given all the required information.

-- 
You are receiving this mail because:
You are watching the assignee of the bug.
___
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

Re: [PATCHv8 0/8] System Cache support for GPU and required SMMU support

[Will Deacon]

On Tue, Nov 17, 2020 at 08:00:39PM +0530, Sai Prakash Ranjan wrote:
> Some hardware variants contain a system cache or the last level
> cache(llc). This cache is typically a large block which is shared
> by multiple clients on the SOC. GPU uses the system cache to cache
> both the GPU data buffers(like textures) as well the SMMU pagetables.
> This helps with improved render performance as well as lower power
> consumption by reducing the bus traffic to the system memory.
> 
> The system cache architecture allows the cache to be split into slices
> which then be used by multiple SOC clients. This patch series is an
> effort to enable and use two of those slices preallocated for the GPU,
> one for the GPU data buffers and another for the GPU SMMU hardware
> pagetables.
> 
> Patch 1 - Patch 6 adds system cache support in SMMU and GPU driver.
> Patch 7 and 8 are minor cleanups for arm-smmu impl.
> 
> Changes in v8:
>  * Introduce a generic domain attribute for pagetable config (Will)
>  * Rename quirk to more generic IO_PGTABLE_QUIRK_ARM_OUTER_WBWA (Will)
>  * Move non-strict mode to use new struct domain_attr_io_pgtbl_config (Will)

Modulo some minor comments I've made, this looks good to me. What is the
plan for merging it? I can take the IOMMU parts, but patches 4-6 touch the
MSM GPU driver and I'd like to avoid conflicts with that.

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

Re: [PATCHv8 3/8] iommu/arm-smmu: Move non-strict mode to use domain_attr_io_pgtbl_cfg

[Will Deacon]

On Tue, Nov 17, 2020 at 08:00:42PM +0530, Sai Prakash Ranjan wrote:
> Now that we have a struct domain_attr_io_pgtbl_cfg with quirks,
> use that for non_strict mode as well thereby removing the need
> for more members of arm_smmu_domain in the future.
> 
> Signed-off-by: Sai Prakash Ranjan 
> ---
>  drivers/iommu/arm/arm-smmu/arm-smmu.c | 7 ++-
>  drivers/iommu/arm/arm-smmu/arm-smmu.h | 1 -
>  2 files changed, 2 insertions(+), 6 deletions(-)
> 
> diff --git a/drivers/iommu/arm/arm-smmu/arm-smmu.c 
> b/drivers/iommu/arm/arm-smmu/arm-smmu.c
> index 7b05782738e2..5f066a1b7221 100644
> --- a/drivers/iommu/arm/arm-smmu/arm-smmu.c
> +++ b/drivers/iommu/arm/arm-smmu/arm-smmu.c
> @@ -786,9 +786,6 @@ static int arm_smmu_init_domain_context(struct 
> iommu_domain *domain,
>   goto out_clear_smmu;
>   }
>  
> - if (smmu_domain->non_strict)
> - pgtbl_cfg.quirks |= IO_PGTABLE_QUIRK_NON_STRICT;
> -
>   if (smmu_domain->pgtbl_cfg.quirks)
>   pgtbl_cfg.quirks |= smmu_domain->pgtbl_cfg.quirks;
>  
> @@ -1527,7 +1524,7 @@ static int arm_smmu_domain_get_attr(struct iommu_domain 
> *domain,
>   case IOMMU_DOMAIN_DMA:
>   switch (attr) {
>   case DOMAIN_ATTR_DMA_USE_FLUSH_QUEUE:
> - *(int *)data = smmu_domain->non_strict;
> + *(int *)data = smmu_domain->pgtbl_cfg.quirks;

Probably better to compare with IO_PGTABLE_QUIRK_NON_STRICT here even though
we only support this one quirk for DMA domains atm.

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

Re: [PATCHv8 2/8] iommu/arm-smmu: Add domain attribute for pagetable configuration

[Will Deacon]

On Tue, Nov 17, 2020 at 08:00:41PM +0530, Sai Prakash Ranjan wrote:
> Add iommu domain attribute for pagetable configuration which
> initially will be used to set quirks like for system cache aka
> last level cache to be used by client drivers like GPU to set
> right attributes for caching the hardware pagetables into the
> system cache and later can be extended to include other page
> table configuration data.
> 
> Signed-off-by: Sai Prakash Ranjan 
> ---
>  drivers/iommu/arm/arm-smmu/arm-smmu.c | 25 +
>  drivers/iommu/arm/arm-smmu/arm-smmu.h |  1 +
>  include/linux/io-pgtable.h|  4 
>  include/linux/iommu.h |  1 +
>  4 files changed, 31 insertions(+)
> 
> diff --git a/drivers/iommu/arm/arm-smmu/arm-smmu.c 
> b/drivers/iommu/arm/arm-smmu/arm-smmu.c
> index 0f28a8614da3..7b05782738e2 100644
> --- a/drivers/iommu/arm/arm-smmu/arm-smmu.c
> +++ b/drivers/iommu/arm/arm-smmu/arm-smmu.c
> @@ -789,6 +789,9 @@ static int arm_smmu_init_domain_context(struct 
> iommu_domain *domain,
>   if (smmu_domain->non_strict)
>   pgtbl_cfg.quirks |= IO_PGTABLE_QUIRK_NON_STRICT;
>  
> + if (smmu_domain->pgtbl_cfg.quirks)
> + pgtbl_cfg.quirks |= smmu_domain->pgtbl_cfg.quirks;
> +
>   pgtbl_ops = alloc_io_pgtable_ops(fmt, _cfg, smmu_domain);
>   if (!pgtbl_ops) {
>   ret = -ENOMEM;
> @@ -1511,6 +1514,12 @@ static int arm_smmu_domain_get_attr(struct 
> iommu_domain *domain,
>   case DOMAIN_ATTR_NESTING:
>   *(int *)data = (smmu_domain->stage == 
> ARM_SMMU_DOMAIN_NESTED);
>   return 0;
> + case DOMAIN_ATTR_IO_PGTABLE_CFG: {
> + struct domain_attr_io_pgtbl_cfg *pgtbl_cfg = data;
> + *pgtbl_cfg = smmu_domain->pgtbl_cfg;
> +
> + return 0;
> + }
>   default:
>   return -ENODEV;
>   }
> @@ -1551,6 +1560,22 @@ static int arm_smmu_domain_set_attr(struct 
> iommu_domain *domain,
>   else
>   smmu_domain->stage = ARM_SMMU_DOMAIN_S1;
>   break;
> + case DOMAIN_ATTR_IO_PGTABLE_CFG: {
> + struct domain_attr_io_pgtbl_cfg *pgtbl_cfg = data;
> +
> + if (smmu_domain->smmu) {
> + ret = -EPERM;
> + goto out_unlock;
> + }
> +
> + if (!pgtbl_cfg) {

Do we really need to check this? If somebody passed us a NULL pointer then
they have a bug and we don't check this for other domain attributes afaict.

> + ret = -ENODEV;
> + goto out_unlock;
> + }
> +
> + smmu_domain->pgtbl_cfg = *pgtbl_cfg;
> + break;
> + }
>   default:
>   ret = -ENODEV;
>   }
> diff --git a/drivers/iommu/arm/arm-smmu/arm-smmu.h 
> b/drivers/iommu/arm/arm-smmu/arm-smmu.h
> index 04288b6fc619..18fbed376afb 100644
> --- a/drivers/iommu/arm/arm-smmu/arm-smmu.h
> +++ b/drivers/iommu/arm/arm-smmu/arm-smmu.h
> @@ -364,6 +364,7 @@ enum arm_smmu_domain_stage {
>  struct arm_smmu_domain {
>   struct arm_smmu_device  *smmu;
>   struct io_pgtable_ops   *pgtbl_ops;
> + struct domain_attr_io_pgtbl_cfg pgtbl_cfg;
>   const struct iommu_flush_ops*flush_ops;
>   struct arm_smmu_cfg cfg;
>   enum arm_smmu_domain_stage  stage;
> diff --git a/include/linux/io-pgtable.h b/include/linux/io-pgtable.h
> index a9a2c59fab37..686b37d48743 100644
> --- a/include/linux/io-pgtable.h
> +++ b/include/linux/io-pgtable.h
> @@ -212,6 +212,10 @@ struct io_pgtable {
>  
>  #define io_pgtable_ops_to_pgtable(x) container_of((x), struct io_pgtable, 
> ops)
>  
> +struct domain_attr_io_pgtbl_cfg {
> + unsigned long quirks;
> +};

nit: Can you rename this to 'struct io_pgtable_domain_attr' please?

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

Re: [PATCHv8 1/8] iommu/io-pgtable-arm: Add support to use system cache

[Will Deacon]

On Tue, Nov 17, 2020 at 08:00:40PM +0530, Sai Prakash Ranjan wrote:
> Add a quirk IO_PGTABLE_QUIRK_ARM_OUTER_WBWA to override
> the attributes set in TCR for the page table walker when
> using system cache.
> 
> Signed-off-by: Sai Prakash Ranjan 
> ---
>  drivers/iommu/io-pgtable-arm.c | 10 --
>  include/linux/io-pgtable.h |  4 
>  2 files changed, 12 insertions(+), 2 deletions(-)
> 
> diff --git a/drivers/iommu/io-pgtable-arm.c b/drivers/iommu/io-pgtable-arm.c
> index a7a9bc08dcd1..7c9ea9d7874a 100644
> --- a/drivers/iommu/io-pgtable-arm.c
> +++ b/drivers/iommu/io-pgtable-arm.c
> @@ -761,7 +761,8 @@ arm_64_lpae_alloc_pgtable_s1(struct io_pgtable_cfg *cfg, 
> void *cookie)
>  
>   if (cfg->quirks & ~(IO_PGTABLE_QUIRK_ARM_NS |
>   IO_PGTABLE_QUIRK_NON_STRICT |
> - IO_PGTABLE_QUIRK_ARM_TTBR1))
> + IO_PGTABLE_QUIRK_ARM_TTBR1 |
> + IO_PGTABLE_QUIRK_ARM_OUTER_WBWA))
>   return NULL;
>  
>   data = arm_lpae_alloc_pgtable(cfg);
> @@ -773,10 +774,15 @@ arm_64_lpae_alloc_pgtable_s1(struct io_pgtable_cfg 
> *cfg, void *cookie)
>   tcr->sh = ARM_LPAE_TCR_SH_IS;
>   tcr->irgn = ARM_LPAE_TCR_RGN_WBWA;
>   tcr->orgn = ARM_LPAE_TCR_RGN_WBWA;
> + if (cfg->quirks & IO_PGTABLE_QUIRK_ARM_OUTER_WBWA)
> + goto out_free_data;
>   } else {
>   tcr->sh = ARM_LPAE_TCR_SH_OS;
>   tcr->irgn = ARM_LPAE_TCR_RGN_NC;
> - tcr->orgn = ARM_LPAE_TCR_RGN_NC;
> + if (!(cfg->quirks & IO_PGTABLE_QUIRK_ARM_OUTER_WBWA))
> + tcr->orgn = ARM_LPAE_TCR_RGN_NC;
> + else
> + tcr->orgn = ARM_LPAE_TCR_RGN_WBWA;
>   }
>  
>   tg1 = cfg->quirks & IO_PGTABLE_QUIRK_ARM_TTBR1;
> diff --git a/include/linux/io-pgtable.h b/include/linux/io-pgtable.h
> index 4cde111e425b..a9a2c59fab37 100644
> --- a/include/linux/io-pgtable.h
> +++ b/include/linux/io-pgtable.h
> @@ -86,6 +86,9 @@ struct io_pgtable_cfg {
>*
>* IO_PGTABLE_QUIRK_ARM_TTBR1: (ARM LPAE format) Configure the table
>*  for use in the upper half of a split address space.
> +  *
> +  * IO_PGTABLE_QUIRK_ARM_OUTER_WBWA: Override the attributes set in TCR 
> for
> +  *  the page table walker when using system cache.

Please can you reword this to say:

  "Override the outer-cacheability attributes set in the TCR for a non-coherent
   page-table walker."

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

Re: [PATCH 18/40] drm/mediatek/mtk_dpi: Remove unused struct definition 'mtk_dpi_encoder_funcs'

[Chun-Kuang Hu]

Hi, Lee:

Lee Jones  於 2020年11月13日 週五 下午9:50寫道：
>
> Fixes the following W=1 kernel build warning(s):
>
>  drivers/gpu/drm/mediatek/mtk_dpi.c:530:39: warning: ‘mtk_dpi_encoder_funcs’ 
> defined but not used [-Wunused-const-variable=]
>

Thanks for this patch, but I've applied another identical patch
"drm/mediatek: mtk_dpi: Fix unused variable 'mtk_dpi_encoder_funcs'"
[1].

[1] 
https://git.kernel.org/pub/scm/linux/kernel/git/chunkuang.hu/linux.git/commit/?h=mediatek-drm-fixes=46b97aed5484a3f44584a10f9e0691bf89d29064

Regards,
Chun-Kuang.

> Cc: Chun-Kuang Hu 
> Cc: Philipp Zabel 
> Cc: David Airlie 
> Cc: Daniel Vetter 
> Cc: Matthias Brugger 
> Cc: Jie Qiu 
> Cc: dri-devel@lists.freedesktop.org
> Cc: linux-arm-ker...@lists.infradead.org
> Cc: linux-media...@lists.infradead.org
> Signed-off-by: Lee Jones 
> ---
>  drivers/gpu/drm/mediatek/mtk_dpi.c | 9 -
>  1 file changed, 9 deletions(-)
>
> diff --git a/drivers/gpu/drm/mediatek/mtk_dpi.c 
> b/drivers/gpu/drm/mediatek/mtk_dpi.c
> index cf11c4850b405..52f11a63a3304 100644
> --- a/drivers/gpu/drm/mediatek/mtk_dpi.c
> +++ b/drivers/gpu/drm/mediatek/mtk_dpi.c
> @@ -522,15 +522,6 @@ static int mtk_dpi_set_display_mode(struct mtk_dpi *dpi,
> return 0;
>  }
>
> -static void mtk_dpi_encoder_destroy(struct drm_encoder *encoder)
> -{
> -   drm_encoder_cleanup(encoder);
> -}
> -
> -static const struct drm_encoder_funcs mtk_dpi_encoder_funcs = {
> -   .destroy = mtk_dpi_encoder_destroy,
> -};
> -
>  static int mtk_dpi_bridge_attach(struct drm_bridge *bridge,
>  enum drm_bridge_attach_flags flags)
>  {
> --
> 2.25.1
>
___
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

[PATCH 2/2] drm: panel: add Khadas TS050 panel driver

[Neil Armstrong]

This add support for the Khadas TS050 1080x1920 5" LCD DSI panel designed to 
work
with the Khadas Edge-V, Captain, VIM3 and VIM3L Single Board Computers.
It provides a MIPI DSI interface to the host, a built-in LED backlight
and touch controller.

The init values was taken from the vendor source tree, comments were added to 
the
know values but most of the init table is undocumented.

Signed-off-by: Neil Armstrong 
---
 drivers/gpu/drm/panel/Kconfig  |  11 +
 drivers/gpu/drm/panel/Makefile |   1 +
 drivers/gpu/drm/panel/panel-khadas-ts050.c | 876 +
 3 files changed, 888 insertions(+)
 create mode 100644 drivers/gpu/drm/panel/panel-khadas-ts050.c

diff --git a/drivers/gpu/drm/panel/Kconfig b/drivers/gpu/drm/panel/Kconfig
index b4e021ea30f9..8fec45b2ce02 100644
--- a/drivers/gpu/drm/panel/Kconfig
+++ b/drivers/gpu/drm/panel/Kconfig
@@ -145,6 +145,17 @@ config DRM_PANEL_JDI_LT070ME05000
  The panel has a 1200(RGB)×1920 (WUXGA) resolution and uses
  24 bit per pixel.
 
+config DRM_PANEL_KHADAS_TS050
+   tristate "Khadas TS050 panel"
+   depends on OF
+   depends on DRM_MIPI_DSI
+   depends on BACKLIGHT_CLASS_DEVICE
+   help
+ Say Y here if you want to enable support for Khadas TS050 TFT-LCD
+ panel module. The panel has a 1080x1920 resolution and uses
+ 24 bit RGB per pixel. It provides a MIPI DSI interface to
+ the host, a built-in LED backlight and touch controller.
+
 config DRM_PANEL_KINGDISPLAY_KD097D04
tristate "Kingdisplay kd097d04 panel"
depends on OF
diff --git a/drivers/gpu/drm/panel/Makefile b/drivers/gpu/drm/panel/Makefile
index ebbf488c7eac..03496695e03f 100644
--- a/drivers/gpu/drm/panel/Makefile
+++ b/drivers/gpu/drm/panel/Makefile
@@ -13,6 +13,7 @@ obj-$(CONFIG_DRM_PANEL_ILITEK_IL9322) += 
panel-ilitek-ili9322.o
 obj-$(CONFIG_DRM_PANEL_ILITEK_ILI9881C) += panel-ilitek-ili9881c.o
 obj-$(CONFIG_DRM_PANEL_INNOLUX_P079ZCA) += panel-innolux-p079zca.o
 obj-$(CONFIG_DRM_PANEL_JDI_LT070ME05000) += panel-jdi-lt070me05000.o
+obj-$(CONFIG_DRM_PANEL_KHADAS_TS050) += panel-khadas-ts050.o
 obj-$(CONFIG_DRM_PANEL_KINGDISPLAY_KD097D04) += panel-kingdisplay-kd097d04.o
 obj-$(CONFIG_DRM_PANEL_LEADTEK_LTK050H3146W) += panel-leadtek-ltk050h3146w.o
 obj-$(CONFIG_DRM_PANEL_LEADTEK_LTK500HD1829) += panel-leadtek-ltk500hd1829.o
diff --git a/drivers/gpu/drm/panel/panel-khadas-ts050.c 
b/drivers/gpu/drm/panel/panel-khadas-ts050.c
new file mode 100644
index ..856fcad69306
--- /dev/null
+++ b/drivers/gpu/drm/panel/panel-khadas-ts050.c
@@ -0,0 +1,876 @@
+// SPDX-License-Identifier: GPL-2.0
+/*
+ * Copyright (C) 2020 BayLibre, SAS
+ * Author: Neil Armstrong 
+ */
+
+#include 
+#include 
+#include 
+#include 
+#include 
+
+#include 
+
+#include 
+#include 
+#include 
+#include 
+#include 
+#include 
+
+struct khadas_ts050_panel {
+   struct drm_panel base;
+   struct mipi_dsi_device *link;
+
+   struct regulator *supply;
+   struct gpio_desc *reset_gpio;
+   struct gpio_desc *enable_gpio;
+
+   bool prepared;
+   bool enabled;
+};
+
+struct khadas_ts050_panel_cmd {
+   u8 cmd;
+   u8 data;
+};
+
+/* Only the CMD1 User Command set is documented */
+static const struct khadas_ts050_panel_cmd init_code[] = {
+   /* Select Unknown CMD Page (Undocumented) */
+   {0xff, 0xee},
+   /* Reload CMD1: Don't reload default value to register */
+   {0xfb, 0x01},
+   {0x1f, 0x45},
+   {0x24, 0x4f},
+   {0x38, 0xc8},
+   {0x39, 0x27},
+   {0x1e, 0x77},
+   {0x1d, 0x0f},
+   {0x7e, 0x71},
+   {0x7c, 0x03},
+   {0xff, 0x00},
+   {0xfb, 0x01},
+   {0x35, 0x01},
+   /* Select CMD2 Page0 (Undocumented) */
+   {0xff, 0x01},
+   /* Reload CMD1: Don't reload default value to register */
+   {0xfb, 0x01},
+   {0x00, 0x01},
+   {0x01, 0x55},
+   {0x02, 0x40},
+   {0x05, 0x40},
+   {0x06, 0x4a},
+   {0x07, 0x24},
+   {0x08, 0x0c},
+   {0x0b, 0x7d},
+   {0x0c, 0x7d},
+   {0x0e, 0xb0},
+   {0x0f, 0xae},
+   {0x11, 0x10},
+   {0x12, 0x10},
+   {0x13, 0x03},
+   {0x14, 0x4a},
+   {0x15, 0x12},
+   {0x16, 0x12},
+   {0x18, 0x00},
+   {0x19, 0x77},
+   {0x1a, 0x55},
+   {0x1b, 0x13},
+   {0x1c, 0x00},
+   {0x1d, 0x00},
+   {0x1e, 0x13},
+   {0x1f, 0x00},
+   {0x23, 0x00},
+   {0x24, 0x00},
+   {0x25, 0x00},
+   {0x26, 0x00},
+   {0x27, 0x00},
+   {0x28, 0x00},
+   {0x35, 0x00},
+   {0x66, 0x00},
+   {0x58, 0x82},
+   {0x59, 0x02},
+   {0x5a, 0x02},
+   {0x5b, 0x02},
+   {0x5c, 0x82},
+   {0x5d, 0x82},
+   {0x5e, 0x02},
+   {0x5f, 0x02},
+   {0x72, 0x31},
+   /* Select CMD2 Page4 (Undocumented) */
+   {0xff, 0x05},
+   /* Reload CMD1: Don't reload default value to register */
+   {0xfb, 0x01},
+   {0x00, 0x01},
+   

[PATCH 1/2] dt-bindings: panel-simple-dsi: add Khadas TS050 panel bindings

[Neil Armstrong]

This add the bindings for the Khadas TS050 1080x1920 5" LCD DSI panel designed 
to work
with the Khadas Edge-V, Captain, VIM3 and VIM3L Single Board Computers.

Signed-off-by: Neil Armstrong 
---
 .../devicetree/bindings/display/panel/panel-simple-dsi.yaml | 2 ++
 1 file changed, 2 insertions(+)

diff --git 
a/Documentation/devicetree/bindings/display/panel/panel-simple-dsi.yaml 
b/Documentation/devicetree/bindings/display/panel/panel-simple-dsi.yaml
index 72e4b6d4d5e1..fbd71669248f 100644
--- a/Documentation/devicetree/bindings/display/panel/panel-simple-dsi.yaml
+++ b/Documentation/devicetree/bindings/display/panel/panel-simple-dsi.yaml
@@ -35,6 +35,8 @@ properties:
   - boe,tv080wum-nl0
 # Innolux P079ZCA 7.85" 768x1024 TFT LCD panel
   - innolux,p079zca
+# Khadas TS050 5" 1080x1920 LCD panel
+  - khadas,ts050
 # Kingdisplay KD097D04 9.7" 1536x2048 TFT LCD panel
   - kingdisplay,kd097d04
 # LG ACX467AKM-7 4.95" 1080×1920 LCD Panel
-- 
2.25.1

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

[PATCH 0/2] drm: panel: add support for the Khadas TS050 panel

[Neil Armstrong]

This add support & bindings for the Khadas TS050 1080x1920 5" LCD DSI panel 
designed
to work with the Khadas Edge-V, Captain, VIM3 and VIM3L Single Board Computers.
It provides a MIPI DSI interface to the host, a built-in LED backlight
and touch controller.

Neil Armstrong (2):
  dt-bindings: panel-simple-dsi: add Khadas TS050 panel bindings
  drm: panel: add Khadas TS050 panel driver

 .../display/panel/panel-simple-dsi.yaml   |   2 +
 drivers/gpu/drm/panel/Kconfig |  11 +
 drivers/gpu/drm/panel/Makefile|   1 +
 drivers/gpu/drm/panel/panel-khadas-ts050.c| 876 ++
 4 files changed, 890 insertions(+)
 create mode 100644 drivers/gpu/drm/panel/panel-khadas-ts050.c

-- 
2.25.1

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

Re: [PATCH 28/30] drm/mediatek/mtk_disp_color: Fix formatting and provide missing member description

[Chun-Kuang Hu]

Hi, Lee:

Lee Jones  於 2020年11月13日 週五 上午3:01寫道：
>
> Fixes the following W=1 kernel build warning(s):
>
>  drivers/gpu/drm/mediatek/mtk_disp_color.c:40: warning: Function parameter or 
> member 'ddp_comp' not described in 'mtk_disp_color'
>  drivers/gpu/drm/mediatek/mtk_disp_color.c:40: warning: Function parameter or 
> member 'crtc' not described in 'mtk_disp_color'
>  drivers/gpu/drm/mediatek/mtk_disp_color.c:40: warning: Function parameter or 
> member 'data' not described in 'mtk_disp_color'
>

Applied to mediatek-drm-next [1], thanks.

[1] 
https://git.kernel.org/pub/scm/linux/kernel/git/chunkuang.hu/linux.git/log/?h=mediatek-drm-next

Regards,
Chun-Kuang.

> Cc: Chun-Kuang Hu 
> Cc: Philipp Zabel 
> Cc: David Airlie 
> Cc: Daniel Vetter 
> Cc: Matthias Brugger 
> Cc: dri-devel@lists.freedesktop.org
> Cc: linux-arm-ker...@lists.infradead.org
> Cc: linux-media...@lists.infradead.org
> Signed-off-by: Lee Jones 
> ---
>  drivers/gpu/drm/mediatek/mtk_disp_color.c | 5 +++--
>  1 file changed, 3 insertions(+), 2 deletions(-)
>
> diff --git a/drivers/gpu/drm/mediatek/mtk_disp_color.c 
> b/drivers/gpu/drm/mediatek/mtk_disp_color.c
> index 3ae9c810845bb..a788ff95ed6e4 100644
> --- a/drivers/gpu/drm/mediatek/mtk_disp_color.c
> +++ b/drivers/gpu/drm/mediatek/mtk_disp_color.c
> @@ -30,8 +30,9 @@ struct mtk_disp_color_data {
>
>  /**
>   * struct mtk_disp_color - DISP_COLOR driver structure
> - * @ddp_comp - structure containing type enum and hardware resources
> - * @crtc - associated crtc to report irq events to
> + * @ddp_comp: structure containing type enum and hardware resources
> + * @crtc: associated crtc to report irq events to
> + * @data: platform colour driver data
>   */
>  struct mtk_disp_color {
> struct mtk_ddp_comp ddp_comp;
> --
> 2.25.1
>
___
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

Re: [PATCH 10/40] drm/mediatek/mtk_drm_drv: Staticise local function invoked by reference

[Chun-Kuang Hu]

Hi, Lee:

Lee Jones  於 2020年11月13日 週五 下午9:49寫道：
>
> Fixes the following W=1 kernel build warning(s):
>
>  drivers/gpu/drm/mediatek/mtk_drm_drv.c:316:24: warning: no previous 
> prototype for ‘mtk_drm_gem_prime_import’ [-Wmissing-prototypes]
>

Applied to mediatek-drm-next [1], thanks.

[1] 
https://git.kernel.org/pub/scm/linux/kernel/git/chunkuang.hu/linux.git/log/?h=mediatek-drm-next

Regards,
Chun-Kuang.

> Cc: Chun-Kuang Hu 
> Cc: Philipp Zabel 
> Cc: David Airlie 
> Cc: Daniel Vetter 
> Cc: Matthias Brugger 
> Cc: Sumit Semwal 
> Cc: "Christian König" 
> Cc: YT SHEN 
> Cc: dri-devel@lists.freedesktop.org
> Cc: linux-arm-ker...@lists.infradead.org
> Cc: linux-media...@lists.infradead.org
> Cc: linux-me...@vger.kernel.org
> Cc: linaro-mm-...@lists.linaro.org
> Signed-off-by: Lee Jones 
> ---
>  drivers/gpu/drm/mediatek/mtk_drm_drv.c | 4 ++--
>  1 file changed, 2 insertions(+), 2 deletions(-)
>
> diff --git a/drivers/gpu/drm/mediatek/mtk_drm_drv.c 
> b/drivers/gpu/drm/mediatek/mtk_drm_drv.c
> index 2e8065b1e2bbc..401311453e7d0 100644
> --- a/drivers/gpu/drm/mediatek/mtk_drm_drv.c
> +++ b/drivers/gpu/drm/mediatek/mtk_drm_drv.c
> @@ -313,8 +313,8 @@ static const struct file_operations mtk_drm_fops = {
>   * We need to override this because the device used to import the memory is
>   * not dev->dev, as drm_gem_prime_import() expects.
>   */
> -struct drm_gem_object *mtk_drm_gem_prime_import(struct drm_device *dev,
> -   struct dma_buf *dma_buf)
> +static struct drm_gem_object *mtk_drm_gem_prime_import(struct drm_device 
> *dev,
> +  struct dma_buf 
> *dma_buf)
>  {
> struct mtk_drm_private *private = dev->dev_private;
>
> --
> 2.25.1
>
___
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

Re: [PATCH 07/40] drm/mediatek/mtk_disp_rdma: Fix formatting and supply missing struct member description

[Chun-Kuang Hu]

Hi, Lee:

Lee Jones  於 2020年11月13日 週五 下午9:49寫道：
>
> Fixes the following W=1 kernel build warning(s):
>
>  drivers/gpu/drm/mediatek/mtk_disp_rdma.c:66: warning: Function parameter or 
> member 'ddp_comp' not described in 'mtk_disp_rdma'
>  drivers/gpu/drm/mediatek/mtk_disp_rdma.c:66: warning: Function parameter or 
> member 'crtc' not described in 'mtk_disp_rdma'
>  drivers/gpu/drm/mediatek/mtk_disp_rdma.c:66: warning: Function parameter or 
> member 'data' not described in 'mtk_disp_rdma'
>

Applied to mediatek-drm-next [1], thanks.

[1] 
https://git.kernel.org/pub/scm/linux/kernel/git/chunkuang.hu/linux.git/log/?h=mediatek-drm-next

Regards,
Chun-Kuang.

> Cc: Chun-Kuang Hu 
> Cc: Philipp Zabel 
> Cc: David Airlie 
> Cc: Daniel Vetter 
> Cc: Matthias Brugger 
> Cc: dri-devel@lists.freedesktop.org
> Cc: linux-arm-ker...@lists.infradead.org
> Cc: linux-media...@lists.infradead.org
> Signed-off-by: Lee Jones 
> ---
>  drivers/gpu/drm/mediatek/mtk_disp_rdma.c | 5 +++--
>  1 file changed, 3 insertions(+), 2 deletions(-)
>
> diff --git a/drivers/gpu/drm/mediatek/mtk_disp_rdma.c 
> b/drivers/gpu/drm/mediatek/mtk_disp_rdma.c
> index e04319fedf463..d46b8ae1d0800 100644
> --- a/drivers/gpu/drm/mediatek/mtk_disp_rdma.c
> +++ b/drivers/gpu/drm/mediatek/mtk_disp_rdma.c
> @@ -56,8 +56,9 @@ struct mtk_disp_rdma_data {
>
>  /**
>   * struct mtk_disp_rdma - DISP_RDMA driver structure
> - * @ddp_comp - structure containing type enum and hardware resources
> - * @crtc - associated crtc to report irq events to
> + * @ddp_comp: structure containing type enum and hardware resources
> + * @crtc: associated crtc to report irq events to
> + * @data: local driver data
>   */
>  struct mtk_disp_rdma {
> struct mtk_ddp_comp ddp_comp;
> --
> 2.25.1
>
___
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

Re: [PATCH 30/30] drm/mediatek/mtk_disp_ovl: Fix formatting and provide missing member description

[Chun-Kuang Hu]

Hi, Lee:

Lee Jones  於 2020年11月13日 週五 上午3:01寫道：
>
> Fixes the following W=1 kernel build warning(s):
>
>  drivers/gpu/drm/mediatek/mtk_disp_ovl.c:75: warning: Function parameter or 
> member 'ddp_comp' not described in 'mtk_disp_ovl'
>  drivers/gpu/drm/mediatek/mtk_disp_ovl.c:75: warning: Function parameter or 
> member 'crtc' not described in 'mtk_disp_ovl'
>  drivers/gpu/drm/mediatek/mtk_disp_ovl.c:75: warning: Function parameter or 
> member 'data' not described in 'mtk_disp_ovl'

Applied to mediatek-drm-next [1], thanks.

[1] 
https://git.kernel.org/pub/scm/linux/kernel/git/chunkuang.hu/linux.git/log/?h=mediatek-drm-next

Regards,
Chun-Kuang.

>
> Cc: Chun-Kuang Hu 
> Cc: Philipp Zabel 
> Cc: David Airlie 
> Cc: Daniel Vetter 
> Cc: Matthias Brugger 
> Cc: dri-devel@lists.freedesktop.org
> Cc: linux-arm-ker...@lists.infradead.org
> Cc: linux-media...@lists.infradead.org
> Signed-off-by: Lee Jones 
> ---
>  drivers/gpu/drm/mediatek/mtk_disp_ovl.c | 5 +++--
>  1 file changed, 3 insertions(+), 2 deletions(-)
>
> diff --git a/drivers/gpu/drm/mediatek/mtk_disp_ovl.c 
> b/drivers/gpu/drm/mediatek/mtk_disp_ovl.c
> index 28651bc579bc9..74ef6fc0528b6 100644
> --- a/drivers/gpu/drm/mediatek/mtk_disp_ovl.c
> +++ b/drivers/gpu/drm/mediatek/mtk_disp_ovl.c
> @@ -65,8 +65,9 @@ struct mtk_disp_ovl_data {
>
>  /**
>   * struct mtk_disp_ovl - DISP_OVL driver structure
> - * @ddp_comp - structure containing type enum and hardware resources
> - * @crtc - associated crtc to report vblank events to
> + * @ddp_comp: structure containing type enum and hardware resources
> + * @crtc: associated crtc to report vblank events to
> + * @data: platform data
>   */
>  struct mtk_disp_ovl {
> struct mtk_ddp_comp ddp_comp;
> --
> 2.25.1
>
___
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

Re: [PATCH] drm/mediatek: Drop local dma_parms

[Chun-Kuang Hu]

Hi, Robin:

Robin Murphy  於 2020年9月4日 週五 上午4:59寫道：
>
> Since commit 9495b7e92f71 ("driver core: platform: Initialize dma_parms
> for platform devices"), struct platform_device already provides a
> dma_parms structure, so we can save allocating another one.
>
> Also the DMA segment size is simply a size, not a bitmask.
>

Applied to mediatek-drm-next [1], thanks.

[1] 
https://git.kernel.org/pub/scm/linux/kernel/git/chunkuang.hu/linux.git/log/?h=mediatek-drm-next

Regards,
Chun-Kuang.

> Signed-off-by: Robin Murphy 
> ---
>  drivers/gpu/drm/mediatek/mtk_drm_drv.c | 23 +++
>  drivers/gpu/drm/mediatek/mtk_drm_drv.h |  2 --
>  2 files changed, 3 insertions(+), 22 deletions(-)
>
> diff --git a/drivers/gpu/drm/mediatek/mtk_drm_drv.c 
> b/drivers/gpu/drm/mediatek/mtk_drm_drv.c
> index 040a8f393fe2..3941f6f6b003 100644
> --- a/drivers/gpu/drm/mediatek/mtk_drm_drv.c
> +++ b/drivers/gpu/drm/mediatek/mtk_drm_drv.c
> @@ -221,21 +221,10 @@ static int mtk_drm_kms_init(struct drm_device *drm)
>  * Configure the DMA segment size to make sure we get contiguous IOVA
>  * when importing PRIME buffers.
>  */
> -   if (!dma_dev->dma_parms) {
> -   private->dma_parms_allocated = true;
> -   dma_dev->dma_parms =
> -   devm_kzalloc(drm->dev, sizeof(*dma_dev->dma_parms),
> -GFP_KERNEL);
> -   }
> -   if (!dma_dev->dma_parms) {
> -   ret = -ENOMEM;
> -   goto err_component_unbind;
> -   }
> -
> -   ret = dma_set_max_seg_size(dma_dev, (unsigned int)DMA_BIT_MASK(32));
> +   ret = dma_set_max_seg_size(dma_dev, UINT_MAX);
> if (ret) {
> dev_err(dma_dev, "Failed to set DMA segment size\n");
> -   goto err_unset_dma_parms;
> +   goto err_component_unbind;
> }
>
> /*
> @@ -246,16 +235,13 @@ static int mtk_drm_kms_init(struct drm_device *drm)
> drm->irq_enabled = true;
> ret = drm_vblank_init(drm, MAX_CRTC);
> if (ret < 0)
> -   goto err_unset_dma_parms;
> +   goto err_component_unbind;
>
> drm_kms_helper_poll_init(drm);
> drm_mode_config_reset(drm);
>
> return 0;
>
> -err_unset_dma_parms:
> -   if (private->dma_parms_allocated)
> -   dma_dev->dma_parms = NULL;
>  err_component_unbind:
> component_unbind_all(drm->dev, drm);
>
> @@ -269,9 +255,6 @@ static void mtk_drm_kms_deinit(struct drm_device *drm)
> drm_kms_helper_poll_fini(drm);
> drm_atomic_helper_shutdown(drm);
>
> -   if (private->dma_parms_allocated)
> -   private->dma_dev->dma_parms = NULL;
> -
> component_unbind_all(drm->dev, drm);
>  }
>
> diff --git a/drivers/gpu/drm/mediatek/mtk_drm_drv.h 
> b/drivers/gpu/drm/mediatek/mtk_drm_drv.h
> index b5be63e53176..6afd0b5f2b92 100644
> --- a/drivers/gpu/drm/mediatek/mtk_drm_drv.h
> +++ b/drivers/gpu/drm/mediatek/mtk_drm_drv.h
> @@ -44,8 +44,6 @@ struct mtk_drm_private {
> struct mtk_ddp_comp *ddp_comp[DDP_COMPONENT_ID_MAX];
> const struct mtk_mmsys_driver_data *data;
> struct drm_atomic_state *suspend_state;
> -
> -   bool dma_parms_allocated;
>  };
>
>  extern struct platform_driver mtk_ddp_driver;
> --
> 2.28.0.dirty
>
___
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

Re: [PATCH 1/2] drm/meson: dw-hdmi: Disable clocks on driver teardown

[Marc Zyngier]

On 2020-11-23 14:03, Jerome Brunet wrote:

On Fri 20 Nov 2020 at 10:42, Marc Zyngier  wrote:


The HDMI driver request clocks early, but never disable them, leaving
the clocks on even when the driver is removed.

Fix it by slightly refactoring the clock code, and register a devm
action that will eventually disable/unprepare the enabled clocks.

Signed-off-by: Marc Zyngier 
---
 drivers/gpu/drm/meson/meson_dw_hdmi.c | 43 
++-

 1 file changed, 29 insertions(+), 14 deletions(-)

diff --git a/drivers/gpu/drm/meson/meson_dw_hdmi.c 
b/drivers/gpu/drm/meson/meson_dw_hdmi.c

index 7f8eea494147..29623b309cb1 100644
--- a/drivers/gpu/drm/meson/meson_dw_hdmi.c
+++ b/drivers/gpu/drm/meson/meson_dw_hdmi.c
@@ -145,8 +145,6 @@ struct meson_dw_hdmi {
struct reset_control *hdmitx_apb;
struct reset_control *hdmitx_ctrl;
struct reset_control *hdmitx_phy;
-   struct clk *hdmi_pclk;
-   struct clk *venci_clk;
struct regulator *hdmi_supply;
u32 irq_stat;
struct dw_hdmi *hdmi;
@@ -946,6 +944,29 @@ static void meson_disable_regulator(void *data)
regulator_disable(data);
 }

+static void meson_disable_clk(void *data)
+{
+   clk_disable_unprepare(data);
+}
+
+static int meson_enable_clk(struct device *dev, char *name)
+{
+   struct clk *clk;
+   int ret;
+
+   clk = devm_clk_get(dev, name);
+   if (IS_ERR(clk)) {
+   dev_err(dev, "Unable to get %s pclk\n", name);
+   return PTR_ERR(clk);
+   }
+
+   ret = clk_prepare_enable(clk);
+   if (!ret)
+   ret = devm_add_action_or_reset(dev, meson_disable_clk,
clk);


Thanks for fixing this Marc.

FYI, while it is fine to declare a function to disable the clocks, a 
quick

cast may avoid it

devm_add_action_or_reset(dev, (void(*)(void *))clk_disable_unprepare, 
clk);


While this works for now, a change to the clk_disable_unprepare()
prototype (such as adding a second argument) would now go completely
unnoticed (after all, you've cast the function, it *must* be correct,
right?), and someone would spend a few hours trying to track down memory
corruption or some other interesting results.

Yes, casting C functions can be hilarious.

I can see a few uses of this hack in the tree, and I have my pop-corn
ready.

Thanks,

M.
--
Jazz is not dead. It just smells funny...
___
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

Re: [PATCH 1/2] drm/meson: dw-hdmi: Disable clocks on driver teardown

[Jerome Brunet]

On Fri 20 Nov 2020 at 10:42, Marc Zyngier  wrote:

> The HDMI driver request clocks early, but never disable them, leaving
> the clocks on even when the driver is removed.
>
> Fix it by slightly refactoring the clock code, and register a devm
> action that will eventually disable/unprepare the enabled clocks.
>
> Signed-off-by: Marc Zyngier 
> ---
>  drivers/gpu/drm/meson/meson_dw_hdmi.c | 43 ++-
>  1 file changed, 29 insertions(+), 14 deletions(-)
>
> diff --git a/drivers/gpu/drm/meson/meson_dw_hdmi.c 
> b/drivers/gpu/drm/meson/meson_dw_hdmi.c
> index 7f8eea494147..29623b309cb1 100644
> --- a/drivers/gpu/drm/meson/meson_dw_hdmi.c
> +++ b/drivers/gpu/drm/meson/meson_dw_hdmi.c
> @@ -145,8 +145,6 @@ struct meson_dw_hdmi {
>   struct reset_control *hdmitx_apb;
>   struct reset_control *hdmitx_ctrl;
>   struct reset_control *hdmitx_phy;
> - struct clk *hdmi_pclk;
> - struct clk *venci_clk;
>   struct regulator *hdmi_supply;
>   u32 irq_stat;
>   struct dw_hdmi *hdmi;
> @@ -946,6 +944,29 @@ static void meson_disable_regulator(void *data)
>   regulator_disable(data);
>  }
>  
> +static void meson_disable_clk(void *data)
> +{
> + clk_disable_unprepare(data);
> +}
> +
> +static int meson_enable_clk(struct device *dev, char *name)
> +{
> + struct clk *clk;
> + int ret;
> +
> + clk = devm_clk_get(dev, name);
> + if (IS_ERR(clk)) {
> + dev_err(dev, "Unable to get %s pclk\n", name);
> + return PTR_ERR(clk);
> + }
> +
> + ret = clk_prepare_enable(clk);
> + if (!ret)
> + ret = devm_add_action_or_reset(dev, meson_disable_clk,
> clk);

Thanks for fixing this Marc.

FYI, while it is fine to declare a function to disable the clocks, a quick
cast may avoid it

devm_add_action_or_reset(dev, (void(*)(void *))clk_disable_unprepare, clk);

> +
> + return ret;
> +}
> +
>  static int meson_dw_hdmi_bind(struct device *dev, struct device *master,
>   void *data)
>  {
> @@ -1026,19 +1047,13 @@ static int meson_dw_hdmi_bind(struct device *dev, 
> struct device *master,
>   if (IS_ERR(meson_dw_hdmi->hdmitx))
>   return PTR_ERR(meson_dw_hdmi->hdmitx);
>  
> - meson_dw_hdmi->hdmi_pclk = devm_clk_get(dev, "isfr");
> - if (IS_ERR(meson_dw_hdmi->hdmi_pclk)) {
> - dev_err(dev, "Unable to get HDMI pclk\n");
> - return PTR_ERR(meson_dw_hdmi->hdmi_pclk);
> - }
> - clk_prepare_enable(meson_dw_hdmi->hdmi_pclk);
> + ret = meson_enable_clk(dev, "isfr");
> + if (ret)
> + return ret;
>  
> - meson_dw_hdmi->venci_clk = devm_clk_get(dev, "venci");
> - if (IS_ERR(meson_dw_hdmi->venci_clk)) {
> - dev_err(dev, "Unable to get venci clk\n");
> - return PTR_ERR(meson_dw_hdmi->venci_clk);
> - }
> - clk_prepare_enable(meson_dw_hdmi->venci_clk);
> + ret = meson_enable_clk(dev, "venci");
> + if (ret)
> + return ret;
>  
>   dw_plat_data->regm = devm_regmap_init(dev, NULL, meson_dw_hdmi,
> _dw_hdmi_regmap_config);

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