[PATCH] drm/vmwgfx: NULL pointer dereference from vmw_cmd_dx_view_define()
If SVGA_3D_CMD_DX_DEFINE_RENDERTARGET_VIEW is called with a surface
ID of SVGA3D_INVALID_ID, the srf struct will remain NULL after
vmw_cmd_res_check(), leading to a null pointer dereference in
vmw_view_add().
Signed-off-by: Murray McAllister
---
drivers/gpu/drm/vmwgfx/vmwgfx_execbuf.c | 4
1 file changed, 4 insertions(+)
diff --git a/drivers/gpu/drm/vmwgfx/vmwgfx_execbuf.c
b/drivers/gpu/drm/vmwgfx/vmwgfx_execbuf.c
index 2ff7ba04d8c8..447afd086206 100644
--- a/drivers/gpu/drm/vmwgfx/vmwgfx_execbuf.c
+++ b/drivers/gpu/drm/vmwgfx/vmwgfx_execbuf.c
@@ -2414,6 +2414,10 @@ static int vmw_cmd_dx_view_define(struct vmw_private
*dev_priv,
return -EINVAL;
cmd = container_of(header, typeof(*cmd), header);
+ if (unlikely(cmd->sid == SVGA3D_INVALID_ID)) {
+ DRM_ERROR("Invalid surface id.\n");
+ return -EINVAL;
+ }
ret = vmw_cmd_res_check(dev_priv, sw_context, vmw_res_surface,
VMW_RES_DIRTY_NONE, user_surface_converter,
&cmd->sid, &srf);
--
2.20.1
[Bug 109345] drm-next-2018-12-14 -Linux PPC
https://bugs.freedesktop.org/show_bug.cgi?id=109345 --- Comment #44 from Allan Cairns --- Created attachment 144231 --> https://bugs.freedesktop.org/attachment.cgi?id=144231&action=edit Log for Good These are the logs for the bisects that booted to Firepro (2nd Card) -- You are receiving this mail because: You are the assignee for the bug.___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
[Bug 109345] drm-next-2018-12-14 -Linux PPC
https://bugs.freedesktop.org/show_bug.cgi?id=109345 --- Comment #43 from Allan Cairns --- Created attachment 144230 --> https://bugs.freedesktop.org/attachment.cgi?id=144230&action=edit Log for bad Christian provided bisect kernels (DRM1 to 11). These examples booted to Southern Island card (1st card) and are therefore marked as bad. -- You are receiving this mail because: You are the assignee for the bug.___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
[radeon-alex:drm-next-5.2-wip 94/126] drivers/gpu/drm/panfrost/panfrost_job.c:388:3: error: too few arguments to function 'drm_sched_stop'
tree: git://people.freedesktop.org/~agd5f/linux.git drm-next-5.2-wip
head: a6be3268e01a878e00f88555d16d65f88471d9e9
commit: 13688847c9d85d358d30dc4d6b128a42c6448106 [94/126] drm/scheduler: rework
job destruction
config: m68k-allmodconfig (attached as .config)
compiler: m68k-linux-gnu-gcc (Debian 7.2.0-11) 7.2.0
reproduce:
wget
https://raw.githubusercontent.com/intel/lkp-tests/master/sbin/make.cross -O
~/bin/make.cross
chmod +x ~/bin/make.cross
git checkout 13688847c9d85d358d30dc4d6b128a42c6448106
# save the attached .config to linux build tree
GCC_VERSION=7.2.0 make.cross ARCH=m68k
If you fix the issue, kindly add following tag
Reported-by: kbuild test robot
All errors (new ones prefixed by >>):
drivers/gpu/drm/panfrost/panfrost_job.c: In function 'panfrost_job_timedout':
>> drivers/gpu/drm/panfrost/panfrost_job.c:388:3: error: too few arguments to
>> function 'drm_sched_stop'
drm_sched_stop(&pfdev->js->queue[i].sched);
^~
In file included from drivers/gpu/drm/panfrost/panfrost_job.c:10:0:
include/drm/gpu_scheduler.h:295:6: note: declared here
void drm_sched_stop(struct drm_gpu_scheduler *sched, struct drm_sched_job
*bad);
^~
vim +/drm_sched_stop +388 drivers/gpu/drm/panfrost/panfrost_job.c
f3ba9122 Rob Herring 2018-09-10 365
f3ba9122 Rob Herring 2018-09-10 366 static void panfrost_job_timedout(struct
drm_sched_job *sched_job)
f3ba9122 Rob Herring 2018-09-10 367 {
f3ba9122 Rob Herring 2018-09-10 368struct panfrost_job *job =
to_panfrost_job(sched_job);
f3ba9122 Rob Herring 2018-09-10 369struct panfrost_device *pfdev =
job->pfdev;
f3ba9122 Rob Herring 2018-09-10 370int js = panfrost_job_get_slot(job);
f3ba9122 Rob Herring 2018-09-10 371int i;
f3ba9122 Rob Herring 2018-09-10 372
f3ba9122 Rob Herring 2018-09-10 373/*
f3ba9122 Rob Herring 2018-09-10 374 * If the GPU managed to complete this
jobs fence, the timeout is
f3ba9122 Rob Herring 2018-09-10 375 * spurious. Bail out.
f3ba9122 Rob Herring 2018-09-10 376 */
f3ba9122 Rob Herring 2018-09-10 377if
(dma_fence_is_signaled(job->done_fence))
f3ba9122 Rob Herring 2018-09-10 378return;
f3ba9122 Rob Herring 2018-09-10 379
f3ba9122 Rob Herring 2018-09-10 380dev_err(pfdev->dev, "gpu sched timeout,
js=%d, status=0x%x, head=0x%x, tail=0x%x, sched_job=%p",
f3ba9122 Rob Herring 2018-09-10 381js,
f3ba9122 Rob Herring 2018-09-10 382job_read(pfdev, JS_STATUS(js)),
f3ba9122 Rob Herring 2018-09-10 383job_read(pfdev, JS_HEAD_LO(js)),
f3ba9122 Rob Herring 2018-09-10 384job_read(pfdev, JS_TAIL_LO(js)),
f3ba9122 Rob Herring 2018-09-10 385sched_job);
f3ba9122 Rob Herring 2018-09-10 386
f3ba9122 Rob Herring 2018-09-10 387for (i = 0; i < NUM_JOB_SLOTS; i++)
f3ba9122 Rob Herring 2018-09-10 @388
drm_sched_stop(&pfdev->js->queue[i].sched);
f3ba9122 Rob Herring 2018-09-10 389
f3ba9122 Rob Herring 2018-09-10 390if (sched_job)
f3ba9122 Rob Herring 2018-09-10 391
drm_sched_increase_karma(sched_job);
f3ba9122 Rob Herring 2018-09-10 392
f3ba9122 Rob Herring 2018-09-10 393/* panfrost_core_dump(pfdev); */
f3ba9122 Rob Herring 2018-09-10 394
f3ba9122 Rob Herring 2018-09-10 395
panfrost_devfreq_record_transition(pfdev, js);
f3ba9122 Rob Herring 2018-09-10 396panfrost_gpu_soft_reset(pfdev);
f3ba9122 Rob Herring 2018-09-10 397
f3ba9122 Rob Herring 2018-09-10 398/* TODO: Re-enable all other address
spaces */
f3ba9122 Rob Herring 2018-09-10 399panfrost_mmu_enable(pfdev, 0);
f3ba9122 Rob Herring 2018-09-10 400panfrost_gpu_power_on(pfdev);
f3ba9122 Rob Herring 2018-09-10 401panfrost_job_enable_interrupts(pfdev);
f3ba9122 Rob Herring 2018-09-10 402
f3ba9122 Rob Herring 2018-09-10 403for (i = 0; i < NUM_JOB_SLOTS; i++)
f3ba9122 Rob Herring 2018-09-10 404
drm_sched_resubmit_jobs(&pfdev->js->queue[i].sched);
f3ba9122 Rob Herring 2018-09-10 405
f3ba9122 Rob Herring 2018-09-10 406/* restart scheduler after GPU is
usable again */
f3ba9122 Rob Herring 2018-09-10 407for (i = 0; i < NUM_JOB_SLOTS; i++)
f3ba9122 Rob Herring 2018-09-10 408
drm_sched_start(&pfdev->js->queue[i].sched, true);
f3ba9122 Rob Herring 2018-09-10 409 }
f3ba9122 Rob Herring 2018-09-10 410
:: The code at line 388 was first introduced by commit
:: f3ba91228e8e917e5bd6c4b72bfe846933d17370 drm/panfrost: Add initial
panfrost driver
:: TO: Rob Herring
:: CC: Rob Herring
---
0-DAY kernel test infrastructureOpen Source Technology Center
https://lists.01.org/pipermail/kbuild-all Intel Corporation
.config.gz
Description: application/gzip
___
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [PATCH 4/4] drm/amdgpu: stop removing BOs from the LRU during CS
Hi,
This patch series doesn't help with the OOM errors due to GDS. Reproducible
with:
AMD_DEBUG=testgdsmm glxgears & AMD_DEBUG=testgdsmm glxgears
Marek
On Fri, May 10, 2019 at 10:13 AM Christian König <
ckoenig.leichtzumer...@gmail.com> wrote:
> This avoids OOM situations when we have lots of threads
> submitting at the same time.
>
> Signed-off-by: Christian König
> ---
> drivers/gpu/drm/amd/amdgpu/amdgpu_cs.c | 2 +-
> 1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_cs.c
> b/drivers/gpu/drm/amd/amdgpu/amdgpu_cs.c
> index a1d6a0721e53..8828d30cd409 100644
> --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_cs.c
> +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_cs.c
> @@ -648,7 +648,7 @@ static int amdgpu_cs_parser_bos(struct
> amdgpu_cs_parser *p,
> }
>
> r = ttm_eu_reserve_buffers(&p->ticket, &p->validated, true,
> - &duplicates, true);
> + &duplicates, false);
> if (unlikely(r != 0)) {
> if (r != -ERESTARTSYS)
> DRM_ERROR("ttm_eu_reserve_buffers failed.\n");
> --
> 2.17.1
>
> ___
> 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
[Bug 110214] radeonsi: xterm scrollback buffer disappears while Shift+PgUp and Shift+PgDn
https://bugs.freedesktop.org/show_bug.cgi?id=110214 --- Comment #91 from komqin...@zoho.eu --- xfce4-terminal and Geany bugs seem to be this https://bugs.freedesktop.org/show_bug.cgi?id=110355 and solved with mesa 19.0.4. -- You are receiving this mail because: You are the assignee for the bug.___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
[Bug 110371] HP Dreamcolor display *Error* No EDID read
https://bugs.freedesktop.org/show_bug.cgi?id=110371 --- Comment #10 from babblebo...@gmail.com --- Best I can tell, and I may be wrong, the error checking code was moved from the DC part straight into DRM which now replicates the exact bug which was reverted by the above DC commits but were never implemented for DRM. https://www.st.com/en/touch-and-display-controllers/stdp8028.html My dreamcolor display uses a STDP8028 chip, istting inbetween the display and the motherboard just behind the screen, to convert a displayport signal coming off the board into a dual channel LVDS to run the display. https://www.st.com/en/touch-and-display-controllers/stdp8028.html The EDID can't be read through this for some reason and it doesn't print any modelines at all for the display so it picks the lowest resolution possible and all the timings are incorrect resulting in the display scramble. I hope the behavior highlighted in the above commit can help someone search for the regression in the new DRM mode setting as it produces the exact same type of scramble and lack of modelines. -- You are receiving this mail because: You are the assignee for the bug.___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
[Bug 203471] Tearing on Raven Ridge and RX560X PRIME setup even with Vsync enabled
https://bugzilla.kernel.org/show_bug.cgi?id=203471 --- Comment #7 from Haxk20 (haxk...@gmail.com) --- (In reply to Michel Dänzer from comment #3) > Please attach the corresponding Xorg log file and the output of glxinfo > (both with and without DRI_PRIME=1). Is there anything more you need to figure out whats casuing this bug ? I will try to play some game on HDMI output and see if its purely eDP issue or it applies to other connectors too. Will post tomorrow. -- 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 v2 00/17] kunit: introduce KUnit, the Linux kernel unit testing framework
On Fri, May 10, 2019 at 5:13 AM Knut Omang wrote: > On Fri, 2019-05-10 at 03:23 -0700, Brendan Higgins wrote: > > > On Fri, May 10, 2019 at 7:49 AM Knut Omang wrote: > > > > > > > > On Thu, 2019-05-09 at 22:18 -0700, Frank Rowand wrote: > > > > > On 5/9/19 4:40 PM, Logan Gunthorpe wrote: > > > > > > > > > > > > > > > > > > On 2019-05-09 5:30 p.m., Theodore Ts'o wrote: > > > > > >> On Thu, May 09, 2019 at 04:20:05PM -0600, Logan Gunthorpe wrote: > > > > > >>> > > > > > >>> The second item, arguably, does have significant overlap with > > > > > >>> kselftest. > > > > > >>> Whether you are running short tests in a light weight UML > > > > > >>> environment or > > > > > >>> higher level tests in an heavier VM the two could be using the > > > > > >>> same > > > > > >>> framework for writing or defining in-kernel tests. It *may* also > > > > > >>> be valuable > > > > > >>> for some people to be able to run all the UML tests in the heavy > > > > > >>> VM > > > > > >>> environment along side other higher level tests. > > > > > >>> > > > > > >>> Looking at the selftests tree in the repo, we already have > > > > > >>> similar items to > > > > > >>> what Kunit is adding as I described in point (2) above. > > > > > >>> kselftest_harness.h > > > > > >>> contains macros like EXPECT_* and ASSERT_* with very similar > > > > > >>> intentions to > > > > > >>> the new KUNIT_EXECPT_* and KUNIT_ASSERT_* macros. > > > > > >>> > > > > > >>> However, the number of users of this harness appears to be quite > > > > > >>> small. Most > > > > > >>> of the code in the selftests tree seems to be a random mismash of > > > > > >>> scripts > > > > > >>> and userspace code so it's not hard to see it as something > > > > > >>> completely > > > > > >>> different from the new Kunit: > > > > > >>> > > > > > >>> $ git grep --files-with-matches kselftest_harness.h * > > > > > >> > > > > > >> To the extent that we can unify how tests are written, I agree that > > > > > >> this would be a good thing. However, you should note that > > > > > >> kselftest_harness.h is currently assums that it will be included in > > > > > >> userspace programs. This is most obviously seen if you look > > > > > >> closely > > > > > >> at the functions defined in the header files which makes calls to > > > > > >> fork(), abort() and fprintf(). > > > > > > > > > > > > Ah, yes. I obviously did not dig deep enough. Using kunit for > > > > > > in-kernel tests and kselftest_harness for userspace tests seems like > > > > > > a sensible line to draw to me. Trying to unify kernel and userspace > > > > > > here sounds like it could be difficult so it's probably not worth > > > > > > forcing the issue unless someone wants to do some really fancy work > > > > > > to get it done. > > > > > > > > > > > > Based on some of the other commenters, I was under the impression > > > > > > that kselftests had in-kernel tests but I'm not sure where or if > > > > > > they > > > > > > exist. > > > > > > > > > > YES, kselftest has in-kernel tests. (Excuse the shouting...) > > > > > > > > > > Here is a likely list of them in the kernel source tree: > > > > > > > > > > $ grep module_init lib/test_*.c > > > > > lib/test_bitfield.c:module_init(test_bitfields) > > > > > lib/test_bitmap.c:module_init(test_bitmap_init); > > > > > lib/test_bpf.c:module_init(test_bpf_init); > > > > > lib/test_debug_virtual.c:module_init(test_debug_virtual_init); > > > > > lib/test_firmware.c:module_init(test_firmware_init); > > > > > lib/test_hash.c:module_init(test_hash_init); /* Does everything */ > > > > > lib/test_hexdump.c:module_init(test_hexdump_init); > > > > > lib/test_ida.c:module_init(ida_checks); > > > > > lib/test_kasan.c:module_init(kmalloc_tests_init); > > > > > lib/test_list_sort.c:module_init(list_sort_test); > > > > > lib/test_memcat_p.c:module_init(test_memcat_p_init); > > > > > lib/test_module.c:static int __init test_module_init(void) > > > > > lib/test_module.c:module_init(test_module_init); > > > > > lib/test_objagg.c:module_init(test_objagg_init); > > > > > lib/test_overflow.c:static int __init test_module_init(void) > > > > > lib/test_overflow.c:module_init(test_module_init); > > > > > lib/test_parman.c:module_init(test_parman_init); > > > > > lib/test_printf.c:module_init(test_printf_init); > > > > > lib/test_rhashtable.c:module_init(test_rht_init); > > > > > lib/test_siphash.c:module_init(siphash_test_init); > > > > > lib/test_sort.c:module_init(test_sort_init); > > > > > lib/test_stackinit.c:module_init(test_stackinit_init); > > > > > lib/test_static_key_base.c:module_init(test_static_key_base_init); > > > > > lib/test_static_keys.c:module_init(test_static_key_init); > > > > > lib/test_string.c:module_init(string_selftest_init); > > > > > lib/test_ubsan.c:module_init(test_ubsan_init); > > > > > lib/test_user_copy.c:module_init(test_user_copy_init); > > > > > lib/test_uuid.c:module_init(test_uuid_init); > > > > > lib/test_vmalloc.c:module_init(vmalloc_test_init) > > > > > li
Re: [PATCH 1/2] mm/hmm: support automatic NUMA balancing
On Fri, May 10, 2019 at 07:53:23PM +, Kuehling, Felix wrote:
> From: Philip Yang
>
> While the page is migrating by NUMA balancing, HMM failed to detect this
> condition and still return the old page. Application will use the new
> page migrated, but driver pass the old page physical address to GPU,
> this crash the application later.
>
> Use pte_protnone(pte) to return this condition and then hmm_vma_do_fault
> will allocate new page.
>
> Signed-off-by: Philip Yang
Reviewed-by: Jérôme Glisse
> ---
> mm/hmm.c | 2 +-
> 1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/mm/hmm.c b/mm/hmm.c
> index 75d2ea906efb..b65c27d5c119 100644
> --- a/mm/hmm.c
> +++ b/mm/hmm.c
> @@ -554,7 +554,7 @@ static int hmm_vma_handle_pmd(struct mm_walk *walk,
>
> static inline uint64_t pte_to_hmm_pfn_flags(struct hmm_range *range, pte_t
> pte)
> {
> - if (pte_none(pte) || !pte_present(pte))
> + if (pte_none(pte) || !pte_present(pte) || pte_protnone(pte))
> return 0;
> return pte_write(pte) ? range->flags[HMM_PFN_VALID] |
> range->flags[HMM_PFN_WRITE] :
> --
> 2.17.1
>
___
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel
[Bug 110661] RX480 idling at over 70°C
https://bugs.freedesktop.org/show_bug.cgi?id=110661 --- Comment #1 from u.ra...@gmail.com --- Created attachment 144222 --> https://bugs.freedesktop.org/attachment.cgi?id=144222&action=edit dmesg -- You are receiving this mail because: You are the assignee for the bug.___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
[Bug 202799] amd/dc: right monitor sometimes never comes back up on dual-display setup after locking session
https://bugzilla.kernel.org/show_bug.cgi?id=202799 --- Comment #9 from Clément Guérin (li...@protonmail.com) --- I'm still seeing this bug with linux 5.1. -- 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 110661] RX480 idling at over 70°C
https://bugs.freedesktop.org/show_bug.cgi?id=110661 Bug ID: 110661 Summary: RX480 idling at over 70°C Product: DRI Version: unspecified Hardware: x86-64 (AMD64) OS: Linux (All) Status: NEW Severity: normal Priority: medium Component: DRM/AMDgpu Assignee: dri-devel@lists.freedesktop.org Reporter: u.ra...@gmail.com Created attachment 144221 --> https://bugs.freedesktop.org/attachment.cgi?id=144221&action=edit PM info As per $SUBJECT, my RX480 displaying an idle KDE Plasma desktop is showing the temperature between 70 and 80°C. GPU-Z in Windows (also idle) usually shows below 40°C. Attaching dmesg, Xorg log, and the PM info from debugfs. -- You are receiving this mail because: You are the assignee for the bug.___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [PATCH 2/2] mm/hmm: Only set FAULT_FLAG_ALLOW_RETRY for non-blocking
On Fri, May 10, 2019 at 07:53:24PM +, Kuehling, Felix wrote:
> Don't set this flag by default in hmm_vma_do_fault. It is set
> conditionally just a few lines below. Setting it unconditionally
> can lead to handle_mm_fault doing a non-blocking fault, returning
> -EBUSY and unlocking mmap_sem unexpectedly.
>
> Signed-off-by: Felix Kuehling
Reviewed-by: Jérôme Glisse
> ---
> mm/hmm.c | 2 +-
> 1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/mm/hmm.c b/mm/hmm.c
> index b65c27d5c119..3c4f1d62202f 100644
> --- a/mm/hmm.c
> +++ b/mm/hmm.c
> @@ -339,7 +339,7 @@ struct hmm_vma_walk {
> static int hmm_vma_do_fault(struct mm_walk *walk, unsigned long addr,
> bool write_fault, uint64_t *pfn)
> {
> - unsigned int flags = FAULT_FLAG_ALLOW_RETRY | FAULT_FLAG_REMOTE;
> + unsigned int flags = FAULT_FLAG_REMOTE;
> struct hmm_vma_walk *hmm_vma_walk = walk->private;
> struct hmm_range *range = hmm_vma_walk->range;
> struct vm_area_struct *vma = walk->vma;
> --
> 2.17.1
>
___
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel
[Bug 110661] RX480 idling at over 70°C
https://bugs.freedesktop.org/show_bug.cgi?id=110661 --- Comment #2 from u.ra...@gmail.com --- Created attachment 144223 --> https://bugs.freedesktop.org/attachment.cgi?id=144223&action=edit Xorg log -- You are receiving this mail because: You are the assignee for the bug.___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
[PATCH 2/2] mm/hmm: Only set FAULT_FLAG_ALLOW_RETRY for non-blocking
Don't set this flag by default in hmm_vma_do_fault. It is set
conditionally just a few lines below. Setting it unconditionally
can lead to handle_mm_fault doing a non-blocking fault, returning
-EBUSY and unlocking mmap_sem unexpectedly.
Signed-off-by: Felix Kuehling
---
mm/hmm.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/mm/hmm.c b/mm/hmm.c
index b65c27d5c119..3c4f1d62202f 100644
--- a/mm/hmm.c
+++ b/mm/hmm.c
@@ -339,7 +339,7 @@ struct hmm_vma_walk {
static int hmm_vma_do_fault(struct mm_walk *walk, unsigned long addr,
bool write_fault, uint64_t *pfn)
{
- unsigned int flags = FAULT_FLAG_ALLOW_RETRY | FAULT_FLAG_REMOTE;
+ unsigned int flags = FAULT_FLAG_REMOTE;
struct hmm_vma_walk *hmm_vma_walk = walk->private;
struct hmm_range *range = hmm_vma_walk->range;
struct vm_area_struct *vma = walk->vma;
--
2.17.1
___
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel
[PATCH 0/2] Two bug-fixes for HMM
These problems were found in AMD-internal testing as we're working on adopting HMM. They are rebased against glisse/hmm-5.2-v3. We'd like to get them applied to a mainline Linux kernel as well as drm-next and amd-staging-drm-next sooner rather than later. Currently the HMM in amd-staging-drm-next is quite far behind hmm-5.2-v3, but the driver changes for HMM are expected to land in 5.2 and will need to be rebased on those HMM changes. I'd like to work out a flow between Jerome, Dave, Alex and myself that allows us to test the latest version of HMM on amd-staging-drm-next so that ideally everything comes together in master without much need for rebasing and retesting. Maybe having Jerome's latest HMM changes in drm-next. However, that may create dependencies where Jerome and Dave need to coordinate their pull- requests for master. Felix Kuehling (1): mm/hmm: Only set FAULT_FLAG_ALLOW_RETRY for non-blocking Philip Yang (1): mm/hmm: support automatic NUMA balancing mm/hmm.c | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) -- 2.17.1 ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
[PATCH 1/2] mm/hmm: support automatic NUMA balancing
From: Philip Yang
While the page is migrating by NUMA balancing, HMM failed to detect this
condition and still return the old page. Application will use the new
page migrated, but driver pass the old page physical address to GPU,
this crash the application later.
Use pte_protnone(pte) to return this condition and then hmm_vma_do_fault
will allocate new page.
Signed-off-by: Philip Yang
---
mm/hmm.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/mm/hmm.c b/mm/hmm.c
index 75d2ea906efb..b65c27d5c119 100644
--- a/mm/hmm.c
+++ b/mm/hmm.c
@@ -554,7 +554,7 @@ static int hmm_vma_handle_pmd(struct mm_walk *walk,
static inline uint64_t pte_to_hmm_pfn_flags(struct hmm_range *range, pte_t pte)
{
- if (pte_none(pte) || !pte_present(pte))
+ if (pte_none(pte) || !pte_present(pte) || pte_protnone(pte))
return 0;
return pte_write(pte) ? range->flags[HMM_PFN_VALID] |
range->flags[HMM_PFN_WRITE] :
--
2.17.1
___
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [RFC PATCH v2 4/5] drm, cgroup: Add total GEM buffer allocation limit
On Fri, May 10, 2019 at 1:48 PM Koenig, Christian wrote: > Well another question is why do we want to prevent that in the first place? > > I mean the worst thing that can happen is that we account a BO multiple > times. That's one of the problems. The other one is the BO outliving the lifetime of a cgroup and there's no good way to un-charge the usage when the BO is free so the count won't be accurate. I have looked into two possible solutions. One is to prevent cgroup from being removed when there are BOs owned by the cgroup still alive (similar to how cgroup removal will fail if it still has processes attached to it.) My concern here is the possibility of not able to remove a cgroup forever due to the lifetime of a BO (continuously being shared and reuse and never die.) Perhaps you can shed some light on this possibility. The other one is to keep track of all the buffers and migrate them to the parent if a cgroup is closed. My concern here is the performance overhead on tracking all the buffers. > And going into the same direction where is the code to handle an open > device file descriptor which is send from one cgroup to another? I looked into this before but I forgot what I found. Perhaps folks familiar with device cgroup can chime in. Actually, just did another quick search right now. Looks like the access is enforced at the inode level (__devcgroup_check_permission) so the fd sent to another cgroup that does not have access to the device should still not have access. Regards, Kenny > Regards, > Christian. > > > > > Regards, > > Kenny > > > >>> On the other hand, if there are expectations for resource management > >>> between containers, I would like to know who is the expected manager > >>> and how does it fit into the concept of container (which enforce some > >>> level of isolation.) One possible manager may be the display server. > >>> But as long as the display server is in a parent cgroup of the apps' > >>> cgroup, the apps can still import handles from the display server > >>> under the current implementation. My understanding is that this is > >>> most likely the case, with the display server simply sitting at the > >>> default/root cgroup. But I certainly want to hear more about other > >>> use cases (for example, is running multiple display servers on a > >>> single host a realistic possibility? Are there people running > >>> multiple display servers inside peer containers? If so, how do they > >>> coordinate resources?) > >> We definitely have situations with multiple display servers running > >> (just think of VR). > >> > >> I just can't say if they currently use cgroups in any way. > >> > >> Thanks, > >> Christian. > >> > >>> I should probably summarize some of these into the commit message. > >>> > >>> Regards, > >>> Kenny > >>> > >>> > >>> > Christian. > > ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [RFC PATCH v2 4/5] drm, cgroup: Add total GEM buffer allocation limit
Am 10.05.19 um 17:25 schrieb Kenny Ho: > [CAUTION: External Email] > > On Fri, May 10, 2019 at 11:08 AM Koenig, Christian > wrote: >> Am 10.05.19 um 16:57 schrieb Kenny Ho: >>> On Fri, May 10, 2019 at 8:28 AM Christian König >>> wrote: Am 09.05.19 um 23:04 schrieb Kenny Ho: >> So the drm cgroup container is separate to other cgroup containers? > In cgroup-v1, which is most widely deployed currently, all controllers > have their own hierarchy (see /sys/fs/cgroup/). In cgroup-v2, the > hierarchy is unified by individual controllers can be disabled (I > believe, I am not super familiar with v2.) > >> In other words as long as userspace doesn't change, this wouldn't have >> any effect? > As far as things like docker and podman is concern, yes. I am not > sure about the behaviour of others like lxc, lxd, etc. because I > haven't used those myself. > >> Well that is unexpected cause then a processes would be in different >> groups for different controllers, but if that's really the case that >> would certainly work. > I believe this is a possibility for v1 and is why folks came up with > the unified hierarchy in v2 to solve some of the issues. > https://www.kernel.org/doc/html/latest/admin-guide/cgroup-v2.html#issues-with-v1-and-rationales-for-v2 Well another question is why do we want to prevent that in the first place? I mean the worst thing that can happen is that we account a BO multiple times. And going into the same direction where is the code to handle an open device file descriptor which is send from one cgroup to another? Regards, Christian. > > Regards, > Kenny > >>> On the other hand, if there are expectations for resource management >>> between containers, I would like to know who is the expected manager >>> and how does it fit into the concept of container (which enforce some >>> level of isolation.) One possible manager may be the display server. >>> But as long as the display server is in a parent cgroup of the apps' >>> cgroup, the apps can still import handles from the display server >>> under the current implementation. My understanding is that this is >>> most likely the case, with the display server simply sitting at the >>> default/root cgroup. But I certainly want to hear more about other >>> use cases (for example, is running multiple display servers on a >>> single host a realistic possibility? Are there people running >>> multiple display servers inside peer containers? If so, how do they >>> coordinate resources?) >> We definitely have situations with multiple display servers running >> (just think of VR). >> >> I just can't say if they currently use cgroups in any way. >> >> Thanks, >> Christian. >> >>> I should probably summarize some of these into the commit message. >>> >>> Regards, >>> Kenny >>> >>> >>> Christian. ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [RFC PATCH v2 0/5] new cgroup controller for gpu/drm subsystem
Am 10.05.19 um 17:07 schrieb Kenny Ho: > [CAUTION: External Email] > > On Fri, May 10, 2019 at 8:31 AM Christian König > wrote: >> I think it is a good approach to try to add a global limit first and >> when that's working go ahead with limiting device specific resources. > What are some of the global drm resource limit/allocation that would > be useful to implement? I would be happy to dig into those. I was thinking about device specific stuff like VRAM etc... What I'm also not clear about is how this should interact with memcg. E.g. do we also need to account BOs in memcg? In theory I would say yes. Christian. > > Regards, > Kenny > > >> The only major issue I can see is on patch #4, see there for further >> details. >> >> Christian. >> >> Am 09.05.19 um 23:04 schrieb Kenny Ho: >>> This is a follow up to the RFC I made last november to introduce a cgroup >>> controller for the GPU/DRM subsystem [a]. The goal is to be able to >>> provide resource management to GPU resources using things like container. >>> The cover letter from v1 is copied below for reference. >>> >>> Usage examples: >>> // set limit for card1 to 1GB >>> sed -i '2s/.*/1073741824/' /sys/fs/cgroup//drm.buffer.total.max >>> >>> // set limit for card0 to 512MB >>> sed -i '1s/.*/536870912/' /sys/fs/cgroup//drm.buffer.total.max >>> >>> >>> v2: >>> * Removed the vendoring concepts >>> * Add limit to total buffer allocation >>> * Add limit to the maximum size of a buffer allocation >>> >>> TODO: process migration >>> TODO: documentations >>> >>> [a]: >>> https://lists.freedesktop.org/archives/dri-devel/2018-November/197106.html >>> >>> v1: cover letter >>> >>> The purpose of this patch series is to start a discussion for a generic >>> cgroup >>> controller for the drm subsystem. The design proposed here is a very early >>> one. >>> We are hoping to engage the community as we develop the idea. >>> >>> >>> Backgrounds >>> == >>> Control Groups/cgroup provide a mechanism for aggregating/partitioning sets >>> of >>> tasks, and all their future children, into hierarchical groups with >>> specialized >>> behaviour, such as accounting/limiting the resources which processes in a >>> cgroup >>> can access[1]. Weights, limits, protections, allocations are the main >>> resource >>> distribution models. Existing cgroup controllers includes cpu, memory, io, >>> rdma, and more. cgroup is one of the foundational technologies that >>> enables the >>> popular container application deployment and management method. >>> >>> Direct Rendering Manager/drm contains code intended to support the needs of >>> complex graphics devices. Graphics drivers in the kernel may make use of DRM >>> functions to make tasks like memory management, interrupt handling and DMA >>> easier, and provide a uniform interface to applications. The DRM has also >>> developed beyond traditional graphics applications to support compute/GPGPU >>> applications. >>> >>> >>> Motivations >>> = >>> As GPU grow beyond the realm of desktop/workstation graphics into areas like >>> data center clusters and IoT, there are increasing needs to monitor and >>> regulate >>> GPU as a resource like cpu, memory and io. >>> >>> Matt Roper from Intel began working on similar idea in early 2018 [2] for >>> the >>> purpose of managing GPU priority using the cgroup hierarchy. While that >>> particular use case may not warrant a standalone drm cgroup controller, >>> there >>> are other use cases where having one can be useful [3]. Monitoring GPU >>> resources such as VRAM and buffers, CU (compute unit [AMD's >>> nomenclature])/EU >>> (execution unit [Intel's nomenclature]), GPU job scheduling [4] can help >>> sysadmins get a better understanding of the applications usage profile. >>> Further >>> usage regulations of the aforementioned resources can also help sysadmins >>> optimize workload deployment on limited GPU resources. >>> >>> With the increased importance of machine learning, data science and other >>> cloud-based applications, GPUs are already in production use in data centers >>> today [5,6,7]. Existing GPU resource management is very course grain, >>> however, >>> as sysadmins are only able to distribute workload on a per-GPU basis [8]. >>> An >>> alternative is to use GPU virtualization (with or without SRIOV) but it >>> generally acts on the entire GPU instead of the specific resources in a GPU. >>> With a drm cgroup controller, we can enable alternate, fine-grain, sub-GPU >>> resource management (in addition to what may be available via GPU >>> virtualization.) >>> >>> In addition to production use, the DRM cgroup can also help with testing >>> graphics application robustness by providing a mean to artificially limit >>> DRM >>> resources availble to the applications. >>> >>> Challenges >>> >>> While there are common infrastructure in DRM that is shared across many >>> vendors >>> (the scheduler [4] for example), there are also aspects of DRM
Re: [GIT PULL] fbdev changes for v5.2
The pull request you sent on Fri, 10 May 2019 18:50:23 +0200: > https://github.com/bzolnier/linux.git tags/fbdev-v5.2 has been merged into torvalds/linux.git: https://git.kernel.org/torvalds/c/cccd559e98c05b669bdc37b01802f920cff1d6dd Thank you! -- Deet-doot-dot, I am a bot. https://korg.wiki.kernel.org/userdoc/prtracker ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
[GIT PULL] fbdev changes for v5.2
Hi Linus, Please pull fbdev changes for v5.2. They are: - 4 small fixes for fb core - updates for udlfb, sm712fb, macfb and atafb drivers - redundant code removals from amba-clcd and atmel_lcdfb drivers - minor fixes/cleanups for other fb drivers Please see the signed tag description for details. Best regards, -- Bartlomiej Zolnierkiewicz Samsung R&D Institute Poland Samsung Electronics The following changes since commit 79a3aaa7b82e3106be97842dedfd8429248896e6: Linux 5.1-rc3 (2019-03-31 14:39:29 -0700) are available in the git repository at: https://github.com/bzolnier/linux.git tags/fbdev-v5.2 for you to fetch changes up to d4a5611743a6f5d126f8cbfdbf29e12fd8d1b73f: video: fbdev: Use dev_get_drvdata() (2019-05-06 15:57:47 +0200) fbdev changes for v5.2: - fix regression in fbcon logo handling on 'quiet' boots (Andreas Schwab) - fix divide-by-zero error in fb_var_to_videomode() (Shile Zhang) - fix 'WARNING in __alloc_pages_nodemask' bug (Jiufei Xue) - list all PCI memory BARs as conflicting apertures (Gerd Hoffmann) - update udlfb driver - fix sleeping inside spinlock, add mutex around rendering calls and remove redundant code (Mikulas Patocka) - update sm712fb driver - fix SM720 support related issues (Yifeng Li) - update macfb driver - fix DAFB colour table pointer initialization and remove redundant code (Finn Thain) - update atafb driver - fix kexec support, use dev_*() calls instead of printk() and remove obsolete module support (Geert Uytterhoeven) - add support to mxsfb driver for skipping display initialization for flicker-free display takeover from bootloader (Melchior Franz) - remove Versatile and Nomadik board families support from amba-clcd driver as they are handled by DRM driver nowadays (Linus Walleij) - remove no longer needed AVR and platform_data support from atmel_lcdfb driver (Alexandre Belloni) - misc fixes (Colin Ian King, Julia Lawall, Gustavo A. R. Silva, Aditya Pakki, Kangjie Lu, YueHaibing) - misc cleanups (Enrico Weigelt, Kefeng Wang) Aditya Pakki (1): omapfb: Fix potential NULL pointer dereference in kmalloc Alexandre Belloni (1): video: fbdev: atmel_lcdfb: drop AVR and platform_data support Andreas Schwab (1): fbcon: Don't reset logo_shown when logo is currently shown Bartlomiej Zolnierkiewicz (1): Merge tag 'v5.1-rc3' of https://git.kernel.org/.../torvalds/linux into fbdev-for-next Colin Ian King (2): video: fbdev: vesafb: fix indentation issue video: fbdev: savage: fix indentation issue Enrico Weigelt, metux IT consult (1): drivers: video: fbdev: Kconfig: pedantic cleanups Finn Thain (3): video/macfb: Remove redundant code video/macfb: Call fb_invert_cmaps() video/macfb: Always initialize DAFB colour table pointer register Geert Uytterhoeven (4): fbdev: atafb: Stop printing virtual screen_base fbdev: atafb: Remove obsolete module support fbdev: atafb: Fix broken frame buffer after kexec fbdev: atafb: Modernize printing of kernel messages Gerd Hoffmann (1): fbdev: list all pci memory bars as conflicting apertures Gustavo A. R. Silva (1): xen, fbfront: mark expected switch fall-through Jiufei Xue (1): fbdev: fix WARNING in __alloc_pages_nodemask bug Julia Lawall (1): omapfb: add missing of_node_put after of_device_is_available Kangjie Lu (2): video: hgafb: fix potential NULL pointer dereference video: imsttfb: fix potential NULL pointer dereferences Kefeng Wang (1): video: fbdev: Use dev_get_drvdata() Linus Walleij (1): video: amba-clcd: Decomission Versatile and Nomadik Melchior Franz (1): fbdev: mxsfb: implement FB_PRE_INIT_FB option Mikulas Patocka (3): udlfb: delete the unused parameter for dlfb_handle_damage udlfb: fix sleeping inside spinlock udlfb: introduce a rendering mutex Shile Zhang (1): fbdev: fix divide error in fb_var_to_videomode Yifeng Li (9): fbdev: sm712fb: fix white screen of death on reboot, don't set CR3B-CR3F fbdev: sm712fb: fix brightness control on reboot, don't set SR30 fbdev: sm712fb: fix VRAM detection, don't set SR70/71/74/75 fbdev: sm712fb: fix boot screen glitch when sm712fb replaces VGA fbdev: sm712fb: fix crashes during framebuffer writes by correctly mapping VRAM fbdev: sm712fb: fix crashes and garbled display during DPMS modesetting fbdev: sm712fb: fix support for 1024x768-16 mode fbdev: sm712fb: use 1024x768 by default on non-MIPS, fix garbled display fbdev: sm712fb: fix memory frequency by avoiding a switch/case fallthrough YueHaibing (3): video: fbdev: pvr2fb: remove set but not used variable 'size' video: fbdev: mxsfb: remove set but not used variable 'line_count' video: fbdev: atmel_lcdfb: remove set but not used var
Re: [PATCH] drm/stm: ltdc: remove clk_round_rate comment
Dear Yannick,
Thank you for your patch,
In your patch, you have removed clk_disable() & clk_enable().
Could you please double confirm ?
thanks
Philippe :-)
On 5/10/19 5:03 PM, Yannick Fertré wrote:
> Clk_round_rate returns rounded clock without changing
> the hardware in any way.
> This function couldn't replace set_rate/get_rate calls.
> Todo comment has been removed & a new log inserted.
>
> Signed-off-by: Yannick Fertré
> ---
> drivers/gpu/drm/stm/ltdc.c | 10 +++---
> 1 file changed, 3 insertions(+), 7 deletions(-)
>
> diff --git a/drivers/gpu/drm/stm/ltdc.c b/drivers/gpu/drm/stm/ltdc.c
> index 97912e2..2f8aa2e 100644
> --- a/drivers/gpu/drm/stm/ltdc.c
> +++ b/drivers/gpu/drm/stm/ltdc.c
> @@ -507,20 +507,16 @@ static bool ltdc_crtc_mode_fixup(struct drm_crtc *crtc,
> struct ltdc_device *ldev = crtc_to_ltdc(crtc);
> int rate = mode->clock * 1000;
>
> - /*
> - * TODO clk_round_rate() does not work yet. When ready, it can
> - * be used instead of clk_set_rate() then clk_get_rate().
> - */
> -
> - clk_disable(ldev->pixel_clk);
> if (clk_set_rate(ldev->pixel_clk, rate) < 0) {
> DRM_ERROR("Cannot set rate (%dHz) for pixel clk\n", rate);
> return false;
> }
> - clk_enable(ldev->pixel_clk);
>
> adjusted_mode->clock = clk_get_rate(ldev->pixel_clk) / 1000;
>
> + DRM_DEBUG_DRIVER("requested clock %dkHz, adjusted clock %dkHz\n",
> + mode->clock, adjusted_mode->clock);
> +
> return true;
> }
>
>
___
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [PATCH] drm/stm: dsi: check hardware version
Dear Yannick,
Thank you for your patch,
Acked-by: Philippe Cornu
Dear Benjamin,
If you are fine with this patch, please push it *after* the patch named
"drm/stm: dsi: add support of an optional regulator" (if I well
understood those two patches)
Thank you
Philippe :-)
On 5/10/19 5:02 PM, Yannick Fertré wrote:
> Check version of DSI hardware IP. Only versions 1.30 & 1.31
> are supported.
>
> Signed-off-by: Yannick Fertré
> ---
> drivers/gpu/drm/stm/dw_mipi_dsi-stm.c | 24 +++-
> 1 file changed, 23 insertions(+), 1 deletion(-)
>
> diff --git a/drivers/gpu/drm/stm/dw_mipi_dsi-stm.c
> b/drivers/gpu/drm/stm/dw_mipi_dsi-stm.c
> index 22bd095..29105e9 100644
> --- a/drivers/gpu/drm/stm/dw_mipi_dsi-stm.c
> +++ b/drivers/gpu/drm/stm/dw_mipi_dsi-stm.c
> @@ -227,7 +227,6 @@ dw_mipi_dsi_get_lane_mbps(void *priv_data, const struct
> drm_display_mode *mode,
> u32 val;
>
> /* Update lane capabilities according to hw version */
> - dsi->hw_version = dsi_read(dsi, DSI_VERSION) & VERSION;
> dsi->lane_min_kbps = LANE_MIN_KBPS;
> dsi->lane_max_kbps = LANE_MAX_KBPS;
> if (dsi->hw_version == HWVER_131) {
> @@ -306,6 +305,7 @@ static int dw_mipi_dsi_stm_probe(struct platform_device
> *pdev)
> {
> struct device *dev = &pdev->dev;
> struct dw_mipi_dsi_stm *dsi;
> + struct clk *pclk;
> struct resource *res;
> int ret;
>
> @@ -347,6 +347,28 @@ static int dw_mipi_dsi_stm_probe(struct platform_device
> *pdev)
> goto err_clk_get;
> }
>
> + pclk = devm_clk_get(dev, "pclk");
> + if (IS_ERR(pclk)) {
> + ret = PTR_ERR(pclk);
> + DRM_ERROR("Unable to get peripheral clock: %d\n", ret);
> + goto err_dsi_probe;
> + }
> +
> + ret = clk_prepare_enable(pclk);
> + if (ret) {
> + DRM_ERROR("%s: Failed to enable peripheral clk\n", __func__);
> + goto err_dsi_probe;
> + }
> +
> + dsi->hw_version = dsi_read(dsi, DSI_VERSION) & VERSION;
> + clk_disable_unprepare(pclk);
> +
> + if (dsi->hw_version != HWVER_130 && dsi->hw_version != HWVER_131) {
> + ret = -ENODEV;
> + DRM_ERROR("bad dsi hardware version\n");
> + goto err_dsi_probe;
> + }
> +
> dw_mipi_dsi_stm_plat_data.base = dsi->base;
> dw_mipi_dsi_stm_plat_data.priv_data = dsi;
>
>
___
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [PATCH v2 3/5] ARM: dts: stm32: add phy-dsi-supply property on stm32mp157c
Dear Yannick, Thank you for your patch, Reviewed-by: Philippe Cornu Philippe :-) On 5/10/19 4:20 PM, Yannick Fertré wrote: > The dsi physical layer is powered by the 1v8 power controller supply. > > Signed-off-by: Yannick Fertré > --- > arch/arm/boot/dts/stm32mp157c.dtsi | 1 + > 1 file changed, 1 insertion(+) > > diff --git a/arch/arm/boot/dts/stm32mp157c.dtsi > b/arch/arm/boot/dts/stm32mp157c.dtsi > index 2afeee6..6b14f1e 100644 > --- a/arch/arm/boot/dts/stm32mp157c.dtsi > +++ b/arch/arm/boot/dts/stm32mp157c.dtsi > @@ -1156,6 +1156,7 @@ > clock-names = "pclk", "ref", "px_clk"; > resets = <&rcc DSI_R>; > reset-names = "apb"; > + phy-dsi-supply = <®18>; > status = "disabled"; > }; > > ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [PATCH v2 2/3] drm/stm: dsi: add support of an optional regulator
Dear Yannick,
Thank you for your patch,
I like better the new shorter commit heading, thank you.
On 5/10/19 4:20 PM, Yannick Fertré wrote:
> Add support of an optional regulator for the phy part of the DSI
> controller.
>
> Signed-off-by: Yannick Fertré
> ---
> drivers/gpu/drm/stm/dw_mipi_dsi-stm.c | 45
> ++-
> 1 file changed, 39 insertions(+), 6 deletions(-)
>
> diff --git a/drivers/gpu/drm/stm/dw_mipi_dsi-stm.c
> b/drivers/gpu/drm/stm/dw_mipi_dsi-stm.c
> index 1bef73e..22bd095 100644
> --- a/drivers/gpu/drm/stm/dw_mipi_dsi-stm.c
> +++ b/drivers/gpu/drm/stm/dw_mipi_dsi-stm.c
> @@ -9,6 +9,7 @@
> #include
> #include
> #include
> +#include
> #include
> #include
> #include
> @@ -76,6 +77,7 @@ struct dw_mipi_dsi_stm {
> u32 hw_version;
> int lane_min_kbps;
> int lane_max_kbps;
> + struct regulator *vdd_supply;
> };
>
> static inline void dsi_write(struct dw_mipi_dsi_stm *dsi, u32 reg, u32 val)
> @@ -318,17 +320,31 @@ static int dw_mipi_dsi_stm_probe(struct platform_device
> *pdev)
> return PTR_ERR(dsi->base);
> }
>
> + dsi->vdd_supply = devm_regulator_get_optional(dev, "phy-dsi");
> + if (IS_ERR(dsi->vdd_supply)) {
> + ret = PTR_ERR(dsi->vdd_supply);
> + if (ret != -EPROBE_DEFER)
> + DRM_ERROR("failed to request regulator: %d\n", ret);
> + return ret;
> + }
> +
> + ret = regulator_enable(dsi->vdd_supply);
> + if (ret) {
> + DRM_ERROR("failed to enable regulator: %d\n", ret);
> + return ret;
> + }
> +
> dsi->pllref_clk = devm_clk_get(dev, "ref");
> if (IS_ERR(dsi->pllref_clk)) {
> ret = PTR_ERR(dsi->pllref_clk);
> - dev_err(dev, "Unable to get pll reference clock: %d\n", ret);
> - return ret;
> + DRM_ERROR("Unable to get pll reference clock: %d\n", ret);
> + goto err_clk_get;
> }
>
> ret = clk_prepare_enable(dsi->pllref_clk);
> if (ret) {
> - dev_err(dev, "%s: Failed to enable pllref_clk\n", __func__);
> - return ret;
> + DRM_ERROR("%s: Failed to enable pllref_clk\n", __func__);
> + goto err_clk_get;
> }
>
> dw_mipi_dsi_stm_plat_data.base = dsi->base;
> @@ -339,11 +355,19 @@ static int dw_mipi_dsi_stm_probe(struct platform_device
> *pdev)
> dsi->dsi = dw_mipi_dsi_probe(pdev, &dw_mipi_dsi_stm_plat_data);
> if (IS_ERR(dsi->dsi)) {
> DRM_ERROR("Failed to initialize mipi dsi host\n");
> - clk_disable_unprepare(dsi->pllref_clk);
> - return PTR_ERR(dsi->dsi);
> + ret = PTR_ERR(dsi->dsi);
> + goto err_dsi_probe;
> }
>
> return 0;
> +
> +err_dsi_probe:
> + clk_disable_unprepare(dsi->pllref_clk);
> +err_clk_get:
> + regulator_disable(dsi->vdd_supply);
> +
> + return ret;
> +
> }
>
> static int dw_mipi_dsi_stm_remove(struct platform_device *pdev)
> @@ -351,6 +375,7 @@ static int dw_mipi_dsi_stm_remove(struct platform_device
> *pdev)
> struct dw_mipi_dsi_stm *dsi = platform_get_drvdata(pdev);
>
> clk_disable_unprepare(dsi->pllref_clk);
> + regulator_disable(dsi->vdd_supply);
> dw_mipi_dsi_remove(dsi->dsi);
for a future patch: we may have a different order
dw_mipi_dsi_remove(dsi->dsi);
clk_disable_unprepare(dsi->pllref_clk);
regulator_disable(dsi->vdd_supply);
>
> return 0;
> @@ -363,6 +388,7 @@ static int __maybe_unused dw_mipi_dsi_stm_suspend(struct
> device *dev)
> DRM_DEBUG_DRIVER("\n");
>
> clk_disable_unprepare(dsi->pllref_clk);
> + regulator_disable(dsi->vdd_supply);
>
> return 0;
> }
> @@ -370,9 +396,16 @@ static int __maybe_unused dw_mipi_dsi_stm_suspend(struct
> device *dev)
> static int __maybe_unudw_mipi_dsi_remove(dsi->dsi);sed
> dw_mipi_dsi_stm_resume(struct device *dev)
> {
> struct dw_mipi_dsi_stm *dsi = dw_mipi_dsi_stm_plat_data.priv_data;
> + int ret;
>
> DRM_DEBUG_DRIVER("\n");
>
> + ret = regulator_enable(dsi->vdd_supply);
> + if (ret) {
> + DRM_ERROR("failed to enable regulator: %d\n", ret);
> + return ret;
> + }
> +
> clk_prepare_enable(dsi->pllref_clk);
for a future patch: we may check clk_prepare_enable return value.
>
> return 0;
>
Acked-by: Philippe Cornu
Philippe :)
___
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [PATCH v2 4/6] drm/fourcc: Pass the format_info pointer to drm_format_plane_cpp
On Fri, May 10, 2019 at 01:08:49PM +0200, Maxime Ripard wrote: > So far, the drm_format_plane_cpp function was operating on the format's > fourcc and was doing a lookup to retrieve the drm_format_info structure and > return the cpp. > > However, this is inefficient since in most cases, we will have the > drm_format_info pointer already available so we shouldn't have to perform a > new lookup. Some drm_fourcc functions also already operate on the > drm_format_info pointer for that reason, so the API is quite inconsistent > there. > > Let's follow the latter pattern and remove the extra lookup while being a > bit more consistent. In order to be extra consistent, also rename that > function to drm_format_info_plane_cpp and to a static function in the > header to match the current policy. Is there any point keeping the function at all? It's just info->cpp[i] no? -- Ville Syrjälä Intel ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [PATCH v2 1/5] dt-bindings: display: stm32: add supply property to DSI controller
Dear Yannick,
Thank you for your patch,
(already ;-)
Reviewed-by: Philippe Cornu
Philippe :)
On 5/10/19 4:20 PM, Yannick Fertré wrote:
> This patch adds documentation of a new property phy-dsi-supply to the
> STM32 DSI controller.
>
> Signed-off-by: Yannick Fertré
> ---
> Documentation/devicetree/bindings/display/st,stm32-ltdc.txt | 3 +++
> 1 file changed, 3 insertions(+)
>
> diff --git a/Documentation/devicetree/bindings/display/st,stm32-ltdc.txt
> b/Documentation/devicetree/bindings/display/st,stm32-ltdc.txt
> index 3eb1b48..60c54da 100644
> --- a/Documentation/devicetree/bindings/display/st,stm32-ltdc.txt
> +++ b/Documentation/devicetree/bindings/display/st,stm32-ltdc.txt
> @@ -40,6 +40,8 @@ Mandatory nodes specific to STM32 DSI:
> - panel or bridge node: A node containing the panel or bridge description as
> documented in [6].
> - port: panel or bridge port node, connected to the DSI output port
> (port@1).
> +Optional properties:
> +- phy-dsi-supply: phandle of the regulator that provides the supply voltage.
>
> Note: You can find more documentation in the following references
> [1] Documentation/devicetree/bindings/clock/clock-bindings.txt
> @@ -101,6 +103,7 @@ Example 2: DSI panel
> clock-names = "pclk", "ref";
> resets = <&rcc STM32F4_APB2_RESET(DSI)>;
> reset-names = "apb";
> + phy-dsi-supply = <®18>;
>
> ports {
> #address-cells = <1>;
>
___
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [PATCH] Revert "vgaarb: Keep adding VGA device in queue"
On 2019-05-10 5:42 p.m., Daniel Vetter wrote: > On Fri, May 10, 2019 at 09:29:58AM -0500, Alex Deucher wrote: >> This breaks multiple graphics cards in the Amigaone x5000 >> on PPC. >> >> This reverts commit 3d42f1ddc47a69c0ce155f9f30d764c4d689a5fa. >> >> Bug: https://bugs.freedesktop.org/show_bug.cgi?id=109345 >> Signed-off-by: Alex Deucher >> CC: Aaron Ma > > Given that the bug is a bit a mess I think we need to add a bit more > context here in the commit message. My understanding: > > Goal of the revert commit was to make the integrated boot device the > primary one, if we can't detect which one is the boot device, instead of > the last one. Which makes some sense. > > Now people have relied on the kernel picking the last one, which usually > is an add-on card, and therefore simply plugging in an add-on card allows > them to overwrite the default choice. Which also makes sense, and since > it's the older behaviour, wins. > > I think it'd be good to add a comment here that this behaviour has become > uapi, e.g. > > /* Add at the front so that we pick the last device as fallback >* default, with the usual result that plug in cards are preferred >* over integrated graphics. */ > > With that (or similar) and more commit message context: The bug reporter's system doesn't have integrated graphics though, just two plug-in cards. It's not clear to me yet that their expectation of Xorg to pick any particular one of them without configuration was justified. -- Earthling Michel Dänzer | https://www.amd.com Libre software enthusiast | Mesa and X developer ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [PATCH v8 4/4] drm/vc4: Allocate binner bo when starting to use the V3D
Hi,
On Thu, 2019-05-09 at 11:39 -0700, Eric Anholt wrote:
> Paul Kocialkowski writes:
>
> > The binner BO is not required until the V3D is in use, so avoid
> > allocating it at probe and do it on the first non-dumb BO allocation.
> >
> > Keep track of which clients are using the V3D and liberate the buffer
> > when there is none left, using a kref. Protect the logic with a
> > mutex to avoid race conditions.
> >
> > The binner BO is created at the time of the first render ioctl and is
> > destroyed when there is no client and no exec job using it left.
> >
> > The Out-Of-Memory (OOM) interrupt also gets some tweaking, to avoid
> > enabling it before having allocated a binner bo.
> >
> > We also want to keep the BO alive during runtime suspend/resume to avoid
> > failing to allocate it at resume. This happens when the CMA pool is
> > full at that point and results in a hard crash.
> >
> > Signed-off-by: Paul Kocialkowski
> > @@ -313,6 +321,49 @@ static int bin_bo_alloc(struct vc4_dev *vc4)
> > return ret;
> > }
> >
> > +int vc4_v3d_bin_bo_get(struct vc4_dev *vc4, bool *used)
> > +{
> > + int ret = 0;
> > +
> > + mutex_lock(&vc4->bin_bo_lock);
> > +
> > + if (used && *used)
> > + goto complete;
>
>
> > +
> > + if (used)
> > + *used = true;
> > +
> > + if (vc4->bin_bo) {
> > + kref_get(&vc4->bin_bo_kref);
> > + goto complete;
> > + }
> > +
> > + ret = bin_bo_alloc(vc4);
>
> I think this block wants to be:
>
> if (vc4->bin_bo)
> kref_get(&vc4->bin_bo_kref);
> else
> ret = bin_bo_alloc(vc4);
>
> if (ret == 0 && used)
> *used = true;
>
> (so we don't flag used if bin_bo_alloc fails)
>
> If you agree, then the series is:
I totally agree, and had definitely lost sight of the return value.
Thanks for the thorough code review once more!
Cheers,
Paul
> Reviewed-by: Eric Anholt
>
> > +
> > +complete:
> > + mutex_unlock(&vc4->bin_bo_lock);
> > +
> > + return ret;
> > +}
--
Paul Kocialkowski, Bootlin
Embedded Linux and kernel engineering
https://bootlin.com
___
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [PATCH] Revert "vgaarb: Keep adding VGA device in queue"
On 2019-05-10 4:29 p.m., Alex Deucher wrote: > This breaks multiple graphics cards in the Amigaone x5000 > on PPC. > > This reverts commit 3d42f1ddc47a69c0ce155f9f30d764c4d689a5fa. > > Bug: https://bugs.freedesktop.org/show_bug.cgi?id=109345 It's not clear to me yet that this was really a regression that requires a revert. It looks like the bug reporter was relying on Xorg with no xorg.conf picking one particular graphics card out of two in the system, and this change resulted in it using the other one instead. I asked for the corresponding Xorg log files to see if they show why Xorg picks one card or the other in each case. -- Earthling Michel Dänzer | https://www.amd.com Libre software enthusiast | Mesa and X developer ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
[Bug 109345] drm-next-2018-12-14 -Linux PPC
https://bugs.freedesktop.org/show_bug.cgi?id=109345 --- Comment #42 from Michel Dänzer --- Allan, please attach the /etc/X11/xorg.conf(.bak) file and the Xorg log files from the good and bad cases. -- You are receiving this mail because: You are the assignee for the bug.___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [PATCH] Revert "vgaarb: Keep adding VGA device in queue"
On Fri, May 10, 2019 at 09:29:58AM -0500, Alex Deucher wrote: > This breaks multiple graphics cards in the Amigaone x5000 > on PPC. > > This reverts commit 3d42f1ddc47a69c0ce155f9f30d764c4d689a5fa. > > Bug: https://bugs.freedesktop.org/show_bug.cgi?id=109345 > Signed-off-by: Alex Deucher > CC: Aaron Ma Given that the bug is a bit a mess I think we need to add a bit more context here in the commit message. My understanding: Goal of the revert commit was to make the integrated boot device the primary one, if we can't detect which one is the boot device, instead of the last one. Which makes some sense. Now people have relied on the kernel picking the last one, which usually is an add-on card, and therefore simply plugging in an add-on card allows them to overwrite the default choice. Which also makes sense, and since it's the older behaviour, wins. I think it'd be good to add a comment here that this behaviour has become uapi, e.g. /* Add at the front so that we pick the last device as fallback * default, with the usual result that plug in cards are preferred * over integrated graphics. */ With that (or similar) and more commit message context: Reviewed-by: Daniel Vetter > --- > drivers/gpu/vga/vgaarb.c | 2 +- > 1 file changed, 1 insertion(+), 1 deletion(-) > > diff --git a/drivers/gpu/vga/vgaarb.c b/drivers/gpu/vga/vgaarb.c > index f2f3ef8af271..8a3c45219a2a 100644 > --- a/drivers/gpu/vga/vgaarb.c > +++ b/drivers/gpu/vga/vgaarb.c > @@ -725,7 +725,7 @@ static bool vga_arbiter_add_pci_device(struct pci_dev > *pdev) > vga_arbiter_check_bridge_sharing(vgadev); > > /* Add to the list */ > - list_add_tail(&vgadev->list, &vga_list); > + list_add(&vgadev->list, &vga_list); > vga_count++; > vgaarb_info(&pdev->dev, "VGA device added: > decodes=%s,owns=%s,locks=%s\n", > vga_iostate_to_str(vgadev->decodes), > -- > 2.20.1 > > ___ > dri-devel mailing list > dri-devel@lists.freedesktop.org > https://lists.freedesktop.org/mailman/listinfo/dri-devel -- Daniel Vetter Software Engineer, Intel Corporation http://blog.ffwll.ch ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [RFC PATCH v2 4/5] drm, cgroup: Add total GEM buffer allocation limit
On Fri, May 10, 2019 at 11:08 AM Koenig, Christian wrote: > Am 10.05.19 um 16:57 schrieb Kenny Ho: > > On Fri, May 10, 2019 at 8:28 AM Christian König > > wrote: > >> Am 09.05.19 um 23:04 schrieb Kenny Ho: > So the drm cgroup container is separate to other cgroup containers? In cgroup-v1, which is most widely deployed currently, all controllers have their own hierarchy (see /sys/fs/cgroup/). In cgroup-v2, the hierarchy is unified by individual controllers can be disabled (I believe, I am not super familiar with v2.) > In other words as long as userspace doesn't change, this wouldn't have > any effect? As far as things like docker and podman is concern, yes. I am not sure about the behaviour of others like lxc, lxd, etc. because I haven't used those myself. > Well that is unexpected cause then a processes would be in different > groups for different controllers, but if that's really the case that > would certainly work. I believe this is a possibility for v1 and is why folks came up with the unified hierarchy in v2 to solve some of the issues. https://www.kernel.org/doc/html/latest/admin-guide/cgroup-v2.html#issues-with-v1-and-rationales-for-v2 Regards, Kenny > > On the other hand, if there are expectations for resource management > > between containers, I would like to know who is the expected manager > > and how does it fit into the concept of container (which enforce some > > level of isolation.) One possible manager may be the display server. > > But as long as the display server is in a parent cgroup of the apps' > > cgroup, the apps can still import handles from the display server > > under the current implementation. My understanding is that this is > > most likely the case, with the display server simply sitting at the > > default/root cgroup. But I certainly want to hear more about other > > use cases (for example, is running multiple display servers on a > > single host a realistic possibility? Are there people running > > multiple display servers inside peer containers? If so, how do they > > coordinate resources?) > > We definitely have situations with multiple display servers running > (just think of VR). > > I just can't say if they currently use cgroups in any way. > > Thanks, > Christian. > > > > > I should probably summarize some of these into the commit message. > > > > Regards, > > Kenny > > > > > > > >> Christian. > >> > ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [PATCH] kernel/locking/semaphore: use wake_q in up()
On Fri, May 10, 2019 at 11:28 AM Petr Mladek wrote:
>
> On Thu 2019-05-09 22:06:33, Daniel Vetter wrote:
> > console_trylock, called from within printk, can be called from pretty
> > much anywhere. Including try_to_wake_up. Note that this isn't common,
> > usually the box is in pretty bad shape at that point already. But it
> > really doesn't help when then lockdep jumps in and spams the logs,
> > potentially obscuring the real backtrace we're really interested in.
> > One case I've seen (slightly simplified backtrace):
> >
> > Fix this specific locking recursion by moving the wake_up_process out
> > from under the semaphore.lock spinlock, using wake_q as recommended by
> > Peter Zijlstra.
>
> It might make sense to mention also the optimization effect mentioned
> by Peter.
>
> > diff --git a/kernel/locking/semaphore.c b/kernel/locking/semaphore.c
> > index 561acdd39960..7a6f33715688 100644
> > --- a/kernel/locking/semaphore.c
> > +++ b/kernel/locking/semaphore.c
> > @@ -169,6 +169,14 @@ int down_timeout(struct semaphore *sem, long timeout)
> > }
> > EXPORT_SYMBOL(down_timeout);
> >
> > +/* Functions for the contended case */
> > +
> > +struct semaphore_waiter {
> > + struct list_head list;
> > + struct task_struct *task;
> > + bool up;
> > +};
> > +
> > /**
> > * up - release the semaphore
> > * @sem: the semaphore to release
> > @@ -179,24 +187,25 @@ EXPORT_SYMBOL(down_timeout);
> > void up(struct semaphore *sem)
> > {
> > unsigned long flags;
> > + struct semaphore_waiter *waiter;
> > + DEFINE_WAKE_Q(wake_q);
>
> We need to call wake_q_init(&wake_q) to make sure that
> it is empty.
DEFINE_WAKE_Q does that already, and if it didn't, I'd wonder how I
managed to boot with this patch. console_lock is usally terribly
contented because thanks to fbcon we must do a full display modeset
while holding it, which takes forever. As long as anyone printks
meanwhile (guaranteed while loading drivers really) you have
contention.
-Daniel
> Best Regards,
> Petr
>
> > raw_spin_lock_irqsave(&sem->lock, flags);
> > - if (likely(list_empty(&sem->wait_list)))
> > + if (likely(list_empty(&sem->wait_list))) {
> > sem->count++;
> > - else
> > - __up(sem);
> > + } else {
> > + waiter = list_first_entry(&sem->wait_list,
> > +struct semaphore_waiter, list);
> > + list_del(&waiter->list);
> > + waiter->up = true;
> > + wake_q_add(&wake_q, waiter->task);
> > + }
> > raw_spin_unlock_irqrestore(&sem->lock, flags);
> > +
> > + wake_up_q(&wake_q);
> > }
> > EXPORT_SYMBOL(up);
> >
> > -/* Functions for the contended case */
> > -
> > -struct semaphore_waiter {
> > - struct list_head list;
> > - struct task_struct *task;
> > - bool up;
> > -};
> > -
> > /*
> > * Because this function is inlined, the 'state' parameter will be
> > * constant, and thus optimised away by the compiler. Likewise the
--
Daniel Vetter
Software Engineer, Intel Corporation
+41 (0) 79 365 57 48 - http://blog.ffwll.ch
___
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [RFC PATCH v2 0/5] new cgroup controller for gpu/drm subsystem
On Fri, May 10, 2019 at 8:31 AM Christian König wrote: > > I think it is a good approach to try to add a global limit first and > when that's working go ahead with limiting device specific resources. What are some of the global drm resource limit/allocation that would be useful to implement? I would be happy to dig into those. Regards, Kenny > The only major issue I can see is on patch #4, see there for further > details. > > Christian. > > Am 09.05.19 um 23:04 schrieb Kenny Ho: > > This is a follow up to the RFC I made last november to introduce a cgroup > > controller for the GPU/DRM subsystem [a]. The goal is to be able to > > provide resource management to GPU resources using things like container. > > The cover letter from v1 is copied below for reference. > > > > Usage examples: > > // set limit for card1 to 1GB > > sed -i '2s/.*/1073741824/' /sys/fs/cgroup//drm.buffer.total.max > > > > // set limit for card0 to 512MB > > sed -i '1s/.*/536870912/' /sys/fs/cgroup//drm.buffer.total.max > > > > > > v2: > > * Removed the vendoring concepts > > * Add limit to total buffer allocation > > * Add limit to the maximum size of a buffer allocation > > > > TODO: process migration > > TODO: documentations > > > > [a]: > > https://lists.freedesktop.org/archives/dri-devel/2018-November/197106.html > > > > v1: cover letter > > > > The purpose of this patch series is to start a discussion for a generic > > cgroup > > controller for the drm subsystem. The design proposed here is a very early > > one. > > We are hoping to engage the community as we develop the idea. > > > > > > Backgrounds > > == > > Control Groups/cgroup provide a mechanism for aggregating/partitioning sets > > of > > tasks, and all their future children, into hierarchical groups with > > specialized > > behaviour, such as accounting/limiting the resources which processes in a > > cgroup > > can access[1]. Weights, limits, protections, allocations are the main > > resource > > distribution models. Existing cgroup controllers includes cpu, memory, io, > > rdma, and more. cgroup is one of the foundational technologies that > > enables the > > popular container application deployment and management method. > > > > Direct Rendering Manager/drm contains code intended to support the needs of > > complex graphics devices. Graphics drivers in the kernel may make use of DRM > > functions to make tasks like memory management, interrupt handling and DMA > > easier, and provide a uniform interface to applications. The DRM has also > > developed beyond traditional graphics applications to support compute/GPGPU > > applications. > > > > > > Motivations > > = > > As GPU grow beyond the realm of desktop/workstation graphics into areas like > > data center clusters and IoT, there are increasing needs to monitor and > > regulate > > GPU as a resource like cpu, memory and io. > > > > Matt Roper from Intel began working on similar idea in early 2018 [2] for > > the > > purpose of managing GPU priority using the cgroup hierarchy. While that > > particular use case may not warrant a standalone drm cgroup controller, > > there > > are other use cases where having one can be useful [3]. Monitoring GPU > > resources such as VRAM and buffers, CU (compute unit [AMD's > > nomenclature])/EU > > (execution unit [Intel's nomenclature]), GPU job scheduling [4] can help > > sysadmins get a better understanding of the applications usage profile. > > Further > > usage regulations of the aforementioned resources can also help sysadmins > > optimize workload deployment on limited GPU resources. > > > > With the increased importance of machine learning, data science and other > > cloud-based applications, GPUs are already in production use in data centers > > today [5,6,7]. Existing GPU resource management is very course grain, > > however, > > as sysadmins are only able to distribute workload on a per-GPU basis [8]. > > An > > alternative is to use GPU virtualization (with or without SRIOV) but it > > generally acts on the entire GPU instead of the specific resources in a GPU. > > With a drm cgroup controller, we can enable alternate, fine-grain, sub-GPU > > resource management (in addition to what may be available via GPU > > virtualization.) > > > > In addition to production use, the DRM cgroup can also help with testing > > graphics application robustness by providing a mean to artificially limit > > DRM > > resources availble to the applications. > > > > Challenges > > > > While there are common infrastructure in DRM that is shared across many > > vendors > > (the scheduler [4] for example), there are also aspects of DRM that are > > vendor > > specific. To accommodate this, we borrowed the mechanism used by the > > cgroup to > > handle different kinds of cgroup controller. > > > > Resources for DRM are also often device (GPU) specific instead of system > > specific and a system may contain more than one GPU. For this, w
Re: [RFC PATCH v2 4/5] drm, cgroup: Add total GEM buffer allocation limit
Am 10.05.19 um 16:57 schrieb Kenny Ho:
> [CAUTION: External Email]
>
> On Fri, May 10, 2019 at 8:28 AM Christian König
> wrote:
>> Am 09.05.19 um 23:04 schrieb Kenny Ho:
>>> + /* only allow bo from the same cgroup or its ancestor to be imported
>>> */
>>> + if (drmcgrp != NULL &&
>>> + !drmcgrp_is_self_or_ancestor(drmcgrp, obj->drmcgrp)) {
>>> + ret = -EACCES;
>>> + goto out_unlock;
>>> + }
>>> +
>> This will most likely go up in flames.
>>
>> If I'm not completely mistaken we already use
>> drm_gem_prime_fd_to_handle() to exchange handles between different
>> cgroups in current container usages.
> This is something that I am interested in getting more details from
> the broader community because the details affect how likely this will
> go up in flames ;). Note that this check does not block sharing of
> handles from cgroup parent to children in the hierarchy, nor does it
> blocks sharing of handles within a cgroup.
>
> I am interested to find out, when existing apps share handles between
> containers, if there are any expectations on resource management.
> Since there are no drm cgroup for current container usage, I expect
> the answer to be no. In this case, the drm cgroup controller can be
> disabled on its own (in the context of cgroup-v2's unified hierarchy),
> or the process can remain at the root for the drm cgroup hierarchy (in
> the context of cgroup-v1.) If I understand the cgroup api correctly,
> that means all process would be part of the root cgroup as far as the
> drm controller is concerned and this block will not come into effect.
> I have verified that this is indeed the current default behaviour of a
> container runtime (runc, which is used by docker, podman and others.)
> The new drm cgroup controller is simply ignored and all processes
> remain at the root of the hierarchy (since there are no other
> cgroups.) I plan to make contributions to runc (so folks can actually
> use this features with docker/podman/k8s, etc.) once things stabilized
> on the kernel side.
So the drm cgroup container is separate to other cgroup containers?
In other words as long as userspace doesn't change, this wouldn't have
any effect?
Well that is unexpected cause then a processes would be in different
groups for different controllers, but if that's really the case that
would certainly work.
> On the other hand, if there are expectations for resource management
> between containers, I would like to know who is the expected manager
> and how does it fit into the concept of container (which enforce some
> level of isolation.) One possible manager may be the display server.
> But as long as the display server is in a parent cgroup of the apps'
> cgroup, the apps can still import handles from the display server
> under the current implementation. My understanding is that this is
> most likely the case, with the display server simply sitting at the
> default/root cgroup. But I certainly want to hear more about other
> use cases (for example, is running multiple display servers on a
> single host a realistic possibility? Are there people running
> multiple display servers inside peer containers? If so, how do they
> coordinate resources?)
We definitely have situations with multiple display servers running
(just think of VR).
I just can't say if they currently use cgroups in any way.
Thanks,
Christian.
>
> I should probably summarize some of these into the commit message.
>
> Regards,
> Kenny
>
>
>
>> Christian.
>>
___
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [PATCH v2 0/8] per-process address spaces for MMUv2
Hi Lucas, On Fri, May 03, 2019 at 01:10:26PM +0200, Guido Günther wrote: > Hi Lucas, > On Wed, Apr 17, 2019 at 03:50:15PM +0200, Lucas Stach wrote: > > > > Hi all, > > > > v1 cover letter: > > > > the following patches finally implement one of the longstanding TODO > > items in the etnaviv driver: per-process address spaces. They are only > > enabled for MMUv2, as switching the MMU context on MMUv1 would require > > a full stop of the FE, which I deemed too expensive to do on potentially > > every submitted commandstream. > > > > For now this only provides better isolation between GPU clients, but it > > is also a big step in the direction of supporting softpin. Softpin will > > later be used by GC7000 userspace drivers to deal with texture descriptors > > without the need to add even more relocation interfaces to the etnaviv > > UAPI. > > > > The changes are big and pretty disruptive, so I acknowledge that they > > aren't prime targets for a quick review, but I would appreciate a > > second pairs of eyes on them. > > > > Changes since v1: > > - fixed an issue where a debugsfs read could run into a kernel NULL > > ptr dereference due to no current MMU context > > - fixed an issue where the current MMU context could be destroyed > > due to the DRM client going away, while it is still in use by > > an active GPU job > > - more extensive testing on GC880, GC2000, GC3000 and GC7000 > > I gave this series a test on GC7000 and it looks good. I'll do some more > testing over the next week. I gave this some more testing and it works nicely. One thing i noticed though are occasional GPU hangs on compositor startup like: [ 58.929906] etnaviv-gpu 3800.gpu: MMU fault status 0x0001 [ 58.936021] etnaviv-gpu 3800.gpu: MMU 0 fault addr 0x00484b80 [ 59.972655] etnaviv-gpu 3800.gpu: recover hung GPU! but these only happen with GALLIUM_DDEBUG=always. I can't reproduce them when either - dropping this patch series - disabling GALLIUM_DDEBUG I see if I can figure out some more details. Cheers, -- Guido ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [PATCH] RFC: console: hack up console_lock more v3
On Fri, May 10, 2019 at 11:15 AM Petr Mladek wrote:
> On Thu 2019-05-09 18:43:12, Daniel Vetter wrote:
> > One thing to keep in mind is that the kernel is already dying, and
> > things will come crashing down later on
>
> This is important information. I havn't seen it mentioned earlier.
I thought I explained that, sorry if this wasn't clear.
> > (I've seen this only in dmesg
> > tails capture in pstore in our CI, i.e. the box died for good). I just
> > want to make sure that the useful information isn't overwritten by
> > more dmesg splats that happen as a consequence of us somehow trying to
> > run things on an offline cpu. Once console_unlock has completed in
> > your above backtrace and the important stuff has gone out I'm totally
> > fine with the kernel just dying. Pulling the wake_up_process out from
> > under the semaphore.lock is enough to prevent lockdep from dumping
> > more stuff while we're trying to print the important things,
>
> With the more stuff you mean the lockdep splat? If yes, it might
> make sense to call debug_locks_off() earlier in panic().
I'm not sure this only happens in panics (I'm honestly not sure at all
why we're complaining about offline cpus). But in general I've seen
piles of dying systems that dump WARNINGS and then eventually panic.
Still would be good to not have a pointless lockdep splat in between.
> > and I think the untangling of the locking hiararchy is useful irrespective
> > of this lockdep splat. Plus Peter doesn't sound like he likes to roll
> > out more printk_deferred kind of things.
> >
> > But if you think I should do the printk_deferred thing too I can look
> > into that. Just not quite sure what that's supposed to look like now.
>
> Your patch might remove the particular lockdep splat. It might be
> worth it (Peter mentioned also an optimization effect). Anyway
> it will not prevent the deadlock.
>
> The only way to avoid the deadlock is to use printk_deferred()
> with the current printk() code.
>
>
> Finally, I have recently worked on similar problem with dying system.
> I sent the following patch for testing. I wonder if it might be
> acceptable upstream:
>
>
> From: Petr Mladek
> Subject: sched/x86: Do not warn about offline CPUs when all are being stopped
> Patch-mainline: No, just for testing
> References: bsc#1104406
>
> The warning about rescheduling offline CPUs cause dealock when
> the CPUs need to get stopped using NMI. It might happen with
> logbuf_lock, locks used by console drivers, especially tty.
> But it might also be caused by a registered kernel message
> dumper, for example, pstore.
>
> The warning is pretty common when there is a high load and
> CPUs are being stopped by native_stop_other_cpus(). But
> they are not really useful in this context. And they scrolls
> the really important messages off the screen.
>
> We need to fix printk() in the long term. But disabling
> the message looks reasonable at least in the meantime.
>
> Signed-off-by: Petr Mladek
> ---
> arch/x86/kernel/smp.c |3 ++-
> 1 file changed, 2 insertions(+), 1 deletion(-)
>
> --- a/arch/x86/kernel/smp.c
> +++ b/arch/x86/kernel/smp.c
> @@ -124,7 +124,8 @@ static bool smp_no_nmi_ipi = false;
> */
> static void native_smp_send_reschedule(int cpu)
> {
> - if (unlikely(cpu_is_offline(cpu))) {
> + if (unlikely(cpu_is_offline(cpu) &&
> +atomic_read(&stopping_cpu) < 0)) {
Hm yeah this sounds like another fix which would stop things from
getting worse when the machine is dying. I can try and stuff this (or
something like this, I honestly have no idea about cpu handling code
so no idea what's reasonable) into our CI and see what happens.
Thanks, Daniel
> WARN(1, "sched: Unexpected reschedule of offline CPU#%d!\n",
> cpu);
> return;
> }
--
Daniel Vetter
Software Engineer, Intel Corporation
+41 (0) 79 365 57 48 - http://blog.ffwll.ch
___
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel
[PATCH] drm/stm: ltdc: remove clk_round_rate comment
Clk_round_rate returns rounded clock without changing
the hardware in any way.
This function couldn't replace set_rate/get_rate calls.
Todo comment has been removed & a new log inserted.
Signed-off-by: Yannick Fertré
---
drivers/gpu/drm/stm/ltdc.c | 10 +++---
1 file changed, 3 insertions(+), 7 deletions(-)
diff --git a/drivers/gpu/drm/stm/ltdc.c b/drivers/gpu/drm/stm/ltdc.c
index 97912e2..2f8aa2e 100644
--- a/drivers/gpu/drm/stm/ltdc.c
+++ b/drivers/gpu/drm/stm/ltdc.c
@@ -507,20 +507,16 @@ static bool ltdc_crtc_mode_fixup(struct drm_crtc *crtc,
struct ltdc_device *ldev = crtc_to_ltdc(crtc);
int rate = mode->clock * 1000;
- /*
-* TODO clk_round_rate() does not work yet. When ready, it can
-* be used instead of clk_set_rate() then clk_get_rate().
-*/
-
- clk_disable(ldev->pixel_clk);
if (clk_set_rate(ldev->pixel_clk, rate) < 0) {
DRM_ERROR("Cannot set rate (%dHz) for pixel clk\n", rate);
return false;
}
- clk_enable(ldev->pixel_clk);
adjusted_mode->clock = clk_get_rate(ldev->pixel_clk) / 1000;
+ DRM_DEBUG_DRIVER("requested clock %dkHz, adjusted clock %dkHz\n",
+mode->clock, adjusted_mode->clock);
+
return true;
}
--
2.7.4
___
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel
[PATCH] drm/stm: dsi: check hardware version
Check version of DSI hardware IP. Only versions 1.30 & 1.31
are supported.
Signed-off-by: Yannick Fertré
---
drivers/gpu/drm/stm/dw_mipi_dsi-stm.c | 24 +++-
1 file changed, 23 insertions(+), 1 deletion(-)
diff --git a/drivers/gpu/drm/stm/dw_mipi_dsi-stm.c
b/drivers/gpu/drm/stm/dw_mipi_dsi-stm.c
index 22bd095..29105e9 100644
--- a/drivers/gpu/drm/stm/dw_mipi_dsi-stm.c
+++ b/drivers/gpu/drm/stm/dw_mipi_dsi-stm.c
@@ -227,7 +227,6 @@ dw_mipi_dsi_get_lane_mbps(void *priv_data, const struct
drm_display_mode *mode,
u32 val;
/* Update lane capabilities according to hw version */
- dsi->hw_version = dsi_read(dsi, DSI_VERSION) & VERSION;
dsi->lane_min_kbps = LANE_MIN_KBPS;
dsi->lane_max_kbps = LANE_MAX_KBPS;
if (dsi->hw_version == HWVER_131) {
@@ -306,6 +305,7 @@ static int dw_mipi_dsi_stm_probe(struct platform_device
*pdev)
{
struct device *dev = &pdev->dev;
struct dw_mipi_dsi_stm *dsi;
+ struct clk *pclk;
struct resource *res;
int ret;
@@ -347,6 +347,28 @@ static int dw_mipi_dsi_stm_probe(struct platform_device
*pdev)
goto err_clk_get;
}
+ pclk = devm_clk_get(dev, "pclk");
+ if (IS_ERR(pclk)) {
+ ret = PTR_ERR(pclk);
+ DRM_ERROR("Unable to get peripheral clock: %d\n", ret);
+ goto err_dsi_probe;
+ }
+
+ ret = clk_prepare_enable(pclk);
+ if (ret) {
+ DRM_ERROR("%s: Failed to enable peripheral clk\n", __func__);
+ goto err_dsi_probe;
+ }
+
+ dsi->hw_version = dsi_read(dsi, DSI_VERSION) & VERSION;
+ clk_disable_unprepare(pclk);
+
+ if (dsi->hw_version != HWVER_130 && dsi->hw_version != HWVER_131) {
+ ret = -ENODEV;
+ DRM_ERROR("bad dsi hardware version\n");
+ goto err_dsi_probe;
+ }
+
dw_mipi_dsi_stm_plat_data.base = dsi->base;
dw_mipi_dsi_stm_plat_data.priv_data = dsi;
--
2.7.4
___
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [RFC PATCH v2 4/5] drm, cgroup: Add total GEM buffer allocation limit
On Fri, May 10, 2019 at 8:28 AM Christian König
wrote:
>
> Am 09.05.19 um 23:04 schrieb Kenny Ho:
> > + /* only allow bo from the same cgroup or its ancestor to be imported
> > */
> > + if (drmcgrp != NULL &&
> > + !drmcgrp_is_self_or_ancestor(drmcgrp, obj->drmcgrp)) {
> > + ret = -EACCES;
> > + goto out_unlock;
> > + }
> > +
>
> This will most likely go up in flames.
>
> If I'm not completely mistaken we already use
> drm_gem_prime_fd_to_handle() to exchange handles between different
> cgroups in current container usages.
This is something that I am interested in getting more details from
the broader community because the details affect how likely this will
go up in flames ;). Note that this check does not block sharing of
handles from cgroup parent to children in the hierarchy, nor does it
blocks sharing of handles within a cgroup.
I am interested to find out, when existing apps share handles between
containers, if there are any expectations on resource management.
Since there are no drm cgroup for current container usage, I expect
the answer to be no. In this case, the drm cgroup controller can be
disabled on its own (in the context of cgroup-v2's unified hierarchy),
or the process can remain at the root for the drm cgroup hierarchy (in
the context of cgroup-v1.) If I understand the cgroup api correctly,
that means all process would be part of the root cgroup as far as the
drm controller is concerned and this block will not come into effect.
I have verified that this is indeed the current default behaviour of a
container runtime (runc, which is used by docker, podman and others.)
The new drm cgroup controller is simply ignored and all processes
remain at the root of the hierarchy (since there are no other
cgroups.) I plan to make contributions to runc (so folks can actually
use this features with docker/podman/k8s, etc.) once things stabilized
on the kernel side.
On the other hand, if there are expectations for resource management
between containers, I would like to know who is the expected manager
and how does it fit into the concept of container (which enforce some
level of isolation.) One possible manager may be the display server.
But as long as the display server is in a parent cgroup of the apps'
cgroup, the apps can still import handles from the display server
under the current implementation. My understanding is that this is
most likely the case, with the display server simply sitting at the
default/root cgroup. But I certainly want to hear more about other
use cases (for example, is running multiple display servers on a
single host a realistic possibility? Are there people running
multiple display servers inside peer containers? If so, how do they
coordinate resources?)
I should probably summarize some of these into the commit message.
Regards,
Kenny
> Christian.
>
> > if (obj->dma_buf) {
> > WARN_ON(obj->dma_buf != dma_buf);
> > } else {
> > diff --git a/include/drm/drm_cgroup.h b/include/drm/drm_cgroup.h
> > index ddb9eab64360..8711b7c5f7bf 100644
> > --- a/include/drm/drm_cgroup.h
> > +++ b/include/drm/drm_cgroup.h
> > @@ -4,12 +4,20 @@
> > #ifndef __DRM_CGROUP_H__
> > #define __DRM_CGROUP_H__
> >
> > +#include
> > +
> > #ifdef CONFIG_CGROUP_DRM
> >
> > int drmcgrp_register_device(struct drm_device *device);
> > -
> > int drmcgrp_unregister_device(struct drm_device *device);
> > -
> > +bool drmcgrp_is_self_or_ancestor(struct drmcgrp *self,
> > + struct drmcgrp *relative);
> > +void drmcgrp_chg_bo_alloc(struct drmcgrp *drmcgrp, struct drm_device *dev,
> > + size_t size);
> > +void drmcgrp_unchg_bo_alloc(struct drmcgrp *drmcgrp, struct drm_device
> > *dev,
> > + size_t size);
> > +bool drmcgrp_bo_can_allocate(struct task_struct *task, struct drm_device
> > *dev,
> > + size_t size);
> > #else
> > static inline int drmcgrp_register_device(struct drm_device *device)
> > {
> > @@ -20,5 +28,27 @@ static inline int drmcgrp_unregister_device(struct
> > drm_device *device)
> > {
> > return 0;
> > }
> > +
> > +static inline bool drmcgrp_is_self_or_ancestor(struct drmcgrp *self,
> > + struct drmcgrp *relative)
> > +{
> > + return false;
> > +}
> > +
> > +static inline void drmcgrp_chg_bo_alloc(struct drmcgrp *drmcgrp,
> > + struct drm_device *dev, size_t size)
> > +{
> > +}
> > +
> > +static inline void drmcgrp_unchg_bo_alloc(struct drmcgrp *drmcgrp,
> > + struct drm_device *dev, size_t size)
> > +{
> > +}
> > +
> > +static inline bool drmcgrp_bo_can_allocate(struct task_struct *task,
> > + struct drm_device *dev, size_t size)
> > +{
> > + return true;
> > +}
> > #endif /* CONFIG_CGROUP_DRM */
> > #endif /* __DRM_CGROUP_H__ */
> > diff --git a/include/drm/drm_gem.h b/include/drm/drm_gem.h
> > index c95727425284..02854c674b5c 100644
> > --- a/include/drm/drm_gem.h
> > ++
Re: [PATCH v7 09/11] drm: uevent for connector status change
On Fri, May 10, 2019 at 2:12 PM Paul Kocialkowski wrote: > > Hi, > > On Tue, 2019-05-07 at 21:57 +0530, Ramalingam C wrote: > > DRM API for generating uevent for a status changes of connector's > > property. > > > > This uevent will have following details related to the status change: > > > > HOTPLUG=1, CONNECTOR= and PROPERTY= > > > > Need ACK from this uevent from userspace consumer. > > So we just had some discussions over on IRC and at about the hotplug > issue and came up with similar ideas: > https://lists.freedesktop.org/archives/dri-devel/2019-May/217408.html > > The conclusions of these discussions so far would be to have a more or > less fine grain of uevent reporting depending on what happened. The > point is that we need to cover different cases: > - one or more properties changed; > - the connector status changed; > - something else about the connector changed (e.g. EDID/modes) > > For the first case, we can send out: > HOTPLUG=1 > CONNECTOR= > PROPERTY= > > and no reprobe is required. > > For the second one, something like: > HOTPLUG=1 > CONNECTOR= > STATUS=Connected/Disconnected > > and a connector probe is needed for connected, but not for > disconnected; > > For the third one, we can only indicate the connector: > HOTPLUG=1 > CONNECTOR= > > and a reprobe of the connector is always needed There's no material difference between this one and the previous one. Plus there's no beenfit in supplying the actual value of the property, i.e. we can reuse the same PROPERTY= trick. Here's why: - A side effect of forcing a probe on a connector is that you get to read all the properties, so supplying them is kinda pointless. - You can read STATUS without forcing a reprobe, if you want to avoid the reprobe for disconnected. I'd kinda not recommend that though, feels a bit like overoptimizing. And for reasonable connectors (i.e. dp) reprobing a disconnected output is fast. HDMI is ... less reasonable unfortunately, but oh well. - There's no way to only reprobe status, you can only ever reprobe everything with the current ioctl and implementations. Having an option to reprobe only parts of it doesn't seem useful to me (we need to read the EDID anyway, and that's the expensive part of reprobing in almost all cases). In a way PROPERTY= simply tells userspace that it needs to reprobe this connector. At that point we need to figure out whether this is a good uapi or not, and that's where the epoch comes in. There's two reasons for an epoch: - We need it internally because I'm not goinig to wire a new return value through hundreds of connector probe functions. It's much easier to have an epoch counter which we set from e.g. drm_set_edid and similar functions that update probe state. - If userspace misses an event and there's no epoch, we're forcing userspace to reprobe everything. Use case would be if a compositor is switched away we probably don't want to piss of the current compositor by blocking it's own probe kernel calls by doing our own (probe is single-threaded in the kernel through the dev->mode_config.mutex). If it can read the epoch property (which it can do without forcing a reprobe) userspace would know which connectors it needs to check and reprobe. Hence why epoch, it's a bit more robust userspace api. Ofc you could also require that userspace needs to keep parsing all uevents and make a list of all connectors it needs to reprobe when it's back to being the active compositor. But just comparing a current epoch with the one you cached from the last full probe is much easier. Another thing: None of this we can for connectors with unreliable hdp. Or at least you'll piss of users if you cache always. The sad thing is that HDMI is unreliable, at least on some machines/screen combos (you never get a hpd irq if you plug in/unplug). So real compositors still need to reprobe when the user asks for it. igt can probably get away without reprobing. -Daniel > Then we still have the legacy case: > HOTPLUG=1 > > where userspace is expected to reprobe all the connectors. > > I think this would deserve to be a separate series on its own. So I am > proposing to take this one off your plate and come up with another > seres implementing this proposal. What do you think? > > Cheers, > > Paul > > > v2: > > Minor fixes at KDoc comments [Daniel] > > v3: > > Check the property is really attached with connector [Daniel] > > > > Signed-off-by: Ramalingam C > > Reviewed-by: Daniel Vetter > > --- > > drivers/gpu/drm/drm_sysfs.c | 35 +++ > > include/drm/drm_sysfs.h | 5 - > > 2 files changed, 39 insertions(+), 1 deletion(-) > > > > diff --git a/drivers/gpu/drm/drm_sysfs.c b/drivers/gpu/drm/drm_sysfs.c > > index 18b1ac442997..63fa951a20db 100644 > > --- a/drivers/gpu/drm/drm_sysfs.c > > +++ b/drivers/gpu/drm/drm_sysfs.c > > @@ -21,6 +21,7 @@ > > #include > > #include > > #include "drm_internal.h" > > +#include "drm_crtc_internal.h" > > > > #define to_drm_minor(d) de
Re: Improving DRM connector hotplug uevents
On Fri, 2019-05-10 at 09:56 +0200, Daniel Vetter wrote: > On Fri, May 10, 2019 at 9:42 AM Paul Kocialkowski > wrote: > > Hi, > > > > So this thread has drifted off from IGT to a general DRM improvement, > > so renaming the thread and adding some CCs. > > > > It's about avoiding full reprobes (which may include slow EDID reads, > > etc) when a connector change was detected, by providing information > > about the connector and the new state to userspace. This way, userspace > > can reprobe the changed connector alone (or not reprobe at all in case > > of a disconnect). > > > > It feels like there is growing temptation to have per-driver hacks to > > avoid full reprobes when the driver knows that nothing changed, but I > > think it makes more sense to fixup the uevent we send to make sure that > > userspace knows what to probe exactly. > > > > On Fri, 2019-05-10 at 06:55 +, Ser, Simon wrote: > > > On Thu, 2019-05-09 at 11:56 -0400, Lyude Paul wrote: > > > > I'm in support of this as well, we really could use better hotplug > > > > events. > > > > Feel free to cc patches to me for review :) > > > > > > Any idea(s) how the API would look like? > > > > From a quick chat on IRC yesterday, it seems that just adding entries > > to the uevent would do and would be backwards-compatible (well, > > provided that the userspace uevent parsers will not fail if something > > extra to "HOTPLUG=1" is provided). > > > > Currently, the notification is sent globally at each round of detected > > connector change (which may concern multiple connectors). So I think we > > should just list the connectors that changed in the uevent and their > > new state. So my proposal here is something like: > > > > HOTPLUG=1 > > CONNECTOR_ID=47 > > STATUS=Connected > > CONNECTOR_ID=48 > > STATUS=Disconnected > > > > Where each STATUS entry would refer to the previous CONNECTOR_ID entry. > > > > What do you think? > > We have the better uevent already, all we need is wiring up, lots of > code, and userspace for it. Ok, not quite, patch is still in flight: > > https://patchwork.kernel.org/patch/10906677/ Excellent. I can help with a userspace implementation if needed. ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [PATCH] of/device: add blacklist for iommu dma_ops
On Tue, Dec 4, 2018 at 2:29 PM Rob Herring wrote:
>
> On Sat, Dec 1, 2018 at 10:54 AM Rob Clark wrote:
> >
> > This solves a problem we see with drm/msm, caused by getting
> > iommu_dma_ops while we attach our own domain and manage it directly at
> > the iommu API level:
> >
> > [0038] user address but active_mm is swapper
> > Internal error: Oops: 9605 [#1] PREEMPT SMP
> > Modules linked in:
> > CPU: 7 PID: 70 Comm: kworker/7:1 Tainted: GW 4.19.3 #90
> > Hardware name: xxx (DT)
> > Workqueue: events deferred_probe_work_func
> > pstate: 80c9 (Nzcv daif +PAN +UAO)
> > pc : iommu_dma_map_sg+0x7c/0x2c8
> > lr : iommu_dma_map_sg+0x40/0x2c8
> > sp : ff80095eb4f0
> > x29: ff80095eb4f0 x28:
> > x27: ffc0f9431578 x26:
> > x25: x24: 0003
> > x23: 0001 x22: ffc0fa9ac010
> > x21: x20: ffc0fab40980
> > x19: ffc0fab40980 x18: 0003
> > x17: 01c4 x16: 0007
> > x15: 000e x14:
> > x13: x12: 0028
> > x11: 0101010101010101 x10: 7f7f7f7f7f7f7f7f
> > x9 : x8 : ffc0fab409a0
> > x7 : x6 : 0002
> > x5 : 0001 x4 :
> > x3 : 0001 x2 : 0002
> > x1 : ffc0f9431578 x0 :
> > Process kworker/7:1 (pid: 70, stack limit = 0x17d08ffb)
> > Call trace:
> >iommu_dma_map_sg+0x7c/0x2c8
> >__iommu_map_sg_attrs+0x70/0x84
> >get_pages+0x170/0x1e8
> >msm_gem_get_iova+0x8c/0x128
> >_msm_gem_kernel_new+0x6c/0xc8
> >msm_gem_kernel_new+0x4c/0x58
> >dsi_tx_buf_alloc_6g+0x4c/0x8c
> >msm_dsi_host_modeset_init+0xc8/0x108
> >msm_dsi_modeset_init+0x54/0x18c
> >_dpu_kms_drm_obj_init+0x430/0x474
> >dpu_kms_hw_init+0x5f8/0x6b4
> >msm_drm_bind+0x360/0x6c8
> >try_to_bring_up_master.part.7+0x28/0x70
> >component_master_add_with_match+0xe8/0x124
> >msm_pdev_probe+0x294/0x2b4
> >platform_drv_probe+0x58/0xa4
> >really_probe+0x150/0x294
> >driver_probe_device+0xac/0xe8
> >__device_attach_driver+0xa4/0xb4
> >bus_for_each_drv+0x98/0xc8
> >__device_attach+0xac/0x12c
> >device_initial_probe+0x24/0x30
> >bus_probe_device+0x38/0x98
> >deferred_probe_work_func+0x78/0xa4
> >process_one_work+0x24c/0x3dc
> >worker_thread+0x280/0x360
> >kthread+0x134/0x13c
> >ret_from_fork+0x10/0x18
> > Code: d284 91000725 6b17039f 5400048a (f9401f40)
> > ---[ end trace f22dda57f3648e2c ]---
> > Kernel panic - not syncing: Fatal exception
> > SMP: stopping secondary CPUs
> > Kernel Offset: disabled
> > CPU features: 0x0,22802a18
> > Memory Limit: none
> >
> > The problem is that when drm/msm does it's own iommu_attach_device(),
> > now the domain returned by iommu_get_domain_for_dev() is drm/msm's
> > domain, and it doesn't have domain->iova_cookie.
> >
> > We kind of avoided this problem prior to sdm845/dpu because the iommu
> > was attached to the mdp node in dt, which is a child of the toplevel
> > mdss node (which corresponds to the dev passed in dma_map_sg()). But
> > with sdm845, now the iommu is attached at the mdss level so we hit the
> > iommu_dma_ops in dma_map_sg().
> >
> > But auto allocating/attaching a domain before the driver is probed was
> > already a blocking problem for enabling per-context pagetables for the
> > GPU. This problem is also now solved with this patch.
> >
> > Fixes: 97890ba9289c dma-mapping: detect and configure IOMMU in
> > of_dma_configure
> > Tested-by: Douglas Anderson
> > Signed-off-by: Rob Clark
> > ---
> > This is an alternative/replacement for [1]. What it lacks in elegance
> > it makes up for in practicality ;-)
> >
> > [1] https://patchwork.freedesktop.org/patch/264930/
> >
> > drivers/of/device.c | 22 ++
> > 1 file changed, 22 insertions(+)
> >
> > diff --git a/drivers/of/device.c b/drivers/of/device.c
> > index 5957cd4fa262..15ffee00fb22 100644
> > --- a/drivers/of/device.c
> > +++ b/drivers/of/device.c
> > @@ -72,6 +72,14 @@ int of_device_add(struct platform_device *ofdev)
> > return device_add(&ofdev->dev);
> > }
> >
> > +static const struct of_device_id iommu_blacklist[] = {
> > + { .compatible = "qcom,mdp4" },
> > + { .compatible = "qcom,mdss" },
> > + { .compatible = "qcom,sdm845-mdss" },
> > + { .compatible = "qcom,adreno" },
> > + {}
> > +};
>
> Not completely clear to whether this is still needed or not, but this
> really won't scale. Why can't the driver for these devices override
> whatever has been setup by default?
>
fwiw, at the moment it is not needed, but it will become needed again
to implement per-context pagetables (although I suppose for this we
only need to blacklist qcom,adreno and not also the display nodes).
T
[PATCH] Revert "vgaarb: Keep adding VGA device in queue"
This breaks multiple graphics cards in the Amigaone x5000 on PPC. This reverts commit 3d42f1ddc47a69c0ce155f9f30d764c4d689a5fa. Bug: https://bugs.freedesktop.org/show_bug.cgi?id=109345 Signed-off-by: Alex Deucher CC: Aaron Ma --- drivers/gpu/vga/vgaarb.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/drivers/gpu/vga/vgaarb.c b/drivers/gpu/vga/vgaarb.c index f2f3ef8af271..8a3c45219a2a 100644 --- a/drivers/gpu/vga/vgaarb.c +++ b/drivers/gpu/vga/vgaarb.c @@ -725,7 +725,7 @@ static bool vga_arbiter_add_pci_device(struct pci_dev *pdev) vga_arbiter_check_bridge_sharing(vgadev); /* Add to the list */ - list_add_tail(&vgadev->list, &vga_list); + list_add(&vgadev->list, &vga_list); vga_count++; vgaarb_info(&pdev->dev, "VGA device added: decodes=%s,owns=%s,locks=%s\n", vga_iostate_to_str(vgadev->decodes), -- 2.20.1 ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: Improving DRM connector hotplug uevents
Hi, Resending this with the right CC. Also changed my mind in the meantime and my latest proposal is at: https://lists.freedesktop.org/archives/dri-devel/2019-May/217442.html On Fri, 2019-05-10 at 10:06 +0200, Daniel Vetter wrote: > On Fri, May 10, 2019 at 10:01 AM Paul Kocialkowski > wrote: > > Hi, > > > > On Fri, 2019-05-10 at 09:56 +0200, Daniel Vetter wrote: > > > On Fri, May 10, 2019 at 9:42 AM Paul Kocialkowski > > > wrote: > > > > Hi, > > > > > > > > So this thread has drifted off from IGT to a general DRM improvement, > > > > so renaming the thread and adding some CCs. > > > > > > > > It's about avoiding full reprobes (which may include slow EDID reads, > > > > etc) when a connector change was detected, by providing information > > > > about the connector and the new state to userspace. This way, userspace > > > > can reprobe the changed connector alone (or not reprobe at all in case > > > > of a disconnect). > > > > > > > > It feels like there is growing temptation to have per-driver hacks to > > > > avoid full reprobes when the driver knows that nothing changed, but I > > > > think it makes more sense to fixup the uevent we send to make sure that > > > > userspace knows what to probe exactly. > > > > > > > > On Fri, 2019-05-10 at 06:55 +, Ser, Simon wrote: > > > > > On Thu, 2019-05-09 at 11:56 -0400, Lyude Paul wrote: > > > > > > I'm in support of this as well, we really could use better hotplug > > > > > > events. > > > > > > Feel free to cc patches to me for review :) > > > > > > > > > > Any idea(s) how the API would look like? > > > > > > > > From a quick chat on IRC yesterday, it seems that just adding entries > > > > to the uevent would do and would be backwards-compatible (well, > > > > provided that the userspace uevent parsers will not fail if something > > > > extra to "HOTPLUG=1" is provided). > > > > > > > > Currently, the notification is sent globally at each round of detected > > > > connector change (which may concern multiple connectors). So I think we > > > > should just list the connectors that changed in the uevent and their > > > > new state. So my proposal here is something like: > > > > > > > > HOTPLUG=1 > > > > CONNECTOR_ID=47 > > > > STATUS=Connected > > > > CONNECTOR_ID=48 > > > > STATUS=Disconnected > > > > > > > > Where each STATUS entry would refer to the previous CONNECTOR_ID entry. > > > > > > > > What do you think? > > > > > > We have the better uevent already, all we need is wiring up, lots of > > > code, and userspace for it. Ok, not quite, patch is still in flight: > > > > > > https://patchwork.kernel.org/patch/10906677/ > > > > Ah great to see that! > > > > > The real trouble isn't the new event, but making sure it goes out for > > > anything that might have changed. There's a lot more connector status > > > than just the STATUS property. > > > > Why though? I thought this was a hotplug interface, not a general > > "something-about-the-connector-changed". To me, hotplug is pretty > > binary: either it's connected, either it's not. > > > > Do we already send out hotplug uevents when something else than the > > status changed? If so, maybe we should consider handling the rest > > separately from hotplug. > > We have plenty of bugs around "changed screen over s/r and no output". > Imo if we rework hotplug, we should get all the mistakes right. I think suspend/resume is one specific case where hotplug just can't be detected by the driver. I think one solution to that would be to always issue a hotplug even for all connectors on resume. I don't really see how we could do better than that anyway. > > Looking at drm_probe_helper.c's output_poll_execute, my understanding > > is that drm_kms_helper_hotplug_event is only called when the status > > changed, not other props. > > Yeah, and that's why userspace has to reprobe all the time. I'm a bit confused then, how can userspace be notified that something else than the connected status changed if that's the only case where a uevent is issued? If something else changed that needs userspace attention, maybe we could send out a uevent with the CONNECTOR_ID but without the STATUS part so that userspace has to reprobe that connector to know what's what. > One idea > that we floated around is to have an epoche counter, which changes > every time anything with the connector changes wrt hotplug related > state (edid, sink id, whatever). The more specific hotplug uevent > would then give you an event on that epoch counter, so userspace knows > it needs to reprobe that specific connector for all the new state. That would work, but I find that a bit overkill for the issue at hand. At least I'm not convinced that something simpler would be a bad fit. The idea of adding STATUS to the uevent is just to provide a fast-path to avoid reprobe for the basic unplug case. If it's a plug or something more complex that has happened, then I think userspace should do a reprobe of the connector anyway. If we prop
[PATCH v2 2/3] drm/stm: dsi: add support of an optional regulator
Add support of an optional regulator for the phy part of the DSI
controller.
Signed-off-by: Yannick Fertré
---
drivers/gpu/drm/stm/dw_mipi_dsi-stm.c | 45 ++-
1 file changed, 39 insertions(+), 6 deletions(-)
diff --git a/drivers/gpu/drm/stm/dw_mipi_dsi-stm.c
b/drivers/gpu/drm/stm/dw_mipi_dsi-stm.c
index 1bef73e..22bd095 100644
--- a/drivers/gpu/drm/stm/dw_mipi_dsi-stm.c
+++ b/drivers/gpu/drm/stm/dw_mipi_dsi-stm.c
@@ -9,6 +9,7 @@
#include
#include
#include
+#include
#include
#include
#include
@@ -76,6 +77,7 @@ struct dw_mipi_dsi_stm {
u32 hw_version;
int lane_min_kbps;
int lane_max_kbps;
+ struct regulator *vdd_supply;
};
static inline void dsi_write(struct dw_mipi_dsi_stm *dsi, u32 reg, u32 val)
@@ -318,17 +320,31 @@ static int dw_mipi_dsi_stm_probe(struct platform_device
*pdev)
return PTR_ERR(dsi->base);
}
+ dsi->vdd_supply = devm_regulator_get_optional(dev, "phy-dsi");
+ if (IS_ERR(dsi->vdd_supply)) {
+ ret = PTR_ERR(dsi->vdd_supply);
+ if (ret != -EPROBE_DEFER)
+ DRM_ERROR("failed to request regulator: %d\n", ret);
+ return ret;
+ }
+
+ ret = regulator_enable(dsi->vdd_supply);
+ if (ret) {
+ DRM_ERROR("failed to enable regulator: %d\n", ret);
+ return ret;
+ }
+
dsi->pllref_clk = devm_clk_get(dev, "ref");
if (IS_ERR(dsi->pllref_clk)) {
ret = PTR_ERR(dsi->pllref_clk);
- dev_err(dev, "Unable to get pll reference clock: %d\n", ret);
- return ret;
+ DRM_ERROR("Unable to get pll reference clock: %d\n", ret);
+ goto err_clk_get;
}
ret = clk_prepare_enable(dsi->pllref_clk);
if (ret) {
- dev_err(dev, "%s: Failed to enable pllref_clk\n", __func__);
- return ret;
+ DRM_ERROR("%s: Failed to enable pllref_clk\n", __func__);
+ goto err_clk_get;
}
dw_mipi_dsi_stm_plat_data.base = dsi->base;
@@ -339,11 +355,19 @@ static int dw_mipi_dsi_stm_probe(struct platform_device
*pdev)
dsi->dsi = dw_mipi_dsi_probe(pdev, &dw_mipi_dsi_stm_plat_data);
if (IS_ERR(dsi->dsi)) {
DRM_ERROR("Failed to initialize mipi dsi host\n");
- clk_disable_unprepare(dsi->pllref_clk);
- return PTR_ERR(dsi->dsi);
+ ret = PTR_ERR(dsi->dsi);
+ goto err_dsi_probe;
}
return 0;
+
+err_dsi_probe:
+ clk_disable_unprepare(dsi->pllref_clk);
+err_clk_get:
+ regulator_disable(dsi->vdd_supply);
+
+ return ret;
+
}
static int dw_mipi_dsi_stm_remove(struct platform_device *pdev)
@@ -351,6 +375,7 @@ static int dw_mipi_dsi_stm_remove(struct platform_device
*pdev)
struct dw_mipi_dsi_stm *dsi = platform_get_drvdata(pdev);
clk_disable_unprepare(dsi->pllref_clk);
+ regulator_disable(dsi->vdd_supply);
dw_mipi_dsi_remove(dsi->dsi);
return 0;
@@ -363,6 +388,7 @@ static int __maybe_unused dw_mipi_dsi_stm_suspend(struct
device *dev)
DRM_DEBUG_DRIVER("\n");
clk_disable_unprepare(dsi->pllref_clk);
+ regulator_disable(dsi->vdd_supply);
return 0;
}
@@ -370,9 +396,16 @@ static int __maybe_unused dw_mipi_dsi_stm_suspend(struct
device *dev)
static int __maybe_unused dw_mipi_dsi_stm_resume(struct device *dev)
{
struct dw_mipi_dsi_stm *dsi = dw_mipi_dsi_stm_plat_data.priv_data;
+ int ret;
DRM_DEBUG_DRIVER("\n");
+ ret = regulator_enable(dsi->vdd_supply);
+ if (ret) {
+ DRM_ERROR("failed to enable regulator: %d\n", ret);
+ return ret;
+ }
+
clk_prepare_enable(dsi->pllref_clk);
return 0;
--
2.7.4
___
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [PATCH 1/4] drm/ttm: Make LRU removal optional.
Stupid me, patch #3 has a rebase error so please ignore this version.
@Marek: This set is fixing the GDS problem for my constructed test case,
you probably try it also in a real world scenario when it lands.
Christian.
Am 10.05.19 um 16:13 schrieb Christian König:
We are already doing this for DMA-buf imports and also for
amdgpu VM BOs for quite a while now.
If this doesn't run into any problems we are probably going
to stop removing BOs from the LRU altogether.
Signed-off-by: Christian König
---
.../gpu/drm/amd/amdgpu/amdgpu_amdkfd_gpuvm.c | 9 +
drivers/gpu/drm/amd/amdgpu/amdgpu_cs.c| 2 +-
drivers/gpu/drm/amd/amdgpu/amdgpu_csa.c | 2 +-
drivers/gpu/drm/amd/amdgpu/amdgpu_gem.c | 4 ++--
drivers/gpu/drm/qxl/qxl_release.c | 2 +-
drivers/gpu/drm/radeon/radeon_gem.c | 2 +-
drivers/gpu/drm/radeon/radeon_object.c| 2 +-
drivers/gpu/drm/ttm/ttm_execbuf_util.c| 20 +++
drivers/gpu/drm/virtio/virtgpu_ioctl.c| 2 +-
drivers/gpu/drm/vmwgfx/vmwgfx_resource.c | 3 ++-
drivers/gpu/drm/vmwgfx/vmwgfx_validation.h| 2 +-
include/drm/ttm/ttm_bo_driver.h | 5 -
include/drm/ttm/ttm_execbuf_util.h| 3 ++-
13 files changed, 34 insertions(+), 24 deletions(-)
diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_amdkfd_gpuvm.c
b/drivers/gpu/drm/amd/amdgpu/amdgpu_amdkfd_gpuvm.c
index e1cae4a37113..647e18f9e136 100644
--- a/drivers/gpu/drm/amd/amdgpu/amdgpu_amdkfd_gpuvm.c
+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_amdkfd_gpuvm.c
@@ -574,7 +574,7 @@ static int reserve_bo_and_vm(struct kgd_mem *mem,
amdgpu_vm_get_pd_bo(vm, &ctx->list, &ctx->vm_pd[0]);
ret = ttm_eu_reserve_buffers(&ctx->ticket, &ctx->list,
-false, &ctx->duplicates);
+false, &ctx->duplicates, true);
if (!ret)
ctx->reserved = true;
else {
@@ -647,7 +647,7 @@ static int reserve_bo_and_cond_vms(struct kgd_mem *mem,
}
ret = ttm_eu_reserve_buffers(&ctx->ticket, &ctx->list,
-false, &ctx->duplicates);
+false, &ctx->duplicates, true);
if (!ret)
ctx->reserved = true;
else
@@ -1800,7 +1800,8 @@ static int validate_invalid_user_pages(struct
amdkfd_process_info *process_info)
}
/* Reserve all BOs and page tables for validation */
- ret = ttm_eu_reserve_buffers(&ticket, &resv_list, false, &duplicates);
+ ret = ttm_eu_reserve_buffers(&ticket, &resv_list, false, &duplicates,
+true);
WARN(!list_empty(&duplicates), "Duplicates should be empty");
if (ret)
goto out_free;
@@ -2006,7 +2007,7 @@ int amdgpu_amdkfd_gpuvm_restore_process_bos(void *info,
struct dma_fence **ef)
}
ret = ttm_eu_reserve_buffers(&ctx.ticket, &ctx.list,
-false, &duplicate_save);
+false, &duplicate_save, true);
if (ret) {
pr_debug("Memory eviction: TTM Reserve Failed. Try again\n");
goto ttm_reserve_fail;
diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_cs.c
b/drivers/gpu/drm/amd/amdgpu/amdgpu_cs.c
index d0e221c8d940..a1d6a0721e53 100644
--- a/drivers/gpu/drm/amd/amdgpu/amdgpu_cs.c
+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_cs.c
@@ -648,7 +648,7 @@ static int amdgpu_cs_parser_bos(struct amdgpu_cs_parser *p,
}
r = ttm_eu_reserve_buffers(&p->ticket, &p->validated, true,
- &duplicates);
+ &duplicates, true);
if (unlikely(r != 0)) {
if (r != -ERESTARTSYS)
DRM_ERROR("ttm_eu_reserve_buffers failed.\n");
diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_csa.c
b/drivers/gpu/drm/amd/amdgpu/amdgpu_csa.c
index 54dd02a898b9..06f83cac0d3a 100644
--- a/drivers/gpu/drm/amd/amdgpu/amdgpu_csa.c
+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_csa.c
@@ -79,7 +79,7 @@ int amdgpu_map_static_csa(struct amdgpu_device *adev, struct
amdgpu_vm *vm,
list_add(&csa_tv.head, &list);
amdgpu_vm_get_pd_bo(vm, &list, &pd);
- r = ttm_eu_reserve_buffers(&ticket, &list, true, NULL);
+ r = ttm_eu_reserve_buffers(&ticket, &list, true, NULL, true);
if (r) {
DRM_ERROR("failed to reserve CSA,PD BOs: err=%d\n", r);
return r;
diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_gem.c
b/drivers/gpu/drm/amd/amdgpu/amdgpu_gem.c
index 7b840367004c..d513a5ad03dd 100644
--- a/drivers/gpu/drm/amd/amdgpu/amdgpu_gem.c
+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_gem.c
@@ -171,7 +171,7 @@ void amdgpu_gem_object_close(struct drm_gem_object *obj,
amdgpu_vm_get_pd_bo(vm, &list, &vm_pd);
- r = ttm_eu_reserve_buffers(&ticket, &list, false, &duplicates);
+ r = ttm_eu_reserve_buff
[PATCH v2 3/5] ARM: dts: stm32: add phy-dsi-supply property on stm32mp157c
The dsi physical layer is powered by the 1v8 power controller supply. Signed-off-by: Yannick Fertré --- arch/arm/boot/dts/stm32mp157c.dtsi | 1 + 1 file changed, 1 insertion(+) diff --git a/arch/arm/boot/dts/stm32mp157c.dtsi b/arch/arm/boot/dts/stm32mp157c.dtsi index 2afeee6..6b14f1e 100644 --- a/arch/arm/boot/dts/stm32mp157c.dtsi +++ b/arch/arm/boot/dts/stm32mp157c.dtsi @@ -1156,6 +1156,7 @@ clock-names = "pclk", "ref", "px_clk"; resets = <&rcc DSI_R>; reset-names = "apb"; + phy-dsi-supply = <®18>; status = "disabled"; }; -- 2.7.4 ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
[PATCH v2 5/5] ARM: dts: stm32: remove phy-dsi-supply property on stm32mp157c-dk2 board
This property is already defined into stm32mp157c.dtsi file.
Signed-off-by: Yannick Fertré
---
arch/arm/boot/dts/stm32mp157c-dk2.dts | 1 -
1 file changed, 1 deletion(-)
diff --git a/arch/arm/boot/dts/stm32mp157c-dk2.dts
b/arch/arm/boot/dts/stm32mp157c-dk2.dts
index 020ea0f..09f6e7b 100644
--- a/arch/arm/boot/dts/stm32mp157c-dk2.dts
+++ b/arch/arm/boot/dts/stm32mp157c-dk2.dts
@@ -17,7 +17,6 @@
#address-cells = <1>;
#size-cells = <0>;
status = "okay";
- phy-dsi-supply = <®18>;
ports {
#address-cells = <1>;
--
2.7.4
___
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel
[PATCH v2 4/5] ARM: dts: stm32: move fixe regulators reg11 & reg18
Move regulators reg11 & reg18 from device-tree files stm32mp157c-ed1.dts
& stm32mp157c-dk2.dts to file stm32mp157c.dtsi.
Signed-off-by: Yannick Fertré
---
arch/arm/boot/dts/stm32mp157c-dk2.dts | 8
arch/arm/boot/dts/stm32mp157c-ed1.dts | 16
arch/arm/boot/dts/stm32mp157c.dtsi| 16
3 files changed, 16 insertions(+), 24 deletions(-)
diff --git a/arch/arm/boot/dts/stm32mp157c-dk2.dts
b/arch/arm/boot/dts/stm32mp157c-dk2.dts
index 20ea601..020ea0f 100644
--- a/arch/arm/boot/dts/stm32mp157c-dk2.dts
+++ b/arch/arm/boot/dts/stm32mp157c-dk2.dts
@@ -11,14 +11,6 @@
/ {
model = "STMicroelectronics STM32MP157C-DK2 Discovery Board";
compatible = "st,stm32mp157c-dk2", "st,stm32mp157";
-
- reg18: reg18 {
- compatible = "regulator-fixed";
- regulator-name = "reg18";
- regulator-min-microvolt = <180>;
- regulator-max-microvolt = <180>;
- regulator-always-on;
- };
};
&dsi {
diff --git a/arch/arm/boot/dts/stm32mp157c-ed1.dts
b/arch/arm/boot/dts/stm32mp157c-ed1.dts
index 62a8c78..f41189c 100644
--- a/arch/arm/boot/dts/stm32mp157c-ed1.dts
+++ b/arch/arm/boot/dts/stm32mp157c-ed1.dts
@@ -27,22 +27,6 @@
serial0 = &uart4;
};
- reg11: reg11 {
- compatible = "regulator-fixed";
- regulator-name = "reg11";
- regulator-min-microvolt = <110>;
- regulator-max-microvolt = <110>;
- regulator-always-on;
- };
-
- reg18: reg18 {
- compatible = "regulator-fixed";
- regulator-name = "reg18";
- regulator-min-microvolt = <180>;
- regulator-max-microvolt = <180>;
- regulator-always-on;
- };
-
sd_switch: regulator-sd_switch {
compatible = "regulator-gpio";
regulator-name = "sd_switch";
diff --git a/arch/arm/boot/dts/stm32mp157c.dtsi
b/arch/arm/boot/dts/stm32mp157c.dtsi
index 6b14f1e..aaac51cd 100644
--- a/arch/arm/boot/dts/stm32mp157c.dtsi
+++ b/arch/arm/boot/dts/stm32mp157c.dtsi
@@ -11,6 +11,22 @@
#address-cells = <1>;
#size-cells = <1>;
+ reg11: reg11 {
+ compatible = "regulator-fixed";
+ regulator-name = "reg11";
+ regulator-min-microvolt = <110>;
+ regulator-max-microvolt = <110>;
+ regulator-always-on;
+ };
+
+ reg18: reg18 {
+ compatible = "regulator-fixed";
+ regulator-name = "reg18";
+ regulator-min-microvolt = <180>;
+ regulator-max-microvolt = <180>;
+ regulator-always-on;
+ };
+
cpus {
#address-cells = <1>;
#size-cells = <0>;
--
2.7.4
___
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel
[PATCH v2 1/5] dt-bindings: display: stm32: add supply property to DSI controller
This patch adds documentation of a new property phy-dsi-supply to the
STM32 DSI controller.
Signed-off-by: Yannick Fertré
---
Documentation/devicetree/bindings/display/st,stm32-ltdc.txt | 3 +++
1 file changed, 3 insertions(+)
diff --git a/Documentation/devicetree/bindings/display/st,stm32-ltdc.txt
b/Documentation/devicetree/bindings/display/st,stm32-ltdc.txt
index 3eb1b48..60c54da 100644
--- a/Documentation/devicetree/bindings/display/st,stm32-ltdc.txt
+++ b/Documentation/devicetree/bindings/display/st,stm32-ltdc.txt
@@ -40,6 +40,8 @@ Mandatory nodes specific to STM32 DSI:
- panel or bridge node: A node containing the panel or bridge description as
documented in [6].
- port: panel or bridge port node, connected to the DSI output port (port@1).
+Optional properties:
+- phy-dsi-supply: phandle of the regulator that provides the supply voltage.
Note: You can find more documentation in the following references
[1] Documentation/devicetree/bindings/clock/clock-bindings.txt
@@ -101,6 +103,7 @@ Example 2: DSI panel
clock-names = "pclk", "ref";
resets = <&rcc STM32F4_APB2_RESET(DSI)>;
reset-names = "apb";
+ phy-dsi-supply = <®18>;
ports {
#address-cells = <1>;
--
2.7.4
___
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel
[PATCH v2 0/5] Add supply property for DSI controller
The DSI controller needs a new property that powers its physical layer. Binding has been updated to documented this property. Device tree of stm32mp157c soc. Move reg18 & reg11 to stm32mp157c device tree file. Remove property phy-dsi-supply property to stm32mp157c-dk2.dts file. Changes in v2: - rename patch drm/stm: dsi: add support of an optional regulator - rework dw_mipi_dsi-stm probe sequence Yannick Fertré (5): dt-bindings: display: stm32: add supply property to DSI controller drm/stm: dsi: add support of an optional regulator ARM: dts: stm32: add phy-dsi-supply property on stm32mp157c ARM: dts: stm32: move fixe regulators reg11 & reg18 ARM: dts: stm32: remove phy-dsi-supply property on stm32mp157c-dk2 board .../devicetree/bindings/display/st,stm32-ltdc.txt| 3 +++ arch/arm/boot/dts/stm32mp157c-dk2.dts| 9 - arch/arm/boot/dts/stm32mp157c-ed1.dts| 16 arch/arm/boot/dts/stm32mp157c.dtsi | 17 + drivers/gpu/drm/stm/dw_mipi_dsi-stm.c | 45 +++--- 5 files changed, 40 insertions(+), 25 deletions(-) -- 2.7.4 ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
[Bug 109345] drm-next-2018-12-14 -Linux PPC
https://bugs.freedesktop.org/show_bug.cgi?id=109345 --- Comment #41 from Alex Deucher --- I'll send out a patch to revert the change. -- You are receiving this mail because: You are the assignee for the bug.___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
[PATCH 1/4] drm/ttm: Make LRU removal optional.
We are already doing this for DMA-buf imports and also for
amdgpu VM BOs for quite a while now.
If this doesn't run into any problems we are probably going
to stop removing BOs from the LRU altogether.
Signed-off-by: Christian König
---
.../gpu/drm/amd/amdgpu/amdgpu_amdkfd_gpuvm.c | 9 +
drivers/gpu/drm/amd/amdgpu/amdgpu_cs.c| 2 +-
drivers/gpu/drm/amd/amdgpu/amdgpu_csa.c | 2 +-
drivers/gpu/drm/amd/amdgpu/amdgpu_gem.c | 4 ++--
drivers/gpu/drm/qxl/qxl_release.c | 2 +-
drivers/gpu/drm/radeon/radeon_gem.c | 2 +-
drivers/gpu/drm/radeon/radeon_object.c| 2 +-
drivers/gpu/drm/ttm/ttm_execbuf_util.c| 20 +++
drivers/gpu/drm/virtio/virtgpu_ioctl.c| 2 +-
drivers/gpu/drm/vmwgfx/vmwgfx_resource.c | 3 ++-
drivers/gpu/drm/vmwgfx/vmwgfx_validation.h| 2 +-
include/drm/ttm/ttm_bo_driver.h | 5 -
include/drm/ttm/ttm_execbuf_util.h| 3 ++-
13 files changed, 34 insertions(+), 24 deletions(-)
diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_amdkfd_gpuvm.c
b/drivers/gpu/drm/amd/amdgpu/amdgpu_amdkfd_gpuvm.c
index e1cae4a37113..647e18f9e136 100644
--- a/drivers/gpu/drm/amd/amdgpu/amdgpu_amdkfd_gpuvm.c
+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_amdkfd_gpuvm.c
@@ -574,7 +574,7 @@ static int reserve_bo_and_vm(struct kgd_mem *mem,
amdgpu_vm_get_pd_bo(vm, &ctx->list, &ctx->vm_pd[0]);
ret = ttm_eu_reserve_buffers(&ctx->ticket, &ctx->list,
-false, &ctx->duplicates);
+false, &ctx->duplicates, true);
if (!ret)
ctx->reserved = true;
else {
@@ -647,7 +647,7 @@ static int reserve_bo_and_cond_vms(struct kgd_mem *mem,
}
ret = ttm_eu_reserve_buffers(&ctx->ticket, &ctx->list,
-false, &ctx->duplicates);
+false, &ctx->duplicates, true);
if (!ret)
ctx->reserved = true;
else
@@ -1800,7 +1800,8 @@ static int validate_invalid_user_pages(struct
amdkfd_process_info *process_info)
}
/* Reserve all BOs and page tables for validation */
- ret = ttm_eu_reserve_buffers(&ticket, &resv_list, false, &duplicates);
+ ret = ttm_eu_reserve_buffers(&ticket, &resv_list, false, &duplicates,
+true);
WARN(!list_empty(&duplicates), "Duplicates should be empty");
if (ret)
goto out_free;
@@ -2006,7 +2007,7 @@ int amdgpu_amdkfd_gpuvm_restore_process_bos(void *info,
struct dma_fence **ef)
}
ret = ttm_eu_reserve_buffers(&ctx.ticket, &ctx.list,
-false, &duplicate_save);
+false, &duplicate_save, true);
if (ret) {
pr_debug("Memory eviction: TTM Reserve Failed. Try again\n");
goto ttm_reserve_fail;
diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_cs.c
b/drivers/gpu/drm/amd/amdgpu/amdgpu_cs.c
index d0e221c8d940..a1d6a0721e53 100644
--- a/drivers/gpu/drm/amd/amdgpu/amdgpu_cs.c
+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_cs.c
@@ -648,7 +648,7 @@ static int amdgpu_cs_parser_bos(struct amdgpu_cs_parser *p,
}
r = ttm_eu_reserve_buffers(&p->ticket, &p->validated, true,
- &duplicates);
+ &duplicates, true);
if (unlikely(r != 0)) {
if (r != -ERESTARTSYS)
DRM_ERROR("ttm_eu_reserve_buffers failed.\n");
diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_csa.c
b/drivers/gpu/drm/amd/amdgpu/amdgpu_csa.c
index 54dd02a898b9..06f83cac0d3a 100644
--- a/drivers/gpu/drm/amd/amdgpu/amdgpu_csa.c
+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_csa.c
@@ -79,7 +79,7 @@ int amdgpu_map_static_csa(struct amdgpu_device *adev, struct
amdgpu_vm *vm,
list_add(&csa_tv.head, &list);
amdgpu_vm_get_pd_bo(vm, &list, &pd);
- r = ttm_eu_reserve_buffers(&ticket, &list, true, NULL);
+ r = ttm_eu_reserve_buffers(&ticket, &list, true, NULL, true);
if (r) {
DRM_ERROR("failed to reserve CSA,PD BOs: err=%d\n", r);
return r;
diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_gem.c
b/drivers/gpu/drm/amd/amdgpu/amdgpu_gem.c
index 7b840367004c..d513a5ad03dd 100644
--- a/drivers/gpu/drm/amd/amdgpu/amdgpu_gem.c
+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_gem.c
@@ -171,7 +171,7 @@ void amdgpu_gem_object_close(struct drm_gem_object *obj,
amdgpu_vm_get_pd_bo(vm, &list, &vm_pd);
- r = ttm_eu_reserve_buffers(&ticket, &list, false, &duplicates);
+ r = ttm_eu_reserve_buffers(&ticket, &list, false, &duplicates, true);
if (r) {
dev_err(adev->dev, "leaking bo va because "
"we fail to reserve bo (%d)\n", r);
@@ -608,7 +608,7 @@ int amdgpu_gem_va_ioctl(struct drm_device *dev, void *d
[PATCH 2/4] drm/ttm: fix busy memory to fail other user v7
From: Chunming Zhou
heavy gpu job could occupy memory long time, which lead other user fail to get
memory.
basically pick up Christian idea:
1. Reserve the BO in DC using a ww_mutex ticket (trivial).
2. If we then run into this EBUSY condition in TTM check if the BO we need
memory for (or rather the ww_mutex of its reservation object) has a ticket
assigned.
3. If we have a ticket we grab a reference to the first BO on the LRU, drop the
LRU lock and try to grab the reservation lock with the ticket.
4. If getting the reservation lock with the ticket succeeded we check if the BO
is still the first one on the LRU in question (the BO could have moved).
5. If the BO is still the first one on the LRU in question we try to evict it
as we would evict any other BO.
6. If any of the "If's" above fail we just back off and return -EBUSY.
v2: fix some minor check
v3: address Christian v2 comments.
v4: fix some missing
v5: handle first_bo unlock and bo_get/put
v6: abstract unified iterate function, and handle all possible usecase not only
pinned bo.
v7: pass request bo->resv to ttm_bo_evict_first
Change-Id: I21423fb922f885465f13833c41df1e134364a8e7
Signed-off-by: Chunming Zhou
---
drivers/gpu/drm/ttm/ttm_bo.c | 111 +--
1 file changed, 94 insertions(+), 17 deletions(-)
diff --git a/drivers/gpu/drm/ttm/ttm_bo.c b/drivers/gpu/drm/ttm/ttm_bo.c
index 2845fceb2fbd..e217a45d2841 100644
--- a/drivers/gpu/drm/ttm/ttm_bo.c
+++ b/drivers/gpu/drm/ttm/ttm_bo.c
@@ -766,11 +766,13 @@ EXPORT_SYMBOL(ttm_bo_eviction_valuable);
* b. Otherwise, trylock it.
*/
static bool ttm_bo_evict_swapout_allowable(struct ttm_buffer_object *bo,
- struct ttm_operation_ctx *ctx, bool *locked)
+ struct ttm_operation_ctx *ctx, bool *locked, bool *busy)
{
bool ret = false;
*locked = false;
+ if (busy)
+ *busy = false;
if (bo->resv == ctx->resv) {
reservation_object_assert_held(bo->resv);
if (ctx->flags & TTM_OPT_FLAG_ALLOW_RES_EVICT
@@ -779,35 +781,46 @@ static bool ttm_bo_evict_swapout_allowable(struct
ttm_buffer_object *bo,
} else {
*locked = reservation_object_trylock(bo->resv);
ret = *locked;
+ if (!ret && busy)
+ *busy = true;
}
return ret;
}
-static int ttm_mem_evict_first(struct ttm_bo_device *bdev,
- uint32_t mem_type,
- const struct ttm_place *place,
- struct ttm_operation_ctx *ctx)
+static struct ttm_buffer_object*
+ttm_mem_find_evitable_bo(struct ttm_bo_device *bdev,
+struct ttm_mem_type_manager *man,
+const struct ttm_place *place,
+struct ttm_operation_ctx *ctx,
+struct ttm_buffer_object **first_bo,
+bool *locked)
{
- struct ttm_bo_global *glob = bdev->glob;
- struct ttm_mem_type_manager *man = &bdev->man[mem_type];
struct ttm_buffer_object *bo = NULL;
- bool locked = false;
- unsigned i;
- int ret;
+ int i;
- spin_lock(&glob->lru_lock);
+ if (first_bo)
+ *first_bo = NULL;
for (i = 0; i < TTM_MAX_BO_PRIORITY; ++i) {
list_for_each_entry(bo, &man->lru[i], lru) {
- if (!ttm_bo_evict_swapout_allowable(bo, ctx, &locked))
+ bool busy = false;
+
+ if (!ttm_bo_evict_swapout_allowable(bo, ctx, locked,
+ &busy)) {
+ if (first_bo && !(*first_bo) && busy) {
+ ttm_bo_get(bo);
+ *first_bo = bo;
+ }
continue;
+ }
if (place && !bdev->driver->eviction_valuable(bo,
place)) {
- if (locked)
+ if (*locked)
reservation_object_unlock(bo->resv);
continue;
}
+
break;
}
@@ -818,9 +831,67 @@ static int ttm_mem_evict_first(struct ttm_bo_device *bdev,
bo = NULL;
}
+ return bo;
+}
+
+static int ttm_mem_evict_first(struct ttm_bo_device *bdev,
+ uint32_t mem_type,
+ const struct ttm_place *place,
+ struct ttm_operation_ctx *ctx,
+ struct reservation_object *request_resv)
+{
+ struct ttm_bo_global *glob = bdev->glob;
+ struct ttm_mem_type_manager *man = &bde
[PATCH 3/4] drm/amd/display: use ttm_eu_reserve_buffers instead of amdgpu_bo_reserve
From: Chunming Zhou
add ticket for display bo, so that it can preempt busy bo.
Change-Id: I9f031cdcc8267de00e819ae303baa0a52df8ebb9
Signed-off-by: Chunming Zhou
Reviewed-by: Christian König
---
.../gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c | 21 ++-
1 file changed, 16 insertions(+), 5 deletions(-)
diff --git a/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c
b/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c
index d7f9d3998641..f881f6678495 100644
--- a/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c
+++ b/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c
@@ -4176,6 +4176,9 @@ static int dm_plane_helper_prepare_fb(struct drm_plane
*plane,
struct amdgpu_device *adev;
struct amdgpu_bo *rbo;
struct dm_plane_state *dm_plane_state_new, *dm_plane_state_old;
+ struct list_head list;
+ struct ttm_validate_buffer tv;
+ struct ww_acquire_ctx ticket;
uint64_t tiling_flags;
uint32_t domain;
int r;
@@ -4192,9 +4195,17 @@ static int dm_plane_helper_prepare_fb(struct drm_plane
*plane,
obj = new_state->fb->obj[0];
rbo = gem_to_amdgpu_bo(obj);
adev = amdgpu_ttm_adev(rbo->tbo.bdev);
- r = amdgpu_bo_reserve(rbo, false);
- if (unlikely(r != 0))
+ INIT_LIST_HEAD(&list);
+
+ tv.bo = &rbo->tbo;
+ tv.num_shared = 1;
+ list_add(&tv.head, &list);
+
+ r = ttm_eu_reserve_buffers(&ticket, &list, false, NULL);
+ if (r) {
+ dev_err(adev->dev, "fail to reserve bo (%d)\n", r);
return r;
+ }
if (plane->type != DRM_PLANE_TYPE_CURSOR)
domain = amdgpu_display_supported_domains(adev);
@@ -4205,21 +4216,21 @@ static int dm_plane_helper_prepare_fb(struct drm_plane
*plane,
if (unlikely(r != 0)) {
if (r != -ERESTARTSYS)
DRM_ERROR("Failed to pin framebuffer with error %d\n",
r);
- amdgpu_bo_unreserve(rbo);
+ ttm_eu_backoff_reservation(&ticket, &list);
return r;
}
r = amdgpu_ttm_alloc_gart(&rbo->tbo);
if (unlikely(r != 0)) {
amdgpu_bo_unpin(rbo);
- amdgpu_bo_unreserve(rbo);
+ ttm_eu_backoff_reservation(&ticket, &list);
DRM_ERROR("%p bind failed\n", rbo);
return r;
}
amdgpu_bo_get_tiling_flags(rbo, &tiling_flags);
- amdgpu_bo_unreserve(rbo);
+ ttm_eu_backoff_reservation(&ticket, &list);
afb->address = amdgpu_bo_gpu_offset(rbo);
--
2.17.1
___
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel
[PATCH 4/4] drm/amdgpu: stop removing BOs from the LRU during CS
This avoids OOM situations when we have lots of threads
submitting at the same time.
Signed-off-by: Christian König
---
drivers/gpu/drm/amd/amdgpu/amdgpu_cs.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_cs.c
b/drivers/gpu/drm/amd/amdgpu/amdgpu_cs.c
index a1d6a0721e53..8828d30cd409 100644
--- a/drivers/gpu/drm/amd/amdgpu/amdgpu_cs.c
+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_cs.c
@@ -648,7 +648,7 @@ static int amdgpu_cs_parser_bos(struct amdgpu_cs_parser *p,
}
r = ttm_eu_reserve_buffers(&p->ticket, &p->validated, true,
- &duplicates, true);
+ &duplicates, false);
if (unlikely(r != 0)) {
if (r != -ERESTARTSYS)
DRM_ERROR("ttm_eu_reserve_buffers failed.\n");
--
2.17.1
___
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel
[Bug 110639] enc->enc_pic.enc_pic_order_cnt_type always zero even if pic->pic_order_cnt_type non-zero that application set
https://bugs.freedesktop.org/show_bug.cgi?id=110639 wangfengjuan changed: What|Removed |Added Version|18.0|19.1 -- You are receiving this mail because: You are the assignee for the bug.___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [RFC PATCH v2 0/5] new cgroup controller for gpu/drm subsystem
That looks better than I thought it would be. I think it is a good approach to try to add a global limit first and when that's working go ahead with limiting device specific resources. The only major issue I can see is on patch #4, see there for further details. Christian. Am 09.05.19 um 23:04 schrieb Kenny Ho: This is a follow up to the RFC I made last november to introduce a cgroup controller for the GPU/DRM subsystem [a]. The goal is to be able to provide resource management to GPU resources using things like container. The cover letter from v1 is copied below for reference. Usage examples: // set limit for card1 to 1GB sed -i '2s/.*/1073741824/' /sys/fs/cgroup//drm.buffer.total.max // set limit for card0 to 512MB sed -i '1s/.*/536870912/' /sys/fs/cgroup//drm.buffer.total.max v2: * Removed the vendoring concepts * Add limit to total buffer allocation * Add limit to the maximum size of a buffer allocation TODO: process migration TODO: documentations [a]: https://lists.freedesktop.org/archives/dri-devel/2018-November/197106.html v1: cover letter The purpose of this patch series is to start a discussion for a generic cgroup controller for the drm subsystem. The design proposed here is a very early one. We are hoping to engage the community as we develop the idea. Backgrounds == Control Groups/cgroup provide a mechanism for aggregating/partitioning sets of tasks, and all their future children, into hierarchical groups with specialized behaviour, such as accounting/limiting the resources which processes in a cgroup can access[1]. Weights, limits, protections, allocations are the main resource distribution models. Existing cgroup controllers includes cpu, memory, io, rdma, and more. cgroup is one of the foundational technologies that enables the popular container application deployment and management method. Direct Rendering Manager/drm contains code intended to support the needs of complex graphics devices. Graphics drivers in the kernel may make use of DRM functions to make tasks like memory management, interrupt handling and DMA easier, and provide a uniform interface to applications. The DRM has also developed beyond traditional graphics applications to support compute/GPGPU applications. Motivations = As GPU grow beyond the realm of desktop/workstation graphics into areas like data center clusters and IoT, there are increasing needs to monitor and regulate GPU as a resource like cpu, memory and io. Matt Roper from Intel began working on similar idea in early 2018 [2] for the purpose of managing GPU priority using the cgroup hierarchy. While that particular use case may not warrant a standalone drm cgroup controller, there are other use cases where having one can be useful [3]. Monitoring GPU resources such as VRAM and buffers, CU (compute unit [AMD's nomenclature])/EU (execution unit [Intel's nomenclature]), GPU job scheduling [4] can help sysadmins get a better understanding of the applications usage profile. Further usage regulations of the aforementioned resources can also help sysadmins optimize workload deployment on limited GPU resources. With the increased importance of machine learning, data science and other cloud-based applications, GPUs are already in production use in data centers today [5,6,7]. Existing GPU resource management is very course grain, however, as sysadmins are only able to distribute workload on a per-GPU basis [8]. An alternative is to use GPU virtualization (with or without SRIOV) but it generally acts on the entire GPU instead of the specific resources in a GPU. With a drm cgroup controller, we can enable alternate, fine-grain, sub-GPU resource management (in addition to what may be available via GPU virtualization.) In addition to production use, the DRM cgroup can also help with testing graphics application robustness by providing a mean to artificially limit DRM resources availble to the applications. Challenges While there are common infrastructure in DRM that is shared across many vendors (the scheduler [4] for example), there are also aspects of DRM that are vendor specific. To accommodate this, we borrowed the mechanism used by the cgroup to handle different kinds of cgroup controller. Resources for DRM are also often device (GPU) specific instead of system specific and a system may contain more than one GPU. For this, we borrowed some of the ideas from RDMA cgroup controller. Approach === To experiment with the idea of a DRM cgroup, we would like to start with basic accounting and statistics, then continue to iterate and add regulating mechanisms into the driver. [1] https://www.kernel.org/doc/Documentation/cgroup-v1/cgroups.txt [2] https://lists.freedesktop.org/archives/intel-gfx/2018-January/153156.html [3] https://www.spinics.net/lists/cgroups/msg20720.html [4] https://elixir.bootlin.com/linux/latest/source/drivers/gpu/drm/scheduler [5] https://kubernetes.io/docs/tasks/manage-gpus/scheduli
Re: [RFC PATCH v2 5/5] drm, cgroup: Add peak GEM buffer allocation limit
Am 09.05.19 um 23:04 schrieb Kenny Ho:
This new drmcgrp resource limits the largest GEM buffer that can be
allocated in a cgroup.
Change-Id: I0830d56775568e1cf215b56cc892d5e7945e9f25
Signed-off-by: Kenny Ho
---
include/linux/cgroup_drm.h | 2 ++
kernel/cgroup/drm.c| 59 ++
2 files changed, 61 insertions(+)
diff --git a/include/linux/cgroup_drm.h b/include/linux/cgroup_drm.h
index fe14ba7bb1cf..57c07a148975 100644
--- a/include/linux/cgroup_drm.h
+++ b/include/linux/cgroup_drm.h
@@ -16,8 +16,10 @@
struct drmcgrp_device_resource {
/* for per device stats */
s64 bo_stats_total_allocated;
+ size_t bo_stats_peak_allocated;
s64 bo_limits_total_allocated;
+ size_t bo_limits_peak_allocated;
Why s64 for the total limit and size_t for the peak allocation?
Christian.
};
struct drmcgrp {
diff --git a/kernel/cgroup/drm.c b/kernel/cgroup/drm.c
index bc3abff09113..5c7e1b8059ce 100644
--- a/kernel/cgroup/drm.c
+++ b/kernel/cgroup/drm.c
@@ -17,6 +17,7 @@ struct drmcgrp_device {
struct mutexmutex;
s64 bo_limits_total_allocated_default;
+ size_t bo_limits_peak_allocated_default;
};
#define DRMCG_CTF_PRIV_SIZE 3
@@ -24,6 +25,7 @@ struct drmcgrp_device {
enum drmcgrp_res_type {
DRMCGRP_TYPE_BO_TOTAL,
+ DRMCGRP_TYPE_BO_PEAK,
};
enum drmcgrp_file_type {
@@ -72,6 +74,9 @@ static inline int init_drmcgrp_single(struct drmcgrp
*drmcgrp, int i)
if (known_drmcgrp_devs[i] != NULL) {
ddr->bo_limits_total_allocated =
known_drmcgrp_devs[i]->bo_limits_total_allocated_default;
+
+ ddr->bo_limits_peak_allocated =
+ known_drmcgrp_devs[i]->bo_limits_peak_allocated_default;
}
return 0;
@@ -131,6 +136,9 @@ static inline void drmcgrp_print_stats(struct
drmcgrp_device_resource *ddr,
case DRMCGRP_TYPE_BO_TOTAL:
seq_printf(sf, "%lld\n", ddr->bo_stats_total_allocated);
break;
+ case DRMCGRP_TYPE_BO_PEAK:
+ seq_printf(sf, "%zu\n", ddr->bo_stats_peak_allocated);
+ break;
default:
seq_puts(sf, "\n");
break;
@@ -149,6 +157,9 @@ static inline void drmcgrp_print_limits(struct
drmcgrp_device_resource *ddr,
case DRMCGRP_TYPE_BO_TOTAL:
seq_printf(sf, "%lld\n", ddr->bo_limits_total_allocated);
break;
+ case DRMCGRP_TYPE_BO_PEAK:
+ seq_printf(sf, "%zu\n", ddr->bo_limits_peak_allocated);
+ break;
default:
seq_puts(sf, "\n");
break;
@@ -167,6 +178,9 @@ static inline void drmcgrp_print_default(struct
drmcgrp_device *ddev,
case DRMCGRP_TYPE_BO_TOTAL:
seq_printf(sf, "%lld\n",
ddev->bo_limits_total_allocated_default);
break;
+ case DRMCGRP_TYPE_BO_PEAK:
+ seq_printf(sf, "%zu\n", ddev->bo_limits_peak_allocated_default);
+ break;
default:
seq_puts(sf, "\n");
break;
@@ -182,6 +196,11 @@ static inline void drmcgrp_print_help(int cardNum, struct
seq_file *sf,
"Total amount of buffer allocation in bytes for card%d\n",
cardNum);
break;
+ case DRMCGRP_TYPE_BO_PEAK:
+ seq_printf(sf,
+ "Largest buffer allocation in bytes for card%d\n",
+ cardNum);
+ break;
default:
seq_puts(sf, "\n");
break;
@@ -254,6 +273,10 @@ ssize_t drmcgrp_bo_limit_write(struct kernfs_open_file
*of, char *buf,
if (val < 0) continue;
ddr->bo_limits_total_allocated = val;
break;
+ case DRMCGRP_TYPE_BO_PEAK:
+if (val < 0) continue;
+ ddr->bo_limits_peak_allocated = val;
+ break;
default:
break;
}
@@ -300,6 +323,33 @@ struct cftype files[] = {
.private = (DRMCGRP_TYPE_BO_TOTAL << DRMCG_CTF_PRIV_SIZE) |
DRMCGRP_FTYPE_MAX,
},
+ {
+ .name = "buffer.peak.stats",
+ .seq_show = drmcgrp_bo_show,
+ .private = (DRMCGRP_TYPE_BO_PEAK << DRMCG_CTF_PRIV_SIZE) |
+ DRMCGRP_FTYPE_STATS,
+ },
+ {
+ .name = "buffer.peak.default",
+ .seq_show = drmcgrp_bo_show,
+ .flags = CFTYPE_ONLY_ON_ROOT,
+ .private = (DRMCGRP_TYPE_BO_PEAK << DRMCG_CTF_PRIV_SIZE) |
+ DRMCGRP_FTYPE_DEFAULT,
+ },
+ {
+ .name
Re: [RFC PATCH v2 4/5] drm, cgroup: Add total GEM buffer allocation limit
Am 09.05.19 um 23:04 schrieb Kenny Ho:
The drm resource being measured and limited here is the GEM buffer
objects. User applications allocate and free these buffers. In
addition, a process can allocate a buffer and share it with another
process. The consumer of a shared buffer can also outlive the
allocator of the buffer.
For the purpose of cgroup accounting and limiting, ownership of the
buffer is deemed to be the cgroup for which the allocating process
belongs to. There is one limit per drm device.
In order to prevent the buffer outliving the cgroup that owns it, a
process is prevented from importing buffers that are not own by the
process' cgroup or the ancestors of the process' cgroup.
For this resource, the control files are prefixed with drm.buffer.total.
There are four control file types,
stats (ro) - display current measured values for a resource
max (rw) - limits for a resource
default (ro, root cgroup only) - default values for a resource
help (ro, root cgroup only) - help string for a resource
Each file is multi-lined with one entry/line per drm device.
Usage examples:
// set limit for card1 to 1GB
sed -i '2s/.*/1073741824/' /sys/fs/cgroup//drm.buffer.total.max
// set limit for card0 to 512MB
sed -i '1s/.*/536870912/' /sys/fs/cgroup//drm.buffer.total.max
Change-Id: I4c249d06d45ec709d6481d4cbe87c5168545c5d0
Signed-off-by: Kenny Ho
---
drivers/gpu/drm/amd/amdgpu/amdgpu_object.c | 4 +
drivers/gpu/drm/drm_gem.c | 7 +
drivers/gpu/drm/drm_prime.c| 9 +
include/drm/drm_cgroup.h | 34 ++-
include/drm/drm_gem.h | 11 +
include/linux/cgroup_drm.h | 3 +
kernel/cgroup/drm.c| 280 +
7 files changed, 346 insertions(+), 2 deletions(-)
diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_object.c
b/drivers/gpu/drm/amd/amdgpu/amdgpu_object.c
index 93b2c5a48a71..b4c078b7ad63 100644
--- a/drivers/gpu/drm/amd/amdgpu/amdgpu_object.c
+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_object.c
@@ -34,6 +34,7 @@
#include
#include
#include
+#include
#include "amdgpu.h"
#include "amdgpu_trace.h"
#include "amdgpu_amdkfd.h"
@@ -446,6 +447,9 @@ static int amdgpu_bo_do_create(struct amdgpu_device *adev,
if (!amdgpu_bo_validate_size(adev, size, bp->domain))
return -ENOMEM;
+ if (!drmcgrp_bo_can_allocate(current, adev->ddev, size))
+ return -ENOMEM;
+
*bo_ptr = NULL;
acc_size = ttm_bo_dma_acc_size(&adev->mman.bdev, size,
diff --git a/drivers/gpu/drm/drm_gem.c b/drivers/gpu/drm/drm_gem.c
index 6a80db077dc6..cbd49bf34dcf 100644
--- a/drivers/gpu/drm/drm_gem.c
+++ b/drivers/gpu/drm/drm_gem.c
@@ -37,10 +37,12 @@
#include
#include
#include
+#include
#include
#include
#include
#include
+#include
#include "drm_internal.h"
/** @file drm_gem.c
@@ -154,6 +156,9 @@ void drm_gem_private_object_init(struct drm_device *dev,
obj->handle_count = 0;
obj->size = size;
drm_vma_node_reset(&obj->vma_node);
+
+ obj->drmcgrp = get_drmcgrp(current);
+ drmcgrp_chg_bo_alloc(obj->drmcgrp, dev, size);
}
EXPORT_SYMBOL(drm_gem_private_object_init);
@@ -804,6 +809,8 @@ drm_gem_object_release(struct drm_gem_object *obj)
if (obj->filp)
fput(obj->filp);
+ drmcgrp_unchg_bo_alloc(obj->drmcgrp, obj->dev, obj->size);
+
drm_gem_free_mmap_offset(obj);
}
EXPORT_SYMBOL(drm_gem_object_release);
diff --git a/drivers/gpu/drm/drm_prime.c b/drivers/gpu/drm/drm_prime.c
index 231e3f6d5f41..faed5611a1c6 100644
--- a/drivers/gpu/drm/drm_prime.c
+++ b/drivers/gpu/drm/drm_prime.c
@@ -32,6 +32,7 @@
#include
#include
#include
+#include
#include "drm_internal.h"
@@ -794,6 +795,7 @@ int drm_gem_prime_fd_to_handle(struct drm_device *dev,
{
struct dma_buf *dma_buf;
struct drm_gem_object *obj;
+ struct drmcgrp *drmcgrp = get_drmcgrp(current);
int ret;
dma_buf = dma_buf_get(prime_fd);
@@ -818,6 +820,13 @@ int drm_gem_prime_fd_to_handle(struct drm_device *dev,
goto out_unlock;
}
+ /* only allow bo from the same cgroup or its ancestor to be imported */
+ if (drmcgrp != NULL &&
+ !drmcgrp_is_self_or_ancestor(drmcgrp, obj->drmcgrp)) {
+ ret = -EACCES;
+ goto out_unlock;
+ }
+
This will most likely go up in flames.
If I'm not completely mistaken we already use
drm_gem_prime_fd_to_handle() to exchange handles between different
cgroups in current container usages.
Christian.
if (obj->dma_buf) {
WARN_ON(obj->dma_buf != dma_buf);
} else {
diff --git a/include/drm/drm_cgroup.h b/include/drm/drm_cgroup.h
index ddb9eab64360..8711b7c5f7bf 100644
--- a/include/drm/drm_cgroup.h
+++ b/include/drm/drm_cgroup.h
@@ -4,12 +4,20 @@
#ifndef __DRM_CGROUP_H__
#define __DRM_CGROUP_H__
Re: [PATCH v7 09/11] drm: uevent for connector status change
Hi,
On Tue, 2019-05-07 at 21:57 +0530, Ramalingam C wrote:
> DRM API for generating uevent for a status changes of connector's
> property.
>
> This uevent will have following details related to the status change:
>
> HOTPLUG=1, CONNECTOR= and PROPERTY=
>
> Need ACK from this uevent from userspace consumer.
So we just had some discussions over on IRC and at about the hotplug
issue and came up with similar ideas:
https://lists.freedesktop.org/archives/dri-devel/2019-May/217408.html
The conclusions of these discussions so far would be to have a more or
less fine grain of uevent reporting depending on what happened. The
point is that we need to cover different cases:
- one or more properties changed;
- the connector status changed;
- something else about the connector changed (e.g. EDID/modes)
For the first case, we can send out:
HOTPLUG=1
CONNECTOR=
PROPERTY=
and no reprobe is required.
For the second one, something like:
HOTPLUG=1
CONNECTOR=
STATUS=Connected/Disconnected
and a connector probe is needed for connected, but not for
disconnected;
For the third one, we can only indicate the connector:
HOTPLUG=1
CONNECTOR=
and a reprobe of the connector is always needed
Then we still have the legacy case:
HOTPLUG=1
where userspace is expected to reprobe all the connectors.
I think this would deserve to be a separate series on its own. So I am
proposing to take this one off your plate and come up with another
seres implementing this proposal. What do you think?
Cheers,
Paul
> v2:
> Minor fixes at KDoc comments [Daniel]
> v3:
> Check the property is really attached with connector [Daniel]
>
> Signed-off-by: Ramalingam C
> Reviewed-by: Daniel Vetter
> ---
> drivers/gpu/drm/drm_sysfs.c | 35 +++
> include/drm/drm_sysfs.h | 5 -
> 2 files changed, 39 insertions(+), 1 deletion(-)
>
> diff --git a/drivers/gpu/drm/drm_sysfs.c b/drivers/gpu/drm/drm_sysfs.c
> index 18b1ac442997..63fa951a20db 100644
> --- a/drivers/gpu/drm/drm_sysfs.c
> +++ b/drivers/gpu/drm/drm_sysfs.c
> @@ -21,6 +21,7 @@
> #include
> #include
> #include "drm_internal.h"
> +#include "drm_crtc_internal.h"
>
> #define to_drm_minor(d) dev_get_drvdata(d)
> #define to_drm_connector(d) dev_get_drvdata(d)
> @@ -320,6 +321,9 @@ void drm_sysfs_lease_event(struct drm_device *dev)
> * Send a uevent for the DRM device specified by @dev. Currently we only
> * set HOTPLUG=1 in the uevent environment, but this could be expanded to
> * deal with other types of events.
> + *
> + * Any new uapi should be using the drm_sysfs_connector_status_event()
> + * for uevents on connector status change.
> */
> void drm_sysfs_hotplug_event(struct drm_device *dev)
> {
> @@ -332,6 +336,37 @@ void drm_sysfs_hotplug_event(struct drm_device *dev)
> }
> EXPORT_SYMBOL(drm_sysfs_hotplug_event);
>
> +/**
> + * drm_sysfs_connector_status_event - generate a DRM uevent for connector
> + * property status change
> + * @connector: connector on which property status changed
> + * @property: connector property whoes status changed.
> + *
> + * Send a uevent for the DRM device specified by @dev. Currently we
> + * set HOTPLUG=1 and connector id along with the attached property id
> + * related to the status change.
> + */
> +void drm_sysfs_connector_status_event(struct drm_connector *connector,
> + struct drm_property *property)
> +{
> + struct drm_device *dev = connector->dev;
> + char hotplug_str[] = "HOTPLUG=1", conn_id[30], prop_id[30];
> + char *envp[4] = { hotplug_str, conn_id, prop_id, NULL };
> +
> + WARN_ON(!drm_mode_obj_find_prop_id(&connector->base,
> +property->base.id));
> +
> + snprintf(conn_id, ARRAY_SIZE(conn_id),
> + "CONNECTOR=%u", connector->base.id);
> + snprintf(prop_id, ARRAY_SIZE(prop_id),
> + "PROPERTY=%u", property->base.id);
> +
> + DRM_DEBUG("generating connector status event\n");
> +
> + kobject_uevent_env(&dev->primary->kdev->kobj, KOBJ_CHANGE, envp);
> +}
> +EXPORT_SYMBOL(drm_sysfs_connector_status_event);
> +
> static void drm_sysfs_release(struct device *dev)
> {
> kfree(dev);
> diff --git a/include/drm/drm_sysfs.h b/include/drm/drm_sysfs.h
> index 4f311e836cdc..d454ef617b2c 100644
> --- a/include/drm/drm_sysfs.h
> +++ b/include/drm/drm_sysfs.h
> @@ -4,10 +4,13 @@
>
> struct drm_device;
> struct device;
> +struct drm_connector;
> +struct drm_property;
>
> int drm_class_device_register(struct device *dev);
> void drm_class_device_unregister(struct device *dev);
>
> void drm_sysfs_hotplug_event(struct drm_device *dev);
> -
> +void drm_sysfs_connector_status_event(struct drm_connector *connector,
> + struct drm_property *property);
> #endif
--
Paul Kocialkowski, Bootlin
Embedded Linux and kernel engineering
https://bootlin.com
___
dri-devel mailing list
[Bug 110248] IGT Meson build fails when "-Dbuild_tests=false" option is used
https://bugs.freedesktop.org/show_bug.cgi?id=110248 --- Comment #2 from Eero Tamminen --- Thanks, I can verify that the Meson doesn't anymore fail. However, it doesn't seem to disable all tests: $ grep tests intel-gpu-tools_build.log Message: Build tests: No [3/328] Compiling C object 'lib/tests/lib@tests@@igt_can_fail_simple@exe/igt_can_fail_simple.c.o'. [4/328] Compiling C object 'lib/tests/lib@tests@@igt_timeout@exe/igt_timeout.c.o'. [5/328] Compiling C object 'lib/tests/lib@tests@@igt_can_fail@exe/igt_can_fail.c.o'. [6/328] Compiling C object 'lib/tests/lib@tests@@igt_no_subtest@exe/igt_no_subtest.c.o'. [7/328] Compiling C object 'lib/tests/lib@tests@@igt_simple_test_subtests@exe/igt_simple_test_subtests.c.o'. [8/328] Compiling C object 'lib/tests/lib@tests@@igt_list_only@exe/igt_list_only.c.o'. [10/328] Compiling C object 'lib/tests/lib@tests@@igt_fork_helper@exe/igt_fork_helper.c.o'. [11/328] Compiling C object 'lib/tests/lib@tests@@igt_invalid_subtest_name@exe/igt_invalid_subtest_name.c.o'. [12/328] Compiling C object 'lib/tests/lib@tests@@igt_segfault@exe/igt_segfault.c.o'. [15/328] Compiling C object 'lib/tests/lib@tests@@igt_no_exit@exe/igt_no_exit.c.o'. [17/328] Compiling C object 'lib/tests/lib@tests@@igt_fork@exe/igt_fork.c.o'. [18/328] Compiling C object 'lib/tests/lib@tests@@igt_subtest_group@exe/igt_subtest_group.c.o'. [22/328] Compiling C object 'lib/tests/lib@tests@@igt_exit_handler@exe/igt_exit_handler.c.o'. [23/328] Compiling C object 'lib/tests/lib@tests@@igt_simulation@exe/igt_simulation.c.o'. [30/328] Compiling C object 'lib/tests/lib@tests@@igt_hdmi_inject@exe/igt_hdmi_inject.c.o'. [37/328] Compiling C object 'lib/tests/lib@tests@@igt_stats@exe/igt_stats.c.o'. [45/328] Compiling C object 'lib/tests/lib@tests@@igt_assert@exe/igt_assert.c.o'. Are these built on purpose even with tests building disabled? -- You are receiving this mail because: You are the assignee for the bug.___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [PATCH][next] drm/amdgpu: fix return of an uninitialized value in variable ret
On Fri, May 10, 2019 at 11:08:42AM +0100, Colin King wrote:
> From: Colin Ian King
>
> In the case where is_enable is false and lo_base_addr is non-zero the
> variable ret has not been initialized and is being checked for non-zero
> and potentially garbage is being returned. Fix this by not returning
> ret but instead returning -EINVAL on the zero lo_base_addr case.
>
> Addresses-Coverity: ("Uninitialized scalar variable")
> Fixes: a6ac0b44bab9 ("drm/amdgpu: add df perfmon regs and funcs for xgmi")
> Signed-off-by: Colin Ian King
> ---
> drivers/gpu/drm/amd/amdgpu/df_v3_6.c | 5 +
> 1 file changed, 1 insertion(+), 4 deletions(-)
>
> diff --git a/drivers/gpu/drm/amd/amdgpu/df_v3_6.c
> b/drivers/gpu/drm/amd/amdgpu/df_v3_6.c
> index a5c3558869fb..8c09bf994acd 100644
> --- a/drivers/gpu/drm/amd/amdgpu/df_v3_6.c
> +++ b/drivers/gpu/drm/amd/amdgpu/df_v3_6.c
> @@ -398,10 +398,7 @@ static int df_v3_6_start_xgmi_link_cntr(struct
> amdgpu_device *adev,
> NULL);
>
> if (lo_base_addr == 0)
> - ret = -EINVAL;
> -
> - if (ret)
> - return ret;
> + return -EINVAL;
From a naive reading of the code without knowing the hardware spec then
you would probably think that lo_base_addr can also be uninitialized.
regards,
dan carpenter
___
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel
[PATCH v2 5/6] drm/fourcc: Pass the format_info pointer to drm_format_plane_width/height
So far, the drm_format_plane_height/width functions were operating on the
format's fourcc and was doing a lookup to retrieve the drm_format_info
structure and return the cpp.
However, this is inefficient since in most cases, we will have the
drm_format_info pointer already available so we shouldn't have to perform a
new lookup. Some drm_fourcc functions also already operate on the
drm_format_info pointer for that reason, so the API is quite inconsistent
there.
Let's follow the latter pattern and remove the extra lookup while being a
bit more consistent.
In order to be extra consistent, also rename that function to
drm_format_info_plane_cpp and to a static function in the header to match
the current policy. The parameters order have also be changed to match the
other functions prototype.
Reviewed-by: Emil Velikov
Signed-off-by: Maxime Ripard
---
drivers/gpu/drm/drm_fourcc.c | 48 +
drivers/gpu/drm/meson/meson_overlay.c | 12 +++
include/drm/drm_fourcc.h | 46 +--
3 files changed, 50 insertions(+), 56 deletions(-)
diff --git a/drivers/gpu/drm/drm_fourcc.c b/drivers/gpu/drm/drm_fourcc.c
index 5f63fc74e265..35b459d186c5 100644
--- a/drivers/gpu/drm/drm_fourcc.c
+++ b/drivers/gpu/drm/drm_fourcc.c
@@ -333,54 +333,6 @@ drm_get_format_info(struct drm_device *dev,
EXPORT_SYMBOL(drm_get_format_info);
/**
- * drm_format_plane_width - width of the plane given the first plane
- * @width: width of the first plane
- * @format: pixel format
- * @plane: plane index
- *
- * Returns:
- * The width of @plane, given that the width of the first plane is @width.
- */
-int drm_format_plane_width(int width, uint32_t format, int plane)
-{
- const struct drm_format_info *info;
-
- info = drm_format_info(format);
- if (!info || plane >= info->num_planes)
- return 0;
-
- if (plane == 0)
- return width;
-
- return width / info->hsub;
-}
-EXPORT_SYMBOL(drm_format_plane_width);
-
-/**
- * drm_format_plane_height - height of the plane given the first plane
- * @height: height of the first plane
- * @format: pixel format
- * @plane: plane index
- *
- * Returns:
- * The height of @plane, given that the height of the first plane is @height.
- */
-int drm_format_plane_height(int height, uint32_t format, int plane)
-{
- const struct drm_format_info *info;
-
- info = drm_format_info(format);
- if (!info || plane >= info->num_planes)
- return 0;
-
- if (plane == 0)
- return height;
-
- return height / info->vsub;
-}
-EXPORT_SYMBOL(drm_format_plane_height);
-
-/**
* drm_format_info_block_width - width in pixels of block.
* @info: pixel format info
* @plane: plane index
diff --git a/drivers/gpu/drm/meson/meson_overlay.c
b/drivers/gpu/drm/meson/meson_overlay.c
index fb8515b2860c..55b3f2f2e608 100644
--- a/drivers/gpu/drm/meson/meson_overlay.c
+++ b/drivers/gpu/drm/meson/meson_overlay.c
@@ -466,8 +466,8 @@ static void meson_overlay_atomic_update(struct drm_plane
*plane,
priv->viu.vd1_addr2 = gem->paddr + fb->offsets[2];
priv->viu.vd1_stride2 = fb->pitches[2];
priv->viu.vd1_height2 =
- drm_format_plane_height(fb->height,
- fb->format->format, 2);
+ drm_format_info_plane_height(fb->format,
+ fb->height, 2);
DRM_DEBUG("plane 2 addr 0x%x stride %d height %d\n",
priv->viu.vd1_addr2,
priv->viu.vd1_stride2,
@@ -478,8 +478,8 @@ static void meson_overlay_atomic_update(struct drm_plane
*plane,
priv->viu.vd1_addr1 = gem->paddr + fb->offsets[1];
priv->viu.vd1_stride1 = fb->pitches[1];
priv->viu.vd1_height1 =
- drm_format_plane_height(fb->height,
- fb->format->format, 1);
+ drm_format_info_plane_height(fb->format,
+ fb->height, 1);
DRM_DEBUG("plane 1 addr 0x%x stride %d height %d\n",
priv->viu.vd1_addr1,
priv->viu.vd1_stride1,
@@ -490,8 +490,8 @@ static void meson_overlay_atomic_update(struct drm_plane
*plane,
priv->viu.vd1_addr0 = gem->paddr + fb->offsets[0];
priv->viu.vd1_stride0 = fb->pitches[0];
priv->viu.vd1_height0 =
- drm_format_plane_height(fb->height,
- fb->format->format, 0);
+ drm_format_info_plane_height(fb->format,
+ fb->height, 0);
DRM_DEBUG("plane 0 addr 0x%x stride %d height %d\n",
priv->viu.vd1_addr0,
[PATCH v2 1/6] drm/rockchip: Change the scl_vop_cal_scl_fac to pass drm_format_info
The Rockchip VOP driver has a function, scl_vop_cal_scl_fac, that will
lookup the drm_format_info structure from the fourcc passed to it by its
caller.
However, its only caller already derefences the drm_format_info structure
it has access to to retrieve that fourcc. Change the prototype of that
function to pass the drm_format_info structure directly, removing the need
for an extra lookup.
Suggested-by: Philipp Zabel
Signed-off-by: Maxime Ripard
---
drivers/gpu/drm/rockchip/rockchip_drm_vop.c | 11 ---
1 file changed, 4 insertions(+), 7 deletions(-)
diff --git a/drivers/gpu/drm/rockchip/rockchip_drm_vop.c
b/drivers/gpu/drm/rockchip/rockchip_drm_vop.c
index 20a9c296d027..9c0d6b367709 100644
--- a/drivers/gpu/drm/rockchip/rockchip_drm_vop.c
+++ b/drivers/gpu/drm/rockchip/rockchip_drm_vop.c
@@ -315,14 +315,13 @@ static uint16_t scl_vop_cal_scale(enum scale_mode mode,
uint32_t src,
static void scl_vop_cal_scl_fac(struct vop *vop, const struct vop_win_data
*win,
uint32_t src_w, uint32_t src_h, uint32_t dst_w,
-uint32_t dst_h, uint32_t pixel_format)
+uint32_t dst_h, const struct drm_format_info *info)
{
uint16_t yrgb_hor_scl_mode, yrgb_ver_scl_mode;
uint16_t cbcr_hor_scl_mode = SCALE_NONE;
uint16_t cbcr_ver_scl_mode = SCALE_NONE;
- int hsub = drm_format_horz_chroma_subsampling(pixel_format);
- int vsub = drm_format_vert_chroma_subsampling(pixel_format);
- const struct drm_format_info *info;
+ int hsub = drm_format_horz_chroma_subsampling(info->format);
+ int vsub = drm_format_vert_chroma_subsampling(info->format);
bool is_yuv = false;
uint16_t cbcr_src_w = src_w / hsub;
uint16_t cbcr_src_h = src_h / vsub;
@@ -331,8 +330,6 @@ static void scl_vop_cal_scl_fac(struct vop *vop, const
struct vop_win_data *win,
uint32_t val;
int vskiplines;
- info = drm_format_info(pixel_format);
-
if (info->is_yuv)
is_yuv = true;
@@ -856,7 +853,7 @@ static void vop_plane_atomic_update(struct drm_plane *plane,
if (win->phy->scl)
scl_vop_cal_scl_fac(vop, win, actual_w, actual_h,
drm_rect_width(dest), drm_rect_height(dest),
- fb->format->format);
+ fb->format);
VOP_WIN_SET(vop, win, act_info, act_info);
VOP_WIN_SET(vop, win, dsp_info, dsp_info);
base-commit: a802303934b3bd4df6e2fc8bf2e4ebced1c37556
--
git-series 0.9.1
___
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel
[PATCH v2 2/6] drm: Remove users of drm_format_num_planes
drm_format_num_planes() is basically a lookup in the drm_format_info table
plus an access to the num_planes field of the appropriate entry.
Most drivers are using this function while having access to the entry
already, which means that we will perform an unnecessary lookup. Removing
the call to drm_format_num_planes is therefore more efficient.
Some drivers will not have access to that entry in the function, but in
this case the overhead is minimal (we just have to call drm_format_info()
to perform the lookup) and we can even avoid multiple, inefficient lookups
in some places that need multiple fields from the drm_format_info
structure.
Reviewed-by: Emil Velikov
Reviewed-by: Paul Kocialkowski
Signed-off-by: Maxime Ripard
---
drivers/gpu/drm/arm/malidp_mw.c | 2 +-
drivers/gpu/drm/armada/armada_fb.c | 3 ++-
drivers/gpu/drm/drm_fourcc.c| 16
drivers/gpu/drm/mediatek/mtk_drm_fb.c | 6 --
drivers/gpu/drm/meson/meson_overlay.c | 2 +-
drivers/gpu/drm/msm/disp/dpu1/dpu_formats.c | 9 ++---
drivers/gpu/drm/msm/disp/mdp5/mdp5_smp.c| 3 ++-
drivers/gpu/drm/msm/msm_fb.c| 8 ++--
drivers/gpu/drm/omapdrm/omap_fb.c | 4 +++-
drivers/gpu/drm/rockchip/rockchip_drm_fb.c | 6 +++---
drivers/gpu/drm/tegra/fb.c | 3 ++-
drivers/gpu/drm/vc4/vc4_plane.c | 2 +-
drivers/gpu/drm/zte/zx_plane.c | 4 +---
include/drm/drm_fourcc.h| 1 -
14 files changed, 32 insertions(+), 37 deletions(-)
diff --git a/drivers/gpu/drm/arm/malidp_mw.c b/drivers/gpu/drm/arm/malidp_mw.c
index 5f102bdaf841..2e812525025d 100644
--- a/drivers/gpu/drm/arm/malidp_mw.c
+++ b/drivers/gpu/drm/arm/malidp_mw.c
@@ -158,7 +158,7 @@ malidp_mw_encoder_atomic_check(struct drm_encoder *encoder,
return -EINVAL;
}
- n_planes = drm_format_num_planes(fb->format->format);
+ n_planes = fb->format->num_planes;
for (i = 0; i < n_planes; i++) {
struct drm_gem_cma_object *obj = drm_fb_cma_get_gem_obj(fb, i);
/* memory write buffers are never rotated */
diff --git a/drivers/gpu/drm/armada/armada_fb.c
b/drivers/gpu/drm/armada/armada_fb.c
index 058ac7d9920f..a2f6472eb482 100644
--- a/drivers/gpu/drm/armada/armada_fb.c
+++ b/drivers/gpu/drm/armada/armada_fb.c
@@ -87,6 +87,7 @@ struct armada_framebuffer *armada_framebuffer_create(struct
drm_device *dev,
struct drm_framebuffer *armada_fb_create(struct drm_device *dev,
struct drm_file *dfile, const struct drm_mode_fb_cmd2 *mode)
{
+ const struct drm_format_info *info = drm_get_format_info(dev, mode);
struct armada_gem_object *obj;
struct armada_framebuffer *dfb;
int ret;
@@ -97,7 +98,7 @@ struct drm_framebuffer *armada_fb_create(struct drm_device
*dev,
mode->pitches[2]);
/* We can only handle a single plane at the moment */
- if (drm_format_num_planes(mode->pixel_format) > 1 &&
+ if (info->num_planes > 1 &&
(mode->handles[0] != mode->handles[1] ||
mode->handles[0] != mode->handles[2])) {
ret = -EINVAL;
diff --git a/drivers/gpu/drm/drm_fourcc.c b/drivers/gpu/drm/drm_fourcc.c
index 6ea55fb4526d..873c0001d8c8 100644
--- a/drivers/gpu/drm/drm_fourcc.c
+++ b/drivers/gpu/drm/drm_fourcc.c
@@ -333,22 +333,6 @@ drm_get_format_info(struct drm_device *dev,
EXPORT_SYMBOL(drm_get_format_info);
/**
- * drm_format_num_planes - get the number of planes for format
- * @format: pixel format (DRM_FORMAT_*)
- *
- * Returns:
- * The number of planes used by the specified pixel format.
- */
-int drm_format_num_planes(uint32_t format)
-{
- const struct drm_format_info *info;
-
- info = drm_format_info(format);
- return info ? info->num_planes : 1;
-}
-EXPORT_SYMBOL(drm_format_num_planes);
-
-/**
* drm_format_plane_cpp - determine the bytes per pixel value
* @format: pixel format (DRM_FORMAT_*)
* @plane: plane index
diff --git a/drivers/gpu/drm/mediatek/mtk_drm_fb.c
b/drivers/gpu/drm/mediatek/mtk_drm_fb.c
index e20fcaef2851..68fdef8b12bd 100644
--- a/drivers/gpu/drm/mediatek/mtk_drm_fb.c
+++ b/drivers/gpu/drm/mediatek/mtk_drm_fb.c
@@ -32,10 +32,11 @@ static struct drm_framebuffer
*mtk_drm_framebuffer_init(struct drm_device *dev,
const struct drm_mode_fb_cmd2 *mode,
struct drm_gem_object *obj)
{
+ const struct drm_format_info *info = drm_get_format_info(dev, mode);
struct drm_framebuffer *fb;
int ret;
- if (drm_format_num_planes(mode->pixel_format) != 1)
+ if (info->num_planes != 1)
return ERR_PTR(-EINVAL);
fb = kzalloc(sizeof(*fb), GFP_KERNEL);
@@ -88,6 +89,7 @@ struct drm_framebuffer *mtk_drm_mode_fb_create(struct
drm_device *dev,
struct drm_file *file,
[PATCH v2 3/6] drm: Remove users of drm_format_(horz|vert)_chroma_subsampling
drm_format_horz_chroma_subsampling and drm_format_vert_chroma_subsampling
are basically a lookup in the drm_format_info table plus an access to the
hsub and vsub fields of the appropriate entry.
Most drivers are using this function while having access to the entry
already, which means that we will perform an unnecessary lookup. Removing
the call to these functions is therefore more efficient.
Some drivers will not have access to that entry in the function, but in
this case the overhead is minimal (we just have to call drm_format_info()
to perform the lookup) and we can even avoid multiple, inefficient lookups
in some places that need multiple fields from the drm_format_info
structure.
This is amplified by the fact that most of the time the callers will have
to retrieve both the vsub and hsub fields, meaning that they would perform
twice the lookup.
Reviewed-by: Emil Velikov
Reviewed-by: Paul Kocialkowski
Reviewed-by: Philipp Zabel
Signed-off-by: Maxime Ripard
---
drivers/gpu/drm/arm/malidp_planes.c | 6 +--
drivers/gpu/drm/atmel-hlcdc/atmel_hlcdc_plane.c | 9 +
drivers/gpu/drm/drm_fourcc.c| 34 +--
drivers/gpu/drm/imx/ipuv3-plane.c | 15 +++-
drivers/gpu/drm/msm/disp/dpu1/dpu_plane.c | 9 +
drivers/gpu/drm/msm/disp/mdp5/mdp5_plane.c | 24 +
drivers/gpu/drm/msm/disp/mdp5/mdp5_smp.c| 2 +-
drivers/gpu/drm/msm/msm_fb.c| 8 +---
drivers/gpu/drm/rockchip/rockchip_drm_fb.c | 9 +
drivers/gpu/drm/rockchip/rockchip_drm_vop.c | 10 ++---
drivers/gpu/drm/tegra/fb.c | 9 +
drivers/gpu/drm/vc4/vc4_plane.c | 13 ++-
include/drm/drm_fourcc.h| 2 +-
13 files changed, 38 insertions(+), 112 deletions(-)
diff --git a/drivers/gpu/drm/arm/malidp_planes.c
b/drivers/gpu/drm/arm/malidp_planes.c
index d42e0ea9a303..8f89813d08c1 100644
--- a/drivers/gpu/drm/arm/malidp_planes.c
+++ b/drivers/gpu/drm/arm/malidp_planes.c
@@ -233,8 +233,7 @@ bool malidp_format_mod_supported(struct drm_device *drm,
}
}
- if ((drm_format_horz_chroma_subsampling(format) != 1) ||
- (drm_format_vert_chroma_subsampling(format) != 1)) {
+ if ((info->hsub != 1) || (info->vsub != 1)) {
if (!(format == DRM_FORMAT_YUV420_10BIT &&
(map->features &
MALIDP_DEVICE_AFBC_YUV_420_10_SUPPORT_SPLIT))) {
DRM_DEBUG_KMS("Formats which are sub-sampled
should never be split\n");
@@ -244,8 +243,7 @@ bool malidp_format_mod_supported(struct drm_device *drm,
}
if (modifier & AFBC_CBR) {
- if ((drm_format_horz_chroma_subsampling(format) == 1) ||
- (drm_format_vert_chroma_subsampling(format) == 1)) {
+ if ((info->hsub == 1) || (info->vsub == 1)) {
DRM_DEBUG_KMS("Formats which are not sub-sampled should
not have CBR set\n");
return false;
}
diff --git a/drivers/gpu/drm/atmel-hlcdc/atmel_hlcdc_plane.c
b/drivers/gpu/drm/atmel-hlcdc/atmel_hlcdc_plane.c
index e836e2de35ce..fdd607ad27fe 100644
--- a/drivers/gpu/drm/atmel-hlcdc/atmel_hlcdc_plane.c
+++ b/drivers/gpu/drm/atmel-hlcdc/atmel_hlcdc_plane.c
@@ -603,8 +603,6 @@ static int atmel_hlcdc_plane_atomic_check(struct drm_plane
*p,
const struct drm_display_mode *mode;
struct drm_crtc_state *crtc_state;
unsigned int tmp;
- int hsub = 1;
- int vsub = 1;
int ret;
int i;
@@ -642,13 +640,10 @@ static int atmel_hlcdc_plane_atomic_check(struct
drm_plane *p,
if (state->nplanes > ATMEL_HLCDC_LAYER_MAX_PLANES)
return -EINVAL;
- hsub = drm_format_horz_chroma_subsampling(fb->format->format);
- vsub = drm_format_vert_chroma_subsampling(fb->format->format);
-
for (i = 0; i < state->nplanes; i++) {
unsigned int offset = 0;
- int xdiv = i ? hsub : 1;
- int ydiv = i ? vsub : 1;
+ int xdiv = i ? fb->format->hsub : 1;
+ int ydiv = i ? fb->format->vsub : 1;
state->bpp[i] = fb->format->cpp[i];
if (!state->bpp[i])
diff --git a/drivers/gpu/drm/drm_fourcc.c b/drivers/gpu/drm/drm_fourcc.c
index 873c0001d8c8..e4a2c8372c8b 100644
--- a/drivers/gpu/drm/drm_fourcc.c
+++ b/drivers/gpu/drm/drm_fourcc.c
@@ -353,40 +353,6 @@ int drm_format_plane_cpp(uint32_t format, int plane)
EXPORT_SYMBOL(drm_format_plane_cpp);
/**
- * drm_format_horz_chroma_subsampling - get the horizontal chroma subsampling
factor
- * @format: pixel format (DRM_FORMAT_*)
- *
- * Returns:
- * The horizontal chroma subsampling factor for the
- * specified pixel format.
- */
-int drm_format_horz_chroma_subsampling(uint32_t format)
-{
- const struct drm_format_info *i
[PATCH v2 4/6] drm/fourcc: Pass the format_info pointer to drm_format_plane_cpp
So far, the drm_format_plane_cpp function was operating on the format's
fourcc and was doing a lookup to retrieve the drm_format_info structure and
return the cpp.
However, this is inefficient since in most cases, we will have the
drm_format_info pointer already available so we shouldn't have to perform a
new lookup. Some drm_fourcc functions also already operate on the
drm_format_info pointer for that reason, so the API is quite inconsistent
there.
Let's follow the latter pattern and remove the extra lookup while being a
bit more consistent. In order to be extra consistent, also rename that
function to drm_format_info_plane_cpp and to a static function in the
header to match the current policy.
Reviewed-by: Emil Velikov
Signed-off-by: Maxime Ripard
---
drivers/gpu/drm/amd/amdgpu/amdgpu_fb.c | 4 +++-
drivers/gpu/drm/arm/malidp_hw.c| 3 ++-
drivers/gpu/drm/arm/malidp_planes.c| 2 +-
drivers/gpu/drm/drm_client.c | 3 ++-
drivers/gpu/drm/drm_fb_helper.c| 2 +-
drivers/gpu/drm/drm_format_helper.c| 4 ++--
drivers/gpu/drm/drm_fourcc.c | 20
drivers/gpu/drm/i915/intel_sprite.c| 3 ++-
drivers/gpu/drm/mediatek/mtk_drm_fb.c | 2 +-
drivers/gpu/drm/msm/disp/mdp5/mdp5_crtc.c | 3 ++-
drivers/gpu/drm/msm/disp/mdp5/mdp5_smp.c | 2 +-
drivers/gpu/drm/msm/msm_fb.c | 2 +-
drivers/gpu/drm/radeon/radeon_fb.c | 4 +++-
drivers/gpu/drm/rockchip/rockchip_drm_fb.c | 2 +-
drivers/gpu/drm/stm/ltdc.c | 2 +-
drivers/gpu/drm/tegra/fb.c | 2 +-
drivers/gpu/drm/zte/zx_plane.c | 2 +-
include/drm/drm_fourcc.h | 18 +-
18 files changed, 42 insertions(+), 38 deletions(-)
diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_fb.c
b/drivers/gpu/drm/amd/amdgpu/amdgpu_fb.c
index e47609218839..06e73a343724 100644
--- a/drivers/gpu/drm/amd/amdgpu/amdgpu_fb.c
+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_fb.c
@@ -121,6 +121,8 @@ static int amdgpufb_create_pinned_object(struct
amdgpu_fbdev *rfbdev,
struct drm_mode_fb_cmd2 *mode_cmd,
struct drm_gem_object **gobj_p)
{
+ const struct drm_format_info *info = drm_get_format_info(dev,
+mode_cmd);
struct amdgpu_device *adev = rfbdev->adev;
struct drm_gem_object *gobj = NULL;
struct amdgpu_bo *abo = NULL;
@@ -131,7 +133,7 @@ static int amdgpufb_create_pinned_object(struct
amdgpu_fbdev *rfbdev,
int height = mode_cmd->height;
u32 cpp;
- cpp = drm_format_plane_cpp(mode_cmd->pixel_format, 0);
+ cpp = drm_format_info_plane_cpp(info, 0);
/* need to align pitch with crtc limits */
mode_cmd->pitches[0] = amdgpu_align_pitch(adev, mode_cmd->width, cpp,
diff --git a/drivers/gpu/drm/arm/malidp_hw.c b/drivers/gpu/drm/arm/malidp_hw.c
index 8df12e9a33bb..1c9e869f4c52 100644
--- a/drivers/gpu/drm/arm/malidp_hw.c
+++ b/drivers/gpu/drm/arm/malidp_hw.c
@@ -382,7 +382,8 @@ static void malidp500_modeset(struct malidp_hw_device
*hwdev, struct videomode *
int malidp_format_get_bpp(u32 fmt)
{
- int bpp = drm_format_plane_cpp(fmt, 0) * 8;
+ const struct drm_format_info *info = drm_format_info(fmt);
+ int bpp = drm_format_info_plane_cpp(info, 0) * 8;
if (bpp == 0) {
switch (fmt) {
diff --git a/drivers/gpu/drm/arm/malidp_planes.c
b/drivers/gpu/drm/arm/malidp_planes.c
index 8f89813d08c1..361c02988375 100644
--- a/drivers/gpu/drm/arm/malidp_planes.c
+++ b/drivers/gpu/drm/arm/malidp_planes.c
@@ -227,7 +227,7 @@ bool malidp_format_mod_supported(struct drm_device *drm,
if (modifier & AFBC_SPLIT) {
if (!info->is_yuv) {
- if (drm_format_plane_cpp(format, 0) <= 2) {
+ if (drm_format_info_plane_cpp(info, 0) <= 2) {
DRM_DEBUG_KMS("RGB formats <= 16bpp are not
supported with SPLIT\n");
return false;
}
diff --git a/drivers/gpu/drm/drm_client.c b/drivers/gpu/drm/drm_client.c
index f20d1dda3961..169d8eeaa662 100644
--- a/drivers/gpu/drm/drm_client.c
+++ b/drivers/gpu/drm/drm_client.c
@@ -243,6 +243,7 @@ static void drm_client_buffer_delete(struct
drm_client_buffer *buffer)
static struct drm_client_buffer *
drm_client_buffer_create(struct drm_client_dev *client, u32 width, u32 height,
u32 format)
{
+ const struct drm_format_info *info = drm_format_info(format);
struct drm_mode_create_dumb dumb_args = { };
struct drm_device *dev = client->dev;
struct drm_client_buffer *buffer;
@@ -258,7 +259,7 @@ drm_client_buffer_create(struct drm_client_dev *client, u32
width, u32 height, u
dumb_args.width = width;
dumb_args.height = height;
- dumb_args
Re: [PATCH 09/16] mmc: sdhci-xenon: use new match_string() helper/macro
On Fri, May 10, 2019 at 09:13:26AM +, Ardelean, Alexandru wrote:
> On Wed, 2019-05-08 at 16:26 +0300, Alexandru Ardelean wrote:
> > On Wed, 2019-05-08 at 15:20 +0300, Dan Carpenter wrote:
> > >
> > >
> > > On Wed, May 08, 2019 at 02:28:35PM +0300, Alexandru Ardelean wrote:
> > > > -static const char * const phy_types[] = {
> > > > - "emmc 5.0 phy",
> > > > - "emmc 5.1 phy"
> > > > -};
> > > > -
> > > > enum xenon_phy_type_enum {
> > > > EMMC_5_0_PHY,
> > > > EMMC_5_1_PHY,
> > > > NR_PHY_TYPES
> > >
> > > There is no need for NR_PHY_TYPES now so you could remove that as well.
> > >
> >
> > I thought the same.
> > The only reason to keep NR_PHY_TYPES, is for potential future patches,
> > where it would be just 1 addition
> >
> > enum xenon_phy_type_enum {
> > EMMC_5_0_PHY,
> > EMMC_5_1_PHY,
> > + EMMC_5_2_PHY,
> > NR_PHY_TYPES
> > }
> >
> > Depending on style/preference of how to do enums (allow comma on last
> > enum
> > or not allow comma on last enum value), adding new enum values woudl be 2
> > additions + 1 deletion lines.
> >
> > enum xenon_phy_type_enum {
> > EMMC_5_0_PHY,
> > - EMMC_5_1_PHY
> > + EMM
> > C_5_1_PHY,
> > + EMMC_5_2_PHY
> > }
> >
> > Either way (leave NR_PHY_TYPES or remove NR_PHY_TYPES) is fine from my
> > side.
> >
>
> Preference on this ?
> If no objection [nobody insists] I would keep.
>
> I don't feel strongly about it [dropping NR_PHY_TYPES or not].
If you end up resending the series could you remove it, but if not then
it's not worth it.
regards,
dan carpenter
___
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel
[PATCH v2 6/6] drm: Replace instances of drm_format_info by drm_get_format_info
drm_get_format_info directly calls into drm_format_info, but takes directly
a struct drm_mode_fb_cmd2 pointer, instead of the fourcc directly. It's
shorter to not dereference it, and we can customise the behaviour at the
driver level if we want to, so let's switch to it where it makes sense.
Reviewed-by: Emil Velikov
Reviewed-by: Paul Kocialkowski
Signed-off-by: Maxime Ripard
---
drivers/gpu/drm/amd/amdgpu/amdgpu_fb.c | 4 ++--
drivers/gpu/drm/gma500/framebuffer.c | 2 +-
drivers/gpu/drm/omapdrm/omap_fb.c | 2 +-
3 files changed, 4 insertions(+), 4 deletions(-)
diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_fb.c
b/drivers/gpu/drm/amd/amdgpu/amdgpu_fb.c
index 06e73a343724..6edae6458be8 100644
--- a/drivers/gpu/drm/amd/amdgpu/amdgpu_fb.c
+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_fb.c
@@ -121,8 +121,7 @@ static int amdgpufb_create_pinned_object(struct
amdgpu_fbdev *rfbdev,
struct drm_mode_fb_cmd2 *mode_cmd,
struct drm_gem_object **gobj_p)
{
- const struct drm_format_info *info = drm_get_format_info(dev,
-mode_cmd);
+ const struct drm_format_info *info;
struct amdgpu_device *adev = rfbdev->adev;
struct drm_gem_object *gobj = NULL;
struct amdgpu_bo *abo = NULL;
@@ -133,6 +132,7 @@ static int amdgpufb_create_pinned_object(struct
amdgpu_fbdev *rfbdev,
int height = mode_cmd->height;
u32 cpp;
+ info = drm_get_format_info(adev->ddev, mode_cmd);
cpp = drm_format_info_plane_cpp(info, 0);
/* need to align pitch with crtc limits */
diff --git a/drivers/gpu/drm/gma500/framebuffer.c
b/drivers/gpu/drm/gma500/framebuffer.c
index a9d3a4a30ab8..1794ab90b2cc 100644
--- a/drivers/gpu/drm/gma500/framebuffer.c
+++ b/drivers/gpu/drm/gma500/framebuffer.c
@@ -232,7 +232,7 @@ static int psb_framebuffer_init(struct drm_device *dev,
* Reject unknown formats, YUV formats, and formats with more than
* 4 bytes per pixel.
*/
- info = drm_format_info(mode_cmd->pixel_format);
+ info = drm_get_format_info(dev, mode_cmd);
if (!info || !info->depth || info->cpp[0] > 4)
return -EINVAL;
diff --git a/drivers/gpu/drm/omapdrm/omap_fb.c
b/drivers/gpu/drm/omapdrm/omap_fb.c
index cfb641363a32..6557b2d6e16e 100644
--- a/drivers/gpu/drm/omapdrm/omap_fb.c
+++ b/drivers/gpu/drm/omapdrm/omap_fb.c
@@ -339,7 +339,7 @@ struct drm_framebuffer *omap_framebuffer_init(struct
drm_device *dev,
dev, mode_cmd, mode_cmd->width, mode_cmd->height,
(char *)&mode_cmd->pixel_format);
- format = drm_format_info(mode_cmd->pixel_format);
+ format = drm_get_format_info(dev, mode_cmd);
for (i = 0; i < ARRAY_SIZE(formats); i++) {
if (formats[i] == mode_cmd->pixel_format)
--
git-series 0.9.1
___
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel
[Bug 110248] IGT Meson build fails when "-Dbuild_tests=false" option is used
https://bugs.freedesktop.org/show_bug.cgi?id=110248 Petri Latvala changed: What|Removed |Added Resolution|--- |FIXED Status|NEW |RESOLVED --- Comment #1 from Petri Latvala --- commit 159c69e5318e934b5f1d83939191c8756280b758 Author: Petri Latvala Date: Fri Apr 26 14:05:41 2019 +0300 runner: Disable building if tests are not built -- You are receiving this mail because: You are the assignee for the bug.___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [PATCH v2 00/17] kunit: introduce KUnit, the Linux kernel unit testing framework
On Thu, May 09, 2019 at 10:11:01PM -0700, Frank Rowand wrote: > >> You *can* run in-kernel test using modules; but there is no framework > >> for the in-kernel code found in the test modules, which means each of > >> the in-kernel code has to create their own in-kernel test > >> infrastructure. > > The kselftest in-kernel tests follow a common pattern. As such, there > is a framework. So we may have different definitions of "framework". In my book, code reuse by "cut and paste" does not make a framework. Could they be rewritten to *use* a framework, whether it be KTF or KUnit? Sure! But they are not using a framework *today*. > This next two paragraphs you ignored entirely in your reply: > > > Why create an entire new subsystem (KUnit) when you can add a header > > file (and .c code as appropriate) that outputs the proper TAP formatted > > results from kselftest kernel test modules? And you keep ignoring my main observation, which is that spinning up a VM, letting systemd start, mounting a root file system, etc., is all unnecessary overhead which takes time. This is important to me, because developer velocity is extremely important if you are doing test driven development. Yes, you can manually unload a module, recompile the module, somehow get the module back into the VM (perhaps by using virtio-9p), and then reloading the module with the in-kernel test code, and the restart the test. BUT: (a) even if it is faster, it requires a lot of manual steps, and would be very hard to automate, and (b) if the test code ever OOPS or triggers a lockdep warning, you will need to restart the VM, and so this involves all of the VM restart overhead, plus trying to automate determining when you actually do need to restart the VM versus unloading and reloading the module. It's clunky. Being able to do the equivalent of "make && make check" is a really big deal. And "make check" needs to go fast. You keep ignore this point, perhaps because you don't care about this issue? Which is fine, and why we may just need to agree to disagree. Cheers, - Ted P.S. Running scripts is Turing-equivalent, so it's self-evident that *anything* you can do with other test frameworks you can somehow do in kselftests. That argument doesn't impress me, and why I do consider it quite flippant. (Heck, /bin/vi is Turing equivalent so we could use vi to as a kernel test framework. Or we could use emacs. Let's not. :-) The question is whether it is the most best and most efficient way to do that testing. And developer velocity is a really big part of my evaluation function when judging whether or a test framework is fit for that purpose. ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [PATCH v2 06/17] kbuild: enable building KUnit
> On Fri, May 10, 2019 at 7:27 PM Brendan Higgins > wrote: > > > > > On Thu, May 2, 2019 at 8:03 AM Brendan Higgins > > > wrote: > > > > > > > > Add KUnit to root Kconfig and Makefile allowing it to actually be built. > > > > > > > > Signed-off-by: Brendan Higgins > > > > > > You need to make sure > > > to not break git-bisect'abililty. > > > > > > > > > With this commit, I see build error. > > > > > > CC kunit/test.o > > > kunit/test.c:11:10: fatal error: os.h: No such file or directory > > > #include > > > ^~ > > > compilation terminated. > > > make[1]: *** [scripts/Makefile.build;279: kunit/test.o] Error 1 > > > make: *** [Makefile;1763: kunit/] Error 2 > > > > Nice catch! That header shouldn't even be in there. > > > > Sorry about that. I will have it fixed in the next revision. > > > BTW, I applied whole of this series > to my kernel.org repository. > > 0day bot started to report issues. > I hope several reports reached you, > and they are useful to fix your code. Yep, I have received several. They are very helpful. I greatly appreciate it. Thanks! ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [PATCH v2 06/17] kbuild: enable building KUnit
> On Thu, May 2, 2019 at 8:03 AM Brendan Higgins > wrote: > > > > Add KUnit to root Kconfig and Makefile allowing it to actually be built. > > > > Signed-off-by: Brendan Higgins > > You need to make sure > to not break git-bisect'abililty. > > > With this commit, I see build error. > > CC kunit/test.o > kunit/test.c:11:10: fatal error: os.h: No such file or directory > #include > ^~ > compilation terminated. > make[1]: *** [scripts/Makefile.build;279: kunit/test.o] Error 1 > make: *** [Makefile;1763: kunit/] Error 2 Nice catch! That header shouldn't even be in there. Sorry about that. I will have it fixed in the next revision. ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [PATCH v2 00/17] kunit: introduce KUnit, the Linux kernel unit testing framework
> On Thu, May 2, 2019 at 8:02 AM Brendan Higgins > wrote: > > > > ## TLDR > > > > I rebased the last patchset on 5.1-rc7 in hopes that we can get this in > > 5.2. > > > > Shuah, I think you, Greg KH, and myself talked off thread, and we agreed > > we would merge through your tree when the time came? Am I remembering > > correctly? > > > > ## Background > > > > This patch set proposes KUnit, a lightweight unit testing and mocking > > framework for the Linux kernel. > > > > Unlike Autotest and kselftest, KUnit is a true unit testing framework; > > it does not require installing the kernel on a test machine or in a VM > > and does not require tests to be written in userspace running on a host > > kernel. Additionally, KUnit is fast: From invocation to completion KUnit > > can run several dozen tests in under a second. Currently, the entire > > KUnit test suite for KUnit runs in under a second from the initial > > invocation (build time excluded). > > > > KUnit is heavily inspired by JUnit, Python's unittest.mock, and > > Googletest/Googlemock for C++. KUnit provides facilities for defining > > unit test cases, grouping related test cases into test suites, providing > > common infrastructure for running tests, mocking, spying, and much more. > > > > ## What's so special about unit testing? > > > > A unit test is supposed to test a single unit of code in isolation, > > hence the name. There should be no dependencies outside the control of > > the test; this means no external dependencies, which makes tests orders > > of magnitudes faster. Likewise, since there are no external dependencies, > > there are no hoops to jump through to run the tests. Additionally, this > > makes unit tests deterministic: a failing unit test always indicates a > > problem. Finally, because unit tests necessarily have finer granularity, > > they are able to test all code paths easily solving the classic problem > > of difficulty in exercising error handling code. > > > > ## Is KUnit trying to replace other testing frameworks for the kernel? > > > > No. Most existing tests for the Linux kernel are end-to-end tests, which > > have their place. A well tested system has lots of unit tests, a > > reasonable number of integration tests, and some end-to-end tests. KUnit > > is just trying to address the unit test space which is currently not > > being addressed. > > > > ## More information on KUnit > > > > There is a bunch of documentation near the end of this patch set that > > describes how to use KUnit and best practices for writing unit tests. > > For convenience I am hosting the compiled docs here: > > https://google.github.io/kunit-docs/third_party/kernel/docs/ > > Additionally for convenience, I have applied these patches to a branch: > > https://kunit.googlesource.com/linux/+/kunit/rfc/v5.1-rc7/v1 > > The repo may be cloned with: > > git clone https://kunit.googlesource.com/linux > > This patchset is on the kunit/rfc/v5.1-rc7/v1 branch. > > > > ## Changes Since Last Version > > > > None. I just rebased the last patchset on v5.1-rc7. > > > > -- > > 2.21.0.593.g511ec345e18-goog > > > > The following is the log of 'git am' of this series. > I see several 'new blank line at EOF' warnings. > > > > masahiro@pug:~/workspace/bsp/linux$ git am ~/Downloads/*.patch > Applying: kunit: test: add KUnit test runner core > Applying: kunit: test: add test resource management API > Applying: kunit: test: add string_stream a std::stream like string builder > .git/rebase-apply/patch:223: new blank line at EOF. > + > warning: 1 line adds whitespace errors. > Applying: kunit: test: add kunit_stream a std::stream like logger > Applying: kunit: test: add the concept of expectations > .git/rebase-apply/patch:475: new blank line at EOF. > + > warning: 1 line adds whitespace errors. > Applying: kbuild: enable building KUnit > Applying: kunit: test: add initial tests > .git/rebase-apply/patch:203: new blank line at EOF. > + > warning: 1 line adds whitespace errors. > Applying: kunit: test: add support for test abort > .git/rebase-apply/patch:453: new blank line at EOF. > + > warning: 1 line adds whitespace errors. > Applying: kunit: test: add tests for kunit test abort > Applying: kunit: test: add the concept of assertions > .git/rebase-apply/patch:518: new blank line at EOF. > + > warning: 1 line adds whitespace errors. > Applying: kunit: test: add test managed resource tests > Applying: kunit: tool: add Python wrappers for running KUnit tests > .git/rebase-apply/patch:457: new blank line at EOF. > + > warning: 1 line adds whitespace errors. > Applying: kunit: defconfig: add defconfigs for building KUnit tests > Applying: Documentation: kunit: add documentation for KUnit > .git/rebase-apply/patch:71: new blank line at EOF. > + > .git/rebase-apply/patch:209: new blank line at EOF. > + > .git/rebase-apply/patch:848: new blank line at EOF. > + > warning: 3 lines add whitespace errors. > Applying: MAINTAINERS: add entry for KUnit the unit testing framework > Appl
Re: [PATCH v2 00/17] kunit: introduce KUnit, the Linux kernel unit testing framework
> On Fri, May 10, 2019 at 7:49 AM Knut Omang wrote: > > > > On Thu, 2019-05-09 at 22:18 -0700, Frank Rowand wrote: > > > On 5/9/19 4:40 PM, Logan Gunthorpe wrote: > > > > > > > > > > > > On 2019-05-09 5:30 p.m., Theodore Ts'o wrote: > > > >> On Thu, May 09, 2019 at 04:20:05PM -0600, Logan Gunthorpe wrote: > > > >>> > > > >>> The second item, arguably, does have significant overlap with > > > >>> kselftest. > > > >>> Whether you are running short tests in a light weight UML environment > > > >>> or > > > >>> higher level tests in an heavier VM the two could be using the same > > > >>> framework for writing or defining in-kernel tests. It *may* also be > > > >>> valuable > > > >>> for some people to be able to run all the UML tests in the heavy VM > > > >>> environment along side other higher level tests. > > > >>> > > > >>> Looking at the selftests tree in the repo, we already have similar > > > >>> items to > > > >>> what Kunit is adding as I described in point (2) above. > > > >>> kselftest_harness.h > > > >>> contains macros like EXPECT_* and ASSERT_* with very similar > > > >>> intentions to > > > >>> the new KUNIT_EXECPT_* and KUNIT_ASSERT_* macros. > > > >>> > > > >>> However, the number of users of this harness appears to be quite > > > >>> small. Most > > > >>> of the code in the selftests tree seems to be a random mismash of > > > >>> scripts > > > >>> and userspace code so it's not hard to see it as something completely > > > >>> different from the new Kunit: > > > >>> > > > >>> $ git grep --files-with-matches kselftest_harness.h * > > > >> > > > >> To the extent that we can unify how tests are written, I agree that > > > >> this would be a good thing. However, you should note that > > > >> kselftest_harness.h is currently assums that it will be included in > > > >> userspace programs. This is most obviously seen if you look closely > > > >> at the functions defined in the header files which makes calls to > > > >> fork(), abort() and fprintf(). > > > > > > > > Ah, yes. I obviously did not dig deep enough. Using kunit for > > > > in-kernel tests and kselftest_harness for userspace tests seems like > > > > a sensible line to draw to me. Trying to unify kernel and userspace > > > > here sounds like it could be difficult so it's probably not worth > > > > forcing the issue unless someone wants to do some really fancy work > > > > to get it done. > > > > > > > > Based on some of the other commenters, I was under the impression > > > > that kselftests had in-kernel tests but I'm not sure where or if they > > > > exist. > > > > > > YES, kselftest has in-kernel tests. (Excuse the shouting...) > > > > > > Here is a likely list of them in the kernel source tree: > > > > > > $ grep module_init lib/test_*.c > > > lib/test_bitfield.c:module_init(test_bitfields) > > > lib/test_bitmap.c:module_init(test_bitmap_init); > > > lib/test_bpf.c:module_init(test_bpf_init); > > > lib/test_debug_virtual.c:module_init(test_debug_virtual_init); > > > lib/test_firmware.c:module_init(test_firmware_init); > > > lib/test_hash.c:module_init(test_hash_init); /* Does everything */ > > > lib/test_hexdump.c:module_init(test_hexdump_init); > > > lib/test_ida.c:module_init(ida_checks); > > > lib/test_kasan.c:module_init(kmalloc_tests_init); > > > lib/test_list_sort.c:module_init(list_sort_test); > > > lib/test_memcat_p.c:module_init(test_memcat_p_init); > > > lib/test_module.c:static int __init test_module_init(void) > > > lib/test_module.c:module_init(test_module_init); > > > lib/test_objagg.c:module_init(test_objagg_init); > > > lib/test_overflow.c:static int __init test_module_init(void) > > > lib/test_overflow.c:module_init(test_module_init); > > > lib/test_parman.c:module_init(test_parman_init); > > > lib/test_printf.c:module_init(test_printf_init); > > > lib/test_rhashtable.c:module_init(test_rht_init); > > > lib/test_siphash.c:module_init(siphash_test_init); > > > lib/test_sort.c:module_init(test_sort_init); > > > lib/test_stackinit.c:module_init(test_stackinit_init); > > > lib/test_static_key_base.c:module_init(test_static_key_base_init); > > > lib/test_static_keys.c:module_init(test_static_key_init); > > > lib/test_string.c:module_init(string_selftest_init); > > > lib/test_ubsan.c:module_init(test_ubsan_init); > > > lib/test_user_copy.c:module_init(test_user_copy_init); > > > lib/test_uuid.c:module_init(test_uuid_init); > > > lib/test_vmalloc.c:module_init(vmalloc_test_init) > > > lib/test_xarray.c:module_init(xarray_checks); > > > > > > > > > > If they do exists, it seems like it would make sense to > > > > convert those to kunit and have Kunit tests run-able in a VM or > > > > baremetal instance. > > > > > > They already run in a VM. > > > > > > They already run on bare metal. > > > > > > They already run in UML. > > > > > > This is not to say that KUnit does not make sense. But I'm still trying > > > to get a better description of the KUnit features (and there are > > > some). > > > > FYI, I have a master st
[PATCH][next] drm/amdgpu: fix return of an uninitialized value in variable ret
From: Colin Ian King
In the case where is_enable is false and lo_base_addr is non-zero the
variable ret has not been initialized and is being checked for non-zero
and potentially garbage is being returned. Fix this by not returning
ret but instead returning -EINVAL on the zero lo_base_addr case.
Addresses-Coverity: ("Uninitialized scalar variable")
Fixes: a6ac0b44bab9 ("drm/amdgpu: add df perfmon regs and funcs for xgmi")
Signed-off-by: Colin Ian King
---
drivers/gpu/drm/amd/amdgpu/df_v3_6.c | 5 +
1 file changed, 1 insertion(+), 4 deletions(-)
diff --git a/drivers/gpu/drm/amd/amdgpu/df_v3_6.c
b/drivers/gpu/drm/amd/amdgpu/df_v3_6.c
index a5c3558869fb..8c09bf994acd 100644
--- a/drivers/gpu/drm/amd/amdgpu/df_v3_6.c
+++ b/drivers/gpu/drm/amd/amdgpu/df_v3_6.c
@@ -398,10 +398,7 @@ static int df_v3_6_start_xgmi_link_cntr(struct
amdgpu_device *adev,
NULL);
if (lo_base_addr == 0)
- ret = -EINVAL;
-
- if (ret)
- return ret;
+ return -EINVAL;
lo_val = RREG32_PCIE(lo_base_addr);
--
2.20.1
___
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel
[drm-intel:drm-intel-next-queued 5/6] drivers/gpu/drm/i915/intel_hdcp.c:1406 hdcp2_authenticate_repeater_topology() warn: should this be a bitwise op?
tree: git://anongit.freedesktop.org/drm-intel drm-intel-next-queued
head: c16fd9be70faf3c49a61700efd16018dd910e390
commit: f26ae6a652f2e75a3a12ac22b7da5797978436c4 [5/6] drm/i915: SRM revocation
check for HDCP1.4 and 2.2
If you fix the issue, kindly add following tag
Reported-by: kbuild test robot
Reported-by: Dan Carpenter
smatch warnings:
drivers/gpu/drm/i915/intel_hdcp.c:1406 hdcp2_authenticate_repeater_topology()
warn: should this be a bitwise op?
git remote add drm-intel git://anongit.freedesktop.org/drm-intel
git remote update drm-intel
git checkout f26ae6a652f2e75a3a12ac22b7da5797978436c4
vim +1406 drivers/gpu/drm/i915/intel_hdcp.c
d849178e Ramalingam C 2019-02-16 1367 static
d849178e Ramalingam C 2019-02-16 1368 int
hdcp2_authenticate_repeater_topology(struct intel_connector *connector)
d849178e Ramalingam C 2019-02-16 1369 {
d849178e Ramalingam C 2019-02-16 1370 struct intel_digital_port
*intel_dig_port = conn_to_dig_port(connector);
d849178e Ramalingam C 2019-02-16 1371 struct intel_hdcp *hdcp =
&connector->hdcp;
f26ae6a6 Ramalingam C 2019-05-07 1372 struct drm_device *dev =
connector->base.dev;
d849178e Ramalingam C 2019-02-16 1373 union {
d849178e Ramalingam C 2019-02-16 1374 struct
hdcp2_rep_send_receiverid_list recvid_list;
d849178e Ramalingam C 2019-02-16 1375 struct
hdcp2_rep_send_ack rep_ack;
d849178e Ramalingam C 2019-02-16 1376 } msgs;
d849178e Ramalingam C 2019-02-16 1377 const struct intel_hdcp_shim
*shim = hdcp->shim;
f26ae6a6 Ramalingam C 2019-05-07 1378 u32 seq_num_v, device_cnt;
d849178e Ramalingam C 2019-02-16 1379 u8 *rx_info;
d849178e Ramalingam C 2019-02-16 1380 int ret;
d849178e Ramalingam C 2019-02-16 1381
d849178e Ramalingam C 2019-02-16 1382 ret =
shim->read_2_2_msg(intel_dig_port, HDCP_2_2_REP_SEND_RECVID_LIST,
d849178e Ramalingam C 2019-02-16 1383
&msgs.recvid_list, sizeof(msgs.recvid_list));
d849178e Ramalingam C 2019-02-16 1384 if (ret < 0)
d849178e Ramalingam C 2019-02-16 1385 return ret;
d849178e Ramalingam C 2019-02-16 1386
d849178e Ramalingam C 2019-02-16 1387 rx_info =
msgs.recvid_list.rx_info;
d849178e Ramalingam C 2019-02-16 1388
d849178e Ramalingam C 2019-02-16 1389 if
(HDCP_2_2_MAX_CASCADE_EXCEEDED(rx_info[1]) ||
d849178e Ramalingam C 2019-02-16 1390
HDCP_2_2_MAX_DEVS_EXCEEDED(rx_info[1])) {
d849178e Ramalingam C 2019-02-16 1391 DRM_DEBUG_KMS("Topology
Max Size Exceeded\n");
d849178e Ramalingam C 2019-02-16 1392 return -EINVAL;
d849178e Ramalingam C 2019-02-16 1393 }
d849178e Ramalingam C 2019-02-16 1394
d849178e Ramalingam C 2019-02-16 1395 /* Converting and Storing the
seq_num_v to local variable as DWORD */
0de655ca Ramalingam C 2019-05-07 1396 seq_num_v =
0de655ca Ramalingam C 2019-05-07 1397
drm_hdcp_be24_to_cpu((const u8 *)msgs.recvid_list.seq_num_v);
d849178e Ramalingam C 2019-02-16 1398
d849178e Ramalingam C 2019-02-16 1399 if (seq_num_v <
hdcp->seq_num_v) {
d849178e Ramalingam C 2019-02-16 1400 /* Roll over of the
seq_num_v from repeater. Reauthenticate. */
d849178e Ramalingam C 2019-02-16 1401
DRM_DEBUG_KMS("Seq_num_v roll over.\n");
d849178e Ramalingam C 2019-02-16 1402 return -EINVAL;
d849178e Ramalingam C 2019-02-16 1403 }
d849178e Ramalingam C 2019-02-16 1404
f26ae6a6 Ramalingam C 2019-05-07 1405 device_cnt =
HDCP_2_2_DEV_COUNT_HI(rx_info[0]) << 4 ||
^^
Bitwise OR | was probably intended.
f26ae6a6 Ramalingam C 2019-05-07 @1406
HDCP_2_2_DEV_COUNT_LO(rx_info[1]);
f26ae6a6 Ramalingam C 2019-05-07 1407 if
(drm_hdcp_check_ksvs_revoked(dev, msgs.recvid_list.receiver_ids,
f26ae6a6 Ramalingam C 2019-05-07 1408
device_cnt)) {
f26ae6a6 Ramalingam C 2019-05-07 1409 DRM_ERROR("Revoked
receiver ID(s) is in list\n");
f26ae6a6 Ramalingam C 2019-05-07 1410 return -EPERM;
f26ae6a6 Ramalingam C 2019-05-07 1411 }
f26ae6a6 Ramalingam C 2019-05-07 1412
d849178e Ramalingam C 2019-02-16 1413 ret =
hdcp2_verify_rep_topology_prepare_ack(connector,
d849178e Ramalingam C 2019-02-16 1414
&msgs.recvid_list,
d849178e Ramalingam C 2019-02-16 1415
&msgs.rep_ack);
d849178e Ramalingam C 2019-02-16 1416 if (ret < 0)
d849178e Ramalingam C 2019-02-16 1417 return ret;
d849178e Ramalingam C 2019-02-16 1418
d849178e Ramalingam C 2019-02-16 1419 hdcp->se
[drm-intel:drm-intel-next-queued 4/6] drivers/gpu/drm/drm_hdcp.c:190 drm_hdcp_parse_hdcp2_srm() warn: mask and shift to zero
tree: git://anongit.freedesktop.org/drm-intel drm-intel-next-queued
head: c16fd9be70faf3c49a61700efd16018dd910e390
commit: 6498bf5800a302ef69e7f4914e727893f278bb2f [4/6] drm: revocation check at
drm subsystem
If you fix the issue, kindly add following tag
Reported-by: kbuild test robot
Reported-by: Dan Carpenter
smatch warnings:
drivers/gpu/drm/drm_hdcp.c:190 drm_hdcp_parse_hdcp2_srm() warn: mask and shift
to zero
git remote add drm-intel git://anongit.freedesktop.org/drm-intel
git remote update drm-intel
git checkout 6498bf5800a302ef69e7f4914e727893f278bb2f
vim +190 drivers/gpu/drm/drm_hdcp.c
6498bf58 Ramalingam C 2019-05-07 151 static int
drm_hdcp_parse_hdcp2_srm(const u8 *buf, size_t count)
6498bf58 Ramalingam C 2019-05-07 152 {
6498bf58 Ramalingam C 2019-05-07 153 struct hdcp_srm_header *header;
6498bf58 Ramalingam C 2019-05-07 154 u32 vrl_length, ksv_count, ksv_sz;
6498bf58 Ramalingam C 2019-05-07 155
6498bf58 Ramalingam C 2019-05-07 156 if (count < (sizeof(struct
hdcp_srm_header) +
6498bf58 Ramalingam C 2019-05-07 157 DRM_HDCP_2_VRL_LENGTH_SIZE +
DRM_HDCP_2_DCP_SIG_SIZE)) {
6498bf58 Ramalingam C 2019-05-07 158 DRM_ERROR("Invalid blob
length\n");
6498bf58 Ramalingam C 2019-05-07 159 return -EINVAL;
6498bf58 Ramalingam C 2019-05-07 160 }
6498bf58 Ramalingam C 2019-05-07 161
6498bf58 Ramalingam C 2019-05-07 162 header = (struct hdcp_srm_header *)buf;
6498bf58 Ramalingam C 2019-05-07 163 DRM_DEBUG("SRM ID: 0x%x, SRM Ver: 0x%x,
SRM Gen No: 0x%x\n",
6498bf58 Ramalingam C 2019-05-07 164 header->srm_id &
DRM_HDCP_SRM_ID_MASK,
6498bf58 Ramalingam C 2019-05-07 165
be16_to_cpu(header->srm_version), header->srm_gen_no);
6498bf58 Ramalingam C 2019-05-07 166
6498bf58 Ramalingam C 2019-05-07 167 if (header->reserved)
6498bf58 Ramalingam C 2019-05-07 168 return -EINVAL;
6498bf58 Ramalingam C 2019-05-07 169
6498bf58 Ramalingam C 2019-05-07 170 buf = buf + sizeof(*header);
6498bf58 Ramalingam C 2019-05-07 171 vrl_length = get_vrl_length(buf);
6498bf58 Ramalingam C 2019-05-07 172
6498bf58 Ramalingam C 2019-05-07 173 if (count < (sizeof(struct
hdcp_srm_header) + vrl_length) ||
6498bf58 Ramalingam C 2019-05-07 174 vrl_length <
(DRM_HDCP_2_VRL_LENGTH_SIZE +
6498bf58 Ramalingam C 2019-05-07 175 DRM_HDCP_2_DCP_SIG_SIZE)) {
6498bf58 Ramalingam C 2019-05-07 176 DRM_ERROR("Invalid blob length
or vrl length\n");
6498bf58 Ramalingam C 2019-05-07 177 return -EINVAL;
6498bf58 Ramalingam C 2019-05-07 178 }
6498bf58 Ramalingam C 2019-05-07 179
6498bf58 Ramalingam C 2019-05-07 180 /* Length of the all vrls combined */
6498bf58 Ramalingam C 2019-05-07 181 vrl_length -=
(DRM_HDCP_2_VRL_LENGTH_SIZE +
6498bf58 Ramalingam C 2019-05-07 182 DRM_HDCP_2_DCP_SIG_SIZE);
6498bf58 Ramalingam C 2019-05-07 183
6498bf58 Ramalingam C 2019-05-07 184 if (!vrl_length) {
6498bf58 Ramalingam C 2019-05-07 185 DRM_ERROR("No vrl found\n");
6498bf58 Ramalingam C 2019-05-07 186 return -EINVAL;
6498bf58 Ramalingam C 2019-05-07 187 }
6498bf58 Ramalingam C 2019-05-07 188
6498bf58 Ramalingam C 2019-05-07 189 buf += DRM_HDCP_2_VRL_LENGTH_SIZE;
6498bf58 Ramalingam C 2019-05-07 @190 ksv_count = (*buf << 2) |
DRM_HDCP_2_KSV_COUNT_2_LSBITS(*(buf + 1));
#define DRM_HDCP_2_KSV_COUNT_2_LSBITS(byte) (((byte) & 0xC) >> 6)
0xC >> 6 is always zero.
6498bf58 Ramalingam C 2019-05-07 191 if (!ksv_count) {
6498bf58 Ramalingam C 2019-05-07 192 DRM_DEBUG("Revoked KSV count is
0\n");
6498bf58 Ramalingam C 2019-05-07 193 return count;
6498bf58 Ramalingam C 2019-05-07 194 }
6498bf58 Ramalingam C 2019-05-07 195
---
0-DAY kernel test infrastructureOpen Source Technology Center
https://lists.01.org/pipermail/kbuild-all Intel Corporation
___
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel
[dma-buf v3 3/3] dma-buf: add show_fdinfo handler
From: Greg Hackmann
The show_fdinfo handler exports the same information available through
debugfs on a per-buffer basis.
Signed-off-by: Greg Hackmann
Signed-off-by: Chenbo Feng
---
drivers/dma-buf/dma-buf.c | 15 +++
1 file changed, 15 insertions(+)
diff --git a/drivers/dma-buf/dma-buf.c b/drivers/dma-buf/dma-buf.c
index c1da5f9ce44d..c4efc272fc34 100644
--- a/drivers/dma-buf/dma-buf.c
+++ b/drivers/dma-buf/dma-buf.c
@@ -381,6 +381,20 @@ static long dma_buf_ioctl(struct file *file,
}
}
+static void dma_buf_show_fdinfo(struct seq_file *m, struct file *file)
+{
+ struct dma_buf *dmabuf = file->private_data;
+
+ seq_printf(m, "size:\t%zu\n", dmabuf->size);
+ /* Don't count the temporary reference taken inside procfs seq_show */
+ seq_printf(m, "count:\t%ld\n", file_count(dmabuf->file) - 1);
+ seq_printf(m, "exp_name:\t%s\n", dmabuf->exp_name);
+ mutex_lock(&dmabuf->lock);
+ if (dmabuf->name)
+ seq_printf(m, "name:\t%s\n", dmabuf->name);
+ mutex_unlock(&dmabuf->lock);
+}
+
static const struct file_operations dma_buf_fops = {
.release= dma_buf_release,
.mmap = dma_buf_mmap_internal,
@@ -390,6 +404,7 @@ static const struct file_operations dma_buf_fops = {
#ifdef CONFIG_COMPAT
.compat_ioctl = dma_buf_ioctl,
#endif
+ .show_fdinfo= dma_buf_show_fdinfo,
};
/*
--
2.21.0.1020.gf2820cf01a-goog
___
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [PATCH v2 00/17] kunit: introduce KUnit, the Linux kernel unit testing framework
Hi Ted, I'll try answering this again. The first time I was a little flippant in part of my answer because I thought your comments somewhat flippant. This time I'll provide a more complete answer. On 5/8/19 7:13 PM, Frank Rowand wrote: > On 5/8/19 6:58 PM, Theodore Ts'o wrote: >> On Wed, May 08, 2019 at 05:43:35PM -0700, Frank Rowand wrote: * begin context from Greg KH that you snipped * On 5/7/19 1:01 AM, Greg KH wrote: << note that I have snipped my original question above this point >> > > kselftest provides no in-kernel framework for testing kernel code > specifically. That should be what kunit provides, an "easy" way to > write in-kernel tests for things. * end context from Greg KH that you snipped * >>> kselftest provides a mechanism for in-kernel tests via modules. For >>> example, see: >>> >>> tools/testing/selftests/vm/run_vmtests invokes: >>> tools/testing/selftests/vm/test_vmalloc.sh >>> loads module: >>> test_vmalloc >>> (which is built from lib/test_vmalloc.c if CONFIG_TEST_VMALLOC) >> >> The majority of the kselftests are implemented as userspace programs. My flippant answer: > Non-argument. This time: My reply to Greg was pointing out that in-kernel tests do exist in kselftest. Your comment that the majority of kselftests are implemented as userspace programs has no bearing on whether kselftest support in-kernel tests. It does not counter the fact the kselftest supports in-kernel tests. >> You *can* run in-kernel test using modules; but there is no framework >> for the in-kernel code found in the test modules, which means each of >> the in-kernel code has to create their own in-kernel test >> infrastructure. The kselftest in-kernel tests follow a common pattern. As such, there is a framework. This next two paragraphs you ignored entirely in your reply: > Why create an entire new subsystem (KUnit) when you can add a header > file (and .c code as appropriate) that outputs the proper TAP formatted > results from kselftest kernel test modules? > > There are already a multitude of in kernel test modules used by > kselftest. It would be good if they all used a common TAP compliant > mechanism to report results. >> That's much like saying you can use vice grips to turn a nut or >> bolt-head. You *can*, but it might be that using a monkey wrench >> would be a much better tool that is much easier. >> >> What would you say to a wood worker objecting that a toolbox should >> contain a monkey wrench because he already knows how to use vise >> grips, and his tiny brain shouldn't be forced to learn how to use a >> wrench when he knows how to use a vise grip, which is a perfectly good >> tool? >> >> If you want to use vice grips as a hammer, screwdriver, monkey wrench, >> etc. there's nothing stopping you from doing that. But it's not fair >> to object to other people who might want to use better tools. >> >> The reality is that we have a lot of testing tools. It's not just >> kselftests. There is xfstests for file system code, blktests for >> block layer tests, etc. We use the right tool for the right job. My flippant answer: > More specious arguments. This time: I took your answer as a straw man, and had no desire to spend time countering a straw man. I am not proposing using a vice grips (to use your analogy). I am saying that maybe the monkey wrench already exists. My point of this whole thread has been to try to get the submitter to provide a better, more complete explanation of how and why KUnit is a better tool. I have not yet objected to the number (and differences among) the many sub-system tests. I am questioning whether there is a need for another _test framework_ for in-kernel testing. If there is something important that KUnit provides that does not exist in existing frameworks then the next question would be to ask how to implement that important thing (improve the existing framework, replace the existing framework, or have two frameworks). I still think it is premature to ask this question until we first know the answer to what unique features KUnit adds (and apparently until we know what the existing framework provides). -Frank > > -Frank > >> >> - Ted >> > > ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [PATCH v2 06/17] kbuild: enable building KUnit
On Thu, May 2, 2019 at 8:03 AM Brendan Higgins wrote: > > Add KUnit to root Kconfig and Makefile allowing it to actually be built. > > Signed-off-by: Brendan Higgins You need to make sure to not break git-bisect'abililty. With this commit, I see build error. CC kunit/test.o kunit/test.c:11:10: fatal error: os.h: No such file or directory #include ^~ compilation terminated. make[1]: *** [scripts/Makefile.build;279: kunit/test.o] Error 1 make: *** [Makefile;1763: kunit/] Error 2 > --- > Kconfig | 2 ++ > Makefile | 2 +- > 2 files changed, 3 insertions(+), 1 deletion(-) > > diff --git a/Kconfig b/Kconfig > index 48a80beab6853..10428501edb78 100644 > --- a/Kconfig > +++ b/Kconfig > @@ -30,3 +30,5 @@ source "crypto/Kconfig" > source "lib/Kconfig" > > source "lib/Kconfig.debug" > + > +source "kunit/Kconfig" > diff --git a/Makefile b/Makefile > index 2b99679148dc7..77368f498d84c 100644 > --- a/Makefile > +++ b/Makefile > @@ -969,7 +969,7 @@ endif > PHONY += prepare0 > > ifeq ($(KBUILD_EXTMOD),) > -core-y += kernel/ certs/ mm/ fs/ ipc/ security/ crypto/ block/ > +core-y += kernel/ certs/ mm/ fs/ ipc/ security/ crypto/ block/ kunit/ > > vmlinux-dirs := $(patsubst %/,%,$(filter %/, $(init-y) $(init-m) \ > $(core-y) $(core-m) $(drivers-y) $(drivers-m) \ > -- > 2.21.0.593.g511ec345e18-goog > -- Best Regards Masahiro Yamada ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
[dma-buf v3 0/3] Improve the dma-buf tracking
Currently, all dma-bufs share the same anonymous inode. While we can count
how many dma-buf fds or mappings a process has, we can't get the size of
the backing buffers or tell if two entries point to the same dma-buf. And
in debugfs, we can get a per-buffer breakdown of size and reference count,
but can't tell which processes are actually holding the references to each
buffer.
To resolve the issue above and provide better method for userspace to track
the dma-buf usage across different processes, the following changes are
proposed in dma-buf kernel side. First of all, replace the singleton inode
inside the dma-buf subsystem with a mini-filesystem, and assign each
dma-buf a unique inode out of this filesystem. With this change, calling
stat(2) on each entry gives the caller a unique ID (st_ino), the buffer's
size (st_size), and even the number of pages assigned to each dma-buffer.
Secoundly, add the inode information to /sys/kernel/debug/dma_buf/bufinfo
so in the case where a buffer is mmap()ed into a process’s address space
but all remaining fds have been closed, we can still get the dma-buf
information and try to accociate it with the process by searching the
proc/pid/maps and looking for the corresponding inode number exposed in
dma-buf debug fs. Thirdly, created an ioctl to assign names to dma-bufs
which lets userspace assign short names (e.g., "CAMERA") to buffers. This
information can be extremely helpful for tracking and accounting shared
buffers based on their usage and original purpose. Last but not least, add
dma-buf information to /proc/pid/fdinfo by adding a show_fdinfo() handler
to dma_buf_file_operations. The handler will print the file_count and name
of each buffer.
Change in v2:
* Add a check to prevent changing dma-buf name when it is attached to
devices.
* Fixed some compile warnings
Change in v3:
* Removed the GET_DMA_BUF_NAME ioctls, add the dma_buf pointer to dentry
d_fsdata so the name can be displayed in proc/pid/maps and
proc/pid/map_files.
Greg Hackmann (3):
dma-buf: give each buffer a full-fledged inode
dma-buf: add DMA_BUF_{GET,SET}_NAME ioctls
dma-buf: add show_fdinfo handler
drivers/dma-buf/dma-buf.c| 122 +--
include/linux/dma-buf.h | 5 +-
include/uapi/linux/dma-buf.h | 3 +
include/uapi/linux/magic.h | 1 +
4 files changed, 124 insertions(+), 7 deletions(-)
--
2.21.0.1020.gf2820cf01a-goog
___
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel
[dma-buf v3 2/3] dma-buf: add DMA_BUF_{GET,SET}_NAME ioctls
From: Greg Hackmann
This patch adds complimentary DMA_BUF_SET_NAME and DMA_BUF_GET_NAME
ioctls, which lets userspace processes attach a free-form name to each
buffer.
This information can be extremely helpful for tracking and accounting
shared buffers. For example, on Android, we know what each buffer will
be used for at allocation time: GL, multimedia, camera, etc. The
userspace allocator can use DMA_BUF_SET_NAME to associate that
information with the buffer, so we can later give developers a
breakdown of how much memory they're allocating for graphics, camera,
etc.
Signed-off-by: Greg Hackmann
Signed-off-by: Chenbo Feng
---
drivers/dma-buf/dma-buf.c| 49 +---
include/linux/dma-buf.h | 5 +++-
include/uapi/linux/dma-buf.h | 3 +++
3 files changed, 53 insertions(+), 4 deletions(-)
diff --git a/drivers/dma-buf/dma-buf.c b/drivers/dma-buf/dma-buf.c
index ffd5a2ad7d6f..c1da5f9ce44d 100644
--- a/drivers/dma-buf/dma-buf.c
+++ b/drivers/dma-buf/dma-buf.c
@@ -48,8 +48,24 @@ struct dma_buf_list {
static struct dma_buf_list db_list;
+static char *dmabuffs_dname(struct dentry *dentry, char *buffer, int buflen)
+{
+ struct dma_buf *dmabuf;
+ char name[DMA_BUF_NAME_LEN];
+ size_t ret = 0;
+
+ dmabuf = dentry->d_fsdata;
+ mutex_lock(&dmabuf->lock);
+ if (dmabuf->name)
+ ret = strlcpy(name, dmabuf->name, DMA_BUF_NAME_LEN);
+ mutex_unlock(&dmabuf->lock);
+
+ return dynamic_dname(dentry, buffer, buflen, "/%s:%s",
+dentry->d_name.name, ret > 0 ? name : "");
+}
+
static const struct dentry_operations dma_buf_dentry_ops = {
- .d_dname = simple_dname,
+ .d_dname = dmabuffs_dname,
};
static struct vfsmount *dma_buf_mnt;
@@ -297,6 +313,27 @@ static __poll_t dma_buf_poll(struct file *file, poll_table
*poll)
return events;
}
+static long dma_buf_set_name(struct dma_buf *dmabuf, const char __user *buf)
+{
+ char *name = strndup_user(buf, DMA_BUF_NAME_LEN);
+ long ret = 0;
+
+ if (IS_ERR(name))
+ return PTR_ERR(name);
+
+ mutex_lock(&dmabuf->lock);
+ if (!list_empty(&dmabuf->attachments)) {
+ ret = -EBUSY;
+ goto out_unlock;
+ }
+ kfree(dmabuf->name);
+ dmabuf->name = name;
+
+out_unlock:
+ mutex_unlock(&dmabuf->lock);
+ return ret;
+}
+
static long dma_buf_ioctl(struct file *file,
unsigned int cmd, unsigned long arg)
{
@@ -335,6 +372,10 @@ static long dma_buf_ioctl(struct file *file,
ret = dma_buf_begin_cpu_access(dmabuf, direction);
return ret;
+
+ case DMA_BUF_SET_NAME:
+ return dma_buf_set_name(dmabuf, (const char __user *)arg);
+
default:
return -ENOTTY;
}
@@ -376,6 +417,7 @@ static struct file *dma_buf_getfile(struct dma_buf *dmabuf,
int flags)
goto err_alloc_file;
file->f_flags = flags & (O_ACCMODE | O_NONBLOCK);
file->private_data = dmabuf;
+ file->f_path.dentry->d_fsdata = dmabuf;
return file;
@@ -1082,12 +1124,13 @@ static int dma_buf_debug_show(struct seq_file *s, void
*unused)
continue;
}
- seq_printf(s, "%08zu\t%08x\t%08x\t%08ld\t%s\t%08lu\n",
+ seq_printf(s, "%08zu\t%08x\t%08x\t%08ld\t%s\t%08lu\t%s\n",
buf_obj->size,
buf_obj->file->f_flags, buf_obj->file->f_mode,
file_count(buf_obj->file),
buf_obj->exp_name,
- file_inode(buf_obj->file)->i_ino);
+ file_inode(buf_obj->file)->i_ino,
+ buf_obj->name ?: "");
robj = buf_obj->resv;
while (true) {
diff --git a/include/linux/dma-buf.h b/include/linux/dma-buf.h
index 58725f890b5b..582998e19df6 100644
--- a/include/linux/dma-buf.h
+++ b/include/linux/dma-buf.h
@@ -255,10 +255,12 @@ struct dma_buf_ops {
* @file: file pointer used for sharing buffers across, and for refcounting.
* @attachments: list of dma_buf_attachment that denotes all devices attached.
* @ops: dma_buf_ops associated with this buffer object.
- * @lock: used internally to serialize list manipulation, attach/detach and
vmap/unmap
+ * @lock: used internally to serialize list manipulation, attach/detach and
+ *vmap/unmap, and accesses to name
* @vmapping_counter: used internally to refcnt the vmaps
* @vmap_ptr: the current vmap ptr if vmapping_counter > 0
* @exp_name: name of the exporter; useful for debugging.
+ * @name: userspace-provided name; useful for accounting and debugging.
* @owner: pointer to exporter module; used for refcounting when exporter is a
* kernel module.
* @list_node: node for dma_buf accounting and
[dma-buf v3 1/3] dma-buf: give each buffer a full-fledged inode
From: Greg Hackmann
By traversing /proc/*/fd and /proc/*/map_files, processes with CAP_ADMIN
can get a lot of fine-grained data about how shmem buffers are shared
among processes. stat(2) on each entry gives the caller a unique
ID (st_ino), the buffer's size (st_size), and even the number of pages
currently charged to the buffer (st_blocks / 512).
In contrast, all dma-bufs share the same anonymous inode. So while we
can count how many dma-buf fds or mappings a process has, we can't get
the size of the backing buffers or tell if two entries point to the same
dma-buf. On systems with debugfs, we can get a per-buffer breakdown of
size and reference count, but can't tell which processes are actually
holding the references to each buffer.
Replace the singleton inode with full-fledged inodes allocated by
alloc_anon_inode(). This involves creating and mounting a
mini-pseudo-filesystem for dma-buf, following the example in fs/aio.c.
Signed-off-by: Greg Hackmann
Signed-off-by: Chenbo Feng
---
drivers/dma-buf/dma-buf.c | 63 ++
include/uapi/linux/magic.h | 1 +
2 files changed, 58 insertions(+), 6 deletions(-)
diff --git a/drivers/dma-buf/dma-buf.c b/drivers/dma-buf/dma-buf.c
index 7c858020d14b..ffd5a2ad7d6f 100644
--- a/drivers/dma-buf/dma-buf.c
+++ b/drivers/dma-buf/dma-buf.c
@@ -34,8 +34,10 @@
#include
#include
#include
+#include
#include
+#include
static inline int is_dma_buf_file(struct file *);
@@ -46,6 +48,25 @@ struct dma_buf_list {
static struct dma_buf_list db_list;
+static const struct dentry_operations dma_buf_dentry_ops = {
+ .d_dname = simple_dname,
+};
+
+static struct vfsmount *dma_buf_mnt;
+
+static struct dentry *dma_buf_fs_mount(struct file_system_type *fs_type,
+ int flags, const char *name, void *data)
+{
+ return mount_pseudo(fs_type, "dmabuf:", NULL, &dma_buf_dentry_ops,
+ DMA_BUF_MAGIC);
+}
+
+static struct file_system_type dma_buf_fs_type = {
+ .name = "dmabuf",
+ .mount = dma_buf_fs_mount,
+ .kill_sb = kill_anon_super,
+};
+
static int dma_buf_release(struct inode *inode, struct file *file)
{
struct dma_buf *dmabuf;
@@ -338,6 +359,31 @@ static inline int is_dma_buf_file(struct file *file)
return file->f_op == &dma_buf_fops;
}
+static struct file *dma_buf_getfile(struct dma_buf *dmabuf, int flags)
+{
+ struct file *file;
+ struct inode *inode = alloc_anon_inode(dma_buf_mnt->mnt_sb);
+
+ if (IS_ERR(inode))
+ return ERR_CAST(inode);
+
+ inode->i_size = dmabuf->size;
+ inode_set_bytes(inode, dmabuf->size);
+
+ file = alloc_file_pseudo(inode, dma_buf_mnt, "dmabuf",
+flags, &dma_buf_fops);
+ if (IS_ERR(file))
+ goto err_alloc_file;
+ file->f_flags = flags & (O_ACCMODE | O_NONBLOCK);
+ file->private_data = dmabuf;
+
+ return file;
+
+err_alloc_file:
+ iput(inode);
+ return file;
+}
+
/**
* DOC: dma buf device access
*
@@ -433,8 +479,7 @@ struct dma_buf *dma_buf_export(const struct
dma_buf_export_info *exp_info)
}
dmabuf->resv = resv;
- file = anon_inode_getfile("dmabuf", &dma_buf_fops, dmabuf,
- exp_info->flags);
+ file = dma_buf_getfile(dmabuf, exp_info->flags);
if (IS_ERR(file)) {
ret = PTR_ERR(file);
goto err_dmabuf;
@@ -1025,8 +1070,8 @@ static int dma_buf_debug_show(struct seq_file *s, void
*unused)
return ret;
seq_puts(s, "\nDma-buf Objects:\n");
- seq_printf(s, "%-8s\t%-8s\t%-8s\t%-8s\texp_name\n",
- "size", "flags", "mode", "count");
+ seq_printf(s, "%-8s\t%-8s\t%-8s\t%-8s\texp_name\t%-8s\n",
+ "size", "flags", "mode", "count", "ino");
list_for_each_entry(buf_obj, &db_list.head, list_node) {
ret = mutex_lock_interruptible(&buf_obj->lock);
@@ -1037,11 +1082,12 @@ static int dma_buf_debug_show(struct seq_file *s, void
*unused)
continue;
}
- seq_printf(s, "%08zu\t%08x\t%08x\t%08ld\t%s\n",
+ seq_printf(s, "%08zu\t%08x\t%08x\t%08ld\t%s\t%08lu\n",
buf_obj->size,
buf_obj->file->f_flags, buf_obj->file->f_mode,
file_count(buf_obj->file),
- buf_obj->exp_name);
+ buf_obj->exp_name,
+ file_inode(buf_obj->file)->i_ino);
robj = buf_obj->resv;
while (true) {
@@ -1136,6 +1182,10 @@ static inline void dma_buf_uninit_debugfs(void)
static int __init dma_buf_init(void)
{
+ dma_buf_mnt = kern_mount(&dma_buf_fs_type);
+ if (IS_ERR(dma_buf_mnt))
+ return PTR_ERR(dma_buf_mnt);
+
mutex_init(&db_l
Re: [PATCH v2 00/17] kunit: introduce KUnit, the Linux kernel unit testing framework
On 5/9/19 4:40 PM, Logan Gunthorpe wrote: > > > On 2019-05-09 5:30 p.m., Theodore Ts'o wrote: >> On Thu, May 09, 2019 at 04:20:05PM -0600, Logan Gunthorpe wrote: >>> >>> The second item, arguably, does have significant overlap with kselftest. >>> Whether you are running short tests in a light weight UML environment or >>> higher level tests in an heavier VM the two could be using the same >>> framework for writing or defining in-kernel tests. It *may* also be valuable >>> for some people to be able to run all the UML tests in the heavy VM >>> environment along side other higher level tests. >>> >>> Looking at the selftests tree in the repo, we already have similar items to >>> what Kunit is adding as I described in point (2) above. kselftest_harness.h >>> contains macros like EXPECT_* and ASSERT_* with very similar intentions to >>> the new KUNIT_EXECPT_* and KUNIT_ASSERT_* macros. >>> >>> However, the number of users of this harness appears to be quite small. Most >>> of the code in the selftests tree seems to be a random mismash of scripts >>> and userspace code so it's not hard to see it as something completely >>> different from the new Kunit: >>> >>> $ git grep --files-with-matches kselftest_harness.h * >> >> To the extent that we can unify how tests are written, I agree that >> this would be a good thing. However, you should note that >> kselftest_harness.h is currently assums that it will be included in >> userspace programs. This is most obviously seen if you look closely >> at the functions defined in the header files which makes calls to >> fork(), abort() and fprintf(). > > Ah, yes. I obviously did not dig deep enough. Using kunit for > in-kernel tests and kselftest_harness for userspace tests seems like > a sensible line to draw to me. Trying to unify kernel and userspace > here sounds like it could be difficult so it's probably not worth > forcing the issue unless someone wants to do some really fancy work > to get it done. > > Based on some of the other commenters, I was under the impression > that kselftests had in-kernel tests but I'm not sure where or if they > exist. YES, kselftest has in-kernel tests. (Excuse the shouting...) Here is a likely list of them in the kernel source tree: $ grep module_init lib/test_*.c lib/test_bitfield.c:module_init(test_bitfields) lib/test_bitmap.c:module_init(test_bitmap_init); lib/test_bpf.c:module_init(test_bpf_init); lib/test_debug_virtual.c:module_init(test_debug_virtual_init); lib/test_firmware.c:module_init(test_firmware_init); lib/test_hash.c:module_init(test_hash_init);/* Does everything */ lib/test_hexdump.c:module_init(test_hexdump_init); lib/test_ida.c:module_init(ida_checks); lib/test_kasan.c:module_init(kmalloc_tests_init); lib/test_list_sort.c:module_init(list_sort_test); lib/test_memcat_p.c:module_init(test_memcat_p_init); lib/test_module.c:static int __init test_module_init(void) lib/test_module.c:module_init(test_module_init); lib/test_objagg.c:module_init(test_objagg_init); lib/test_overflow.c:static int __init test_module_init(void) lib/test_overflow.c:module_init(test_module_init); lib/test_parman.c:module_init(test_parman_init); lib/test_printf.c:module_init(test_printf_init); lib/test_rhashtable.c:module_init(test_rht_init); lib/test_siphash.c:module_init(siphash_test_init); lib/test_sort.c:module_init(test_sort_init); lib/test_stackinit.c:module_init(test_stackinit_init); lib/test_static_key_base.c:module_init(test_static_key_base_init); lib/test_static_keys.c:module_init(test_static_key_init); lib/test_string.c:module_init(string_selftest_init); lib/test_ubsan.c:module_init(test_ubsan_init); lib/test_user_copy.c:module_init(test_user_copy_init); lib/test_uuid.c:module_init(test_uuid_init); lib/test_vmalloc.c:module_init(vmalloc_test_init) lib/test_xarray.c:module_init(xarray_checks); > If they do exists, it seems like it would make sense to > convert those to kunit and have Kunit tests run-able in a VM or > baremetal instance. They already run in a VM. They already run on bare metal. They already run in UML. This is not to say that KUnit does not make sense. But I'm still trying to get a better description of the KUnit features (and there are some). -Frank ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [PATCH] kernel/locking/semaphore: use wake_q in up()
On (05/09/19 22:06), Daniel Vetter wrote:
[..]
> +/* Functions for the contended case */
> +
> +struct semaphore_waiter {
> + struct list_head list;
> + struct task_struct *task;
> + bool up;
> +};
> +
> /**
> * up - release the semaphore
> * @sem: the semaphore to release
> @@ -179,24 +187,25 @@ EXPORT_SYMBOL(down_timeout);
> void up(struct semaphore *sem)
> {
> unsigned long flags;
> + struct semaphore_waiter *waiter;
> + DEFINE_WAKE_Q(wake_q);
>
> raw_spin_lock_irqsave(&sem->lock, flags);
> - if (likely(list_empty(&sem->wait_list)))
> + if (likely(list_empty(&sem->wait_list))) {
> sem->count++;
> - else
> - __up(sem);
> + } else {
> + waiter = list_first_entry(&sem->wait_list,
> +struct semaphore_waiter, list);
> + list_del(&waiter->list);
> + waiter->up = true;
> + wake_q_add(&wake_q, waiter->task);
> + }
> raw_spin_unlock_irqrestore(&sem->lock, flags);
So the new code still can printk/WARN under sem->lock in some buggy
cases.
E.g.
wake_q_add()
get_task_struct()
refcount_inc_checked()
WARN_ONCE()
Are we fine with that?
-ss
___
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [PATCH v2 00/17] kunit: introduce KUnit, the Linux kernel unit testing framework
On Thu, 2019-05-09 at 22:18 -0700, Frank Rowand wrote: > On 5/9/19 4:40 PM, Logan Gunthorpe wrote: > > > > > > On 2019-05-09 5:30 p.m., Theodore Ts'o wrote: > >> On Thu, May 09, 2019 at 04:20:05PM -0600, Logan Gunthorpe wrote: > >>> > >>> The second item, arguably, does have significant overlap with kselftest. > >>> Whether you are running short tests in a light weight UML environment or > >>> higher level tests in an heavier VM the two could be using the same > >>> framework for writing or defining in-kernel tests. It *may* also be > >>> valuable > >>> for some people to be able to run all the UML tests in the heavy VM > >>> environment along side other higher level tests. > >>> > >>> Looking at the selftests tree in the repo, we already have similar items > >>> to > >>> what Kunit is adding as I described in point (2) above. > >>> kselftest_harness.h > >>> contains macros like EXPECT_* and ASSERT_* with very similar intentions to > >>> the new KUNIT_EXECPT_* and KUNIT_ASSERT_* macros. > >>> > >>> However, the number of users of this harness appears to be quite small. > >>> Most > >>> of the code in the selftests tree seems to be a random mismash of scripts > >>> and userspace code so it's not hard to see it as something completely > >>> different from the new Kunit: > >>> > >>> $ git grep --files-with-matches kselftest_harness.h * > >> > >> To the extent that we can unify how tests are written, I agree that > >> this would be a good thing. However, you should note that > >> kselftest_harness.h is currently assums that it will be included in > >> userspace programs. This is most obviously seen if you look closely > >> at the functions defined in the header files which makes calls to > >> fork(), abort() and fprintf(). > > > > Ah, yes. I obviously did not dig deep enough. Using kunit for > > in-kernel tests and kselftest_harness for userspace tests seems like > > a sensible line to draw to me. Trying to unify kernel and userspace > > here sounds like it could be difficult so it's probably not worth > > forcing the issue unless someone wants to do some really fancy work > > to get it done. > > > > Based on some of the other commenters, I was under the impression > > that kselftests had in-kernel tests but I'm not sure where or if they > > exist. > > YES, kselftest has in-kernel tests. (Excuse the shouting...) > > Here is a likely list of them in the kernel source tree: > > $ grep module_init lib/test_*.c > lib/test_bitfield.c:module_init(test_bitfields) > lib/test_bitmap.c:module_init(test_bitmap_init); > lib/test_bpf.c:module_init(test_bpf_init); > lib/test_debug_virtual.c:module_init(test_debug_virtual_init); > lib/test_firmware.c:module_init(test_firmware_init); > lib/test_hash.c:module_init(test_hash_init); /* Does everything */ > lib/test_hexdump.c:module_init(test_hexdump_init); > lib/test_ida.c:module_init(ida_checks); > lib/test_kasan.c:module_init(kmalloc_tests_init); > lib/test_list_sort.c:module_init(list_sort_test); > lib/test_memcat_p.c:module_init(test_memcat_p_init); > lib/test_module.c:static int __init test_module_init(void) > lib/test_module.c:module_init(test_module_init); > lib/test_objagg.c:module_init(test_objagg_init); > lib/test_overflow.c:static int __init test_module_init(void) > lib/test_overflow.c:module_init(test_module_init); > lib/test_parman.c:module_init(test_parman_init); > lib/test_printf.c:module_init(test_printf_init); > lib/test_rhashtable.c:module_init(test_rht_init); > lib/test_siphash.c:module_init(siphash_test_init); > lib/test_sort.c:module_init(test_sort_init); > lib/test_stackinit.c:module_init(test_stackinit_init); > lib/test_static_key_base.c:module_init(test_static_key_base_init); > lib/test_static_keys.c:module_init(test_static_key_init); > lib/test_string.c:module_init(string_selftest_init); > lib/test_ubsan.c:module_init(test_ubsan_init); > lib/test_user_copy.c:module_init(test_user_copy_init); > lib/test_uuid.c:module_init(test_uuid_init); > lib/test_vmalloc.c:module_init(vmalloc_test_init) > lib/test_xarray.c:module_init(xarray_checks); > > > > If they do exists, it seems like it would make sense to > > convert those to kunit and have Kunit tests run-able in a VM or > > baremetal instance. > > They already run in a VM. > > They already run on bare metal. > > They already run in UML. > > This is not to say that KUnit does not make sense. But I'm still trying > to get a better description of the KUnit features (and there are > some). FYI, I have a master student who looks at converting some of these to KTF, such as for instance the XArray tests, which lended themselves quite good to a semi-automated conversion. The result is also a somewhat more compact code as well as the flexibility provided by the Googletest executor and the KTF frameworks, such as running selected tests, output formatting, debugging features etc. Knut > -Frank ___ dri-devel mailing list dri-devel@lists.freedeskt
Re: [PATCH v2 3/7] lib/hexdump.c: Optionally suppress lines of repeated bytes
On Wed, 2019-05-08 at 17:58 -0700, Randy Dunlap wrote: > On 5/8/19 12:01 AM, Alastair D'Silva wrote: > > From: Alastair D'Silva > > > > Some buffers may only be partially filled with useful data, while > > the rest > > is padded (typically with 0x00 or 0xff). > > > > This patch introduces a flag to allow the supression of lines of > > repeated > > bytes, which are replaced with '** Skipped %u bytes of value 0x%x > > **' > > > > An inline wrapper function is provided for backwards compatibility > > with > > existing code, which maintains the original behaviour. > > > > Signed-off-by: Alastair D'Silva > > --- > > include/linux/printk.h | 25 +--- > > lib/hexdump.c | 91 > > -- > > 2 files changed, 99 insertions(+), 17 deletions(-) > > > > Hi, > Did you do "make htmldocs" or something similar on this? > > > diff --git a/lib/hexdump.c b/lib/hexdump.c > > index 3943507bc0e9..d61a1e4f19fa 100644 > > --- a/lib/hexdump.c > > +++ b/lib/hexdump.c > > @@ -212,8 +212,44 @@ int hex_dump_to_buffer(const void *buf, size_t > > len, int rowsize, int groupsize, > > EXPORT_SYMBOL(hex_dump_to_buffer); > > > > #ifdef CONFIG_PRINTK > > + > > +/** > > + * Check if a buffer contains only a single byte value > > + * @buf: pointer to the buffer > > + * @len: the size of the buffer in bytes > > + * @val: outputs the value if if the bytes are identical > > Does this work without a function name? > Documentation/doc-guide/kernel-doc.rst says the general format is: > > /** >* function_name() - Brief description of function. >* @arg1: Describe the first argument. >* @arg2: Describe the second argument. >*One can provide multiple line descriptions >*for arguments. >* > > > + */ > > /** > > - * print_hex_dump - print a text hex dump to syslog for a binary > > blob of data > > + * print_hex_dump_ext: dump a binary blob of data to syslog in > > hexadecimal > > Also not in the general documented format. > Thanks Randy, I'll address these. -- Alastair D'Silva mob: 0423 762 819 skype: alastair_dsilva Twitter: @EvilDeece blog: http://alastair.d-silva.org ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [PATCH v2 00/17] kunit: introduce KUnit, the Linux kernel unit testing framework
On Fri, May 10, 2019 at 7:49 AM Knut Omang wrote: > > On Thu, 2019-05-09 at 22:18 -0700, Frank Rowand wrote: > > On 5/9/19 4:40 PM, Logan Gunthorpe wrote: > > > > > > > > > On 2019-05-09 5:30 p.m., Theodore Ts'o wrote: > > >> On Thu, May 09, 2019 at 04:20:05PM -0600, Logan Gunthorpe wrote: > > >>> > > >>> The second item, arguably, does have significant overlap with kselftest. > > >>> Whether you are running short tests in a light weight UML environment or > > >>> higher level tests in an heavier VM the two could be using the same > > >>> framework for writing or defining in-kernel tests. It *may* also be > > >>> valuable > > >>> for some people to be able to run all the UML tests in the heavy VM > > >>> environment along side other higher level tests. > > >>> > > >>> Looking at the selftests tree in the repo, we already have similar > > >>> items to > > >>> what Kunit is adding as I described in point (2) above. > > >>> kselftest_harness.h > > >>> contains macros like EXPECT_* and ASSERT_* with very similar intentions > > >>> to > > >>> the new KUNIT_EXECPT_* and KUNIT_ASSERT_* macros. > > >>> > > >>> However, the number of users of this harness appears to be quite small. > > >>> Most > > >>> of the code in the selftests tree seems to be a random mismash of > > >>> scripts > > >>> and userspace code so it's not hard to see it as something completely > > >>> different from the new Kunit: > > >>> > > >>> $ git grep --files-with-matches kselftest_harness.h * > > >> > > >> To the extent that we can unify how tests are written, I agree that > > >> this would be a good thing. However, you should note that > > >> kselftest_harness.h is currently assums that it will be included in > > >> userspace programs. This is most obviously seen if you look closely > > >> at the functions defined in the header files which makes calls to > > >> fork(), abort() and fprintf(). > > > > > > Ah, yes. I obviously did not dig deep enough. Using kunit for > > > in-kernel tests and kselftest_harness for userspace tests seems like > > > a sensible line to draw to me. Trying to unify kernel and userspace > > > here sounds like it could be difficult so it's probably not worth > > > forcing the issue unless someone wants to do some really fancy work > > > to get it done. > > > > > > Based on some of the other commenters, I was under the impression > > > that kselftests had in-kernel tests but I'm not sure where or if they > > > exist. > > > > YES, kselftest has in-kernel tests. (Excuse the shouting...) > > > > Here is a likely list of them in the kernel source tree: > > > > $ grep module_init lib/test_*.c > > lib/test_bitfield.c:module_init(test_bitfields) > > lib/test_bitmap.c:module_init(test_bitmap_init); > > lib/test_bpf.c:module_init(test_bpf_init); > > lib/test_debug_virtual.c:module_init(test_debug_virtual_init); > > lib/test_firmware.c:module_init(test_firmware_init); > > lib/test_hash.c:module_init(test_hash_init); /* Does everything */ > > lib/test_hexdump.c:module_init(test_hexdump_init); > > lib/test_ida.c:module_init(ida_checks); > > lib/test_kasan.c:module_init(kmalloc_tests_init); > > lib/test_list_sort.c:module_init(list_sort_test); > > lib/test_memcat_p.c:module_init(test_memcat_p_init); > > lib/test_module.c:static int __init test_module_init(void) > > lib/test_module.c:module_init(test_module_init); > > lib/test_objagg.c:module_init(test_objagg_init); > > lib/test_overflow.c:static int __init test_module_init(void) > > lib/test_overflow.c:module_init(test_module_init); > > lib/test_parman.c:module_init(test_parman_init); > > lib/test_printf.c:module_init(test_printf_init); > > lib/test_rhashtable.c:module_init(test_rht_init); > > lib/test_siphash.c:module_init(siphash_test_init); > > lib/test_sort.c:module_init(test_sort_init); > > lib/test_stackinit.c:module_init(test_stackinit_init); > > lib/test_static_key_base.c:module_init(test_static_key_base_init); > > lib/test_static_keys.c:module_init(test_static_key_init); > > lib/test_string.c:module_init(string_selftest_init); > > lib/test_ubsan.c:module_init(test_ubsan_init); > > lib/test_user_copy.c:module_init(test_user_copy_init); > > lib/test_uuid.c:module_init(test_uuid_init); > > lib/test_vmalloc.c:module_init(vmalloc_test_init) > > lib/test_xarray.c:module_init(xarray_checks); > > > > > > > If they do exists, it seems like it would make sense to > > > convert those to kunit and have Kunit tests run-able in a VM or > > > baremetal instance. > > > > They already run in a VM. > > > > They already run on bare metal. > > > > They already run in UML. > > > > This is not to say that KUnit does not make sense. But I'm still trying > > to get a better description of the KUnit features (and there are > > some). > > FYI, I have a master student who looks at converting some of these to KTF, > such as for > instance the XArray tests, which lended themselves quite good to a > semi-automated > conversion. > > The result is also a somewhat more compact code as well as
Re: Improving DRM connector hotplug uevents
On Fri, May 10, 2019 at 10:01 AM Paul Kocialkowski wrote: > > Hi, > > On Fri, 2019-05-10 at 09:56 +0200, Daniel Vetter wrote: > > On Fri, May 10, 2019 at 9:42 AM Paul Kocialkowski > > wrote: > > > Hi, > > > > > > So this thread has drifted off from IGT to a general DRM improvement, > > > so renaming the thread and adding some CCs. > > > > > > It's about avoiding full reprobes (which may include slow EDID reads, > > > etc) when a connector change was detected, by providing information > > > about the connector and the new state to userspace. This way, userspace > > > can reprobe the changed connector alone (or not reprobe at all in case > > > of a disconnect). > > > > > > It feels like there is growing temptation to have per-driver hacks to > > > avoid full reprobes when the driver knows that nothing changed, but I > > > think it makes more sense to fixup the uevent we send to make sure that > > > userspace knows what to probe exactly. > > > > > > On Fri, 2019-05-10 at 06:55 +, Ser, Simon wrote: > > > > On Thu, 2019-05-09 at 11:56 -0400, Lyude Paul wrote: > > > > > I'm in support of this as well, we really could use better hotplug > > > > > events. > > > > > Feel free to cc patches to me for review :) > > > > > > > > Any idea(s) how the API would look like? > > > > > > From a quick chat on IRC yesterday, it seems that just adding entries > > > to the uevent would do and would be backwards-compatible (well, > > > provided that the userspace uevent parsers will not fail if something > > > extra to "HOTPLUG=1" is provided). > > > > > > Currently, the notification is sent globally at each round of detected > > > connector change (which may concern multiple connectors). So I think we > > > should just list the connectors that changed in the uevent and their > > > new state. So my proposal here is something like: > > > > > > HOTPLUG=1 > > > CONNECTOR_ID=47 > > > STATUS=Connected > > > CONNECTOR_ID=48 > > > STATUS=Disconnected > > > > > > Where each STATUS entry would refer to the previous CONNECTOR_ID entry. > > > > > > What do you think? > > > > We have the better uevent already, all we need is wiring up, lots of > > code, and userspace for it. Ok, not quite, patch is still in flight: > > > > https://patchwork.kernel.org/patch/10906677/ > > Ah great to see that! > > > The real trouble isn't the new event, but making sure it goes out for > > anything that might have changed. There's a lot more connector status > > than just the STATUS property. > > Why though? I thought this was a hotplug interface, not a general > "something-about-the-connector-changed". To me, hotplug is pretty > binary: either it's connected, either it's not. > > Do we already send out hotplug uevents when something else than the > status changed? If so, maybe we should consider handling the rest > separately from hotplug. We have plenty of bugs around "changed screen over s/r and no output". Imo if we rework hotplug, we should get all the mistakes right. > Looking at drm_probe_helper.c's output_poll_execute, my understanding > is that drm_kms_helper_hotplug_event is only called when the status > changed, not other props. Yeah, and that's why userspace has to reprobe all the time. One idea that we floated around is to have an epoche counter, which changes every time anything with the connector changes wrt hotplug related state (edid, sink id, whatever). The more specific hotplug uevent would then give you an event on that epoch counter, so userspace knows it needs to reprobe that specific connector for all the new state. -Daniel > Cheers, > > Paul > > > -Daniel > > > > > Cheers, > > > > > > Paul > > > > > > > > On Thu, 2019-05-09 at 14:24 +0200, Paul Kocialkowski wrote: > > > > > > Hi, > > > > > > > > > > > > On Thu, 2019-05-09 at 15:08 +0300, Imre Deak wrote: > > > > > > > On Thu, May 09, 2019 at 09:25:41AM +0200, Paul Kocialkowski wrote: > > > > > > > > On Wed, 2019-05-08 at 11:24 +0300, Imre Deak wrote: > > > > > > > > > After scheduling an HPD toggle event, make sure that we wait > > > > > > > > > for the > > > > > > > > > hotplug event for each connector that may be sent by the > > > > > > > > > driver. > > > > > > > > > > > > > > > > > > Depending on the scheduling there could be 1 event or as many > > > > > > > > > events > > > > > > > > > as > > > > > > > > > connectors we scheduled an HPD toggle event on, depending on > > > > > > > > > the > > > > > > > > > timing. > > > > > > > > > So if we don't yet see the expected connector state on a given > > > > > > > > > connector > > > > > > > > > try to wait for an additional hotplug event and > > > > > > > > > reprobe/recheck the > > > > > > > > > state. > > > > > > > > > > > > > > > > This changes makes good sense to me, so: > > > > > > > > > > > > > > > > Reviewed-By: Paul Kocialkowski > > > > > > > > > > > > > > > > And it brings back hard feelings about the hotplug interface we > > > > > > > > have > > > > > > > > today... > > > > > > > > > > > > > > Yes, I was surpri
Re: Improving DRM connector hotplug uevents
Hi, On Fri, 2019-05-10 at 09:56 +0200, Daniel Vetter wrote: > On Fri, May 10, 2019 at 9:42 AM Paul Kocialkowski > wrote: > > Hi, > > > > So this thread has drifted off from IGT to a general DRM improvement, > > so renaming the thread and adding some CCs. > > > > It's about avoiding full reprobes (which may include slow EDID reads, > > etc) when a connector change was detected, by providing information > > about the connector and the new state to userspace. This way, userspace > > can reprobe the changed connector alone (or not reprobe at all in case > > of a disconnect). > > > > It feels like there is growing temptation to have per-driver hacks to > > avoid full reprobes when the driver knows that nothing changed, but I > > think it makes more sense to fixup the uevent we send to make sure that > > userspace knows what to probe exactly. > > > > On Fri, 2019-05-10 at 06:55 +, Ser, Simon wrote: > > > On Thu, 2019-05-09 at 11:56 -0400, Lyude Paul wrote: > > > > I'm in support of this as well, we really could use better hotplug > > > > events. > > > > Feel free to cc patches to me for review :) > > > > > > Any idea(s) how the API would look like? > > > > From a quick chat on IRC yesterday, it seems that just adding entries > > to the uevent would do and would be backwards-compatible (well, > > provided that the userspace uevent parsers will not fail if something > > extra to "HOTPLUG=1" is provided). > > > > Currently, the notification is sent globally at each round of detected > > connector change (which may concern multiple connectors). So I think we > > should just list the connectors that changed in the uevent and their > > new state. So my proposal here is something like: > > > > HOTPLUG=1 > > CONNECTOR_ID=47 > > STATUS=Connected > > CONNECTOR_ID=48 > > STATUS=Disconnected > > > > Where each STATUS entry would refer to the previous CONNECTOR_ID entry. > > > > What do you think? > > We have the better uevent already, all we need is wiring up, lots of > code, and userspace for it. Ok, not quite, patch is still in flight: > > https://patchwork.kernel.org/patch/10906677/ Ah great to see that! > The real trouble isn't the new event, but making sure it goes out for > anything that might have changed. There's a lot more connector status > than just the STATUS property. Why though? I thought this was a hotplug interface, not a general "something-about-the-connector-changed". To me, hotplug is pretty binary: either it's connected, either it's not. Do we already send out hotplug uevents when something else than the status changed? If so, maybe we should consider handling the rest separately from hotplug. Looking at drm_probe_helper.c's output_poll_execute, my understanding is that drm_kms_helper_hotplug_event is only called when the status changed, not other props. Cheers, Paul > -Daniel > > > Cheers, > > > > Paul > > > > > > On Thu, 2019-05-09 at 14:24 +0200, Paul Kocialkowski wrote: > > > > > Hi, > > > > > > > > > > On Thu, 2019-05-09 at 15:08 +0300, Imre Deak wrote: > > > > > > On Thu, May 09, 2019 at 09:25:41AM +0200, Paul Kocialkowski wrote: > > > > > > > On Wed, 2019-05-08 at 11:24 +0300, Imre Deak wrote: > > > > > > > > After scheduling an HPD toggle event, make sure that we wait > > > > > > > > for the > > > > > > > > hotplug event for each connector that may be sent by the driver. > > > > > > > > > > > > > > > > Depending on the scheduling there could be 1 event or as many > > > > > > > > events > > > > > > > > as > > > > > > > > connectors we scheduled an HPD toggle event on, depending on the > > > > > > > > timing. > > > > > > > > So if we don't yet see the expected connector state on a given > > > > > > > > connector > > > > > > > > try to wait for an additional hotplug event and reprobe/recheck > > > > > > > > the > > > > > > > > state. > > > > > > > > > > > > > > This changes makes good sense to me, so: > > > > > > > > > > > > > > Reviewed-By: Paul Kocialkowski > > > > > > > > > > > > > > And it brings back hard feelings about the hotplug interface we > > > > > > > have > > > > > > > today... > > > > > > > > > > > > Yes, I was surprised too not find any way to identify which > > > > > > connector(s) > > > > > > the hotplug event is associated with. We discussed with Ville if > > > > > > it'd > > > > > > make sense to add a connector ID map to the uevent, but not sure if > > > > > > it'd > > > > > > be useful outside of this test, or how that would align with Chris' > > > > > > connector probe state generation idea. > > > > > > > > > > Either way, I'm definitely in favor of having something more reliable > > > > > to expose to userspace. Having something to correlate each event to > > > > > one > > > > > (or more) connector(s) sounds good to me. > > > > > > > > > > And I think there is a general need for this, it's not just IGT. > > > > > > > > > > Currently, every userspace application using DRM has to do a full > > > > > reprobe once a hotplug uevent i
Re: Improving DRM connector hotplug uevents
On Fri, May 10, 2019 at 9:42 AM Paul Kocialkowski wrote: > > Hi, > > So this thread has drifted off from IGT to a general DRM improvement, > so renaming the thread and adding some CCs. > > It's about avoiding full reprobes (which may include slow EDID reads, > etc) when a connector change was detected, by providing information > about the connector and the new state to userspace. This way, userspace > can reprobe the changed connector alone (or not reprobe at all in case > of a disconnect). > > It feels like there is growing temptation to have per-driver hacks to > avoid full reprobes when the driver knows that nothing changed, but I > think it makes more sense to fixup the uevent we send to make sure that > userspace knows what to probe exactly. > > On Fri, 2019-05-10 at 06:55 +, Ser, Simon wrote: > > On Thu, 2019-05-09 at 11:56 -0400, Lyude Paul wrote: > > > I'm in support of this as well, we really could use better hotplug events. > > > Feel free to cc patches to me for review :) > > > > Any idea(s) how the API would look like? > > From a quick chat on IRC yesterday, it seems that just adding entries > to the uevent would do and would be backwards-compatible (well, > provided that the userspace uevent parsers will not fail if something > extra to "HOTPLUG=1" is provided). > > Currently, the notification is sent globally at each round of detected > connector change (which may concern multiple connectors). So I think we > should just list the connectors that changed in the uevent and their > new state. So my proposal here is something like: > > HOTPLUG=1 > CONNECTOR_ID=47 > STATUS=Connected > CONNECTOR_ID=48 > STATUS=Disconnected > > Where each STATUS entry would refer to the previous CONNECTOR_ID entry. > > What do you think? We have the better uevent already, all we need is wiring up, lots of code, and userspace for it. Ok, not quite, patch is still in flight: https://patchwork.kernel.org/patch/10906677/ The real trouble isn't the new event, but making sure it goes out for anything that might have changed. There's a lot more connector status than just the STATUS property. -Daniel > > Cheers, > > Paul > > > > On Thu, 2019-05-09 at 14:24 +0200, Paul Kocialkowski wrote: > > > > Hi, > > > > > > > > On Thu, 2019-05-09 at 15:08 +0300, Imre Deak wrote: > > > > > On Thu, May 09, 2019 at 09:25:41AM +0200, Paul Kocialkowski wrote: > > > > > > On Wed, 2019-05-08 at 11:24 +0300, Imre Deak wrote: > > > > > > > After scheduling an HPD toggle event, make sure that we wait for > > > > > > > the > > > > > > > hotplug event for each connector that may be sent by the driver. > > > > > > > > > > > > > > Depending on the scheduling there could be 1 event or as many > > > > > > > events > > > > > > > as > > > > > > > connectors we scheduled an HPD toggle event on, depending on the > > > > > > > timing. > > > > > > > So if we don't yet see the expected connector state on a given > > > > > > > connector > > > > > > > try to wait for an additional hotplug event and reprobe/recheck > > > > > > > the > > > > > > > state. > > > > > > > > > > > > This changes makes good sense to me, so: > > > > > > > > > > > > Reviewed-By: Paul Kocialkowski > > > > > > > > > > > > And it brings back hard feelings about the hotplug interface we have > > > > > > today... > > > > > > > > > > Yes, I was surprised too not find any way to identify which > > > > > connector(s) > > > > > the hotplug event is associated with. We discussed with Ville if it'd > > > > > make sense to add a connector ID map to the uevent, but not sure if > > > > > it'd > > > > > be useful outside of this test, or how that would align with Chris' > > > > > connector probe state generation idea. > > > > > > > > Either way, I'm definitely in favor of having something more reliable > > > > to expose to userspace. Having something to correlate each event to one > > > > (or more) connector(s) sounds good to me. > > > > > > > > And I think there is a general need for this, it's not just IGT. > > > > > > > > Currently, every userspace application using DRM has to do a full > > > > reprobe once a hotplug uevent is received, which is really not optimal. > > > > If an entry identifying the connector was also provided, userspace > > > > could only reprobe that connector. The step after that would be having > > > > the indication of whether the connector was connected or disconnected > > > > directly in uevent too, so that no probing needs to be done at all when > > > > a given connector is disconnected. > > > > > > > > Anyway, if you're interested in the idea and that Ville and Chris are > > > > in favor, I really think it would be worth fixing that. And we'd only > > > > be adding new elements to uevent, so that would stay backward- > > > > compatible too. > > > > > > > > Cheers, > > > > > > > > Paul > > > > > > > > > > > v2: > > > > > > > - s/igt_assert(x >= y)/igt_assert_lte(y, x)/ to see the actual > > > > > > > limits > > > > > > > in > > > > > > > the debugging outp
Re: [PATCH] kernel/locking/semaphore: use wake_q in up()
On Fri, May 10, 2019 at 7:50 AM Sergey Senozhatsky
wrote:
>
> On (05/09/19 22:06), Daniel Vetter wrote:
> [..]
> > +/* Functions for the contended case */
> > +
> > +struct semaphore_waiter {
> > + struct list_head list;
> > + struct task_struct *task;
> > + bool up;
> > +};
> > +
> > /**
> > * up - release the semaphore
> > * @sem: the semaphore to release
> > @@ -179,24 +187,25 @@ EXPORT_SYMBOL(down_timeout);
> > void up(struct semaphore *sem)
> > {
> > unsigned long flags;
> > + struct semaphore_waiter *waiter;
> > + DEFINE_WAKE_Q(wake_q);
> >
> > raw_spin_lock_irqsave(&sem->lock, flags);
> > - if (likely(list_empty(&sem->wait_list)))
> > + if (likely(list_empty(&sem->wait_list))) {
> > sem->count++;
> > - else
> > - __up(sem);
> > + } else {
> > + waiter = list_first_entry(&sem->wait_list,
> > +struct semaphore_waiter, list);
> > + list_del(&waiter->list);
> > + waiter->up = true;
> > + wake_q_add(&wake_q, waiter->task);
> > + }
> > raw_spin_unlock_irqrestore(&sem->lock, flags);
>
> So the new code still can printk/WARN under sem->lock in some buggy
> cases.
>
> E.g.
> wake_q_add()
> get_task_struct()
> refcount_inc_checked()
>WARN_ONCE()
>
> Are we fine with that?
Hm not great. It's not as bad as the one I'm trying to fix (or not the
same at least), because with the wake up chain we have a few locks in
there. Which allows lockdep to connect the loop and complain, even
when we never actually hit that specific recursion. I.e. once hitting
a WARN_ON from try_to_wake_up is enough, plus a totally separate
callchain can then close the semaphore.lock->scheduler locks part.
Your chain only goes boom if it happens from the console_lock's up.
wake_q_add_safe would be an option, but then we somehow need to
arrange for down to call get_task_struct(current) and releasing that,
but only if there's no waker who needs that task ref. Sounds tricky
... Also not sure we want to stuff that trickery into the generic
semaphore code.
-Daniel
--
Daniel Vetter
Software Engineer, Intel Corporation
+41 (0) 79 365 57 48 - http://blog.ffwll.ch
___
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel
Improving DRM connector hotplug uevents
Hi, So this thread has drifted off from IGT to a general DRM improvement, so renaming the thread and adding some CCs. It's about avoiding full reprobes (which may include slow EDID reads, etc) when a connector change was detected, by providing information about the connector and the new state to userspace. This way, userspace can reprobe the changed connector alone (or not reprobe at all in case of a disconnect). It feels like there is growing temptation to have per-driver hacks to avoid full reprobes when the driver knows that nothing changed, but I think it makes more sense to fixup the uevent we send to make sure that userspace knows what to probe exactly. On Fri, 2019-05-10 at 06:55 +, Ser, Simon wrote: > On Thu, 2019-05-09 at 11:56 -0400, Lyude Paul wrote: > > I'm in support of this as well, we really could use better hotplug events. > > Feel free to cc patches to me for review :) > > Any idea(s) how the API would look like? From a quick chat on IRC yesterday, it seems that just adding entries to the uevent would do and would be backwards-compatible (well, provided that the userspace uevent parsers will not fail if something extra to "HOTPLUG=1" is provided). Currently, the notification is sent globally at each round of detected connector change (which may concern multiple connectors). So I think we should just list the connectors that changed in the uevent and their new state. So my proposal here is something like: HOTPLUG=1 CONNECTOR_ID=47 STATUS=Connected CONNECTOR_ID=48 STATUS=Disconnected Where each STATUS entry would refer to the previous CONNECTOR_ID entry. What do you think? Cheers, Paul > > On Thu, 2019-05-09 at 14:24 +0200, Paul Kocialkowski wrote: > > > Hi, > > > > > > On Thu, 2019-05-09 at 15:08 +0300, Imre Deak wrote: > > > > On Thu, May 09, 2019 at 09:25:41AM +0200, Paul Kocialkowski wrote: > > > > > On Wed, 2019-05-08 at 11:24 +0300, Imre Deak wrote: > > > > > > After scheduling an HPD toggle event, make sure that we wait for the > > > > > > hotplug event for each connector that may be sent by the driver. > > > > > > > > > > > > Depending on the scheduling there could be 1 event or as many events > > > > > > as > > > > > > connectors we scheduled an HPD toggle event on, depending on the > > > > > > timing. > > > > > > So if we don't yet see the expected connector state on a given > > > > > > connector > > > > > > try to wait for an additional hotplug event and reprobe/recheck the > > > > > > state. > > > > > > > > > > This changes makes good sense to me, so: > > > > > > > > > > Reviewed-By: Paul Kocialkowski > > > > > > > > > > And it brings back hard feelings about the hotplug interface we have > > > > > today... > > > > > > > > Yes, I was surprised too not find any way to identify which connector(s) > > > > the hotplug event is associated with. We discussed with Ville if it'd > > > > make sense to add a connector ID map to the uevent, but not sure if it'd > > > > be useful outside of this test, or how that would align with Chris' > > > > connector probe state generation idea. > > > > > > Either way, I'm definitely in favor of having something more reliable > > > to expose to userspace. Having something to correlate each event to one > > > (or more) connector(s) sounds good to me. > > > > > > And I think there is a general need for this, it's not just IGT. > > > > > > Currently, every userspace application using DRM has to do a full > > > reprobe once a hotplug uevent is received, which is really not optimal. > > > If an entry identifying the connector was also provided, userspace > > > could only reprobe that connector. The step after that would be having > > > the indication of whether the connector was connected or disconnected > > > directly in uevent too, so that no probing needs to be done at all when > > > a given connector is disconnected. > > > > > > Anyway, if you're interested in the idea and that Ville and Chris are > > > in favor, I really think it would be worth fixing that. And we'd only > > > be adding new elements to uevent, so that would stay backward- > > > compatible too. > > > > > > Cheers, > > > > > > Paul > > > > > > > > > v2: > > > > > > - s/igt_assert(x >= y)/igt_assert_lte(y, x)/ to see the actual > > > > > > limits > > > > > > in > > > > > > the debugging output. (Lyude) > > > > > > > > > > > > Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=110534 > > > > > > Cc: Paul Kocialkowski > > > > > > Cc: Lyude Paul > > > > > > Signed-off-by: Imre Deak > > > > > > Reviewed-by: Lyude Paul > > > > > > --- > > > > > > tests/kms_chamelium.c | 45 > > > > > > +- > > > > > > - > > > > > > 1 file changed, 39 insertions(+), 6 deletions(-) > > > > > > > > > > > > diff --git a/tests/kms_chamelium.c b/tests/kms_chamelium.c > > > > > > index 714e5e06..502f1efa 100644 > > > > > > --- a/tests/kms_chamelium.c > > > > > > +++ b/tests/kms_chamelium.c > > > > > > @@ -284,11 +284,32 @@ test_edid_re
[PATCH][next] drm/amdgpu: fix spelling mistake "retrived" -> "retrieved"
From: Colin Ian King
There is a spelling mistake in a DRM_ERROR error message. Fix this.
Signed-off-by: Colin Ian King
---
drivers/gpu/drm/amd/amdgpu/df_v3_6.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/drivers/gpu/drm/amd/amdgpu/df_v3_6.c
b/drivers/gpu/drm/amd/amdgpu/df_v3_6.c
index 7d375f8dcce6..a5c3558869fb 100644
--- a/drivers/gpu/drm/amd/amdgpu/df_v3_6.c
+++ b/drivers/gpu/drm/amd/amdgpu/df_v3_6.c
@@ -194,7 +194,7 @@ static void df_v3_6_pmc_get_ctrl_settings(struct
amdgpu_device *adev,
return;
if ((*lo_base_addr == 0) || (*hi_base_addr == 0)) {
- DRM_ERROR("DF PMC addressing not retrived! Lo: %x, Hi: %x",
+ DRM_ERROR("DF PMC addressing not retrieved! Lo: %x, Hi: %x",
*lo_base_addr, *hi_base_addr);
return;
}
--
2.20.1
___
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel
