[Bug 200695] Blank screen on RX 580 with amdgpu.dc=1 enabled (no displays detected)

2020-06-16 Thread bugzilla-daemon
https://bugzilla.kernel.org/show_bug.cgi?id=200695

--- Comment #45 from Sergey Kondakov (virtuous...@gmail.com) ---
(In reply to Alex Deucher from comment #44)
> There is no support for analog displays in DC.

Which was a horrible decision. Luckily, decent DP->VGA adapters actually work
even with proper ≥1080p ≥85fps CRTs. But it still has some problems with DVI
ports and/or HDMI->DVI adapters or certain monitors. Seems my issue with no
output at all in UEFI mode was a separate one and has to do with BIOS's signing
DRM nonsense. Hacked VBIOS works fine with UEFI… if you don't count just
initializing sole DP port at boot… which has backup/special-purpose CRT
connected and not real primary display in DVI.

But in either mode with dc=1 it just doesn't want to acknowledge my BENQ
G2320HDB connected with a simple HDMI->DVI adapter, as if it doesn't exist, and
there is no second DVI port. Maybe HDCP shenanigans ?

There is also another issue with amdgpu failing to initialize and hanging
display output at boot, if 64-bit PCIe addressing ("above 4G decoding") is
enabled, unless pcie=nocrs specified for kernel but that's a third issue and
likely has to do with crappy BIOS of my new unlicensed LGA2011 motherboard
(even though it works fine on Windows):
amdgpu: [powerplay] 
 failed to send message 254 ret is 0 
amdgpu: [powerplay] SMU load firmware failed
amdgpu: [powerplay] fw load failed
smu firmware loading failed
amdgpu :03:00.0: amdgpu_device_ip_init failed
amdgpu :03:00.0: Fatal error during GPU init
[drm] amdgpu: finishing device.

-- 
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 200695] Blank screen on RX 580 with amdgpu.dc=1 enabled (no displays detected)

2020-06-16 Thread bugzilla-daemon
https://bugzilla.kernel.org/show_bug.cgi?id=200695

--- Comment #44 from Alex Deucher (alexdeuc...@gmail.com) ---
There is no support for analog displays in DC.

-- 
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 208209] [amdgpu] driver crash -- enable_link_dp -- RX 570

2020-06-16 Thread bugzilla-daemon
https://bugzilla.kernel.org/show_bug.cgi?id=208209

--- Comment #3 from Alex Deucher (alexdeuc...@gmail.com) ---
Please attach your xorg log and dmesg output.

-- 
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: [drm/mgag200] e44e907dd8: phoronix-test-suite.glmark2.800x600.score -64.9% regression

2020-06-16 Thread Rong Chen



On 6/16/20 9:49 PM, Thomas Zimmermann wrote:

Hi

Am 16.06.20 um 05:10 schrieb Rong Chen:


On 6/16/20 4:58 AM, Emil Velikov wrote:

Hi all,

On Thu, 4 Jun 2020 at 08:11, kernel test robot 
wrote:

Greeting,

FYI, we noticed a -64.9% regression of
phoronix-test-suite.glmark2.800x600.score due to commit:


On one hand, I'm really happy to see performance testing happening
although this report is missing various crucial pieces of information.


commit: e44e907dd8f937313d35615d799d54162c56d173 ("[PATCH v3 05/15]
drm/mgag200: Split MISC register update into PLL selection, SYNC and
I/O")
url:
https://github.com/0day-ci/linux/commits/Thomas-Zimmermann/drm-mgag200-Convert-to-atomic-modesetting/20200515-163744

base: git://anongit.freedesktop.org/drm/drm-tip drm-tip

in testcase: phoronix-test-suite
on test machine: 16 threads Intel(R) Xeon(R) CPU X5570 @ 2.93GHz with
48G memory
with following parameters:

  need_x: true

Replace "need_x" with the Xorg version as seen in `Xorg -version'.

# Xorg -version
/bin/sh: warning: setlocale: LC_ALL: cannot change locale (en_US.UTF-8)

X.Org X Server 1.20.4
X Protocol Version 11, Revision 0
Build Operating System: Linux 4.9.0-8-amd64 x86_64 Debian
Current Operating System: Linux lkp-nhm-2ep1
5.7.0-rc5-01428-ge44e907dd8f937 #1 SMP Tue Jun 2 19:51:38 CST 2020 x86_64
Kernel command line:  ip=lkp-nhm-2ep1::dhcp
root=/dev/disk/by-id/wwn-0x55cd2e4123123127-part2
rootflags=subvol=debian-x86_64-phoronix
remote_rootfs=internal-lkp-server:/osimage/debian/debian-x86_64-phoronix
user=lkp
job=/lkp/jobs/scheduled/lkp-nhm-2ep1/phoronix-test-suite-performance-true-glmark2-1.1.0-ucode=0x1d-debian-x86_64-phoronix-e44e907dd8f937313d35615d799d54162c56d173-20200616-56456-1kgmjzm-0.yaml
ARCH=x86_64 kconfig=x86_64-rhel-7.6
branch=linux-devel/devel-hourly-2020051600
commit=e44e907dd8f937313d35615d799d54162c56d173
BOOT_IMAGE=/pkg/linux/x86_64-rhel-7.6/gcc-7/e44e907dd8f937313d35615d799d54162c56d173/vmlinuz-5.7.0-rc5-01428-ge44e907dd8f937
console=ttyS1,115200 console=tty0 max_uptime=3600
RESULT_ROOT=/result/phoronix-test-suite/performance-true-glmark2-1.1.0-ucode=0x1d/lkp-nhm-2ep1/debian-x86_64-phoronix/x86_64-rhel-7.6/gcc-7/e44e907dd8f937313d35615d799d54162c56d173/4
LKP_SERVER=inn nokaslr selinux=0 debug apic=debug sysrq_always_enabled
rcupdate.rcu_cpu_stall_timeout=100 net.ifnames=0 printk.devkmsg=on
panic=-1 softlockup_panic=1 nmi_watchdog=panic oops=panic load_ramdisk=2
prompt_ramdisk=0 drbd.minor_count=8 systemd.log_level=err
ignore_loglevel console=tty0 earlyprintk=ttyS0,115200
console=ttyS0,115200 vga=normal rw
Build Date: 05 March 2019  08:11:12PM
xorg-server 2:1.20.4-1 (https://www.debian.org/support)
Current version of pixman: 0.36.0
     Before reporting problems, check http://wiki.x.org
     to make sure that you have the latest version.


  test: glmark2-1.1.0
  cpufreq_governor: performance
  ucode: 0x1d

test-description: The Phoronix Test Suite is the most comprehensive
testing and benchmarking platform available that provides an
extensible framework for which new tests can be easily added.
test-url: http://www.phoronix-test-suite.com/


Please remove the test description and url. They don't add any value.

Mention which Mesa version is used as well as on what GPU. The output
of lspci and glxinfo will help here.

Attached please find the outputs of lspci and glxinfo


For this particular test - there is no Mesa/upstream driver for this
GPU, so I imagine one of the swrast drivers was used. Which one -
swrast (classic, softpipe, llvmpipe, swr) or kms_swrast.
The output of `LD_DEBUG=libs glxinfo  |& grep _dri.so` will help here.

# LD_DEBUG=libs glxinfo  |& grep _dri.so
   2132: calling init: /usr/lib/i386-linux-gnu/dri/swrast_dri.so
   2132: calling fini: /usr/lib/i386-linux-gnu/dri/swrast_dri.so [0]

Best Regards,
Rong Chen

Thanks for testing. If I send out a patch, could you try it?


Yes, we can test the new patch if still needed.

Best Regards,
Rong Chen



Best regards
Thomas


commit:
    bef2303526 ("drm/mgag200: Move mode-setting code into separate
helper function")
    e44e907dd8 ("drm/mgag200: Split MISC register update into PLL
selection, SYNC and I/O")


Actually the offending commit has a subtle change of behaviour - it
adds an extra MGAREG_MISC_RAMMAPEN.
That is not documented and I've failed to spot it during review.

Thomas - shall we revert that line in itself or at least add an inline
comment why it is needed?


    100
+-+
     90 |-+    +  +   +.+  +    + +    +  +
:   |
    | :    :  :   : :  :    : :    :  :
:   |
     80 |-:    :  :   : :  :    : :    :  :
:   |
     70 |-::   : ::   :  : :   :: ::   : ::
:    |
    |: :  : :    : : :   :    : :  

Re: [drm/mgag200] e44e907dd8: phoronix-test-suite.glmark2.800x600.score -64.9% regression

2020-06-16 Thread Rong Chen

Hi Emil,

Thanks for the guidance, we'll add these information in future reports.

Best Regards,
Rong Chen

On 6/16/20 9:49 PM, Emil Velikov wrote:

Hi Rong,

Thanks for the prompt reply and information. Can I suggest including
the suggested information in future reports.
I've included a command for each one, to aid automating things.

Namely:
Xorg: 1.20.4 (or None)
$ which Xorg 2>/dev/null  || echo None && Xorg -version |& grep -o "X
Server.*" | sed "s/X Server//"

Mesa: 20.0.7
$ grep -E -o "Mesa [1-9]+.*" | head -n1 | sed "s/Mesa//"

Mesa module: swrast_dri.so
$ basename `LD_DEBUG=libs glxinfo |& grep _dri.so | head -n1 | cut -f3 -d:`

Mesa device: llvmpipe (LLVM 10.0.0, 128 bits) (0x)
$ glxinfo | grep -i device | cut -f2 -d:

GPU: Matrox Electronics ...
$ lspci -nn | grep -E "VGA|Display|3D" | cut -f2- -d:

Thanks
-Emil


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


[Bug 208209] [amdgpu] driver crash -- enable_link_dp -- RX 570

2020-06-16 Thread bugzilla-daemon
https://bugzilla.kernel.org/show_bug.cgi?id=208209

--- Comment #2 from max.bruc...@gmail.com ---
I am using X, there aren't any interesting logs within the prior 12 hours or
since.
[901953.262] (II) AMDGPU(0): EDID vendor "SAM", prod id 3427
[901953.262] (II) AMDGPU(0): Using hsync ranges from config file
[901953.262] (II) AMDGPU(0): Using vrefresh ranges from config file
[901953.262] (II) AMDGPU(0): Printing DDC gathered Modelines:
[901953.262] (II) AMDGPU(0): Modeline "3840x2160"x0.0  533.25  3840 3888 3920
4000  2160 2163 2168  +hsync -vsync (133.3 kHz eP)
[901953.262] (II) AMDGPU(0): Modeline "1920x1080"x0.0  148.50  1920 2008 2052
2200  1080 1084 1089 1125 +hsync +vsync (67.5 kHz e)
[901953.262] (II) AMDGPU(0): Modeline "2560x1440"x0.0  241.50  2560 2608 2640
2720  1440 1443 1448 1481 +hsync -vsync (88.8 kHz e)
[901953.262] (II) AMDGPU(0): Modeline "800x600"x0.0   40.00  800 840 968 1056 
600 601 605 628 +hsync +vsync (37.9 kHz e)
[901953.262] (II) AMDGPU(0): Modeline "800x600"x0.0   36.00  800 824 896 1024 
600 601 603 625 +hsync +vsync (35.2 kHz e)
[901953.262] (II) AMDGPU(0): Modeline "640x480"x0.0   31.50  640 656 720 840 
480 481 484 500 -hsync -vsync (37.5 kHz e)
[901953.262] (II) AMDGPU(0): Modeline "640x480"x0.0   31.50  640 664 704 832 
480 489 492 520 -hsync -vsync (37.9 kHz e)
[901953.262] (II) AMDGPU(0): Modeline "640x480"x0.0   30.24  640 704 768 864 
480 483 486 525 -hsync -vsync (35.0 kHz e)
[901953.262] (II) AMDGPU(0): Modeline "640x480"x0.0   25.18  640 656 752 800 
480 490 492 525 -hsync -vsync (31.5 kHz e)
[901953.262] (II) AMDGPU(0): Modeline "720x400"x0.0   28.32  720 738 846 900 
400 412 414 449 -hsync +vsync (31.5 kHz e)
[901953.262] (II) AMDGPU(0): Modeline "1280x1024"x0.0  135.00  1280 1296 1440
1688  1024 1025 1028 1066 +hsync +vsync (80.0 kHz e)
[901953.262] (II) AMDGPU(0): Modeline "1024x768"x0.0   78.75  1024 1040 1136
1312  768 769 772 800 +hsync +vsync (60.0 kHz e)
[901953.262] (II) AMDGPU(0): Modeline "1024x768"x0.0   75.00  1024 1048 1184
1328  768 771 777 806 -hsync -vsync (56.5 kHz e)
[901953.262] (II) AMDGPU(0): Modeline "1024x768"x0.0   65.00  1024 1048 1184
1344  768 771 777 806 -hsync -vsync (48.4 kHz e)
[901953.262] (II) AMDGPU(0): Modeline "832x624"x0.0   57.28  832 864 928 1152 
624 625 628 667 -hsync -vsync (49.7 kHz e)
[901953.262] (II) AMDGPU(0): Modeline "800x600"x0.0   49.50  800 816 896 1056 
600 601 604 625 +hsync +vsync (46.9 kHz e)
[901953.262] (II) AMDGPU(0): Modeline "800x600"x0.0   50.00  800 856 976 1040 
600 637 643 666 +hsync +vsync (48.1 kHz e)
[901953.262] (II) AMDGPU(0): Modeline "1152x864"x0.0  108.00  1152 1216 1344
1600  864 865 868 900 +hsync +vsync (67.5 kHz e)
[901953.262] (II) AMDGPU(0): Modeline "1280x800"x0.0   83.50  1280 1352 1480
1680  800 803 809 831 -hsync +vsync (49.7 kHz e)
[901953.262] (II) AMDGPU(0): Modeline "1280x720"x60.0   74.48  1280 1336 1472
1664  720 721 724 746 -hsync +vsync (44.8 kHz e)
[901953.262] (II) AMDGPU(0): Modeline "1280x1024"x0.0  108.00  1280 1328 1440
1688  1024 1025 1028 1066 +hsync +vsync (64.0 kHz e)
[901953.262] (II) AMDGPU(0): Modeline "1600x900"x60.0  119.00  1600 1696 1864
2128  900 901 904 932 -hsync +vsync (55.9 kHz e)
[901953.262] (II) AMDGPU(0): Modeline "1680x1050"x0.0  146.25  1680 1784 1960
2240  1050 1053 1059 1089 -hsync +vsync (65.3 kHz e)
[901953.262] (II) AMDGPU(0): Modeline "1440x900"x0.0  106.50  1440 1520 1672
1904  900 903 909 934 -hsync +vsync (55.9 kHz e)
[901953.813] (II) AMDGPU(0): EDID vendor "SAM", prod id 3427
[901953.813] (II) AMDGPU(0): Using hsync ranges from config file
[901953.813] (II) AMDGPU(0): Using vrefresh ranges from config file
[901953.813] (II) AMDGPU(0): Printing DDC gathered Modelines:
[901953.813] (II) AMDGPU(0): Modeline "3840x2160"x0.0  533.25  3840 3888 3920
4000  2160 2163 2168  +hsync -vsync (133.3 kHz eP)
[901953.813] (II) AMDGPU(0): Modeline "1920x1080"x0.0  148.50  1920 2008 2052
2200  1080 1084 1089 1125 +hsync +vsync (67.5 kHz e)
[901953.813] (II) AMDGPU(0): Modeline "2560x1440"x0.0  241.50  2560 2608 2640
2720  1440 1443 1448 1481 +hsync -vsync (88.8 kHz e)
[901953.813] (II) AMDGPU(0): Modeline "800x600"x0.0   40.00  800 840 968 1056 
600 601 605 628 +hsync +vsync (37.9 kHz e)
[901953.813] (II) AMDGPU(0): Modeline "800x600"x0.0   36.00  800 824 896 1024 
600 601 603 625 +hsync +vsync (35.2 kHz e)
[901953.813] (II) AMDGPU(0): Modeline "640x480"x0.0   31.50  640 656 720 840 
480 481 484 500 -hsync -vsync (37.5 kHz e)
[901953.813] (II) AMDGPU(0): Modeline "640x480"x0.0   31.50  640 664 704 832 
480 489 492 520 -hsync -vsync (37.9 kHz e)
[901953.813] (II) AMDGPU(0): Modeline "640x480"x0.0   30.24  640 704 768 864 
480 483 486 525 -hsync -vsync (35.0 kHz e)
[901953.813] (II) AMDGPU(0): Modeline "640x480"x0.0   25.18  640 656 752 800 
480 490 492 525 -hsync -vsync (31.5 kHz e)
[901953.813] (II) AMDGPU(0): Modeline "720x400"x0.0   28.32  720 738 846 900 
400 412 414 449 -hsync +vsync (31.5 kHz e)
[901953.813] (II) AMDGPU(0): Modeline 

linux-next: build failure after merge of the drm-misc tree

2020-06-16 Thread Stephen Rothwell
Hi all,

After merging the drm-misc tree, today's linux-next build (x86_64
allmodconfig) failed like this:

drivers/gpu/drm/amd/amdgpu/amdgpu_amdkfd_gpuvm.c: In function 
'amdgpu_amdkfd_gpuvm_free_memory_of_gpu':
drivers/gpu/drm/amd/amdgpu/amdgpu_amdkfd_gpuvm.c:1357:2: error: implicit 
declaration of function 'drm_gem_object_put_unlocked'; did you mean 
'drm_gem_object_put_locked'? [-Werror=implicit-function-declaration]
 1357 |  drm_gem_object_put_unlocked(>bo->tbo.base);
  |  ^~~
  |  drm_gem_object_put_locked

Caused by commit

  ab15d56e27be ("drm: remove transient drm_gem_object_put_unlocked()")

interacting with commit

  fd9a9f8801de ("drm/amdgpu: Use GEM obj reference for KFD BOs")

from Linus' tree.

I have applied the following merge fix up patch for today.

From: Stephen Rothwell 
Date: Wed, 17 Jun 2020 10:55:32 +1000
Subject: [PATCH] drm/amdgpu: remove stray drm_gem_object_put_unlocked

Signed-off-by: Stephen Rothwell 
---
 drivers/gpu/drm/amd/amdgpu/amdgpu_amdkfd_gpuvm.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_amdkfd_gpuvm.c 
b/drivers/gpu/drm/amd/amdgpu/amdgpu_amdkfd_gpuvm.c
index b91b5171270f..9015c7b76d60 100644
--- a/drivers/gpu/drm/amd/amdgpu/amdgpu_amdkfd_gpuvm.c
+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_amdkfd_gpuvm.c
@@ -1354,7 +1354,7 @@ int amdgpu_amdkfd_gpuvm_free_memory_of_gpu(
}
 
/* Free the BO*/
-   drm_gem_object_put_unlocked(>bo->tbo.base);
+   drm_gem_object_put(>bo->tbo.base);
mutex_destroy(>lock);
kfree(mem);
 
-- 
2.26.2

-- 
Cheers,
Stephen Rothwell


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


linux-next: manual merge of the drm-misc tree with Linus' tree

2020-06-16 Thread Stephen Rothwell
Hi all,

Today's linux-next merge of the drm-misc tree got a conflict in:

  drivers/gpu/drm/nouveau/nouveau_display.c

between commit:

  183405879255 ("drm/nouveau/kms: Remove field nvbo from struct 
nouveau_framebuffer")

from Linus' tree and commit:

  cdc194cebd71 ("drm/nouveau: remove _unlocked suffix in 
drm_gem_object_put_unlocked")

from the drm-misc tree.

I fixed it up (the former just removed one of the functions modified
by the latter) and can carry the fix as necessary. This is now fixed as
far as linux-next is concerned, but any non trivial conflicts should be
mentioned to your upstream maintainer when your tree is submitted for
merging.  You may also want to consider cooperating with the maintainer
of the conflicting tree to minimise any particularly complex conflicts.

-- 
Cheers,
Stephen Rothwell


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


Re: [PATCH 4/4] drm: pl111: Update documentation

2020-06-16 Thread Philip Li
On Tue, Jun 16, 2020 at 05:12:51PM +0100, Emil Velikov wrote:
> Hi all,
> 
> Inspired by Linus and my personal confusion with the report, allow me
> to put some suggestions. Followed by an illustrative example.
thanks for all the suggestions, we will take them into consideration
to gradually improve the report.

> 
> On Wed, 10 Jun 2020 at 08:38, kernel test robot  wrote:
> >
> > Hi Linus,
> >
> > I love your patch! Perhaps something to improve:
> >
> > [auto build test WARNING on drm-exynos/exynos-drm-next]
> > [also build test WARNING on drm-intel/for-linux-next 
> > tegra-drm/drm/tegra/for-next linus/master v5.7 next-20200609]
> One thing which was always inclear me - is the warning identical in
> all the branches listed.
yes, they contain same warning, we will clarify this.

> 
> > [cannot apply to drm-tip/drm-tip drm/drm-next]
> > [if your patch is applied to the wrong git tree, please drop us a note to 
> > help
> > improve the system. BTW, we also suggest to use '--base' option to specify 
> > the
> > base tree in git format-patch, please see 
> > https://stackoverflow.com/a/37406982]
> >
> Reference to the manual tends to be better than an old SO thread.
got it, we will refer to your later example to enhance here.

> 
> > url:
> > https://github.com/0day-ci/linux/commits/Linus-Walleij/drm-pl111-Credit-where-credit-is-due/20200610-041025
> > base:   
> > https://git.kernel.org/pub/scm/linux/kernel/git/daeinki/drm-exynos.git 
> > exynos-drm-next
> > reproduce: make htmldocs
> >
> > If you fix the issue, kindly add following tag as appropriate
> > Reported-by: kernel test robot 
> >
> > All warnings (new ones prefixed by >>, old ones prefixed by <<):
> >
> Please do not list old warnings/errors. They only distract and dilute
> what/where the problem is.
early on, there's a request to view the old warning of modpost related
issue in case it is a false positive due to modpost output format changing.

we will think of this to limit to modpost only.

> 
> 
> > vim +1 drivers/gpu/drm/pl111/pl111_drv.c
> >
> > e559355a9da60a Thomas Gleixner 2019-06-01  @1  // SPDX-License-Identifier: 
> > GPL-2.0-only
> > bed41005e6174d Tom Cooksey 2017-04-12   2  /*
> > bed41005e6174d Tom Cooksey 2017-04-12   3   * (C) COPYRIGHT 2012-2013 
> > ARM Limited. All rights reserved.
> > bed41005e6174d Tom Cooksey 2017-04-12   4   *
> > bed41005e6174d Tom Cooksey 2017-04-12   5   * Parts of this file were 
> > based on sources as follows:
> > bed41005e6174d Tom Cooksey 2017-04-12   6   *
> > bed41005e6174d Tom Cooksey 2017-04-12   7   * Copyright (c) 2006-2008 
> > Intel Corporation
> > bed41005e6174d Tom Cooksey 2017-04-12   8   * Copyright (c) 2007 Dave 
> > Airlie 
> > bed41005e6174d Tom Cooksey 2017-04-12   9   * Copyright (C) 2011 Texas 
> > Instruments
> > bed41005e6174d Tom Cooksey 2017-04-12  10   */
> > bed41005e6174d Tom Cooksey 2017-04-12  11
> >
> > :: The code at line 1 was first introduced by commit
> > :: e559355a9da60a2388893d9e9da66c79fd604b9a treewide: Replace GPLv2 
> > boilerplate/reference with SPDX - rule 443
> >
> > :: TO: Thomas Gleixner 
> > :: CC: Greg Kroah-Hartman 
> >
> All of this seems useful when debugging the kernel test robot itself.
> Which in this case is misleading as hell.
thanks, we will check above 4 lines with : to adjust if needed.

> 
> 
> Here is something which is much shorter, with clearer structure and
> reads much easier.
this is very helpful and can provide a different perspective for how
the report is. We will learn from it to imporve the report.

> 
> ---
> Hi Linus,
> 
> This is an automated message from your friendly test robot.
> We've noticed some issues with your patch although being a robot,
> kindly double-check the results.
> 
> Branches:
> [if the warning/errors are the same group them, otherwise split them
> in separate sections]
> 
> - drm-exynos/exynos-drm-next [1]: WARNING
> - drm-intel/for-linux-next [2]: WARNING
> -  : WARNING
> drivers/gpu/drm/pl111/pl111_drv.c:1: warning: 'ARM PrimeCell PL111
> CLCD Driver' not found
> - tegra-drm/drm/tegra/for-next [3]: WARNING
> some other warning
> - drm-tip/drm-tip [4] drm/drm-next [5]: cannot apply series
> 
> Note: when submitting patches, please use `--base` as documented in
> https://git-scm.com/docs/git-format-patch.
> 
> 
> To reproduce:
>  - wget https://url/to/build/toolchain // when applicable
>  - wget https://url/to/config // when applicable
>  - make htmldocs // use explicit make command instead of magic shell
> script, as seen in the cross build reports
> 
> 
> If you fix the issue, kindly add following tag as appropriate
> Reported-by: kernel test robot 
> 
> Thank you
> The LKP team
> 
> [1] URI to the drm-exynos/exynos-drm-next repo
> [2] URI to the drm-intel/for-linux-next repo
> [3] URI to the ...
> 
___
dri-devel mailing list
dri-devel@lists.freedesktop.org

[Bug 200695] Blank screen on RX 580 with amdgpu.dc=1 enabled (no displays detected)

2020-06-16 Thread bugzilla-daemon
https://bugzilla.kernel.org/show_bug.cgi?id=200695

Lukas Fink (lukas.fi...@gmail.com) changed:

   What|Removed |Added

 CC||lukas.fi...@gmail.com

--- Comment #43 from Lukas Fink (lukas.fi...@gmail.com) ---
(In reply to babgozd from comment #38)
> I am following this thread since it has created. My R9 380 with DVI-I output
> is connected to my VGA monitor with DVI-I male to VGA female adapter and I
> am getting black/blank screen while booting with amdgpu.dc=1 which is
> default since kernel 4.17. It is fine with amdgpu.dc=0. I can provide logs
> too if any kernel contributor wants to investigate the problem any further.

To me it seems, as if the DVI-I port is somehow misdetected/misconfigured as a
DVI-D port. With dc=0 I find the files "card1-DVI-I-1" and "card1-DVI-D-1" in
"/sys/class/drm/card1". With dc=1 there are "card1-DVI-D-1" and
"card1-DVI-D-2".

It appears to correctly negotiate resolution and refresh rate over DDC, but
because it only outputs the digital signal, a display connected over VGA
receives nothing and turns off.

-- 
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 v6 1/5] dt-bindings: msm/dp: add bindings of DP/DP-PLL driver for Snapdragon

2020-06-16 Thread tanmay

Thanks Stephen for your answers.

On 2020-06-16 03:55, Stephen Boyd wrote:

Quoting tan...@codeaurora.org (2020-06-11 13:07:09)

On 2020-06-09 19:15, Stephen Boyd wrote:
> Quoting Tanmay Shah (2020-06-08 20:38:18)
>> diff --git
>> a/Documentation/devicetree/bindings/display/msm/dp-sc7180.yaml
>> b/Documentation/devicetree/bindings/display/msm/dp-sc7180.yaml
>> new file mode 100644
>> index 000..5fdb915
>> --- /dev/null
>> +++ b/Documentation/devicetree/bindings/display/msm/dp-sc7180.yaml
>
> Typically the file name matches the compatible string. But the
> compatible string is just qcom,dp-display. Maybe the compatible string
> should be qcom,sc7180-dp? Notice that the SoC number comes first as is
> preferred.
>
These bindings will be similar for upcoming SOC as well.
So just for understanding, when we add new SOC do we create new file
with same bidings
with SOC number in new file name?
Instead we can keep this file's name as qcom,dp-display.yaml (same as
compatible const) and we can include SOC number in compatible enum ?
some examples:
https://patchwork.kernel.org/patch/11448357/
https://patchwork.kernel.org/patch/11164619/


Yes that works too. It's really up to robh here.


>
>> @@ -0,0 +1,142 @@
>> +# SPDX-License-Identifier: (GPL-2.0-only  OR BSD-2-Clause)
>> +%YAML 1.2
>> +---
>> +$id: http://devicetree.org/schemas/display/msm/dp-sc7180.yaml#
>> +$schema: http://devicetree.org/meta-schemas/core.yaml#
>> +
>> +title: Qualcomm Display Port Controller.
>> +
>> +maintainers:
>> +  - Chandan Uddaraju 
>> +  - Vara Reddy 
>> +  - Tanmay Shah 
>> +
>> +description: |
>> +  Device tree bindings for MSM Display Port which supports DP host
>> controllers
>> +  that are compatible with VESA Display Port interface specification.
>> +
>> +properties:
>> +  compatible:
>> +items:
>> +  - const: qcom,dp-display
>> +
>> +  cell-index:
>> +description: Specifies the controller instance.
>> +
>> +  reg:
>> +items:
>> +  - description: DP controller registers
>> +
>> +  interrupts:
>> +description: The interrupt signal from the DP block.
>> +
>> +  clocks:
>> +description: List of clock specifiers for clocks needed by the
>> device.
>> +items:
>> +  - description: Display Port AUX clock
>> +  - description: Display Port Link clock
>> +  - description: Link interface clock between DP and PHY
>> +  - description: Display Port Pixel clock
>> +  - description: Root clock generator for pixel clock
>> +
>> +  clock-names:
>> +description: |
>> +  Device clock names in the same order as mentioned in clocks
>> property.
>> +  The required clocks are mentioned below.
>> +items:
>> +  - const: core_aux
>> +  - const: ctrl_link
>> +  - const: ctrl_link_iface
>> +  - const: stream_pixel
>> +  - const: pixel_rcg
>
> Why not just 'pixel'? And why is the root clk generator important? It
> looks like this binding should be using the assigned clock parents
> property instead so that it doesn't have to call clk_set_parent()
> explicitly.
>
Are we talking about renaming stream_pixel to pixel only?
We divide clocks in categories: core, control and stream clock.
Similar terminology will be used in subsequent driver patches as well.

We can remove pixel_rcg use assigned clock parents property and remove
clk_set_parent
from driver.


Cool. Using assigned clock parents is good.



>> +  "#clock-cells":
>> +const: 1
>> +
>> +  vdda-1p2-supply:
>> +description: phandle to vdda 1.2V regulator node.
>> +
>> +  vdda-0p9-supply:
>> +description: phandle to vdda 0.9V regulator node.
>> +
>> +  data-lanes = <0 1>:
>
> Is this correct? We can have =  in the property name? Also feels
> generic and possibly should come from the phy binding instead of from
> the controller binding.
>
We are using this property in DP controller programming sequence such 
as

link training.
So I think we can keep this here.
You are right about . <0 1> part should be in example only. It
was passing through dt_binding_check though.
Here it should be like:
data-lanes:
minItems:1
maxItems:4


Ok.



>> +type: object
>> +description: Maximum number of lanes that can be used for Display
>> port.
>> +
>> +  ports:
>> +description: |
>> +   Contains display port controller endpoint subnode.
>> +   remote-endpoint: |
>> + For port@0, set to phandle of the connected panel/bridge's
>> + input endpoint. For port@1, set to the DPU interface output.
>> + Documentation/devicetree/bindings/graph.txt and
>> +
>> Documentation/devicetree/bindings/media/video-interfaces.txt.
>> +
>> +patternProperties:
>> +  "^aux-cfg([0-9])-settings$":
>> +type: object
>> +description: |
>> +  Specifies the DP AUX configuration [0-9] settings.
>> +  The first entry in this array corresponds to the register
>> offset
>> +  within DP AUX, while the remaining entries indicate the
>> +  programmable values.
>
> I'd prefer this was removed 

Re: [RFC PATCH 2/2] drm: xlnx: driver for Xilinx DSI TX Subsystem

2020-06-16 Thread Laurent Pinchart
Hi GVRao,

Sorry for the delayed reply.

On Tue, Jun 09, 2020 at 02:48:25AM +, Venkateshwar Rao Gannavarapu wrote:
> Hi Laurent,
> 
> Thanks for the review. 
> Please see my comments about D-PHY and bridge driver implementation.
> 
> On Sunday, June 7, 2020 7:55 AM, Laurent Pinchart wrote:
> > On Sun, May 31, 2020 at 05:41:50PM +, Venkateshwar Rao Gannavarapu 
> > wrote:
> >> On Sunday, May 24, 2020 8:38 AM, Laurent Pinchart wrote:
> >>> On Mon, May 04, 2020 at 11:43:48AM -0700, Hyun Kwon wrote:
>  On Mon, 2020-04-20 at 14:20:56 -0700, Venkateshwar Rao Gannavarapu wrote:
> > The Xilinx MIPI DSI Tx Subsystem soft IP is used to display video
> > data from AXI-4 stream interface.
> >
> > It supports upto 4 lanes, optional register interface for the
> > DPHY,
> 
>  I don't see the register interface for dphy support.
> >>>
> >>> I think the D-PHY should be supported through a PHY driver, as it
> >>> seems to be shared between different subsystems.
> >>
> >> IP has the provision to read DPHY register for debug purpose only.
> >> No programming of DPHY is required in subsystem.
> >
> > Do you know if this is the same D-PHY as used in the CSI2-RX subsystem ?
>  
> Same D-PHY core has been used in MIPI CSI2 RXSS, but with different 
> configuration.
>
> > multiple RGB color formats, command mode and video mode.
> > This is a MIPI-DSI host driver and provides DSI bus for panels.
> > This driver also helps to communicate with its panel using panel
> > framework.
> >
> > Signed-off-by: Venkateshwar Rao Gannavarapu
> > 
> > ---
> >  drivers/gpu/drm/xlnx/Kconfig|  11 +
> >  drivers/gpu/drm/xlnx/Makefile   |   2 +
> >  drivers/gpu/drm/xlnx/xlnx_dsi.c | 755 
> > 
> >>>
> >>> Daniel Vetter has recently expressed his opiion that bridge drivers
> >>> should go to drivers/gpu/drm/bridge/. It would then be
> >>> drivers/gpu/drm/bridge/xlnx/. I don't have a strong opinion myself.
> 
> The DSI-TX subsystem IP block is not a bridge driver.
> The DSI-TX subsystem IP block itself contains all the drm encoder/connector
> functionality and it’s the last node in display pipe line.

The DSI-TX subsystem IP block is indeed an encoder from a hardware point
of view, but it's not necessarily the last block in the display
pipeline. While the output of the IP core goes of the the SoC, tt would
be entirely feasible to connect it to a DP to HDMI bridge on the board,
such as the ANX7737 ([1]) for instance. This is why we're pushing for
all encoder (from a hardware point of view) drivers to be implemented as
DRM bridge, in order to make them usable in different display pipelines,
without hardcoding the assumption they will be the last encoder in the
pipeline.

> I didn't see any hard
> requirement to implement it into bridge driver and I see many DSI drivers are
> implemented as encoder drivers.
> Xilinx PL DRM encoder drivers are implemented in modular approach so that
> they can work with any CRTC driver which handles the DMA calls.
> So, at this stage we want to upstream as encoder driver only.
>
> >  3 files changed, 768 insertions(+)  create mode 100644
> > drivers/gpu/drm/xlnx/xlnx_dsi.c

[1] https://www.analogix.com/en/products/convertersbridges/anx7737

-- 
Regards,

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


[PATCH v2 6/6] drm/i915/dp_mst: Ensure the DPCD ACT sent flag is cleared before waiting for it

2020-06-16 Thread Imre Deak
Atm, we clear the ACT sent flag in the sink's DPCD before updating the
sink's payload table, along clearing the payload table updated flag.
The sink is supposed to set this flag once it detects that the source
has completed the ACT sequence (after detecting the 4 required ACT MTPH
symbols sent by the source). As opposed to this 2 DELL monitors I have
set the flag already along the payload table updated flag, which is not
quite correct.

To be sure that the sink has detected the ACT MTPH symbols before
continuing enabling the encoder, clear the ACT sent flag before enabling
or disabling the transcoder VC payload allocation (which is what starts
the ACT sequence).

v2 (Ville):
- Use the correct bit to clear the flags.
- Add code comment explaining the clearing semantics of the ACT handled
  flag.

Cc: Lyude Paul 
Cc: Ville Syrjälä 
Cc: dri-devel@lists.freedesktop.org
Signed-off-by: Imre Deak 
---
 drivers/gpu/drm/drm_dp_mst_topology.c   | 38 +++--
 drivers/gpu/drm/i915/display/intel_dp_mst.c |  2 ++
 include/drm/drm_dp_mst_helper.h |  2 ++
 3 files changed, 40 insertions(+), 2 deletions(-)

diff --git a/drivers/gpu/drm/drm_dp_mst_topology.c 
b/drivers/gpu/drm/drm_dp_mst_topology.c
index b2f5a84b4cfb..1f5d14128c1a 100644
--- a/drivers/gpu/drm/drm_dp_mst_topology.c
+++ b/drivers/gpu/drm/drm_dp_mst_topology.c
@@ -4377,6 +4377,41 @@ void drm_dp_mst_deallocate_vcpi(struct 
drm_dp_mst_topology_mgr *mgr,
 }
 EXPORT_SYMBOL(drm_dp_mst_deallocate_vcpi);
 
+/**
+ * drm_dp_clear_payload_status() - Clears the payload table status flags
+ * @mgr: manager to use
+ *
+ * Clears the payload table ACT handled and table updated flags in the MST 
hub's
+ * DPCD. This function must be called before updating the payload table or
+ * starting the ACT sequence and waiting for the corresponding flags to get
+ * set by the hub.
+ *
+ * Returns:
+ * 0 if the flags got cleared successfully, otherwise a negative error code.
+ */
+int drm_dp_clear_payload_status(struct drm_dp_mst_topology_mgr *mgr)
+{
+   int ret;
+
+   /*
+* Note that the following is based on the DP Standard stating that
+* writing the DP_PAYLOAD_TABLE_UPDATED bit alone will clear both the
+* DP_PAYLOAD_TABLE_UPDATED and the DP_PAYLOAD_ACT_HANDLED flags. This
+* seems to be also the only way to clear DP_PAYLOAD_ACT_HANDLED.
+*/
+   ret = drm_dp_dpcd_writeb(mgr->aux, DP_PAYLOAD_TABLE_UPDATE_STATUS,
+DP_PAYLOAD_TABLE_UPDATED);
+   if (ret < 0) {
+   DRM_DEBUG_DRIVER("Can't clear the ACT handled/table updated 
flags (%d)\n",
+ret);
+   return ret;
+   }
+   WARN_ON(ret != 1);
+
+   return 0;
+}
+EXPORT_SYMBOL(drm_dp_clear_payload_status);
+
 static int drm_dp_dpcd_write_payload(struct drm_dp_mst_topology_mgr *mgr,
 int id, struct drm_dp_payload *payload)
 {
@@ -4384,8 +4419,7 @@ static int drm_dp_dpcd_write_payload(struct 
drm_dp_mst_topology_mgr *mgr,
int ret;
int retries = 0;
 
-   drm_dp_dpcd_writeb(mgr->aux, DP_PAYLOAD_TABLE_UPDATE_STATUS,
-  DP_PAYLOAD_TABLE_UPDATED);
+   drm_dp_clear_payload_status(mgr);
 
payload_alloc[0] = id;
payload_alloc[1] = payload->start_slot;
diff --git a/drivers/gpu/drm/i915/display/intel_dp_mst.c 
b/drivers/gpu/drm/i915/display/intel_dp_mst.c
index 9308b5920780..3c4b0fb10d8b 100644
--- a/drivers/gpu/drm/i915/display/intel_dp_mst.c
+++ b/drivers/gpu/drm/i915/display/intel_dp_mst.c
@@ -323,6 +323,8 @@ static void clear_act_sent(struct intel_dp *intel_dp)
 
intel_de_write(i915, intel_dp->regs.dp_tp_status,
   DP_TP_STATUS_ACT_SENT);
+
+   drm_dp_clear_payload_status(_dp->mst_mgr);
 }
 
 static void wait_for_act_sent(struct intel_dp *intel_dp)
diff --git a/include/drm/drm_dp_mst_helper.h b/include/drm/drm_dp_mst_helper.h
index 8b9eb4db3381..2facb87624bf 100644
--- a/include/drm/drm_dp_mst_helper.h
+++ b/include/drm/drm_dp_mst_helper.h
@@ -763,6 +763,8 @@ int drm_dp_find_vcpi_slots(struct drm_dp_mst_topology_mgr 
*mgr,
   int pbn);
 
 
+int drm_dp_clear_payload_status(struct drm_dp_mst_topology_mgr *mgr);
+
 int drm_dp_update_payload_part1(struct drm_dp_mst_topology_mgr *mgr);
 
 
-- 
2.23.1

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


Re: [PATCH v3 070/105] drm/vc4: hdmi: rework connectors and encoders

2020-06-16 Thread Stefan Wahren
Hi Maxime,

Am 16.06.20 um 14:30 schrieb Maxime Ripard:
> On Sun, Jun 14, 2020 at 06:16:56PM +0200, Stefan Wahren wrote:
>> Am 11.06.20 um 15:34 schrieb Maxime Ripard:
>>> Hi Stefan,
>>>
>>> On Sat, Jun 06, 2020 at 10:06:12AM +0200, Stefan Wahren wrote:
 Hi Maxime,

 Am 05.06.20 um 16:35 schrieb Maxime Ripard:
> Hi Stefan,
>
> On Wed, Jun 03, 2020 at 07:32:30PM +0200, Stefan Wahren wrote:
>> Am 02.06.20 um 17:54 schrieb Maxime Ripard:
>> FWIW this is the first patch which breaks X on my Raspberry Pi 3 B.
>>
>> Here are the bisect results:
>>
>> 587d6e4a529a8d807a5c0bae583dd432d77064d6 bad (black screen, no heartbeat)
>>
>> b0523c7b1c9d0edcd6c0fe6d2cb558a9ad5c60a8 good
>>
>> 2c6a651cac6359cb0244a40d3b7a14e72918f169 good
>>
>> 1705c3cb40906863ec0d24ee5ea5092f5ee2e994 bad (black screen, but 
>> heartbeat)
>>
>> 601527fea6bb226abd088a864e74b25368218e87 good
>>
>> 2165607ede34d229d0cbce916c70c7fb6c0337be good
>>
>> f094f388fc2df848227e2ae648df2c97872df42b good
>>
>> 020de18840a1075b2671736c6cc2e451030fad74 bad (black screen, but 
>> heartbeat)
>>
>> 4c4da3823e4d1a8189e96a59a79451fff372f70b good
>>
>> 020de18840a1075b2671736c6cc2e451030fad74 is the first bad commit
>> commit 020de18840a1075b2671736c6cc2e451030fad74
>> Author: Maxime Ripard 
>> Date:   Mon Jan 6 17:17:29 2020 +0100
>>
>>     drm/vc4: hdmi: rework connectors and encoders
>>    
>>     the vc4_hdmi driver has some custom structures to hold the data it
>> needs to
>>     associate with the drm_encoder and drm_connector structures.
>>    
>>     However, it allocates them separately from the vc4_hdmi structure 
>> which
>>     makes it more complicated than it needs to be.
>>    
>>     Move those structures to be contained by vc4_hdmi and update the code
>>     accordingly.
>>    
>>     Signed-off-by: Maxime Ripard 
> So it looks like there was two issues on the Pi3. The first one was
> causing the timeouts (and therefore likely the black screen but
> heartbeat case you had) and I've fixed it.
>
> However, I can indeed reproduce the case with the black screen / no
> heartbeat you mentionned. My bisection however returns that it's the
> patch "drm/vc4: hdmi: Implement finer-grained hooks" that is at fault.
> I've pushed my updated branch, if you have some spare time, it would be
> great if you could confirm it on your Pi.
 yesterday i checked out your latest rpi4-kms branch, but i was still
 facing similiar issues with my Raspberry Pi 3 and multi_v7_defconfig
 (heartbeat stops, splashscreen freeze, heartbeat is abnormal fast). So i
 tried to bisect but the offending commit didn't cause an issue the
 second time.

 By accident i noticed that a simple reboot seems to hang for at least 8
 minutes (using b0523c7b1c9d0edcd the base of your branch). This usually
 take a few seconds. So i consider this base on linux-next as too
 unstable for reliable testing.

 Is it possible to rebase this on something more stable like linux-5.7 or
 at least drm-misc-next? This should avoid chasing unrelated issues.
>>> I've rebased it on 5.7 here:
>>> https://git.kernel.org/pub/scm/linux/kernel/git/mripard/linux.git/log/?h=rpi4-kms-5.7
>>>
>>> And it looks to be indeed an issue coming from next. That branch can
>>> start the desktop just fine on an RPi3 here. It would be great if you
>>> could confirm on your end.
>>>
>>> Thanks!
>>> Maxime
>> thank you very much. The good news are that the "black screen, but
>> heartbeat" issue and reboot hang are gone. Unfortunately the "no
>> heartbeat" issue is still there.
>>
>> Here are more details about the issue. It doesn't occur everytime. I
>> would guess the probability is about 40 percent, which made bisecting
>> much harder.
> Are you sure about that 40% reliability?
it's more a gut feeling than a statistical analyze. It's definitely not
100% in my setup.
>  I found out that the culprit
> was that the commit we mentionned was actually running atomic_disable
> before our own custom callbacks, meaning that we would run the custom
> callbacks with the clocks and the power domain shut down, resulting in a
> stall.
>
> I was seeing it all the time when X was shutting down the display, but
> maybe you were changing the resolution between the framebuffer console
> or something, and since the power domain is shut down asynchronously, it
> wasn't running fast enough for the next enable to come up and re-enable
> it again?
>
>> It is reproducible on my 2 Raspberry Pi 3 B Rev 1.2. It is
>> also seems independent from the display because the problem occured on
>> my Computer display and my TV.
> But only on HDMI, right?
I only tested it with HDMI displays. All tests without any display were
always successful.
>
> I've pushed a new branch with that fix.

[Bug 208205] [amdgpu] System hang / freeze

2020-06-16 Thread bugzilla-daemon
https://bugzilla.kernel.org/show_bug.cgi?id=208205

--- Comment #3 from Automne von Einzbern (mar...@automne.me) ---
And I don't have X (headless server) however I have a GPU (Ryzen 3200G with
Vega chipset)

-- 
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 208205] [amdgpu] System hang / freeze

2020-06-16 Thread bugzilla-daemon
https://bugzilla.kernel.org/show_bug.cgi?id=208205

--- Comment #2 from Automne von Einzbern (mar...@automne.me) ---
Created attachment 289711
  --> https://bugzilla.kernel.org/attachment.cgi?id=289711=edit
Log before a crash

-- 
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 0/5] 180 degrees rotation support for NVIDIA Tegra DRM

2020-06-16 Thread Emil Velikov
On Tue, 16 Jun 2020 at 18:20, Dmitry Osipenko  wrote:
>
> 16.06.2020 18:48, Emil Velikov пишет:
> > On Tue, 16 Jun 2020 at 12:40, Dmitry Osipenko  wrote:
> >>
> >> 16.06.2020 01:26, Emil Velikov пишет:
> >>> Hi Dmitry,
> >>>
> >>> On Mon, 15 Jun 2020 at 08:28, Dmitry Osipenko  wrote:
> 
>  Hello!
> 
>  This series adds 180° display plane rotation support to the NVIDIA Tegra
>  DRM driver which is needed for devices that have display panel physically
>  mounted upside-down, like Nexus 7 tablet device for example [1]. Since
>  DRM panel rotation is a new thing for a userspace, currently only
>  Opentegra Xorg driver handles the rotated display panel [2], but this
>  is good enough for the start.
> 
>  Note that later on it should be possible to implement a transparent 180°
>  display rotation for Tegra DRM driver which will remove the need to have
>  a bleeding edge userspace that knows how to rotate display planes and I'm
>  slowly working on it. For the starter we can go with the minimal rotation
>  support, so it's not a blocker.
> 
>  This series is based on the work that was made by Derek Basehore for the
>  Mediatek driver [3], his patch is included into this patchset. I added
>  my tested-by tag to the Derek's patch.
> 
>  Please review and apply, thanks in advance!
> 
>  [1] 
>  https://patchwork.ozlabs.org/project/linux-tegra/patch/20200607154327.18589-3-dig...@gmail.com/
>  [2] 
>  https://github.com/grate-driver/xf86-video-opentegra/commit/28eb20a3959bbe5bc3a3b67e55977093fd5114ca
>  [3] https://lkml.org/lkml/2020/3/5/1119
> 
>  Changelog:
> 
>  v2: - Dropped "drm/panel: Set display info in panel attach" patch, which
>    turned out to be obsolete now.
> 
>  - Renamed the cover-latter, hopefully this will fix the bouncing 
>  emails.
> 
>  Derek Basehore (1):
>    drm/panel: Add helper for reading DT rotation
> 
>  Dmitry Osipenko (4):
>    drm/panel: lvds: Set up panel orientation
> >>>
> >>> IMHO it's perfectly reasonable to report the panel orientation to
> >>> userspace, which can apply plane rotation as needed.
> >>>
> >>> Although I see that this series, alike Derek's, has a couple of issues:
> >>>  - only a single panel driver is updated
> >>>  - rotation is _not_ listed as supported property, in said panel
> >>> driver device-tree bindings
> >>>
> >>> My personal inclination is that we should aim for a comprehensive 
> >>> solution:
> >>>  - wire all panel drivers, as currently documented (quick grep list below)
> >>>  - document and wire-up the lvds and boe panels - as proposed by you
> >>> and Derek respectively
> >>>
> >>> HTH
> >>> Emil
> >>>
> >>> Documentation/devicetree/bindings/display/himax,hx8357d.txt:2
> >>> Documentation/devicetree/bindings/display/ilitek,ili9225.txt:2
> >>> Documentation/devicetree/bindings/display/ilitek,ili9341.txt:2
> >>> Documentation/devicetree/bindings/display/ilitek,ili9486.yaml:2
> >>> Documentation/devicetree/bindings/display/multi-inno,mi0283qt.txt:2
> >>> Documentation/devicetree/bindings/display/panel/panel-common.yaml:2
> >>> Documentation/devicetree/bindings/display/sitronix,st7586.txt:1
> >>> Documentation/devicetree/bindings/display/sitronix,st7735r.yaml:2
> >>
> >> Rotation is a common DT panel property that is described in the
> >> panel-common.yaml.
> > The property was introduced almost exclusively for tiny drm panels.
> > Those ones are a bit different from the rest (in panel/) -
> > MIPI-DBI/SPI w/o (not connected at least) an actual GPU.
> >
> > To make it a bit better, the rotation is seemingly performed in the
> > tiny driver itself ouch.
> >
> >> This property is supported by all panel bindings
> >> because these bindings inherent the common properties from the
> >> panel-common.yaml.
> >>
> > Seems like that was an unintentional change with the conversion to YAML.
> > Beforehand only a few selected panels had rotation. Upon closer look -
> > some panels do have follow-up fixes, to remove/limit the implicit
> > inclusion.
>
> Interesting.. my understanding that the rotation property is supposed to
> be a generic property which represents physical orientation of a display
> panel and hence it should be applicable to all panels.
>
You're spot on - it is/should be a generic property.

I believe that in general many panels were mounted in the correct
orientation so the property, kernel and userspace were slow to catch
up. In some cases panels will use flip x+y to denote 180 rotation, yet
lacking the rotation property.
The s6e8aa0 is an example of the last oddity. To make it better, the
two dts in-tree always set both flip x and y.

Tl;Dr: Hysterical raisins

> > Sam seems like you've done most of the YAML conversion. IMHO it would
> > make sense to revisit the patches and inherit common properties only
> > as applicable.
> >
> >> I don't think that it makes sense to 

[PATCH v3] drm/tegra: Add zpos property for cursor planes

2020-06-16 Thread Thierry Reding
From: Thierry Reding 

As of commit 4dc55525b095 ("drm: plane: Verify that no or all planes
have a zpos property") a warning is emitted if there's a mix of planes
with and without a zpos property.

On Tegra, cursor planes are always composited on top of all other
planes, which is why they never had a zpos property attached to them.
However, since the composition order is fixed, this is trivial to
remedy by simply attaching an immutable zpos property to them.

v3: do not hardcode zpos for overlay planes used as cursor (Dmitry)
v2: hardcode cursor plane zpos to 255 instead of 0 (Ville)

Reported-by: Jonathan Hunter 
Signed-off-by: Thierry Reding 
---
 drivers/gpu/drm/tegra/dc.c | 1 +
 1 file changed, 1 insertion(+)

diff --git a/drivers/gpu/drm/tegra/dc.c b/drivers/gpu/drm/tegra/dc.c
index 83f31c6e891c..04d6848d19fc 100644
--- a/drivers/gpu/drm/tegra/dc.c
+++ b/drivers/gpu/drm/tegra/dc.c
@@ -957,6 +957,7 @@ static struct drm_plane 
*tegra_dc_cursor_plane_create(struct drm_device *drm,
}
 
drm_plane_helper_add(>base, _cursor_plane_helper_funcs);
+   drm_plane_create_zpos_immutable_property(>base, 255);
 
return >base;
 }
-- 
2.24.1

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


Re: [RFC PATCH 0/4] DSI/DBI and TinyDRM driver

2020-06-16 Thread Emil Velikov
Hi all,

Allow me to compare this approach with some work Linus W [1] did a
while back, which I've just noticed.

Pauls' approach:

 - Perhaps the shortest one possible
Porting an existing DSI panel to DBI is 3 lines of code - compat line
in the SPI/DSI bridge, a bus_type and
mipi_dsi_maybe_register_tiny_driver() call
The clever use of the DSI type (equal to zero) means that things will
work w/o updating existing dsi hosts and devices in panel/. Yet in the
very unlikely case that the panel does not support DSI, we will still
allow it.

Although thinking about the type I wonder if it can accommodate all use-cases:
Since we can have a device (panel) capable of DSI+SPI it makes sense
for it to expose the type bitmask, not the host. Although, what if the
host itself supports DSI+SPI.?
Now we can extrapolate that with a host (say fimd/exynos I think)
which supports a SPI panel (s6e63m0) while having
of_graph_get_port_by_id(0)?

- Strange (ab)use of the DSI bus for DBI (SPI, 6800, 8080 etc)
We care about existing users (DT) and it sounds unlikely (based on
previous discussion) that DBI + SPI/6800... will make it into DT. So
the current approach seems quite reasonable IMHO.


Linus' approach:
- Clear separation of DSI/SPI
Compat strings are still duplicated, although in separate files

- Minor code motion and slightly more invasive porting overall
Much of the boilerplate can be reduced via helper lib and macros. Even
then it's unlikely we'll reach the 3 line delta as with Paul's
approach.

- Does not handle tiny-dsi (dummy) drm driver
It seems doable, with minor changes


Personally I'm on the fence and a deciding factor for me is if Paul's
approach can handle all the combinations of host/device type support.
That said, the input from people likely to do the work would be highly
appreciated.

Once a decision is made, an illustrative series + todo entry would be
great to have.
-Emil

[1] https://lists.freedesktop.org/archives/dri-devel/2020-May/266079.html
___
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel


Re: [PATCH v2 0/5] 180 degrees rotation support for NVIDIA Tegra DRM

2020-06-16 Thread Laurent Pinchart
On Tue, Jun 16, 2020 at 08:20:57PM +0300, Dmitry Osipenko wrote:
> 16.06.2020 18:48, Emil Velikov пишет:
> > On Tue, 16 Jun 2020 at 12:40, Dmitry Osipenko  wrote:
> >>
> >> 16.06.2020 01:26, Emil Velikov пишет:
> >>> Hi Dmitry,
> >>>
> >>> On Mon, 15 Jun 2020 at 08:28, Dmitry Osipenko  wrote:
> 
>  Hello!
> 
>  This series adds 180° display plane rotation support to the NVIDIA Tegra
>  DRM driver which is needed for devices that have display panel physically
>  mounted upside-down, like Nexus 7 tablet device for example [1]. Since
>  DRM panel rotation is a new thing for a userspace, currently only
>  Opentegra Xorg driver handles the rotated display panel [2], but this
>  is good enough for the start.
> 
>  Note that later on it should be possible to implement a transparent 180°
>  display rotation for Tegra DRM driver which will remove the need to have
>  a bleeding edge userspace that knows how to rotate display planes and I'm
>  slowly working on it. For the starter we can go with the minimal rotation
>  support, so it's not a blocker.
> 
>  This series is based on the work that was made by Derek Basehore for the
>  Mediatek driver [3], his patch is included into this patchset. I added
>  my tested-by tag to the Derek's patch.
> 
>  Please review and apply, thanks in advance!
> 
>  [1] 
>  https://patchwork.ozlabs.org/project/linux-tegra/patch/20200607154327.18589-3-dig...@gmail.com/
>  [2] 
>  https://github.com/grate-driver/xf86-video-opentegra/commit/28eb20a3959bbe5bc3a3b67e55977093fd5114ca
>  [3] https://lkml.org/lkml/2020/3/5/1119
> 
>  Changelog:
> 
>  v2: - Dropped "drm/panel: Set display info in panel attach" patch, which
>    turned out to be obsolete now.
> 
>  - Renamed the cover-latter, hopefully this will fix the bouncing 
>  emails.
> 
>  Derek Basehore (1):
>    drm/panel: Add helper for reading DT rotation
> 
>  Dmitry Osipenko (4):
>    drm/panel: lvds: Set up panel orientation
> >>>
> >>> IMHO it's perfectly reasonable to report the panel orientation to
> >>> userspace, which can apply plane rotation as needed.
> >>>
> >>> Although I see that this series, alike Derek's, has a couple of issues:
> >>>  - only a single panel driver is updated
> >>>  - rotation is _not_ listed as supported property, in said panel
> >>> driver device-tree bindings
> >>>
> >>> My personal inclination is that we should aim for a comprehensive 
> >>> solution:
> >>>  - wire all panel drivers, as currently documented (quick grep list below)
> >>>  - document and wire-up the lvds and boe panels - as proposed by you
> >>> and Derek respectively
> >>>
> >>> HTH
> >>> Emil
> >>>
> >>> Documentation/devicetree/bindings/display/himax,hx8357d.txt:2
> >>> Documentation/devicetree/bindings/display/ilitek,ili9225.txt:2
> >>> Documentation/devicetree/bindings/display/ilitek,ili9341.txt:2
> >>> Documentation/devicetree/bindings/display/ilitek,ili9486.yaml:2
> >>> Documentation/devicetree/bindings/display/multi-inno,mi0283qt.txt:2
> >>> Documentation/devicetree/bindings/display/panel/panel-common.yaml:2
> >>> Documentation/devicetree/bindings/display/sitronix,st7586.txt:1
> >>> Documentation/devicetree/bindings/display/sitronix,st7735r.yaml:2
> >>
> >> Rotation is a common DT panel property that is described in the
> >> panel-common.yaml.
> > The property was introduced almost exclusively for tiny drm panels.
> > Those ones are a bit different from the rest (in panel/) -
> > MIPI-DBI/SPI w/o (not connected at least) an actual GPU.
> > 
> > To make it a bit better, the rotation is seemingly performed in the
> > tiny driver itself ouch.
> > 
> >> This property is supported by all panel bindings
> >> because these bindings inherent the common properties from the
> >> panel-common.yaml.
> >>
> > Seems like that was an unintentional change with the conversion to YAML.
> > Beforehand only a few selected panels had rotation. Upon closer look -
> > some panels do have follow-up fixes, to remove/limit the implicit
> > inclusion.
> 
> Interesting.. my understanding that the rotation property is supposed to
> be a generic property which represents physical orientation of a display
> panel and hence it should be applicable to all panels.

Adding a bit more food for thoughts, the DT rotation property for camera
sensor modules has recently been documented with lots of details. See
https://lore.kernel.org/linux-media/20200509090456.3496481-3-jac...@jmondi.org/,
part of the documentation may be useful for panels.

> > Sam seems like you've done most of the YAML conversion. IMHO it would
> > make sense to revisit the patches and inherit common properties only
> > as applicable.
> > 
> >> I don't think that it makes sense to wire up rotation property to all
> >> panel drivers at once because those drivers will be untested, at least I
> >> don't know anything about those 

Re: [Intel-gfx] [PATCH 7/8] drm/mipi-dbi: Remove ->enabled

2020-06-16 Thread Daniel Vetter
On Tue, Jun 16, 2020 at 3:57 PM Emil Velikov  wrote:
>
> On Tue, 16 Jun 2020 at 07:50, Daniel Vetter  wrote:
> >
> > On Mon, Jun 15, 2020 at 11:35 PM Emil Velikov  
> > wrote:
> > >
> > > Hi Daniel,
> > >
> > > On Fri, 12 Jun 2020 at 17:01, Daniel Vetter  
> > > wrote:
> > > >
> > > > The atomic helpers try really hard to not lose track of things,
> > > > duplicating enabled tracking in the driver is at best confusing.
> > > > Double-enabling or disabling is a bug in atomic helpers.
> > > >
> > > > In the fb_dirty function we can just assume that the fb always exists,
> > > > simple display pipe helpers guarantee that the crtc is only enabled
> > > > together with the output, so we always have a primary plane around.
> > > >
> > > > Now in the update function we need to be a notch more careful, since
> > > > that can also get called when the crtc is off. And we don't want to
> > > > upload frames when that's the case, so filter that out too.
> > > >
> > > > Signed-off-by: Daniel Vetter 
> > > > Cc: Maarten Lankhorst 
> > > > Cc: Maxime Ripard 
> > > > Cc: Thomas Zimmermann 
> > > > Cc: David Airlie 
> > > > Cc: Daniel Vetter 
> > > > Cc: David Lechner 
> > > > ---
> > > >  drivers/gpu/drm/drm_mipi_dbi.c | 16 ++--
> > > >  drivers/gpu/drm/tiny/ili9225.c | 12 +++-
> > > >  drivers/gpu/drm/tiny/st7586.c  | 11 +++
> > > >  include/drm/drm_mipi_dbi.h |  5 -
> > > >  4 files changed, 12 insertions(+), 32 deletions(-)
> > > >
> > > > diff --git a/drivers/gpu/drm/drm_mipi_dbi.c 
> > > > b/drivers/gpu/drm/drm_mipi_dbi.c
> > > > index fd8d672972a9..79532b9a324a 100644
> > > > --- a/drivers/gpu/drm/drm_mipi_dbi.c
> > > > +++ b/drivers/gpu/drm/drm_mipi_dbi.c
> > > > @@ -268,7 +268,7 @@ static void mipi_dbi_fb_dirty(struct 
> > > > drm_framebuffer *fb, struct drm_rect *rect)
> > > > bool full;
> > > > void *tr;
> > > >
> > > > -   if (!dbidev->enabled)
> > > > +   if (WARN_ON(!fb))
> > > > return;
> > > >
> > > AFAICT no other driver has such WARN_ON. Let's drop that - it is
> > > pretty confusing and misleading as-is.
> >
> > Yeah, this is a helper library which might be used wrongly by drivers.
> > That's why I put it in - if you don't put all the various calls
> > together correctly, this should at least catch one case. So really
> > would like to keep this, can I convince you?
>
> There are plenty of similar places where a drm library/helper can be
> misused, lacking a WARN. Nevertheless - sure feel free to keep it.

Yeah I agree, we can't check for everything. Personally I think a
check is warranted in two conditions:
- drivers got it wrong, and the WARNING helps catch driver-bugs we've
seen in the wild. Not really the case here
- drivers do check something as defensive programming, but it's an
invariant enforced by higher levels or helpers. Those I like to
convert to WARNING so that other driver authors learn that this should
never happen. This is such a case imo, I removed a bunch of fb checks
from drivers here.

But yeah I think we should only add WARNING checks if this is actually
something people have gotten wrong, otherwise there's just too many of
them, distracting from the code.
-Daniel
-- 
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: [PATCH] drm/shmem-helper: Only dma-buf imports are private obj

2020-06-16 Thread Daniel Vetter
On Tue, Jun 16, 2020 at 02:06:24PM +0200, Thomas Zimmermann wrote:
> Hi
> 
> Am 16.06.20 um 13:47 schrieb Daniel Vetter:
> > I broke that in my refactoring:
> > 
> > commit 7d2cd72a9aa3df3604cafd169a2d4a525afb68ca
> > Author: Daniel Vetter 
> > Date:   Fri May 29 16:05:42 2020 +0200
> > 
> > drm/shmem-helpers: Simplify dma-buf importing
> > 
> > I'm not entirely sure of the history here, but I suspect that in one
> > of the rebases or when applying the patch I moved the hunk from
> > drm_gem_shmem_prime_import_sg_table(), where it should be, to
> > drm_gem_shmem_create_with_handle(), which is totally wrong.
> > 
> > Remedy this.
> > 
> > Thanks for Thomas for the crucual hint in debugging this.
> > 
> > Reported-by: Thomas Zimmermann 
> > Fixes: 7d2cd72a9aa3 ("drm/shmem-helpers: Simplify dma-buf importing")
> > Cc: Boris Brezillon 
> > Cc: Thomas Zimmermann 
> > Cc: Gerd Hoffmann 
> > Cc: Rob Herring 
> > Cc: Noralf Trønnes 
> > Signed-off-by: Daniel Vetter 
> 
> Tested-by: Thomas Zimmermann 
> Reviewed-by: Thomas Zimmermann 

Now also merged, thanks a lot for your help.
-Daniel

> 
> > ---
> >  drivers/gpu/drm/drm_gem_shmem_helper.c | 4 ++--
> >  1 file changed, 2 insertions(+), 2 deletions(-)
> > 
> > diff --git a/drivers/gpu/drm/drm_gem_shmem_helper.c 
> > b/drivers/gpu/drm/drm_gem_shmem_helper.c
> > index 0a7e3b664bc2..837e0840990c 100644
> > --- a/drivers/gpu/drm/drm_gem_shmem_helper.c
> > +++ b/drivers/gpu/drm/drm_gem_shmem_helper.c
> > @@ -377,7 +377,7 @@ drm_gem_shmem_create_with_handle(struct drm_file 
> > *file_priv,
> > struct drm_gem_shmem_object *shmem;
> > int ret;
> >  
> > -   shmem = __drm_gem_shmem_create(dev, size, true);
> > +   shmem = drm_gem_shmem_create(dev, size);
> > if (IS_ERR(shmem))
> > return shmem;
> >  
> > @@ -730,7 +730,7 @@ drm_gem_shmem_prime_import_sg_table(struct drm_device 
> > *dev,
> > size_t size = PAGE_ALIGN(attach->dmabuf->size);
> > struct drm_gem_shmem_object *shmem;
> >  
> > -   shmem = drm_gem_shmem_create(dev, size);
> > +   shmem = __drm_gem_shmem_create(dev, size, true);
> > if (IS_ERR(shmem))
> > return ERR_CAST(shmem);
> >  
> > 
> 
> -- 
> Thomas Zimmermann
> Graphics Driver Developer
> SUSE Software Solutions Germany GmbH
> Maxfeldstr. 5, 90409 Nürnberg, Germany
> (HRB 36809, AG Nürnberg)
> Geschäftsführer: Felix Imendörffer
> 




-- 
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: [PATCH] drm/shmem-helper: Fix obj->filp derefence

2020-06-16 Thread Daniel Vetter
On Tue, Jun 16, 2020 at 02:10:10PM +0200, Thomas Zimmermann wrote:
> Hi,
> 
> as discussed on IRC, we still need this patch.
> 
> Am 15.06.20 um 17:10 schrieb Daniel Vetter:
> > I broke that in my refactoring:
> > 
> > commit 7d2cd72a9aa3df3604cafd169a2d4a525afb68ca
> > Author: Daniel Vetter 
> > Date:   Fri May 29 16:05:42 2020 +0200
> > 
> > drm/shmem-helpers: Simplify dma-buf importing
> > 
> > Reported-by: Thomas Zimmermann 
> > Fixes: 7d2cd72a9aa3 ("drm/shmem-helpers: Simplify dma-buf importing")
> > Cc: Boris Brezillon 
> > Cc: Thomas Zimmermann 
> > Cc: Gerd Hoffmann 
> > Cc: Rob Herring 
> > Cc: Noralf Trønnes 
> > Signed-off-by: Daniel Vetter 
> > ---
> >  drivers/gpu/drm/drm_gem_shmem_helper.c | 20 +++-
> >  1 file changed, 11 insertions(+), 9 deletions(-)
> > 
> > diff --git a/drivers/gpu/drm/drm_gem_shmem_helper.c 
> > b/drivers/gpu/drm/drm_gem_shmem_helper.c
> > index 0a7e3b664bc2..3e7ee407a17c 100644
> > --- a/drivers/gpu/drm/drm_gem_shmem_helper.c
> > +++ b/drivers/gpu/drm/drm_gem_shmem_helper.c
> > @@ -70,15 +70,17 @@ __drm_gem_shmem_create(struct drm_device *dev, size_t 
> > size, bool private)
> > mutex_init(>vmap_lock);
> > INIT_LIST_HEAD(>madv_list);
> >  
> > -   /*
> > -* Our buffers are kept pinned, so allocating them
> > -* from the MOVABLE zone is a really bad idea, and
> > -* conflicts with CMA. See comments above new_inode()
> > -* why this is required _and_ expected if you're
> > -* going to pin these pages.
> > -*/
> > -   mapping_set_gfp_mask(obj->filp->f_mapping, GFP_HIGHUSER |
> > -__GFP_RETRY_MAYFAIL | __GFP_NOWARN);
> > +   if (!private) {
> 
> I would test for (obj->filp) here, because it's what the branch depends
> on. Your choice. In any case

I was pondering this too, on one hand it's the thing we're using, otoh
it's a direct consequence of the private flag, and the private flag has a
bit the clearer control flow I think - the obj->filp is clear that it's a
NULL check, but it's a lot less clear _why_ it is ok to have obj->filp ==
NULL. Checking for private makes this a bit clearer imo.

But yeah I considered both options. Maybe we should improve the comment in
a follow-up patch? I want to land the bugfix meanwhile, to close the
regression.

> Tested-by: Thomas Zimmermann 
> Reviewed-by: Thomas Zimmermann 

Thanks for testing and reviewing!
-Daniel

> 
> 
> > +   /*
> > +* Our buffers are kept pinned, so allocating them
> > +* from the MOVABLE zone is a really bad idea, and
> > +* conflicts with CMA. See comments above new_inode()
> > +* why this is required _and_ expected if you're
> > +* going to pin these pages.
> > +*/
> > +   mapping_set_gfp_mask(obj->filp->f_mapping, GFP_HIGHUSER |
> > +__GFP_RETRY_MAYFAIL | __GFP_NOWARN);
> > +   }
> >  
> > return shmem;
> >  
> > 
> 
> -- 
> Thomas Zimmermann
> Graphics Driver Developer
> SUSE Software Solutions Germany GmbH
> Maxfeldstr. 5, 90409 Nürnberg, Germany
> (HRB 36809, AG Nürnberg)
> Geschäftsführer: Felix Imendörffer
> 




-- 
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: [PATCH 1/5] drm/omap: Fix suspend resume regression after platform data removal

2020-06-16 Thread Tomi Valkeinen

On 11/06/2020 17:00, Grygorii Strashko wrote:



On 09/06/2020 18:26, Tomi Valkeinen wrote:

On 09/06/2020 18:19, Tony Lindgren wrote:

But there's an extra runtime PM reference (dev.power.usage_count) that seems
to come out of nowhere. So when omap_drm_suspend is finished, there's still
usage_count of 1, and dispc never suspends fully.


Hmm no idea about that. My guess is that there might be an issue that was
masked earlier with omap_device calling the child runtime_suspend.


Yes. It's how PM works. It calls pm_runtime_get_noresume() before starting the suspend of a 
device. So I guess omapdrm's suspend has been broken all the time, but it was "fixed" by omap_device.




I think I might have an idea what is going wrong.

Before:
+--+
|omap_device_pm_domain |
+---+--+--+
     | device  |
     +-+
     | omap_device |
     +-+

omap_device is embedded in DD device and PM handled by omap_device_pm_domain.

static int _od_suspend_noirq(struct device *dev)
{
...

 ret = pm_generic_suspend_noirq(dev);
[1] ^^ device suspend_noirq call

 if (!ret && !pm_runtime_status_suspended(dev)) {
     if (pm_generic_runtime_suspend(dev) == 0) {
[2] ^^ device pm_runtime_suspend force call

     omap_device_idle(pdev);
[3] ^^ omap_device disable
     od->flags |= OMAP_DEVICE_SUSPENDED;
     }
 }

 return ret;
}

Now:
++
|ti sysc dev |
+-+--+
   |
   |
   |   +-+
   |   | device  |
   +-->+ |
   +-+

With new approach the omap_device is not embedded in DD Device anymore,
instead ti-sysc (hwmod replacement) became parent of DD Device.

As result suspend sequence became the following
(Note. All PM runtime PUT calls became NOP during suspend by design):

device
|-> suspend() - in case of dss omap_drm_suspend() and Co if defined
|-> suspend_noirq() - in case of dss *not defined", equal to step [1] above
..

ti sysc dev (ti-sysc is parent, so called after device)
|-> sysc_noirq_suspend
    |-> pm_runtime_force_suspend()
 |-> sysc_runtime_suspend() - equal to step [3] above

And step [2] is missing as of now!

I think, suspend might be fixed if all devices, which are now child of ti-sysc, 
will do
pm_runtime_force_xxx() calls at noirq suspend stage by adding:

 SET_NOIRQ_SYSTEM_SLEEP_PM_OPS(pm_runtime_force_suspend,
   pm_runtime_force_resume)

Am I missing smth?


Isn't this almost exactly the same my patch does? I just used suspend_late and resume_early. Is 
noirq phase better than late & early?


 Tomi

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


Re: [PATCH 6/6] drm/i915/dp_mst: Ensure the DPCD ACT sent flag is cleared before waiting for it

2020-06-16 Thread Imre Deak
On Tue, Jun 16, 2020 at 07:40:47PM +0300, Ville Syrjälä wrote:
> On Tue, Jun 16, 2020 at 07:23:21PM +0300, Ville Syrjälä wrote:
> > On Tue, Jun 16, 2020 at 06:54:41PM +0300, Imre Deak wrote:
> > > On Tue, Jun 16, 2020 at 06:45:46PM +0300, Ville Syrjälä wrote:
> > > > On Tue, Jun 16, 2020 at 05:18:55PM +0300, Imre Deak wrote:
> > > > > Atm, we clear the ACT sent flag in the sink's DPCD before updating the
> > > > > sink's payload table, along clearing the payload table updated flag.
> > > > > The sink is supposed to set this flag once it detects that the source
> > > > > has completed the ACT sequence (after detecting the 4 required ACT 
> > > > > MTPH
> > > > > symbols sent by the source). As opposed to this 2 DELL monitors I have
> > > > > set the flag already along the payload table updated flag, which is 
> > > > > not
> > > > > quite correct.
> > > > > 
> > > > > To be sure that the sink has detected the ACT MTPH symbols before
> > > > > continuing enabling the encoder, clear the ACT sent flag before 
> > > > > enabling
> > > > > or disabling the transcoder VC payload allocation (which is what 
> > > > > starts
> > > > > the ACT sequence).
> > > > > 
> > > > > Cc: Lyude Paul 
> > > > > Cc: Ville Syrjälä 
> > > > > Cc: dri-devel@lists.freedesktop.org
> > > > > Signed-off-by: Imre Deak 
> > > > > ---
> > > > >  drivers/gpu/drm/drm_dp_mst_topology.c   | 31 
> > > > > +++--
> > > > >  drivers/gpu/drm/i915/display/intel_dp_mst.c |  2 ++
> > > > >  include/drm/drm_dp_mst_helper.h |  2 ++
> > > > >  3 files changed, 33 insertions(+), 2 deletions(-)
> > > > > 
> > > > > diff --git a/drivers/gpu/drm/drm_dp_mst_topology.c 
> > > > > b/drivers/gpu/drm/drm_dp_mst_topology.c
> > > > > index b2f5a84b4cfb..e3bf8c9c8267 100644
> > > > > --- a/drivers/gpu/drm/drm_dp_mst_topology.c
> > > > > +++ b/drivers/gpu/drm/drm_dp_mst_topology.c
> > > > > @@ -4377,6 +4377,34 @@ void drm_dp_mst_deallocate_vcpi(struct 
> > > > > drm_dp_mst_topology_mgr *mgr,
> > > > >  }
> > > > >  EXPORT_SYMBOL(drm_dp_mst_deallocate_vcpi);
> > > > >  
> > > > > +/**
> > > > > + * drm_dp_clear_payload_status() - Clears the payload table status 
> > > > > flags
> > > > > + * @mgr: manager to use
> > > > > + *
> > > > > + * Clears the payload table ACT handled and table updated flags in 
> > > > > the MST hub's
> > > > > + * DPCD. This function must be called before updating the payload 
> > > > > table or
> > > > > + * starting the ACT sequence and waiting for the corresponding flags 
> > > > > to get
> > > > > + * set by the hub.
> > > > > + *
> > > > > + * Returns:
> > > > > + * 0 if the flag got cleared successfully, otherwise a negative 
> > > > > error code.
> > > > > + */
> > > > > +int drm_dp_clear_payload_status(struct drm_dp_mst_topology_mgr *mgr)
> > > > > +{
> > > > > + int ret;
> > > > > +
> > > > > + ret = drm_dp_dpcd_writeb(mgr->aux, 
> > > > > DP_PAYLOAD_TABLE_UPDATE_STATUS,
> > > > > +  DP_PAYLOAD_ACT_HANDLED);
> > > > > + if (ret < 0) {
> > > > > + DRM_DEBUG_DRIVER("Can't clear the ACT sent flag 
> > > > > (%d)\n", ret);
> > > > > + return ret;
> > > > > + }
> > > > > + WARN_ON(ret != 1);
> > > > > +
> > > > > + return 0;
> > > > > +}
> > > > > +EXPORT_SYMBOL(drm_dp_clear_payload_status);
> > > > > +
> > > > >  static int drm_dp_dpcd_write_payload(struct drm_dp_mst_topology_mgr 
> > > > > *mgr,
> > > > >int id, struct drm_dp_payload 
> > > > > *payload)
> > > > >  {
> > > > > @@ -4384,8 +4412,7 @@ static int drm_dp_dpcd_write_payload(struct 
> > > > > drm_dp_mst_topology_mgr *mgr,
> > > > >   int ret;
> > > > >   int retries = 0;
> > > > >  
> > > > > - drm_dp_dpcd_writeb(mgr->aux, DP_PAYLOAD_TABLE_UPDATE_STATUS,
> > > > > -DP_PAYLOAD_TABLE_UPDATED);
> > > > 
> > > > We used to clear DP_PAYLOAD_TABLE_UPDATED but now we clear
> > > > DP_PAYLOAD_ACT_HANDLED ?
> > > 
> > > Eek. We should write DP_PAYLOAD_TABLE_UPDATED which is the only way to
> > > clear both the act-handled and the table-updated flags.
> > 
> > Huh. That's a bit crazy. But it is what the spec says.
> 
> In fact, I'd suggest adding a comment explaining this crazyness
> so that the next person doesn't have to wonder why we're never
> clearing the ACT bit.

Ok.

> 
> > 
> > > I tested things
> > > that way but managed to send an old version. Thanks for catching it.
> > > 
> > > > 
> > > > > + drm_dp_clear_payload_status(mgr);
> > > > >  
> > > > >   payload_alloc[0] = id;
> > > > >   payload_alloc[1] = payload->start_slot;
> > > > > diff --git a/drivers/gpu/drm/i915/display/intel_dp_mst.c 
> > > > > b/drivers/gpu/drm/i915/display/intel_dp_mst.c
> > > > > index 9308b5920780..3c4b0fb10d8b 100644
> > > > > --- a/drivers/gpu/drm/i915/display/intel_dp_mst.c
> > > > > +++ b/drivers/gpu/drm/i915/display/intel_dp_mst.c
> > > > > @@ -323,6 +323,8 @@ static void clear_act_sent(struct intel_dp 

Re: [PATCH 6/6] drm/i915/dp_mst: Ensure the DPCD ACT sent flag is cleared before waiting for it

2020-06-16 Thread Ville Syrjälä
On Tue, Jun 16, 2020 at 07:23:21PM +0300, Ville Syrjälä wrote:
> On Tue, Jun 16, 2020 at 06:54:41PM +0300, Imre Deak wrote:
> > On Tue, Jun 16, 2020 at 06:45:46PM +0300, Ville Syrjälä wrote:
> > > On Tue, Jun 16, 2020 at 05:18:55PM +0300, Imre Deak wrote:
> > > > Atm, we clear the ACT sent flag in the sink's DPCD before updating the
> > > > sink's payload table, along clearing the payload table updated flag.
> > > > The sink is supposed to set this flag once it detects that the source
> > > > has completed the ACT sequence (after detecting the 4 required ACT MTPH
> > > > symbols sent by the source). As opposed to this 2 DELL monitors I have
> > > > set the flag already along the payload table updated flag, which is not
> > > > quite correct.
> > > > 
> > > > To be sure that the sink has detected the ACT MTPH symbols before
> > > > continuing enabling the encoder, clear the ACT sent flag before enabling
> > > > or disabling the transcoder VC payload allocation (which is what starts
> > > > the ACT sequence).
> > > > 
> > > > Cc: Lyude Paul 
> > > > Cc: Ville Syrjälä 
> > > > Cc: dri-devel@lists.freedesktop.org
> > > > Signed-off-by: Imre Deak 
> > > > ---
> > > >  drivers/gpu/drm/drm_dp_mst_topology.c   | 31 +++--
> > > >  drivers/gpu/drm/i915/display/intel_dp_mst.c |  2 ++
> > > >  include/drm/drm_dp_mst_helper.h |  2 ++
> > > >  3 files changed, 33 insertions(+), 2 deletions(-)
> > > > 
> > > > diff --git a/drivers/gpu/drm/drm_dp_mst_topology.c 
> > > > b/drivers/gpu/drm/drm_dp_mst_topology.c
> > > > index b2f5a84b4cfb..e3bf8c9c8267 100644
> > > > --- a/drivers/gpu/drm/drm_dp_mst_topology.c
> > > > +++ b/drivers/gpu/drm/drm_dp_mst_topology.c
> > > > @@ -4377,6 +4377,34 @@ void drm_dp_mst_deallocate_vcpi(struct 
> > > > drm_dp_mst_topology_mgr *mgr,
> > > >  }
> > > >  EXPORT_SYMBOL(drm_dp_mst_deallocate_vcpi);
> > > >  
> > > > +/**
> > > > + * drm_dp_clear_payload_status() - Clears the payload table status 
> > > > flags
> > > > + * @mgr: manager to use
> > > > + *
> > > > + * Clears the payload table ACT handled and table updated flags in the 
> > > > MST hub's
> > > > + * DPCD. This function must be called before updating the payload 
> > > > table or
> > > > + * starting the ACT sequence and waiting for the corresponding flags 
> > > > to get
> > > > + * set by the hub.
> > > > + *
> > > > + * Returns:
> > > > + * 0 if the flag got cleared successfully, otherwise a negative error 
> > > > code.
> > > > + */
> > > > +int drm_dp_clear_payload_status(struct drm_dp_mst_topology_mgr *mgr)
> > > > +{
> > > > +   int ret;
> > > > +
> > > > +   ret = drm_dp_dpcd_writeb(mgr->aux, 
> > > > DP_PAYLOAD_TABLE_UPDATE_STATUS,
> > > > +DP_PAYLOAD_ACT_HANDLED);
> > > > +   if (ret < 0) {
> > > > +   DRM_DEBUG_DRIVER("Can't clear the ACT sent flag 
> > > > (%d)\n", ret);
> > > > +   return ret;
> > > > +   }
> > > > +   WARN_ON(ret != 1);
> > > > +
> > > > +   return 0;
> > > > +}
> > > > +EXPORT_SYMBOL(drm_dp_clear_payload_status);
> > > > +
> > > >  static int drm_dp_dpcd_write_payload(struct drm_dp_mst_topology_mgr 
> > > > *mgr,
> > > >  int id, struct drm_dp_payload 
> > > > *payload)
> > > >  {
> > > > @@ -4384,8 +4412,7 @@ static int drm_dp_dpcd_write_payload(struct 
> > > > drm_dp_mst_topology_mgr *mgr,
> > > > int ret;
> > > > int retries = 0;
> > > >  
> > > > -   drm_dp_dpcd_writeb(mgr->aux, DP_PAYLOAD_TABLE_UPDATE_STATUS,
> > > > -  DP_PAYLOAD_TABLE_UPDATED);
> > > 
> > > We used to clear DP_PAYLOAD_TABLE_UPDATED but now we clear
> > > DP_PAYLOAD_ACT_HANDLED ?
> > 
> > Eek. We should write DP_PAYLOAD_TABLE_UPDATED which is the only way to
> > clear both the act-handled and the table-updated flags.
> 
> Huh. That's a bit crazy. But it is what the spec says.

In fact, I'd suggest adding a comment explaining this crazyness
so that the next person doesn't have to wonder why we're never
clearing the ACT bit.

> 
> > I tested things
> > that way but managed to send an old version. Thanks for catching it.
> > 
> > > 
> > > > +   drm_dp_clear_payload_status(mgr);
> > > >  
> > > > payload_alloc[0] = id;
> > > > payload_alloc[1] = payload->start_slot;
> > > > diff --git a/drivers/gpu/drm/i915/display/intel_dp_mst.c 
> > > > b/drivers/gpu/drm/i915/display/intel_dp_mst.c
> > > > index 9308b5920780..3c4b0fb10d8b 100644
> > > > --- a/drivers/gpu/drm/i915/display/intel_dp_mst.c
> > > > +++ b/drivers/gpu/drm/i915/display/intel_dp_mst.c
> > > > @@ -323,6 +323,8 @@ static void clear_act_sent(struct intel_dp 
> > > > *intel_dp)
> > > >  
> > > > intel_de_write(i915, intel_dp->regs.dp_tp_status,
> > > >DP_TP_STATUS_ACT_SENT);
> > > > +
> > > > +   drm_dp_clear_payload_status(_dp->mst_mgr);
> > > >  }
> > > >  
> > > >  static void wait_for_act_sent(struct intel_dp *intel_dp)
> > > 

[Bug 206475] amdgpu under load drop signal to monitor until hard reset

2020-06-16 Thread bugzilla-daemon
https://bugzilla.kernel.org/show_bug.cgi?id=206475

--- Comment #14 from Andrew Ammerlaan (andrewammerl...@riseup.net) ---
I sort of worked around this too.

I changed two things:

1) the iGPU is now the primary GPU, and I use DRI_PRIME=1 to offload to the AMD
gpu. This has reduced the amount of things that are rendered on the AMD card.
This didn't actually fix anything, but it did remove the necessity for a hard
reboot when the AMD GPU does a reset. Now, when the GPU resets only the
applications that are rendered on the AMD card stop working, the desktop and
stuff stay functional. 

2) I added three fans to my PC. Though the card's thermal sensor never reported
that it reached the critical temperature (it went up to 82 Celsius max,
critical is 91 Celsius). There definitely does seem to be a correlation between
high temperatures and the occurrence of the resets. And more fans is always
better anyway.

I still experienced some resets after switching the primary GPU to the iGPU,
but only if I really pushed it to it's limits. I haven't had a single reset
since I added the fans. (Though admittedly I haven't run a decent stress test
yet, so it is still too early to conclude that the problem is completely gone)

Since under-clocking the card worked for you, and adding fans seems to work for
me. I have a hunch that even though the thermal sensor doesn't report
problematic temperatures some parts of the card actually do reach problematic
temperatures nonetheless, which might causes issues leading to a reset.
I'm not sure where the sensor is physically located, but considering that the
card is quite large, it doesn't seem that far fetched to me that there could be
quite a large difference in temperature between two points on the card.

Perhaps this card could benefit from a second thermal sensor or earlier and/or
more aggressive thermal throttling.

-- 
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] drm/panel-simple: fix connector type for LogicPD Type28 Display

2020-06-16 Thread Tomi Valkeinen

On 15/06/2020 17:53, Adam Ford wrote:

On Mon, Jun 15, 2020 at 9:46 AM Fabio Estevam  wrote:


On Mon, Jun 15, 2020 at 10:19 AM Adam Ford  wrote:


The LogicPD Type28 display used by several Logic PD products has not
worked since v5.5.


Maybe you could tell which commit exactly and then put a Fixes tag?


I honestly don't know.  I reached out to the omap mailing list,
because I noted this issue. Tomi V from TI responded with a link that
he posted which fixes this for another display.

https://www.mail-archive.com/dri-devel@lists.freedesktop.org/msg312208.html

I tested that patch and it worked for a different LCD, so I did the
same thing to the Logic PD Type 28 display as well.

My patch and commit message were modeled after his, and his commit
CC's stable with a note about being required for v5.5+

I added him to the CC list, so maybe he knows which hash needs to be
referenced from a fixes tag.  I was hoping to not have to go back and
bisect if it's not required, but I will if necessary.


No, I didn't check when exactly it broke. connector_type was added in v5.5, and my patch applies to 
v5.5, so I set that as stable version. But the WARN comes from panel bridge. Possibly 
89958b7cd9555a5d82556cc9a1f4c62fffda6f96 is the one that adds requirement to have connector_type.


 Tomi

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


Re: [PATCH 6/6] drm/i915/dp_mst: Ensure the DPCD ACT sent flag is cleared before waiting for it

2020-06-16 Thread Ville Syrjälä
On Tue, Jun 16, 2020 at 06:54:41PM +0300, Imre Deak wrote:
> On Tue, Jun 16, 2020 at 06:45:46PM +0300, Ville Syrjälä wrote:
> > On Tue, Jun 16, 2020 at 05:18:55PM +0300, Imre Deak wrote:
> > > Atm, we clear the ACT sent flag in the sink's DPCD before updating the
> > > sink's payload table, along clearing the payload table updated flag.
> > > The sink is supposed to set this flag once it detects that the source
> > > has completed the ACT sequence (after detecting the 4 required ACT MTPH
> > > symbols sent by the source). As opposed to this 2 DELL monitors I have
> > > set the flag already along the payload table updated flag, which is not
> > > quite correct.
> > > 
> > > To be sure that the sink has detected the ACT MTPH symbols before
> > > continuing enabling the encoder, clear the ACT sent flag before enabling
> > > or disabling the transcoder VC payload allocation (which is what starts
> > > the ACT sequence).
> > > 
> > > Cc: Lyude Paul 
> > > Cc: Ville Syrjälä 
> > > Cc: dri-devel@lists.freedesktop.org
> > > Signed-off-by: Imre Deak 
> > > ---
> > >  drivers/gpu/drm/drm_dp_mst_topology.c   | 31 +++--
> > >  drivers/gpu/drm/i915/display/intel_dp_mst.c |  2 ++
> > >  include/drm/drm_dp_mst_helper.h |  2 ++
> > >  3 files changed, 33 insertions(+), 2 deletions(-)
> > > 
> > > diff --git a/drivers/gpu/drm/drm_dp_mst_topology.c 
> > > b/drivers/gpu/drm/drm_dp_mst_topology.c
> > > index b2f5a84b4cfb..e3bf8c9c8267 100644
> > > --- a/drivers/gpu/drm/drm_dp_mst_topology.c
> > > +++ b/drivers/gpu/drm/drm_dp_mst_topology.c
> > > @@ -4377,6 +4377,34 @@ void drm_dp_mst_deallocate_vcpi(struct 
> > > drm_dp_mst_topology_mgr *mgr,
> > >  }
> > >  EXPORT_SYMBOL(drm_dp_mst_deallocate_vcpi);
> > >  
> > > +/**
> > > + * drm_dp_clear_payload_status() - Clears the payload table status flags
> > > + * @mgr: manager to use
> > > + *
> > > + * Clears the payload table ACT handled and table updated flags in the 
> > > MST hub's
> > > + * DPCD. This function must be called before updating the payload table 
> > > or
> > > + * starting the ACT sequence and waiting for the corresponding flags to 
> > > get
> > > + * set by the hub.
> > > + *
> > > + * Returns:
> > > + * 0 if the flag got cleared successfully, otherwise a negative error 
> > > code.
> > > + */
> > > +int drm_dp_clear_payload_status(struct drm_dp_mst_topology_mgr *mgr)
> > > +{
> > > + int ret;
> > > +
> > > + ret = drm_dp_dpcd_writeb(mgr->aux, DP_PAYLOAD_TABLE_UPDATE_STATUS,
> > > +  DP_PAYLOAD_ACT_HANDLED);
> > > + if (ret < 0) {
> > > + DRM_DEBUG_DRIVER("Can't clear the ACT sent flag (%d)\n", ret);
> > > + return ret;
> > > + }
> > > + WARN_ON(ret != 1);
> > > +
> > > + return 0;
> > > +}
> > > +EXPORT_SYMBOL(drm_dp_clear_payload_status);
> > > +
> > >  static int drm_dp_dpcd_write_payload(struct drm_dp_mst_topology_mgr *mgr,
> > >int id, struct drm_dp_payload *payload)
> > >  {
> > > @@ -4384,8 +4412,7 @@ static int drm_dp_dpcd_write_payload(struct 
> > > drm_dp_mst_topology_mgr *mgr,
> > >   int ret;
> > >   int retries = 0;
> > >  
> > > - drm_dp_dpcd_writeb(mgr->aux, DP_PAYLOAD_TABLE_UPDATE_STATUS,
> > > -DP_PAYLOAD_TABLE_UPDATED);
> > 
> > We used to clear DP_PAYLOAD_TABLE_UPDATED but now we clear
> > DP_PAYLOAD_ACT_HANDLED ?
> 
> Eek. We should write DP_PAYLOAD_TABLE_UPDATED which is the only way to
> clear both the act-handled and the table-updated flags.

Huh. That's a bit crazy. But it is what the spec says.

> I tested things
> that way but managed to send an old version. Thanks for catching it.
> 
> > 
> > > + drm_dp_clear_payload_status(mgr);
> > >  
> > >   payload_alloc[0] = id;
> > >   payload_alloc[1] = payload->start_slot;
> > > diff --git a/drivers/gpu/drm/i915/display/intel_dp_mst.c 
> > > b/drivers/gpu/drm/i915/display/intel_dp_mst.c
> > > index 9308b5920780..3c4b0fb10d8b 100644
> > > --- a/drivers/gpu/drm/i915/display/intel_dp_mst.c
> > > +++ b/drivers/gpu/drm/i915/display/intel_dp_mst.c
> > > @@ -323,6 +323,8 @@ static void clear_act_sent(struct intel_dp *intel_dp)
> > >  
> > >   intel_de_write(i915, intel_dp->regs.dp_tp_status,
> > >  DP_TP_STATUS_ACT_SENT);
> > > +
> > > + drm_dp_clear_payload_status(_dp->mst_mgr);
> > >  }
> > >  
> > >  static void wait_for_act_sent(struct intel_dp *intel_dp)
> > > diff --git a/include/drm/drm_dp_mst_helper.h 
> > > b/include/drm/drm_dp_mst_helper.h
> > > index 8b9eb4db3381..2facb87624bf 100644
> > > --- a/include/drm/drm_dp_mst_helper.h
> > > +++ b/include/drm/drm_dp_mst_helper.h
> > > @@ -763,6 +763,8 @@ int drm_dp_find_vcpi_slots(struct 
> > > drm_dp_mst_topology_mgr *mgr,
> > >  int pbn);
> > >  
> > >  
> > > +int drm_dp_clear_payload_status(struct drm_dp_mst_topology_mgr *mgr);
> > > +
> > >  int drm_dp_update_payload_part1(struct drm_dp_mst_topology_mgr *mgr);
> > >  
> > >  
> > > -- 
> > > 2.23.1
> > 
> > -- 
> > Ville Syrjälä
> > 

Re: [PATCH 4/4] drm: pl111: Update documentation

2020-06-16 Thread Emil Velikov
Hi all,

Inspired by Linus and my personal confusion with the report, allow me
to put some suggestions. Followed by an illustrative example.

On Wed, 10 Jun 2020 at 08:38, kernel test robot  wrote:
>
> Hi Linus,
>
> I love your patch! Perhaps something to improve:
>
> [auto build test WARNING on drm-exynos/exynos-drm-next]
> [also build test WARNING on drm-intel/for-linux-next 
> tegra-drm/drm/tegra/for-next linus/master v5.7 next-20200609]
One thing which was always inclear me - is the warning identical in
all the branches listed.

> [cannot apply to drm-tip/drm-tip drm/drm-next]
> [if your patch is applied to the wrong git tree, please drop us a note to help
> improve the system. BTW, we also suggest to use '--base' option to specify the
> base tree in git format-patch, please see 
> https://stackoverflow.com/a/37406982]
>
Reference to the manual tends to be better than an old SO thread.

> url:
> https://github.com/0day-ci/linux/commits/Linus-Walleij/drm-pl111-Credit-where-credit-is-due/20200610-041025
> base:   
> https://git.kernel.org/pub/scm/linux/kernel/git/daeinki/drm-exynos.git 
> exynos-drm-next
> reproduce: make htmldocs
>
> If you fix the issue, kindly add following tag as appropriate
> Reported-by: kernel test robot 
>
> All warnings (new ones prefixed by >>, old ones prefixed by <<):
>
Please do not list old warnings/errors. They only distract and dilute
what/where the problem is.


> vim +1 drivers/gpu/drm/pl111/pl111_drv.c
>
> e559355a9da60a Thomas Gleixner 2019-06-01  @1  // SPDX-License-Identifier: 
> GPL-2.0-only
> bed41005e6174d Tom Cooksey 2017-04-12   2  /*
> bed41005e6174d Tom Cooksey 2017-04-12   3   * (C) COPYRIGHT 2012-2013 ARM 
> Limited. All rights reserved.
> bed41005e6174d Tom Cooksey 2017-04-12   4   *
> bed41005e6174d Tom Cooksey 2017-04-12   5   * Parts of this file were 
> based on sources as follows:
> bed41005e6174d Tom Cooksey 2017-04-12   6   *
> bed41005e6174d Tom Cooksey 2017-04-12   7   * Copyright (c) 2006-2008 
> Intel Corporation
> bed41005e6174d Tom Cooksey 2017-04-12   8   * Copyright (c) 2007 Dave 
> Airlie 
> bed41005e6174d Tom Cooksey 2017-04-12   9   * Copyright (C) 2011 Texas 
> Instruments
> bed41005e6174d Tom Cooksey 2017-04-12  10   */
> bed41005e6174d Tom Cooksey 2017-04-12  11
>
> :: The code at line 1 was first introduced by commit
> :: e559355a9da60a2388893d9e9da66c79fd604b9a treewide: Replace GPLv2 
> boilerplate/reference with SPDX - rule 443
>
> :: TO: Thomas Gleixner 
> :: CC: Greg Kroah-Hartman 
>
All of this seems useful when debugging the kernel test robot itself.
Which in this case is misleading as hell.


Here is something which is much shorter, with clearer structure and
reads much easier.

---
Hi Linus,

This is an automated message from your friendly test robot.
We've noticed some issues with your patch although being a robot,
kindly double-check the results.

Branches:
[if the warning/errors are the same group them, otherwise split them
in separate sections]

- drm-exynos/exynos-drm-next [1]: WARNING
- drm-intel/for-linux-next [2]: WARNING
-  : WARNING
drivers/gpu/drm/pl111/pl111_drv.c:1: warning: 'ARM PrimeCell PL111
CLCD Driver' not found
- tegra-drm/drm/tegra/for-next [3]: WARNING
some other warning
- drm-tip/drm-tip [4] drm/drm-next [5]: cannot apply series

Note: when submitting patches, please use `--base` as documented in
https://git-scm.com/docs/git-format-patch.


To reproduce:
 - wget https://url/to/build/toolchain // when applicable
 - wget https://url/to/config // when applicable
 - make htmldocs // use explicit make command instead of magic shell
script, as seen in the cross build reports


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

Thank you
The LKP team

[1] URI to the drm-exynos/exynos-drm-next repo
[2] URI to the drm-intel/for-linux-next repo
[3] URI to the ...
___
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel


Re: [PATCH v2] drm/tegra: Add zpos property for cursor planes

2020-06-16 Thread Thierry Reding
On Tue, Jun 16, 2020 at 06:54:35PM +0300, Dmitry Osipenko wrote:
> 16.06.2020 15:17, Thierry Reding пишет:
> > From: Thierry Reding 
> > 
> > As of commit 4dc55525b095 ("drm: plane: Verify that no or all planes
> > have a zpos property") a warning is emitted if there's a mix of planes
> > with and without a zpos property.
> > 
> > On Tegra, cursor planes are always composited on top of all other
> > planes, which is why they never had a zpos property attached to them.
> > However, since the composition order is fixed, this is trivial to
> > remedy by simply attaching an immutable zpos property to them.
> > 
> > Changes in v2:
> > - hardcode cursor plane zpos to 255 instead of 0 (Ville)
> > 
> > Reported-by: Jonathan Hunter 
> > Signed-off-by: Thierry Reding 
> > ---
> >  drivers/gpu/drm/tegra/dc.c  | 9 +++--
> >  drivers/gpu/drm/tegra/hub.c | 2 +-
> >  2 files changed, 8 insertions(+), 3 deletions(-)
> > 
> > diff --git a/drivers/gpu/drm/tegra/dc.c b/drivers/gpu/drm/tegra/dc.c
> > index 83f31c6e891c..85408eed4685 100644
> > --- a/drivers/gpu/drm/tegra/dc.c
> > +++ b/drivers/gpu/drm/tegra/dc.c
> > @@ -787,7 +787,7 @@ static struct drm_plane 
> > *tegra_primary_plane_create(struct drm_device *drm,
> > }
> >  
> > drm_plane_helper_add(>base, _plane_helper_funcs);
> > -   drm_plane_create_zpos_property(>base, plane->index, 0, 255);
> > +   drm_plane_create_zpos_property(>base, plane->index, 0, 254);
> >  
> > err = drm_plane_create_rotation_property(>base,
> >  DRM_MODE_ROTATE_0,
> > @@ -957,6 +957,7 @@ static struct drm_plane 
> > *tegra_dc_cursor_plane_create(struct drm_device *drm,
> > }
> >  
> > drm_plane_helper_add(>base, _cursor_plane_helper_funcs);
> > +   drm_plane_create_zpos_immutable_property(>base, 255);
> >  
> > return >base;
> >  }
> > @@ -1074,7 +1075,11 @@ static struct drm_plane 
> > *tegra_dc_overlay_plane_create(struct drm_device *drm,
> > }
> >  
> > drm_plane_helper_add(>base, _plane_helper_funcs);
> > -   drm_plane_create_zpos_property(>base, plane->index, 0, 255);
> > +
> > +   if (!cursor)
> > +   drm_plane_create_zpos_property(>base, plane->index, 0, 
> > 254);
> > +   else
> > +   drm_plane_create_zpos_immutable_property(>base, 255);
> 
> On T20/30 we're are setting the plane's type to CURSOR because we want
> to use one overlay plane for the mouse cursor. Nevertheless, it's still
> a generic overlay plane that can change its z-position, and thus, it's
> wrong to make zpos immutable here.

But it doesn't really make sense for the cursor plane to change z-
position, even if it's technically possible. We do want it to always be
on top anyway. Doing it this way makes the cursor behave the same way
irrespective of the Tegra generation that we're running on.

Yes, that may not fully expose the capabilities of the hardware, but we
are already restricting the hardware capabilities by exposing the
overlay plane as a cursor plane in the first place.

Thierry


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


Re: [RFC PATCH 0/4] DirectX on Linux

2020-06-16 Thread Sasha Levin

On Tue, Jun 16, 2020 at 04:41:22PM +0200, Pavel Machek wrote:

On Tue 2020-06-16 09:28:19, Sasha Levin wrote:

On Tue, Jun 16, 2020 at 12:51:13PM +0200, Pavel Machek wrote:
> Hi!
>
> > > The driver creates the /dev/dxg device, which can be opened by user mode
> > > application and handles their ioctls. The IOCTL interface to the driver
> > > is defined in dxgkmthk.h (Dxgkrnl Graphics Port Driver ioctl
> > > definitions). The interface matches the D3DKMT interface on Windows.
> > > Ioctls are implemented in ioctl.c.
> >
> > Echoing what others said, you're not making a DRM driver. The driver should 
live outside
> > of the DRM code.
> >
>
> Actually, this sounds to me like "this should not be merged into linux 
kernel". I mean,
> we already have DRM API on Linux. We don't want another one, do we?

This driver doesn't have any display functionality.


Graphics cards without displays connected are quite common. I may be
wrong, but I believe we normally handle them using DRM...


This is more similar to the accelerators that live in drivers/misc/
right now.


> And at the very least... this misses API docs for /dev/dxg. Code can't really
> be reviewed without that.

The docs live here: 
https://docs.microsoft.com/en-us/windows-hardware/drivers/ddi/d3dkmthk/


I don't see "/dev/dxg" being metioned there. Plus, kernel API


Right, this is because this entire codebase is just a pipe to the API
I've linked, it doesn't implement anything new on it's own.


documentation should really go to Documentation, and be suitably
licensed.


While I don't mind copying the docs into Documentation, I'm concerned
that over time they will diverge from the docs on the website. This is
similar to how other documentation (such as the virtio spec) live out of
tree to avoid these issues.

w.r.t the licensing, again: this was sent under GPL2 (note the SPDX tags
in each file), and the patches carry a S-O-B by someone who was a
Microsoft employee at the time the patches were sent.

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


Re: [PATCH 6/6] drm/i915/dp_mst: Ensure the DPCD ACT sent flag is cleared before waiting for it

2020-06-16 Thread Imre Deak
On Tue, Jun 16, 2020 at 06:45:46PM +0300, Ville Syrjälä wrote:
> On Tue, Jun 16, 2020 at 05:18:55PM +0300, Imre Deak wrote:
> > Atm, we clear the ACT sent flag in the sink's DPCD before updating the
> > sink's payload table, along clearing the payload table updated flag.
> > The sink is supposed to set this flag once it detects that the source
> > has completed the ACT sequence (after detecting the 4 required ACT MTPH
> > symbols sent by the source). As opposed to this 2 DELL monitors I have
> > set the flag already along the payload table updated flag, which is not
> > quite correct.
> > 
> > To be sure that the sink has detected the ACT MTPH symbols before
> > continuing enabling the encoder, clear the ACT sent flag before enabling
> > or disabling the transcoder VC payload allocation (which is what starts
> > the ACT sequence).
> > 
> > Cc: Lyude Paul 
> > Cc: Ville Syrjälä 
> > Cc: dri-devel@lists.freedesktop.org
> > Signed-off-by: Imre Deak 
> > ---
> >  drivers/gpu/drm/drm_dp_mst_topology.c   | 31 +++--
> >  drivers/gpu/drm/i915/display/intel_dp_mst.c |  2 ++
> >  include/drm/drm_dp_mst_helper.h |  2 ++
> >  3 files changed, 33 insertions(+), 2 deletions(-)
> > 
> > diff --git a/drivers/gpu/drm/drm_dp_mst_topology.c 
> > b/drivers/gpu/drm/drm_dp_mst_topology.c
> > index b2f5a84b4cfb..e3bf8c9c8267 100644
> > --- a/drivers/gpu/drm/drm_dp_mst_topology.c
> > +++ b/drivers/gpu/drm/drm_dp_mst_topology.c
> > @@ -4377,6 +4377,34 @@ void drm_dp_mst_deallocate_vcpi(struct 
> > drm_dp_mst_topology_mgr *mgr,
> >  }
> >  EXPORT_SYMBOL(drm_dp_mst_deallocate_vcpi);
> >  
> > +/**
> > + * drm_dp_clear_payload_status() - Clears the payload table status flags
> > + * @mgr: manager to use
> > + *
> > + * Clears the payload table ACT handled and table updated flags in the MST 
> > hub's
> > + * DPCD. This function must be called before updating the payload table or
> > + * starting the ACT sequence and waiting for the corresponding flags to get
> > + * set by the hub.
> > + *
> > + * Returns:
> > + * 0 if the flag got cleared successfully, otherwise a negative error code.
> > + */
> > +int drm_dp_clear_payload_status(struct drm_dp_mst_topology_mgr *mgr)
> > +{
> > +   int ret;
> > +
> > +   ret = drm_dp_dpcd_writeb(mgr->aux, DP_PAYLOAD_TABLE_UPDATE_STATUS,
> > +DP_PAYLOAD_ACT_HANDLED);
> > +   if (ret < 0) {
> > +   DRM_DEBUG_DRIVER("Can't clear the ACT sent flag (%d)\n", ret);
> > +   return ret;
> > +   }
> > +   WARN_ON(ret != 1);
> > +
> > +   return 0;
> > +}
> > +EXPORT_SYMBOL(drm_dp_clear_payload_status);
> > +
> >  static int drm_dp_dpcd_write_payload(struct drm_dp_mst_topology_mgr *mgr,
> >  int id, struct drm_dp_payload *payload)
> >  {
> > @@ -4384,8 +4412,7 @@ static int drm_dp_dpcd_write_payload(struct 
> > drm_dp_mst_topology_mgr *mgr,
> > int ret;
> > int retries = 0;
> >  
> > -   drm_dp_dpcd_writeb(mgr->aux, DP_PAYLOAD_TABLE_UPDATE_STATUS,
> > -  DP_PAYLOAD_TABLE_UPDATED);
> 
> We used to clear DP_PAYLOAD_TABLE_UPDATED but now we clear
> DP_PAYLOAD_ACT_HANDLED ?

Eek. We should write DP_PAYLOAD_TABLE_UPDATED which is the only way to
clear both the act-handled and the table-updated flags. I tested things
that way but managed to send an old version. Thanks for catching it.

> 
> > +   drm_dp_clear_payload_status(mgr);
> >  
> > payload_alloc[0] = id;
> > payload_alloc[1] = payload->start_slot;
> > diff --git a/drivers/gpu/drm/i915/display/intel_dp_mst.c 
> > b/drivers/gpu/drm/i915/display/intel_dp_mst.c
> > index 9308b5920780..3c4b0fb10d8b 100644
> > --- a/drivers/gpu/drm/i915/display/intel_dp_mst.c
> > +++ b/drivers/gpu/drm/i915/display/intel_dp_mst.c
> > @@ -323,6 +323,8 @@ static void clear_act_sent(struct intel_dp *intel_dp)
> >  
> > intel_de_write(i915, intel_dp->regs.dp_tp_status,
> >DP_TP_STATUS_ACT_SENT);
> > +
> > +   drm_dp_clear_payload_status(_dp->mst_mgr);
> >  }
> >  
> >  static void wait_for_act_sent(struct intel_dp *intel_dp)
> > diff --git a/include/drm/drm_dp_mst_helper.h 
> > b/include/drm/drm_dp_mst_helper.h
> > index 8b9eb4db3381..2facb87624bf 100644
> > --- a/include/drm/drm_dp_mst_helper.h
> > +++ b/include/drm/drm_dp_mst_helper.h
> > @@ -763,6 +763,8 @@ int drm_dp_find_vcpi_slots(struct 
> > drm_dp_mst_topology_mgr *mgr,
> >int pbn);
> >  
> >  
> > +int drm_dp_clear_payload_status(struct drm_dp_mst_topology_mgr *mgr);
> > +
> >  int drm_dp_update_payload_part1(struct drm_dp_mst_topology_mgr *mgr);
> >  
> >  
> > -- 
> > 2.23.1
> 
> -- 
> Ville Syrjälä
> Intel
___
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel


Re: [PATCH v2 0/5] 180 degrees rotation support for NVIDIA Tegra DRM

2020-06-16 Thread Emil Velikov
On Tue, 16 Jun 2020 at 12:40, Dmitry Osipenko  wrote:
>
> 16.06.2020 01:26, Emil Velikov пишет:
> > Hi Dmitry,
> >
> > On Mon, 15 Jun 2020 at 08:28, Dmitry Osipenko  wrote:
> >>
> >> Hello!
> >>
> >> This series adds 180° display plane rotation support to the NVIDIA Tegra
> >> DRM driver which is needed for devices that have display panel physically
> >> mounted upside-down, like Nexus 7 tablet device for example [1]. Since
> >> DRM panel rotation is a new thing for a userspace, currently only
> >> Opentegra Xorg driver handles the rotated display panel [2], but this
> >> is good enough for the start.
> >>
> >> Note that later on it should be possible to implement a transparent 180°
> >> display rotation for Tegra DRM driver which will remove the need to have
> >> a bleeding edge userspace that knows how to rotate display planes and I'm
> >> slowly working on it. For the starter we can go with the minimal rotation
> >> support, so it's not a blocker.
> >>
> >> This series is based on the work that was made by Derek Basehore for the
> >> Mediatek driver [3], his patch is included into this patchset. I added
> >> my tested-by tag to the Derek's patch.
> >>
> >> Please review and apply, thanks in advance!
> >>
> >> [1] 
> >> https://patchwork.ozlabs.org/project/linux-tegra/patch/20200607154327.18589-3-dig...@gmail.com/
> >> [2] 
> >> https://github.com/grate-driver/xf86-video-opentegra/commit/28eb20a3959bbe5bc3a3b67e55977093fd5114ca
> >> [3] https://lkml.org/lkml/2020/3/5/1119
> >>
> >> Changelog:
> >>
> >> v2: - Dropped "drm/panel: Set display info in panel attach" patch, which
> >>   turned out to be obsolete now.
> >>
> >> - Renamed the cover-latter, hopefully this will fix the bouncing 
> >> emails.
> >>
> >> Derek Basehore (1):
> >>   drm/panel: Add helper for reading DT rotation
> >>
> >> Dmitry Osipenko (4):
> >>   drm/panel: lvds: Set up panel orientation
> >
> > IMHO it's perfectly reasonable to report the panel orientation to
> > userspace, which can apply plane rotation as needed.
> >
> > Although I see that this series, alike Derek's, has a couple of issues:
> >  - only a single panel driver is updated
> >  - rotation is _not_ listed as supported property, in said panel
> > driver device-tree bindings
> >
> > My personal inclination is that we should aim for a comprehensive solution:
> >  - wire all panel drivers, as currently documented (quick grep list below)
> >  - document and wire-up the lvds and boe panels - as proposed by you
> > and Derek respectively
> >
> > HTH
> > Emil
> >
> > Documentation/devicetree/bindings/display/himax,hx8357d.txt:2
> > Documentation/devicetree/bindings/display/ilitek,ili9225.txt:2
> > Documentation/devicetree/bindings/display/ilitek,ili9341.txt:2
> > Documentation/devicetree/bindings/display/ilitek,ili9486.yaml:2
> > Documentation/devicetree/bindings/display/multi-inno,mi0283qt.txt:2
> > Documentation/devicetree/bindings/display/panel/panel-common.yaml:2
> > Documentation/devicetree/bindings/display/sitronix,st7586.txt:1
> > Documentation/devicetree/bindings/display/sitronix,st7735r.yaml:2
>
> Rotation is a common DT panel property that is described in the
> panel-common.yaml.
The property was introduced almost exclusively for tiny drm panels.
Those ones are a bit different from the rest (in panel/) -
MIPI-DBI/SPI w/o (not connected at least) an actual GPU.

To make it a bit better, the rotation is seemingly performed in the
tiny driver itself ouch.

> This property is supported by all panel bindings
> because these bindings inherent the common properties from the
> panel-common.yaml.
>
Seems like that was an unintentional change with the conversion to YAML.
Beforehand only a few selected panels had rotation. Upon closer look -
some panels do have follow-up fixes, to remove/limit the implicit
inclusion.

Sam seems like you've done most of the YAML conversion. IMHO it would
make sense to revisit the patches and inherit common properties only
as applicable.

> I don't think that it makes sense to wire up rotation property to all
> panel drivers at once because those drivers will be untested, at least I
> don't know anything about those other panels and can't test them. It
> will be much better to support the rotation on by as-needed basis for
> each panel driver individually.

How about CCing the author and reviewer asking them to test the patch?
The only place where the patches might cause an issue is with tiny,
although patches would still be appreciated.

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


[Bug 206475] amdgpu under load drop signal to monitor until hard reset

2020-06-16 Thread bugzilla-daemon
https://bugzilla.kernel.org/show_bug.cgi?id=206475

--- Comment #13 from Marco (rodomar...@protonmail.com) ---
The only way I had it "fixed" (it's more of a workaround, but it is working) is
to slightly drop the clocks (my GPU has by default a max boost clock of 1430
MHz, I have dropped it to 1340 MHz) and voltages (From 1150 mV to 1040 mV on
peak clocks, however both depends on your specific silicon, this is just my
values). 

Now, if the system post after the downclock of it (sometimes lowering
clocks/voltages triggers a black screen bug at boot when the values are applied
with systemd, not sure if the issue is the same), however, if I can reach the
login screen, the system will work perfectly fine under load with no problem.
Never had a crash while using since.

I do not know if it's a specific issue of my silicon, since after this issue
happened to my card the same applies under Windows system. Repasting didn't
help.

-- 
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 6/6] drm/i915/dp_mst: Ensure the DPCD ACT sent flag is cleared before waiting for it

2020-06-16 Thread Ville Syrjälä
On Tue, Jun 16, 2020 at 05:18:55PM +0300, Imre Deak wrote:
> Atm, we clear the ACT sent flag in the sink's DPCD before updating the
> sink's payload table, along clearing the payload table updated flag.
> The sink is supposed to set this flag once it detects that the source
> has completed the ACT sequence (after detecting the 4 required ACT MTPH
> symbols sent by the source). As opposed to this 2 DELL monitors I have
> set the flag already along the payload table updated flag, which is not
> quite correct.
> 
> To be sure that the sink has detected the ACT MTPH symbols before
> continuing enabling the encoder, clear the ACT sent flag before enabling
> or disabling the transcoder VC payload allocation (which is what starts
> the ACT sequence).
> 
> Cc: Lyude Paul 
> Cc: Ville Syrjälä 
> Cc: dri-devel@lists.freedesktop.org
> Signed-off-by: Imre Deak 
> ---
>  drivers/gpu/drm/drm_dp_mst_topology.c   | 31 +++--
>  drivers/gpu/drm/i915/display/intel_dp_mst.c |  2 ++
>  include/drm/drm_dp_mst_helper.h |  2 ++
>  3 files changed, 33 insertions(+), 2 deletions(-)
> 
> diff --git a/drivers/gpu/drm/drm_dp_mst_topology.c 
> b/drivers/gpu/drm/drm_dp_mst_topology.c
> index b2f5a84b4cfb..e3bf8c9c8267 100644
> --- a/drivers/gpu/drm/drm_dp_mst_topology.c
> +++ b/drivers/gpu/drm/drm_dp_mst_topology.c
> @@ -4377,6 +4377,34 @@ void drm_dp_mst_deallocate_vcpi(struct 
> drm_dp_mst_topology_mgr *mgr,
>  }
>  EXPORT_SYMBOL(drm_dp_mst_deallocate_vcpi);
>  
> +/**
> + * drm_dp_clear_payload_status() - Clears the payload table status flags
> + * @mgr: manager to use
> + *
> + * Clears the payload table ACT handled and table updated flags in the MST 
> hub's
> + * DPCD. This function must be called before updating the payload table or
> + * starting the ACT sequence and waiting for the corresponding flags to get
> + * set by the hub.
> + *
> + * Returns:
> + * 0 if the flag got cleared successfully, otherwise a negative error code.
> + */
> +int drm_dp_clear_payload_status(struct drm_dp_mst_topology_mgr *mgr)
> +{
> + int ret;
> +
> + ret = drm_dp_dpcd_writeb(mgr->aux, DP_PAYLOAD_TABLE_UPDATE_STATUS,
> +  DP_PAYLOAD_ACT_HANDLED);
> + if (ret < 0) {
> + DRM_DEBUG_DRIVER("Can't clear the ACT sent flag (%d)\n", ret);
> + return ret;
> + }
> + WARN_ON(ret != 1);
> +
> + return 0;
> +}
> +EXPORT_SYMBOL(drm_dp_clear_payload_status);
> +
>  static int drm_dp_dpcd_write_payload(struct drm_dp_mst_topology_mgr *mgr,
>int id, struct drm_dp_payload *payload)
>  {
> @@ -4384,8 +4412,7 @@ static int drm_dp_dpcd_write_payload(struct 
> drm_dp_mst_topology_mgr *mgr,
>   int ret;
>   int retries = 0;
>  
> - drm_dp_dpcd_writeb(mgr->aux, DP_PAYLOAD_TABLE_UPDATE_STATUS,
> -DP_PAYLOAD_TABLE_UPDATED);

We used to clear DP_PAYLOAD_TABLE_UPDATED but now we clear
DP_PAYLOAD_ACT_HANDLED ?

> + drm_dp_clear_payload_status(mgr);
>  
>   payload_alloc[0] = id;
>   payload_alloc[1] = payload->start_slot;
> diff --git a/drivers/gpu/drm/i915/display/intel_dp_mst.c 
> b/drivers/gpu/drm/i915/display/intel_dp_mst.c
> index 9308b5920780..3c4b0fb10d8b 100644
> --- a/drivers/gpu/drm/i915/display/intel_dp_mst.c
> +++ b/drivers/gpu/drm/i915/display/intel_dp_mst.c
> @@ -323,6 +323,8 @@ static void clear_act_sent(struct intel_dp *intel_dp)
>  
>   intel_de_write(i915, intel_dp->regs.dp_tp_status,
>  DP_TP_STATUS_ACT_SENT);
> +
> + drm_dp_clear_payload_status(_dp->mst_mgr);
>  }
>  
>  static void wait_for_act_sent(struct intel_dp *intel_dp)
> diff --git a/include/drm/drm_dp_mst_helper.h b/include/drm/drm_dp_mst_helper.h
> index 8b9eb4db3381..2facb87624bf 100644
> --- a/include/drm/drm_dp_mst_helper.h
> +++ b/include/drm/drm_dp_mst_helper.h
> @@ -763,6 +763,8 @@ int drm_dp_find_vcpi_slots(struct drm_dp_mst_topology_mgr 
> *mgr,
>  int pbn);
>  
>  
> +int drm_dp_clear_payload_status(struct drm_dp_mst_topology_mgr *mgr);
> +
>  int drm_dp_update_payload_part1(struct drm_dp_mst_topology_mgr *mgr);
>  
>  
> -- 
> 2.23.1

-- 
Ville Syrjälä
Intel
___
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel


Re: [PATCH v7 3/3] drm/i915/dp: Expose connector VRR monitor range via debugfs

2020-06-16 Thread Emil Velikov
On Mon, 15 Jun 2020 at 22:47, Manasi Navare  wrote:
>
> On Mon, Jun 15, 2020 at 10:36:28PM +0100, Emil Velikov wrote:
> > Hi Manasi,
> >
> > On Sat, 13 Jun 2020 at 00:55, Manasi Navare  
> > wrote:
> > >
> > > From: Bhanuprakash Modem 
> > >
> > > [Why]
> > > It's useful to know the min and max vrr range for IGT testing.
> > >
> > > [How]
> > > Expose the min and max vfreq for the connector via a debugfs file
> > > on the connector, "vrr_range".
> > >
> > > Example usage: cat /sys/kernel/debug/dri/0/DP-1/vrr_range
> > >
> > > v7:
> > > * Fix cmpilation due to rebase
> > > v6:
> > > * Rebase (manasi)
> > > v5:
> > > * Rename to vrr_range to match AMD debugfs
> > > v4:
> > > * Rebase
> > > v3:
> > > * Remove the unnecessary debug print (Manasi)
> > > v2:
> > > * Fix the typo in max_vfreq (Manasi)
> > > * Change the name of node to i915_vrr_info so we can add
> > > other vrr info for more debug info (Manasi)
> > > * Change the VRR capable to display Yes or No (Manasi)
> > > * Fix indentation checkpatch errors (Manasi)
> > >
> > Nit: generally revision log is listed in v2 -> v6 order.
>
> Okay point noted. Will update this in the next rev
>
> >
> > > Signed-off-by: Bhanuprakash Modem 
> > > Signed-off-by: Manasi Navare 
> > > Cc: Jani Nikula 
> > > Cc: Ville Syrjälä 
> > > Tested-by: Manasi Navare 
> > > ---
> > >  .../drm/i915/display/intel_display_debugfs.c  | 22 ++-
> > >  1 file changed, 21 insertions(+), 1 deletion(-)
> > >
> > > diff --git a/drivers/gpu/drm/i915/display/intel_display_debugfs.c 
> > > b/drivers/gpu/drm/i915/display/intel_display_debugfs.c
> > > index 28dd717e943a..2921f7d2a26e 100644
> > > --- a/drivers/gpu/drm/i915/display/intel_display_debugfs.c
> > > +++ b/drivers/gpu/drm/i915/display/intel_display_debugfs.c
> > > @@ -2185,6 +2185,21 @@ static const struct file_operations 
> > > i915_dsc_fec_support_fops = {
> > > .write = i915_dsc_fec_support_write
> > >  };
> > >
> > > +static int vrr_range_show(struct seq_file *m, void *data)
> > > +{
> > > +   struct drm_connector *connector = m->private;
> > > +
> > > +   if (connector->status != connector_status_connected)
> > > +   return -ENODEV;
> > > +
> > > +   seq_printf(m, "Vrr_capable: %s\n", 
> > > yesno(intel_dp_is_vrr_capable(connector)));
> > > +   seq_printf(m, "Min: %u\n", 
> > > (u8)connector->display_info.monitor_range.min_vfreq);
> > > +   seq_printf(m, "Max: %u\n", 
> > > (u8)connector->display_info.monitor_range.max_vfreq);
> > > +
> > > +   return 0;
> > > +}
> > > +DEFINE_SHOW_ATTRIBUTE(vrr_range);
> > > +
> > >  /**
> > >   * intel_connector_debugfs_add - add i915 specific connector debugfs 
> > > files
> > >   * @connector: pointer to a registered drm_connector
> > > @@ -2220,10 +2235,15 @@ int intel_connector_debugfs_add(struct 
> > > drm_connector *connector)
> > > if (INTEL_GEN(dev_priv) >= 10 &&
> > > ((connector->connector_type == DRM_MODE_CONNECTOR_DisplayPort 
> > > &&
> > >   !to_intel_connector(connector)->mst_port) ||
> > > -connector->connector_type == DRM_MODE_CONNECTOR_eDP))
> > > +connector->connector_type == DRM_MODE_CONNECTOR_eDP)) {
> > > debugfs_create_file("i915_dsc_fec_support", S_IRUGO, root,
> > > connector, 
> > > _dsc_fec_support_fops);
> > >
> > > +   if (INTEL_GEN(dev_priv) >= 12)
> > > +   debugfs_create_file("vrr_range", S_IRUGO,
> > > +   root, connector, 
> > > _range_fops);
> > > +   }
> > > +
> >
> > I think this should be added by core drm. Ideally drm will add it
> > automatically for each connector that the driver has called
> > drm_connector_attach_vrr_capable_property() upon.
> >
>
> But in this case drm_connector_attach_vrr_capable_property() is called by 
> individual
> driver since its an optional connector property. So we call this inside i915.

I'm _not_ suggesting that one moves the
drm_connector_attach_vrr_capable_property() call. Simply create the
debugfs file in drm itself.

> Also currently AMD sets this debugfs inside AMD IMO, so setting this here for 
> now.
Let's do the better thing of a) make drm create the file, and b)
remove the AMDGPU specific one.

We're talking about 20-30 lines worth of a patch. Postponing it sounds silly.

> But I agree that can be moved to drm core may be when drm_display_info gets 
> populated
> with min and max, thats where drm can add this?
>
Both min and max are already part of drm_display_info. On the question
of how - check the existing properties (edid_override, force) for
examples.

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


[Bug 208209] [amdgpu] driver crash -- enable_link_dp -- RX 570

2020-06-16 Thread bugzilla-daemon
https://bugzilla.kernel.org/show_bug.cgi?id=208209

Alex Deucher (alexdeuc...@gmail.com) changed:

   What|Removed |Added

 CC||alexdeuc...@gmail.com

--- Comment #1 from Alex Deucher (alexdeuc...@gmail.com) ---
It's not a crash, just a warning.  Please attach your xorg log (if using X) and
dmesg output.

-- 
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 208209] New: [amdgpu] driver crash -- enable_link_dp -- RX 570

2020-06-16 Thread bugzilla-daemon
https://bugzilla.kernel.org/show_bug.cgi?id=208209

Bug ID: 208209
   Summary: [amdgpu] driver crash -- enable_link_dp -- RX 570
   Product: Drivers
   Version: 2.5
Kernel Version: 5.3.0-55-generic
  Hardware: x86-64
OS: Linux
  Tree: Mainline
Status: NEW
  Severity: normal
  Priority: P1
 Component: Video(DRI - non Intel)
  Assignee: drivers_video-...@kernel-bugs.osdl.org
  Reporter: max.bruc...@gmail.com
Regression: No

This crash was nonfatal to the system, but seems to break video playback.
Happens randomly. Let me know if more information needed.

Dmesg log:
[901929.410695] WARNING: CPU: 11 PID: 123042 at
drivers/gpu/drm/amd/amdgpu/../display/dc/core/dc_link_dp.c:1720
decide_link_settings+0x107/0x270 [amdgpu]
[901929.410696] Modules linked in: ufs qnx4 hfsplus hfs minix ntfs msdos jfs
xfs cpuid rfcomm xt_CHECKSUM xt_MASQUERADE xt_conntrack ipt_REJECT
nf_reject_ipv4 xt_tcpudp ip6table_mangle ip6table_nat iptable_mangle
iptable_nat nf_nat nf_conntrack nf_defrag_ipv6 nf_defrag_ipv4 ebtable_filter
ebtables ip6table_filter ip6_tables bridge stp llc rpcsec_gss_krb5 auth_rpcgss
edac_mce_amd kvm_amd ashmem_linux(CE) binder_linux iptable_filter bpfilter cmac
bnep binfmt_misc input_leds snd_hda_codec_realtek snd_hda_codec_generic
ledtrig_audio snd_hda_codec_hdmi snd_hda_intel snd_intel_dspcfg btusb btrtl
snd_hda_codec btbcm snd_hda_core btintel snd_hwdep snd_pcm bluetooth
snd_seq_midi snd_seq_midi_event ecdh_generic ecc kvm snd_rawmidi irqbypass
snd_seq iwlmvm snd_seq_device mac80211 snd_timer libarc4 iwlwifi wmi_bmof snd
cfg80211 soundcore k10temp ccp mac_hid sch_fq_codel parport_pc ppdev lp parport
ip_tables x_tables autofs4 btrfs zstd_compress hid_logitech_hidpp
hid_logitech_dj dm_crypt raid10 raid456
[901929.410732]  async_raid6_recov async_memcpy async_pq async_xor async_tx xor
hid_generic usbhid hid raid6_pq libcrc32c raid1 raid0 multipath linear nfsv4
nfsv3 nfs_acl nfs lockd grace sunrpc fscache bonding amdgpu crct10dif_pclmul
amd_iommu_v2 crc32_pclmul gpu_sched ghash_clmulni_intel ttm aesni_intel
drm_kms_helper aes_x86_64 syscopyarea sysfillrect crypto_simd igb sysimgblt
nvme cryptd fb_sys_fops dca i2c_algo_bit mxm_wmi drm glue_helper nvme_core ahci
i2c_piix4 libahci gpio_amdpt wmi gpio_generic
[901929.410759] CPU: 11 PID: 123042 Comm: kworker/11:0 Kdump: loaded Tainted: G
   WC  E 5.3.0-55-generic #49-Ubuntu
[901929.410760] Hardware name: To Be Filled By O.E.M. To Be Filled By
O.E.M./X399 Taichi, BIOS P3.50 12/24/2018
[901929.410865] Workqueue: events dm_irq_work_func [amdgpu]
[901929.410965] RIP: 0010:decide_link_settings+0x107/0x270 [amdgpu]
[901929.410968] Code: c0 83 fa 0e 77 07 8b 04 95 80 c6 73 c0 89 44 24 14 c7 44
24 10 01 00 00 00 39 43 5c 73 bd 48 c7 c7 e8 d3 77 c0 e8 cc 9c a7 c0 <0f> 0b 83
7b 58 00 0f 85 7d 50 02 00 e9 6a 50 02 00 80 bb e4 00 00
[901929.410969] RSP: 0018:c900165777d0 EFLAGS: 00010246
[901929.410971] RAX: 0024 RBX: 49ecc400 RCX:

[901929.410972] RDX:  RSI: 88905ead7448 RDI:
88905ead7448
[901929.410973] RBP: c90016577818 R08: 88905ead7448 R09:
0004
[901929.410974] R10:  R11: 0001 R12:
c900165777e0
[901929.410975] R13: 00c34830 R14: c90016577838 R15:

[901929.410977] FS:  () GS:88905eac()
knlGS:
[901929.410978] CS:  0010 DS:  ES:  CR0: 80050033
[901929.410979] CR2: 5564ed8d0730 CR3: 001016bb CR4:
003406e0
[901929.410980] Call Trace:
[901929.411082]  enable_link_dp+0x51/0x260 [amdgpu]
[901929.411180]  core_link_enable_stream+0x63c/0x8f0 [amdgpu]
[901929.411284]  ? set_pixel_encoding.isra.0+0x6d/0x160 [amdgpu]
[901929.411381]  dce110_apply_ctx_to_hw+0x501/0x580 [amdgpu]
[901929.411479]  ? dce110_apply_ctx_to_hw+0x501/0x580 [amdgpu]
[901929.411552]  dc_commit_state_no_check+0x228/0x580 [amdgpu]
[901929.411624]  dc_commit_state+0x96/0xb0 [amdgpu]
[901929.411699]  amdgpu_dm_atomic_commit_tail+0x3bf/0xfb0 [amdgpu]
[901929.411768]  ? amdgpu_cgs_read_register+0x14/0x20 [amdgpu]
[901929.411838]  ? dm_read_reg_func+0x25/0x90 [amdgpu]
[901929.411844]  ? _cond_resched+0x19/0x30
[901929.411894]  ? amdgpu_bo_pin_restricted+0x61/0x2a0 [amdgpu]
[901929.411969]  ? dm_plane_helper_prepare_fb+0x1b3/0x290 [amdgpu]
[901929.411972]  ? _cond_resched+0x19/0x30
[901929.411973]  ? wait_for_completion_timeout+0x3a/0x120
[901929.411975]  ? wait_for_completion_interruptible+0x37/0x160
[901929.411983]  commit_tail+0x41/0x70 [drm_kms_helper]
[901929.411990]  ? commit_tail+0x41/0x70 [drm_kms_helper]
[901929.411997]  drm_atomic_helper_commit+0x118/0x120 [drm_kms_helper]
[901929.412070]  amdgpu_dm_atomic_commit+0xb1/0xf0 [amdgpu]
[901929.412085]  drm_atomic_commit+0x4a/0x50 [drm]
[901929.412158]  dm_restore_drm_connector_state+0xe9/0x140 [amdgpu]

[Bug 208205] [amdgpu] System hang / freeze

2020-06-16 Thread bugzilla-daemon
https://bugzilla.kernel.org/show_bug.cgi?id=208205

Alex Deucher (alexdeuc...@gmail.com) changed:

   What|Removed |Added

 CC||alexdeuc...@gmail.com

--- Comment #1 from Alex Deucher (alexdeuc...@gmail.com) ---
Please attach your dmesg output and xorg log (if using X).

-- 
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] drm/mgag200: Don't set in MISC

2020-06-16 Thread Emil Velikov
Hi Thomas,

On Tue, 16 Jun 2020 at 15:26, Thomas Zimmermann  wrote:
>
> The original modesetting code set MISC to 0x2d, which is ,
>  and 
>
> With the conversion to atomic modesetting,  accidentally
> got enabled as well. Revert this change and initialize MISC with a
> constant value of  and . The  bits are set
> in mga_crtc_set_plls(), sync flags are set in mgag200_set_mode_regs().
>

Let's keep the remove (restoring original functionality) and rename
(cosmetics) separate patches. The read has also disappeared, which
should be safe although might be better on it's own.

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


Re: [PATCH][next] drm/i915/selftests: Fix inconsistent IS_ERR and PTR_ERR

2020-06-16 Thread Chris Wilson
Quoting Gustavo A. R. Silva (2020-06-16 15:54:52)
> Fix inconsistent IS_ERR and PTR_ERR in live_timeslice_nopreempt().
> 
> The proper pointer to be passed as argument to PTR_ERR() is ce.
> 
> This bug was detected with the help of Coccinelle.
> 
> Fixes: b72f02d78e4f ("drm/i915/gt: Prevent timeslicing into unpreemptable 
> requests")
> Signed-off-by: Gustavo A. R. Silva 

Fair enough, I had sent out a patch that included this, but never split
it out for early adoption.

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


[PATCH][next] drm/i915/selftests: Fix inconsistent IS_ERR and PTR_ERR

2020-06-16 Thread Gustavo A. R. Silva
Fix inconsistent IS_ERR and PTR_ERR in live_timeslice_nopreempt().

The proper pointer to be passed as argument to PTR_ERR() is ce.

This bug was detected with the help of Coccinelle.

Fixes: b72f02d78e4f ("drm/i915/gt: Prevent timeslicing into unpreemptable 
requests")
Signed-off-by: Gustavo A. R. Silva 
---
 drivers/gpu/drm/i915/gt/selftest_lrc.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/gpu/drm/i915/gt/selftest_lrc.c 
b/drivers/gpu/drm/i915/gt/selftest_lrc.c
index 91543494f595..393339de0910 100644
--- a/drivers/gpu/drm/i915/gt/selftest_lrc.c
+++ b/drivers/gpu/drm/i915/gt/selftest_lrc.c
@@ -1337,7 +1337,7 @@ static int live_timeslice_nopreempt(void *arg)
 
ce = intel_context_create(engine);
if (IS_ERR(ce)) {
-   err = PTR_ERR(rq);
+   err = PTR_ERR(ce);
goto out_spin;
}
 
-- 
2.27.0

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


[PATCH] drm/mgag200: Don't set in MISC

2020-06-16 Thread Thomas Zimmermann
The original modesetting code set MISC to 0x2d, which is ,
 and 

With the conversion to atomic modesetting,  accidentally
got enabled as well. Revert this change and initialize MISC with a
constant value of  and . The  bits are set
in mga_crtc_set_plls(), sync flags are set in mgag200_set_mode_regs().

While at it, also rename the flag constant to match the nameing in
the MGA Programming Manual.

Signed-off-by: Thomas Zimmermann 
Reported-by: kernel test robot 
Suggested-by: Emil Velikov 
Fixes: db05f8d3dc87 ("drm/mgag200: Split MISC register update into PLL 
selection, SYNC and I/O")
Cc: Thomas Zimmermann 
Cc: Sam Ravnborg 
Cc: Emil Velikov 
Cc: Dave Airlie 
Cc: Daniel Vetter 
Cc: Gerd Hoffmann 
Cc: "José Roberto de Souza" 
Cc: Andrzej Pietrasiewicz 
Cc: Rong Chen 
Cc: John Donnelly 
---
 drivers/gpu/drm/mgag200/mgag200_mode.c | 6 ++
 drivers/gpu/drm/mgag200/mgag200_reg.h  | 2 +-
 2 files changed, 3 insertions(+), 5 deletions(-)

diff --git a/drivers/gpu/drm/mgag200/mgag200_mode.c 
b/drivers/gpu/drm/mgag200/mgag200_mode.c
index f16bd278ab7e4..3b7235bd0bcba 100644
--- a/drivers/gpu/drm/mgag200/mgag200_mode.c
+++ b/drivers/gpu/drm/mgag200/mgag200_mode.c
@@ -1018,10 +1018,8 @@ static void mgag200_init_regs(struct mga_device *mdev)
if (mdev->type == G200_EW3)
WREG_ECRT(0x34, 0x5);
 
-   misc = RREG8(MGA_MISC_IN);
-   misc |= MGAREG_MISC_IOADSEL |
-   MGAREG_MISC_RAMMAPEN |
-   MGAREG_MISC_HIGH_PG_SEL;
+   misc = MGAREG_MISC_HPGODDEV |
+  MGAREG_MISC_IOADSEL;
WREG8(MGA_MISC_OUT, misc);
 }
 
diff --git a/drivers/gpu/drm/mgag200/mgag200_reg.h 
b/drivers/gpu/drm/mgag200/mgag200_reg.h
index 29f7194faadc0..f6629e0d4bdf2 100644
--- a/drivers/gpu/drm/mgag200/mgag200_reg.h
+++ b/drivers/gpu/drm/mgag200/mgag200_reg.h
@@ -228,7 +228,7 @@
 #define MGAREG_MISC_CLK_SEL_MGA_PIX(0x2 << 2)
 #define MGAREG_MISC_CLK_SEL_MGA_MSK(0x3 << 2)
 #define MGAREG_MISC_VIDEO_DIS  (0x1 << 4)
-#define MGAREG_MISC_HIGH_PG_SEL(0x1 << 5)
+#define MGAREG_MISC_HPGODDEV   BIT(5)
 #define MGAREG_MISC_HSYNCPOL   BIT(6)
 #define MGAREG_MISC_VSYNCPOL   BIT(7)
 
-- 
2.27.0

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


Re: [drm/mgag200] e44e907dd8: phoronix-test-suite.glmark2.800x600.score -64.9% regression

2020-06-16 Thread Thomas Zimmermann
Hi

Am 15.06.20 um 22:58 schrieb Emil Velikov:
> Hi all,
> 
> On Thu, 4 Jun 2020 at 08:11, kernel test robot  wrote:
>>
>> Greeting,
>>
>> FYI, we noticed a -64.9% regression of 
>> phoronix-test-suite.glmark2.800x600.score due to commit:
>>
> On one hand, I'm really happy to see performance testing happening
> although this report is missing various crucial pieces of information.
> 
>> commit: e44e907dd8f937313d35615d799d54162c56d173 ("[PATCH v3 05/15] 
>> drm/mgag200: Split MISC register update into PLL selection, SYNC and I/O")
>> url: 
>> https://github.com/0day-ci/linux/commits/Thomas-Zimmermann/drm-mgag200-Convert-to-atomic-modesetting/20200515-163744
>> base: git://anongit.freedesktop.org/drm/drm-tip drm-tip
>>
>> in testcase: phoronix-test-suite
>> on test machine: 16 threads Intel(R) Xeon(R) CPU X5570 @ 2.93GHz with 48G 
>> memory
>> with following parameters:
>>
>> need_x: true
> Replace "need_x" with the Xorg version as seen in `Xorg -version'.
> 
>> test: glmark2-1.1.0
>> cpufreq_governor: performance
>> ucode: 0x1d
>>
>> test-description: The Phoronix Test Suite is the most comprehensive testing 
>> and benchmarking platform available that provides an extensible framework 
>> for which new tests can be easily added.
>> test-url: http://www.phoronix-test-suite.com/
>>
> Please remove the test description and url. They don't add any value.
> 
> Mention which Mesa version is used as well as on what GPU. The output
> of lspci and glxinfo will help here.
> 
> For this particular test - there is no Mesa/upstream driver for this
> GPU, so I imagine one of the swrast drivers was used. Which one -
> swrast (classic, softpipe, llvmpipe, swr) or kms_swrast.
> The output of `LD_DEBUG=libs glxinfo  |& grep _dri.so` will help here.
> 
>> commit:
>>   bef2303526 ("drm/mgag200: Move mode-setting code into separate helper 
>> function")
>>   e44e907dd8 ("drm/mgag200: Split MISC register update into PLL selection, 
>> SYNC and I/O")
>>
> 
> Actually the offending commit has a subtle change of behaviour - it
> adds an extra MGAREG_MISC_RAMMAPEN.
> That is not documented and I've failed to spot it during review.
> 
> Thomas - shall we revert that line in itself or at least add an inline
> comment why it is needed?

Oh, well spotted. I'll send out a patch to not set the bit. Hopefully
this will resolve the problem.

> 
>>
>>   100 +-+
>>90 |-++  +   +.+  ++ ++  +   :   |
>>   | ::  :   : :  :: ::  :   :   |
>>80 |-::  :   : :  :: ::  :   :   |
>>70 |-::   : ::   :  : :   :: ::   : ::  :|
>>   |: :  : :: : :   :: :  : :   : :  : :: : :|
>>60 |:+:  : :: : :   :: :  : :   : :  : :: : :|
>>50 |:+:  : :: : :   :: :  : :   : :  : :: : :|
>>40 |:+ : : :   :  : ::   : : :  :   :  : : :   :  : :|
>>   |:  : : :   :  : ::   : : :  :   :  : : :   :  : :O  O O O  O |
>>30 |:+ : : :   :  : ::   : : :  :   :  : : :   :  : :|
>>20 |-+ ::   :  :   : :  :   ::   : :   ::   :  : O : |
>>   |:   : ::  : :   :: ::   : :: |
>>10 |-+  :   : ::  : :   :: ::   : :: |
>> 0 +-+
>>
>>
>>phoronix-test-suite.glmark2.1024x768.score
>>
>>   70 +--+
>>  | ++  +   +..+ ++  ++ ++.+ |
>>   60 |-::  :   :  : ::  :: ::   |
>>  | ::  :   :  : ::  :: ::   |
>>   50 |-::   : ::   :  : ::   :  :   :: ::   :   |
>>  |: :  : :: : ::   : :  : :: :  : :   : :  :|
>>   40 |:+:  : :: : ::   : :  : :: :  : :   : :  :|
>>  |: :  : :: : ::   : :  : :: :  : :   : :  : O  |
>>   30 |:+ : : :   :  : ::   :  : : :: : :  :   :  : :O  O   O  O |
>>  |:  : : :   :  : ::   :  : : :: : :  :   :  : :|
>>   20 |:+ : : :   :  : ::   :  : : :: : :  :   :  : :|
>>  |   ::   :  :   :  : :   ::   :  :   ::   : : O :: |
>>   10 |-+  :   : ::  : ::   :  :   :: :: |
>>  |:   : :  O :O : : O  :   :  : O :: :: |
>>0 +--+
>>
>>
>> [*] bisect-good sample
>> [O] bisect-bad  sample
>>
> Hmm I must be going blind - there isn't 

[PATCH 6/6] drm/i915/dp_mst: Ensure the DPCD ACT sent flag is cleared before waiting for it

2020-06-16 Thread Imre Deak
Atm, we clear the ACT sent flag in the sink's DPCD before updating the
sink's payload table, along clearing the payload table updated flag.
The sink is supposed to set this flag once it detects that the source
has completed the ACT sequence (after detecting the 4 required ACT MTPH
symbols sent by the source). As opposed to this 2 DELL monitors I have
set the flag already along the payload table updated flag, which is not
quite correct.

To be sure that the sink has detected the ACT MTPH symbols before
continuing enabling the encoder, clear the ACT sent flag before enabling
or disabling the transcoder VC payload allocation (which is what starts
the ACT sequence).

Cc: Lyude Paul 
Cc: Ville Syrjälä 
Cc: dri-devel@lists.freedesktop.org
Signed-off-by: Imre Deak 
---
 drivers/gpu/drm/drm_dp_mst_topology.c   | 31 +++--
 drivers/gpu/drm/i915/display/intel_dp_mst.c |  2 ++
 include/drm/drm_dp_mst_helper.h |  2 ++
 3 files changed, 33 insertions(+), 2 deletions(-)

diff --git a/drivers/gpu/drm/drm_dp_mst_topology.c 
b/drivers/gpu/drm/drm_dp_mst_topology.c
index b2f5a84b4cfb..e3bf8c9c8267 100644
--- a/drivers/gpu/drm/drm_dp_mst_topology.c
+++ b/drivers/gpu/drm/drm_dp_mst_topology.c
@@ -4377,6 +4377,34 @@ void drm_dp_mst_deallocate_vcpi(struct 
drm_dp_mst_topology_mgr *mgr,
 }
 EXPORT_SYMBOL(drm_dp_mst_deallocate_vcpi);
 
+/**
+ * drm_dp_clear_payload_status() - Clears the payload table status flags
+ * @mgr: manager to use
+ *
+ * Clears the payload table ACT handled and table updated flags in the MST 
hub's
+ * DPCD. This function must be called before updating the payload table or
+ * starting the ACT sequence and waiting for the corresponding flags to get
+ * set by the hub.
+ *
+ * Returns:
+ * 0 if the flag got cleared successfully, otherwise a negative error code.
+ */
+int drm_dp_clear_payload_status(struct drm_dp_mst_topology_mgr *mgr)
+{
+   int ret;
+
+   ret = drm_dp_dpcd_writeb(mgr->aux, DP_PAYLOAD_TABLE_UPDATE_STATUS,
+DP_PAYLOAD_ACT_HANDLED);
+   if (ret < 0) {
+   DRM_DEBUG_DRIVER("Can't clear the ACT sent flag (%d)\n", ret);
+   return ret;
+   }
+   WARN_ON(ret != 1);
+
+   return 0;
+}
+EXPORT_SYMBOL(drm_dp_clear_payload_status);
+
 static int drm_dp_dpcd_write_payload(struct drm_dp_mst_topology_mgr *mgr,
 int id, struct drm_dp_payload *payload)
 {
@@ -4384,8 +4412,7 @@ static int drm_dp_dpcd_write_payload(struct 
drm_dp_mst_topology_mgr *mgr,
int ret;
int retries = 0;
 
-   drm_dp_dpcd_writeb(mgr->aux, DP_PAYLOAD_TABLE_UPDATE_STATUS,
-  DP_PAYLOAD_TABLE_UPDATED);
+   drm_dp_clear_payload_status(mgr);
 
payload_alloc[0] = id;
payload_alloc[1] = payload->start_slot;
diff --git a/drivers/gpu/drm/i915/display/intel_dp_mst.c 
b/drivers/gpu/drm/i915/display/intel_dp_mst.c
index 9308b5920780..3c4b0fb10d8b 100644
--- a/drivers/gpu/drm/i915/display/intel_dp_mst.c
+++ b/drivers/gpu/drm/i915/display/intel_dp_mst.c
@@ -323,6 +323,8 @@ static void clear_act_sent(struct intel_dp *intel_dp)
 
intel_de_write(i915, intel_dp->regs.dp_tp_status,
   DP_TP_STATUS_ACT_SENT);
+
+   drm_dp_clear_payload_status(_dp->mst_mgr);
 }
 
 static void wait_for_act_sent(struct intel_dp *intel_dp)
diff --git a/include/drm/drm_dp_mst_helper.h b/include/drm/drm_dp_mst_helper.h
index 8b9eb4db3381..2facb87624bf 100644
--- a/include/drm/drm_dp_mst_helper.h
+++ b/include/drm/drm_dp_mst_helper.h
@@ -763,6 +763,8 @@ int drm_dp_find_vcpi_slots(struct drm_dp_mst_topology_mgr 
*mgr,
   int pbn);
 
 
+int drm_dp_clear_payload_status(struct drm_dp_mst_topology_mgr *mgr);
+
 int drm_dp_update_payload_part1(struct drm_dp_mst_topology_mgr *mgr);
 
 
-- 
2.23.1

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


Re: [PATCH 0/6] remove deprecated i2c_new_device API

2020-06-16 Thread Emil Velikov
Hi all,

On Tue, 16 Jun 2020 at 13:12, Daniel Vetter  wrote:
>
> On Mon, Jun 15, 2020 at 09:58:09AM +0200, Wolfram Sang wrote:
> > I want to remove the above API this cycle, and just a few patches have
> > not made it into 5.8-rc1. They have been reviewed and most had been
> > promised to get into linux-next, but well, things happen. So, I hope it
> > is okay for everyone to collect them like this and push them via I2C for
> > 5.8-rc2.
>
> for the drm side of things:
>
> Acked-by: Daniel Vetter 
> >
> > One minor exception is the media documentation patch which I simply have
> > missed so far, but it is trivial.
> >
> > And then, finally, there is the removal of the old API as the final
> > patch. Phew, that's been a long ride.
> >
> > I am open for comments, of course.
> >
> > Happy hacking,
> >
> >Wolfram
> >
> >
> > Wolfram Sang (6):
> >   drm: encoder_slave: fix refcouting error for modules
> >   drm: encoder_slave: use new I2C API

The first two are in drm-misc-next and are to be expected with the 5.9
merge window. As long as that doesn't cause major nuisance proceed as
you prefer.

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


RE: [PATCH] dmabuf: use spinlock to access dmabuf->name

2020-06-16 Thread Ruhl, Michael J
>-Original Message-
>From: dri-devel  On Behalf Of
>Ruhl, Michael J
>Sent: Tuesday, June 16, 2020 9:51 AM
>To: Charan Teja Kalla ; Sumit Semwal
>; open list:DMA BUFFER SHARING FRAMEWORK
>; DRI mailing list de...@lists.freedesktop.org>
>Cc: Linaro MM SIG ;
>vinme...@codeaurora.org; LKML ;
>sta...@vger.kernel.org
>Subject: RE: [PATCH] dmabuf: use spinlock to access dmabuf->name
>
>>-Original Message-
>>From: dri-devel  On Behalf Of
>>Charan Teja Kalla
>>Sent: Thursday, June 11, 2020 9:40 AM
>>To: Sumit Semwal ; open list:DMA BUFFER
>>SHARING FRAMEWORK ; DRI mailing list >de...@lists.freedesktop.org>
>>Cc: Linaro MM SIG ;
>>vinme...@codeaurora.org; LKML ;
>>sta...@vger.kernel.org
>>Subject: [PATCH] dmabuf: use spinlock to access dmabuf->name
>>
>>There exists a sleep-while-atomic bug while accessing the dmabuf->name
>>under mutex in the dmabuffs_dname(). This is caused from the SELinux
>>permissions checks on a process where it tries to validate the inherited
>>files from fork() by traversing them through iterate_fd() (which
>>traverse files under spin_lock) and call
>>match_file(security/selinux/hooks.c) where the permission checks happen.
>>This audit information is logged using dump_common_audit_data() where it
>>calls d_path() to get the file path name. If the file check happen on
>>the dmabuf's fd, then it ends up in ->dmabuffs_dname() and use mutex to
>>access dmabuf->name. The flow will be like below:
>>flush_unauthorized_files()
>>  iterate_fd()
>>spin_lock() --> Start of the atomic section.
>>  match_file()
>>file_has_perm()
>>  avc_has_perm()
>>avc_audit()
>>  slow_avc_audit()
>>  common_lsm_audit()
>>dump_common_audit_data()
>>  audit_log_d_path()
>>d_path()
>>dmabuffs_dname()
>>  mutex_lock()--> Sleep while atomic.
>>
>>Call trace captured (on 4.19 kernels) is below:
>>___might_sleep+0x204/0x208
>>__might_sleep+0x50/0x88
>>__mutex_lock_common+0x5c/0x1068
>>__mutex_lock_common+0x5c/0x1068
>>mutex_lock_nested+0x40/0x50
>>dmabuffs_dname+0xa0/0x170
>>d_path+0x84/0x290
>>audit_log_d_path+0x74/0x130
>>common_lsm_audit+0x334/0x6e8
>>slow_avc_audit+0xb8/0xf8
>>avc_has_perm+0x154/0x218
>>file_has_perm+0x70/0x180
>>match_file+0x60/0x78
>>iterate_fd+0x128/0x168
>>selinux_bprm_committing_creds+0x178/0x248
>>security_bprm_committing_creds+0x30/0x48
>>install_exec_creds+0x1c/0x68
>>load_elf_binary+0x3a4/0x14e0
>>search_binary_handler+0xb0/0x1e0
>>
>>So, use spinlock to access dmabuf->name to avoid sleep-while-atomic.
>>
>>Cc:  [5.3+]
>>Signed-off-by: Charan Teja Reddy 
>>---
>> drivers/dma-buf/dma-buf.c | 13 +++--
>> include/linux/dma-buf.h   |  1 +
>> 2 files changed, 8 insertions(+), 6 deletions(-)
>>
>>diff --git a/drivers/dma-buf/dma-buf.c b/drivers/dma-buf/dma-buf.c
>>index 01ce125..2e0456c 100644
>>--- a/drivers/dma-buf/dma-buf.c
>>+++ b/drivers/dma-buf/dma-buf.c
>>@@ -45,10 +45,10 @@ static char *dmabuffs_dname(struct dentry *dentry,
>>char *buffer, int buflen)
>>  size_t ret = 0;
>>
>>  dmabuf = dentry->d_fsdata;
>>- dma_resv_lock(dmabuf->resv, NULL);
>>+ spin_lock(>name_lock);
>>  if (dmabuf->name)
>>  ret = strlcpy(name, dmabuf->name, DMA_BUF_NAME_LEN);
>>- dma_resv_unlock(dmabuf->resv);
>>+ spin_unlock(>name_lock);
>
>I am not really clear on why you need this lock.
>
>If name == NULL you have no issues.
>If name is real, you have no issues.
>
>If name is freed you will copy garbage, but the only way
>for that to happen is that _set_name or _release have to be called
>at just the right time.
>
>And the above would probably only be an issue if the set_name
>was called, so you will get NULL or a real name.
>
>Is there a reason for the lock here?
>
>Mike

Maybe dmabuf->name = NULL after the kfree(dmabuf->name) in:

dma_buf_release()

Would be sufficient?

M
>>  return dynamic_dname(dentry, buffer, buflen, "/%s:%s",
>>   dentry->d_name.name, ret > 0 ? name : "");
>>@@ -335,7 +335,7 @@ static long dma_buf_set_name(struct dma_buf
>>*dmabuf, const char __user *buf)
>>  if (IS_ERR(name))
>>  return PTR_ERR(name);
>>
>>- dma_resv_lock(dmabuf->resv, NULL);
>>+ spin_lock(>name_lock);
>>  if (!list_empty(>attachments)) {
>>  ret = -EBUSY;
>>  kfree(name);
>>@@ -345,7 +345,7 @@ static long dma_buf_set_name(struct dma_buf
>>*dmabuf, const char __user *buf)
>>  dmabuf->name = name;
>>
>> out_unlock:
>>- dma_resv_unlock(dmabuf->resv);
>>+ spin_unlock(>name_lock);
>>  return ret;
>> }
>>
>>@@ -405,10 +405,10 @@ static void dma_buf_show_fdinfo(struct seq_file
>>*m, struct file *file)
>>  /* 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);
>>- 

Re: [Intel-gfx] [PATCH 7/8] drm/mipi-dbi: Remove ->enabled

2020-06-16 Thread Emil Velikov
On Tue, 16 Jun 2020 at 07:50, Daniel Vetter  wrote:
>
> On Mon, Jun 15, 2020 at 11:35 PM Emil Velikov  
> wrote:
> >
> > Hi Daniel,
> >
> > On Fri, 12 Jun 2020 at 17:01, Daniel Vetter  wrote:
> > >
> > > The atomic helpers try really hard to not lose track of things,
> > > duplicating enabled tracking in the driver is at best confusing.
> > > Double-enabling or disabling is a bug in atomic helpers.
> > >
> > > In the fb_dirty function we can just assume that the fb always exists,
> > > simple display pipe helpers guarantee that the crtc is only enabled
> > > together with the output, so we always have a primary plane around.
> > >
> > > Now in the update function we need to be a notch more careful, since
> > > that can also get called when the crtc is off. And we don't want to
> > > upload frames when that's the case, so filter that out too.
> > >
> > > Signed-off-by: Daniel Vetter 
> > > Cc: Maarten Lankhorst 
> > > Cc: Maxime Ripard 
> > > Cc: Thomas Zimmermann 
> > > Cc: David Airlie 
> > > Cc: Daniel Vetter 
> > > Cc: David Lechner 
> > > ---
> > >  drivers/gpu/drm/drm_mipi_dbi.c | 16 ++--
> > >  drivers/gpu/drm/tiny/ili9225.c | 12 +++-
> > >  drivers/gpu/drm/tiny/st7586.c  | 11 +++
> > >  include/drm/drm_mipi_dbi.h |  5 -
> > >  4 files changed, 12 insertions(+), 32 deletions(-)
> > >
> > > diff --git a/drivers/gpu/drm/drm_mipi_dbi.c 
> > > b/drivers/gpu/drm/drm_mipi_dbi.c
> > > index fd8d672972a9..79532b9a324a 100644
> > > --- a/drivers/gpu/drm/drm_mipi_dbi.c
> > > +++ b/drivers/gpu/drm/drm_mipi_dbi.c
> > > @@ -268,7 +268,7 @@ static void mipi_dbi_fb_dirty(struct drm_framebuffer 
> > > *fb, struct drm_rect *rect)
> > > bool full;
> > > void *tr;
> > >
> > > -   if (!dbidev->enabled)
> > > +   if (WARN_ON(!fb))
> > > return;
> > >
> > AFAICT no other driver has such WARN_ON. Let's drop that - it is
> > pretty confusing and misleading as-is.
>
> Yeah, this is a helper library which might be used wrongly by drivers.
> That's why I put it in - if you don't put all the various calls
> together correctly, this should at least catch one case. So really
> would like to keep this, can I convince you?

There are plenty of similar places where a drm library/helper can be
misused, lacking a WARN. Nevertheless - sure feel free to keep it.

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


Re: [drm/mgag200] e44e907dd8: phoronix-test-suite.glmark2.800x600.score -64.9% regression

2020-06-16 Thread Emil Velikov
Hi Rong,

Thanks for the prompt reply and information. Can I suggest including
the suggested information in future reports.
I've included a command for each one, to aid automating things.

Namely:
Xorg: 1.20.4 (or None)
$ which Xorg 2>/dev/null  || echo None && Xorg -version |& grep -o "X
Server.*" | sed "s/X Server//"

Mesa: 20.0.7
$ grep -E -o "Mesa [1-9]+.*" | head -n1 | sed "s/Mesa//"

Mesa module: swrast_dri.so
$ basename `LD_DEBUG=libs glxinfo |& grep _dri.so | head -n1 | cut -f3 -d:`

Mesa device: llvmpipe (LLVM 10.0.0, 128 bits) (0x)
$ glxinfo | grep -i device | cut -f2 -d:

GPU: Matrox Electronics ...
$ lspci -nn | grep -E "VGA|Display|3D" | cut -f2- -d:

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


RE: [PATCH] dmabuf: use spinlock to access dmabuf->name

2020-06-16 Thread Ruhl, Michael J
>-Original Message-
>From: dri-devel  On Behalf Of
>Charan Teja Kalla
>Sent: Thursday, June 11, 2020 9:40 AM
>To: Sumit Semwal ; open list:DMA BUFFER
>SHARING FRAMEWORK ; DRI mailing list de...@lists.freedesktop.org>
>Cc: Linaro MM SIG ;
>vinme...@codeaurora.org; LKML ;
>sta...@vger.kernel.org
>Subject: [PATCH] dmabuf: use spinlock to access dmabuf->name
>
>There exists a sleep-while-atomic bug while accessing the dmabuf->name
>under mutex in the dmabuffs_dname(). This is caused from the SELinux
>permissions checks on a process where it tries to validate the inherited
>files from fork() by traversing them through iterate_fd() (which
>traverse files under spin_lock) and call
>match_file(security/selinux/hooks.c) where the permission checks happen.
>This audit information is logged using dump_common_audit_data() where it
>calls d_path() to get the file path name. If the file check happen on
>the dmabuf's fd, then it ends up in ->dmabuffs_dname() and use mutex to
>access dmabuf->name. The flow will be like below:
>flush_unauthorized_files()
>  iterate_fd()
>spin_lock() --> Start of the atomic section.
>  match_file()
>file_has_perm()
>  avc_has_perm()
>avc_audit()
>  slow_avc_audit()
>   common_lsm_audit()
> dump_common_audit_data()
>   audit_log_d_path()
> d_path()
>dmabuffs_dname()
>  mutex_lock()--> Sleep while atomic.
>
>Call trace captured (on 4.19 kernels) is below:
>___might_sleep+0x204/0x208
>__might_sleep+0x50/0x88
>__mutex_lock_common+0x5c/0x1068
>__mutex_lock_common+0x5c/0x1068
>mutex_lock_nested+0x40/0x50
>dmabuffs_dname+0xa0/0x170
>d_path+0x84/0x290
>audit_log_d_path+0x74/0x130
>common_lsm_audit+0x334/0x6e8
>slow_avc_audit+0xb8/0xf8
>avc_has_perm+0x154/0x218
>file_has_perm+0x70/0x180
>match_file+0x60/0x78
>iterate_fd+0x128/0x168
>selinux_bprm_committing_creds+0x178/0x248
>security_bprm_committing_creds+0x30/0x48
>install_exec_creds+0x1c/0x68
>load_elf_binary+0x3a4/0x14e0
>search_binary_handler+0xb0/0x1e0
>
>So, use spinlock to access dmabuf->name to avoid sleep-while-atomic.
>
>Cc:  [5.3+]
>Signed-off-by: Charan Teja Reddy 
>---
> drivers/dma-buf/dma-buf.c | 13 +++--
> include/linux/dma-buf.h   |  1 +
> 2 files changed, 8 insertions(+), 6 deletions(-)
>
>diff --git a/drivers/dma-buf/dma-buf.c b/drivers/dma-buf/dma-buf.c
>index 01ce125..2e0456c 100644
>--- a/drivers/dma-buf/dma-buf.c
>+++ b/drivers/dma-buf/dma-buf.c
>@@ -45,10 +45,10 @@ static char *dmabuffs_dname(struct dentry *dentry,
>char *buffer, int buflen)
>   size_t ret = 0;
>
>   dmabuf = dentry->d_fsdata;
>-  dma_resv_lock(dmabuf->resv, NULL);
>+  spin_lock(>name_lock);
>   if (dmabuf->name)
>   ret = strlcpy(name, dmabuf->name, DMA_BUF_NAME_LEN);
>-  dma_resv_unlock(dmabuf->resv);
>+  spin_unlock(>name_lock);

I am not really clear on why you need this lock.

If name == NULL you have no issues.
If name is real, you have no issues.

If name is freed you will copy garbage, but the only way
for that to happen is that _set_name or _release have to be called
at just the right time.

And the above would probably only be an issue if the set_name
was called, so you will get NULL or a real name.

Is there a reason for the lock here?

Mike

>   return dynamic_dname(dentry, buffer, buflen, "/%s:%s",
>dentry->d_name.name, ret > 0 ? name : "");
>@@ -335,7 +335,7 @@ static long dma_buf_set_name(struct dma_buf
>*dmabuf, const char __user *buf)
>   if (IS_ERR(name))
>   return PTR_ERR(name);
>
>-  dma_resv_lock(dmabuf->resv, NULL);
>+  spin_lock(>name_lock);
>   if (!list_empty(>attachments)) {
>   ret = -EBUSY;
>   kfree(name);
>@@ -345,7 +345,7 @@ static long dma_buf_set_name(struct dma_buf
>*dmabuf, const char __user *buf)
>   dmabuf->name = name;
>
> out_unlock:
>-  dma_resv_unlock(dmabuf->resv);
>+  spin_unlock(>name_lock);
>   return ret;
> }
>
>@@ -405,10 +405,10 @@ static void dma_buf_show_fdinfo(struct seq_file
>*m, struct file *file)
>   /* 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);
>-  dma_resv_lock(dmabuf->resv, NULL);
>+  spin_lock(>name_lock);
>   if (dmabuf->name)
>   seq_printf(m, "name:\t%s\n", dmabuf->name);
>-  dma_resv_unlock(dmabuf->resv);
>+  spin_unlock(>name_lock);
> }
>
> static const struct file_operations dma_buf_fops = {
>@@ -546,6 +546,7 @@ struct dma_buf *dma_buf_export(const struct
>dma_buf_export_info *exp_info)
>   dmabuf->size = exp_info->size;
>   dmabuf->exp_name = exp_info->exp_name;
>   dmabuf->owner = exp_info->owner;
>+  spin_lock_init(>name_lock);
>   init_waitqueue_head(>poll);
>   

Re: [drm/mgag200] e44e907dd8: phoronix-test-suite.glmark2.800x600.score -64.9% regression

2020-06-16 Thread Thomas Zimmermann
Hi

Am 16.06.20 um 05:10 schrieb Rong Chen:
> 
> 
> On 6/16/20 4:58 AM, Emil Velikov wrote:
>> Hi all,
>>
>> On Thu, 4 Jun 2020 at 08:11, kernel test robot 
>> wrote:
>>> Greeting,
>>>
>>> FYI, we noticed a -64.9% regression of
>>> phoronix-test-suite.glmark2.800x600.score due to commit:
>>>
>> On one hand, I'm really happy to see performance testing happening
>> although this report is missing various crucial pieces of information.
>>
>>> commit: e44e907dd8f937313d35615d799d54162c56d173 ("[PATCH v3 05/15]
>>> drm/mgag200: Split MISC register update into PLL selection, SYNC and
>>> I/O")
>>> url:
>>> https://github.com/0day-ci/linux/commits/Thomas-Zimmermann/drm-mgag200-Convert-to-atomic-modesetting/20200515-163744
>>>
>>> base: git://anongit.freedesktop.org/drm/drm-tip drm-tip
>>>
>>> in testcase: phoronix-test-suite
>>> on test machine: 16 threads Intel(R) Xeon(R) CPU X5570 @ 2.93GHz with
>>> 48G memory
>>> with following parameters:
>>>
>>>  need_x: true
>> Replace "need_x" with the Xorg version as seen in `Xorg -version'.
> 
> # Xorg -version
> /bin/sh: warning: setlocale: LC_ALL: cannot change locale (en_US.UTF-8)
> 
> X.Org X Server 1.20.4
> X Protocol Version 11, Revision 0
> Build Operating System: Linux 4.9.0-8-amd64 x86_64 Debian
> Current Operating System: Linux lkp-nhm-2ep1
> 5.7.0-rc5-01428-ge44e907dd8f937 #1 SMP Tue Jun 2 19:51:38 CST 2020 x86_64
> Kernel command line:  ip=lkp-nhm-2ep1::dhcp
> root=/dev/disk/by-id/wwn-0x55cd2e4123123127-part2
> rootflags=subvol=debian-x86_64-phoronix
> remote_rootfs=internal-lkp-server:/osimage/debian/debian-x86_64-phoronix
> user=lkp
> job=/lkp/jobs/scheduled/lkp-nhm-2ep1/phoronix-test-suite-performance-true-glmark2-1.1.0-ucode=0x1d-debian-x86_64-phoronix-e44e907dd8f937313d35615d799d54162c56d173-20200616-56456-1kgmjzm-0.yaml
> ARCH=x86_64 kconfig=x86_64-rhel-7.6
> branch=linux-devel/devel-hourly-2020051600
> commit=e44e907dd8f937313d35615d799d54162c56d173
> BOOT_IMAGE=/pkg/linux/x86_64-rhel-7.6/gcc-7/e44e907dd8f937313d35615d799d54162c56d173/vmlinuz-5.7.0-rc5-01428-ge44e907dd8f937
> console=ttyS1,115200 console=tty0 max_uptime=3600
> RESULT_ROOT=/result/phoronix-test-suite/performance-true-glmark2-1.1.0-ucode=0x1d/lkp-nhm-2ep1/debian-x86_64-phoronix/x86_64-rhel-7.6/gcc-7/e44e907dd8f937313d35615d799d54162c56d173/4
> LKP_SERVER=inn nokaslr selinux=0 debug apic=debug sysrq_always_enabled
> rcupdate.rcu_cpu_stall_timeout=100 net.ifnames=0 printk.devkmsg=on
> panic=-1 softlockup_panic=1 nmi_watchdog=panic oops=panic load_ramdisk=2
> prompt_ramdisk=0 drbd.minor_count=8 systemd.log_level=err
> ignore_loglevel console=tty0 earlyprintk=ttyS0,115200
> console=ttyS0,115200 vga=normal rw
> Build Date: 05 March 2019  08:11:12PM
> xorg-server 2:1.20.4-1 (https://www.debian.org/support)
> Current version of pixman: 0.36.0
>     Before reporting problems, check http://wiki.x.org
>     to make sure that you have the latest version.
> 
>>
>>>  test: glmark2-1.1.0
>>>  cpufreq_governor: performance
>>>  ucode: 0x1d
>>>
>>> test-description: The Phoronix Test Suite is the most comprehensive
>>> testing and benchmarking platform available that provides an
>>> extensible framework for which new tests can be easily added.
>>> test-url: http://www.phoronix-test-suite.com/
>>>
>> Please remove the test description and url. They don't add any value.
>>
>> Mention which Mesa version is used as well as on what GPU. The output
>> of lspci and glxinfo will help here.
> 
> Attached please find the outputs of lspci and glxinfo
> 
>>
>> For this particular test - there is no Mesa/upstream driver for this
>> GPU, so I imagine one of the swrast drivers was used. Which one -
>> swrast (classic, softpipe, llvmpipe, swr) or kms_swrast.
>> The output of `LD_DEBUG=libs glxinfo  |& grep _dri.so` will help here.
> 
> # LD_DEBUG=libs glxinfo  |& grep _dri.so
>   2132: calling init: /usr/lib/i386-linux-gnu/dri/swrast_dri.so
>   2132: calling fini: /usr/lib/i386-linux-gnu/dri/swrast_dri.so [0]
> 
> Best Regards,
> Rong Chen

Thanks for testing. If I send out a patch, could you try it?

Best regards
Thomas

> 
>>
>>> commit:
>>>    bef2303526 ("drm/mgag200: Move mode-setting code into separate
>>> helper function")
>>>    e44e907dd8 ("drm/mgag200: Split MISC register update into PLL
>&g

Re: [RFC PATCH 0/4] DirectX on Linux

2020-06-16 Thread Sasha Levin

On Tue, Jun 16, 2020 at 12:51:13PM +0200, Pavel Machek wrote:

Hi!


> The driver creates the /dev/dxg device, which can be opened by user mode
> application and handles their ioctls. The IOCTL interface to the driver
> is defined in dxgkmthk.h (Dxgkrnl Graphics Port Driver ioctl
> definitions). The interface matches the D3DKMT interface on Windows.
> Ioctls are implemented in ioctl.c.

Echoing what others said, you're not making a DRM driver. The driver should 
live outside
of the DRM code.



Actually, this sounds to me like "this should not be merged into linux kernel". 
I mean,
we already have DRM API on Linux. We don't want another one, do we?


This driver doesn't have any display functionality.


And at the very least... this misses API docs for /dev/dxg. Code can't really
be reviewed without that.


The docs live here: 
https://docs.microsoft.com/en-us/windows-hardware/drivers/ddi/d3dkmthk/

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


Re: [RFC PATCH 0/4] DirectX on Linux

2020-06-16 Thread Sasha Levin

On Tue, Jun 16, 2020 at 12:51:56PM +0200, Pavel Machek wrote:

> Having said that, I hit one stumbling block:
> "Further, at this time there are no presentation integration. "
>
> If we upstream this driver as-is into some hyperv specific place, and
> you decide to add presentation integration this is more than likely
> going to mean you will want to interact with dma-bufs and dma-fences.
> If the driver is hidden away in a hyperv place it's likely we won't
> even notice that feature landing until it's too late.
>
> I would like to see a coherent plan for presentation support (not
> code, just an architectural diagram), because I think when you
> contemplate how that works it will change the picture of how this
> driver looks and intergrates into the rest of the Linux graphics
> ecosystem.
>
> As-is I'd rather this didn't land under my purview, since I don't see
> the value this adds to the Linux ecosystem at all, and I think it's
> important when putting a burden on upstream that you provide some
> value.

I also have another concern from a legal standpoint I'd rather not
review the ioctl part of this. I'd probably request under DRI
developers abstain as well.

This is a Windows kernel API being smashed into a Linux driver. I don't want to 
be
tainted by knowledge of an API that I've no idea of the legal status of derived 
works.
(it this all covered patent wise under OIN?)


If you can't look onto it, perhaps it is not suitable to merge into kernel...?

What would be legal requirements so this is "safe to look at"? We should really
require submitter to meet them...


Could you walk me through your view on what the function of the
"Signed-off-by" tag is?

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


Re: [PATCH v2] dma-buf: Move dma_buf_release() from fops to dentry_ops

2020-06-16 Thread Sumit Semwal
Hi Chris,

On Tue, 16 Jun 2020 at 18:20, Chris Wilson  wrote:
>
> Quoting Sumit Semwal (2020-06-16 13:42:13)
> > Hello,
> >
> > If there are no objections to this, I will plan to merge it soon.
>
> I was going to suggest running it against our CI, but that's unavailable
> at the moment.
>
> There's a particularly nasty BUG_ON() in dma_buf_release that we hit
> irregularly, that this might help with.
Thanks for your reply; if the CI is going to be available in a couple
of days, we could wait - it'd be definitely good to see a bug being
splattered out!

> -Chris

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


[PATCH libdrm] intel: sync i915_pciids.h with kernel

2020-06-16 Thread ramadevi . gandi
From: Gandi Ramadevi 

Add DG1 PCI ID

Signed-off-by: Gandi Ramadevi 
---
 intel/i915_pciids.h | 5 +
 1 file changed, 5 insertions(+)

diff --git a/intel/i915_pciids.h b/intel/i915_pciids.h
index 662d8351..724e68a0 100644
--- a/intel/i915_pciids.h
+++ b/intel/i915_pciids.h
@@ -605,4 +605,9 @@
INTEL_VGA_DEVICE(0x9AD9, info), \
INTEL_VGA_DEVICE(0x9AF8, info)
 
+/* DG1 */
+#define INTEL_DG1_IDS(info) \
+INTEL_VGA_DEVICE(0x4905, info), \
+INTEL_VGA_DEVICE(0x4906, info)
+
 #endif /* _I915_PCIIDS_H */
-- 
2.25.1

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


Re: [PATCH v2] dma-buf: Move dma_buf_release() from fops to dentry_ops

2020-06-16 Thread Chris Wilson
Quoting Sumit Semwal (2020-06-16 13:42:13)
> Hello,
> 
> If there are no objections to this, I will plan to merge it soon.

I was going to suggest running it against our CI, but that's unavailable
at the moment.

There's a particularly nasty BUG_ON() in dma_buf_release that we hit
irregularly, that this might help with.
-Chris
___
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel


Re: [PATCH v2] dma-buf: Move dma_buf_release() from fops to dentry_ops

2020-06-16 Thread Sumit Semwal
Hello,

If there are no objections to this, I will plan to merge it soon.

Daniel, Chris, Chenbo?

On Thu, 11 Jun 2020 at 17:14, Sumit Semwal  wrote:
>
> Charan Teja reported a 'use-after-free' in dmabuffs_dname [1], which
> happens if the dma_buf_release() is called while the userspace is
> accessing the dma_buf pseudo fs's dmabuffs_dname() in another process,
> and dma_buf_release() releases the dmabuf object when the last reference
> to the struct file goes away.
>
> I discussed with Arnd Bergmann, and he suggested that rather than tying
> the dma_buf_release() to the file_operations' release(), we can tie it to
> the dentry_operations' d_release(), which will be called when the last ref
> to the dentry is removed.
>
> The path exercised by __fput() calls f_op->release() first, and then calls
> dput, which eventually calls d_op->d_release().
>
> In the 'normal' case, when no userspace access is happening via dma_buf
> pseudo fs, there should be exactly one fd, file, dentry and inode, so
> closing the fd will kill of everything right away.
>
> In the presented case, the dentry's d_release() will be called only when
> the dentry's last ref is released.
>
> Therefore, lets move dma_buf_release() from fops->release() to
> d_ops->d_release()
>
> Many thanks to Arnd for his FS insights :)
>
> [1]: https://lore.kernel.org/patchwork/patch/1238278/
>
> Fixes: bb2bb9030425 ("dma-buf: add DMA_BUF_SET_NAME ioctls")
> Reported-by: syzbot+3643a18836bce555b...@syzkaller.appspotmail.com
> Cc:  [5.3+]
> Cc: Arnd Bergmann 
> Reported-by: Charan Teja Reddy 
> Reviewed-by: Arnd Bergmann 
> Signed-off-by: Sumit Semwal 
>
> ---
> v2: per Arnd: Moved dma_buf_release() above to avoid forward declaration;
>  removed dentry_ops check.
> ---
>  drivers/dma-buf/dma-buf.c | 54 ++-
>  1 file changed, 25 insertions(+), 29 deletions(-)
>
> diff --git a/drivers/dma-buf/dma-buf.c b/drivers/dma-buf/dma-buf.c
> index 01ce125f8e8d..412629601ad3 100644
> --- a/drivers/dma-buf/dma-buf.c
> +++ b/drivers/dma-buf/dma-buf.c
> @@ -54,37 +54,11 @@ static char *dmabuffs_dname(struct dentry *dentry, char 
> *buffer, int buflen)
>  dentry->d_name.name, ret > 0 ? name : "");
>  }
>
> -static const struct dentry_operations dma_buf_dentry_ops = {
> -   .d_dname = dmabuffs_dname,
> -};
> -
> -static struct vfsmount *dma_buf_mnt;
> -
> -static int dma_buf_fs_init_context(struct fs_context *fc)
> -{
> -   struct pseudo_fs_context *ctx;
> -
> -   ctx = init_pseudo(fc, DMA_BUF_MAGIC);
> -   if (!ctx)
> -   return -ENOMEM;
> -   ctx->dops = _buf_dentry_ops;
> -   return 0;
> -}
> -
> -static struct file_system_type dma_buf_fs_type = {
> -   .name = "dmabuf",
> -   .init_fs_context = dma_buf_fs_init_context,
> -   .kill_sb = kill_anon_super,
> -};
> -
> -static int dma_buf_release(struct inode *inode, struct file *file)
> +static void dma_buf_release(struct dentry *dentry)
>  {
> struct dma_buf *dmabuf;
>
> -   if (!is_dma_buf_file(file))
> -   return -EINVAL;
> -
> -   dmabuf = file->private_data;
> +   dmabuf = dentry->d_fsdata;
>
> BUG_ON(dmabuf->vmapping_counter);
>
> @@ -110,9 +84,32 @@ static int dma_buf_release(struct inode *inode, struct 
> file *file)
> module_put(dmabuf->owner);
> kfree(dmabuf->name);
> kfree(dmabuf);
> +}
> +
> +static const struct dentry_operations dma_buf_dentry_ops = {
> +   .d_dname = dmabuffs_dname,
> +   .d_release = dma_buf_release,
> +};
> +
> +static struct vfsmount *dma_buf_mnt;
> +
> +static int dma_buf_fs_init_context(struct fs_context *fc)
> +{
> +   struct pseudo_fs_context *ctx;
> +
> +   ctx = init_pseudo(fc, DMA_BUF_MAGIC);
> +   if (!ctx)
> +   return -ENOMEM;
> +   ctx->dops = _buf_dentry_ops;
> return 0;
>  }
>
> +static struct file_system_type dma_buf_fs_type = {
> +   .name = "dmabuf",
> +   .init_fs_context = dma_buf_fs_init_context,
> +   .kill_sb = kill_anon_super,
> +};
> +
>  static int dma_buf_mmap_internal(struct file *file, struct vm_area_struct 
> *vma)
>  {
> struct dma_buf *dmabuf;
> @@ -412,7 +409,6 @@ static void dma_buf_show_fdinfo(struct seq_file *m, 
> struct file *file)
>  }
>
>  static const struct file_operations dma_buf_fops = {
> -   .release= dma_buf_release,
> .mmap   = dma_buf_mmap_internal,
> .llseek = dma_buf_llseek,
> .poll   = dma_buf_poll,
> --
> 2.27.0
>

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


Re: [PATCH] dmabuf: use spinlock to access dmabuf->name

2020-06-16 Thread Sumit Semwal
Hi Daniel, Chris,

On Thu, 11 Jun 2020 at 19:10, Charan Teja Kalla  wrote:
>
> There exists a sleep-while-atomic bug while accessing the dmabuf->name
> under mutex in the dmabuffs_dname(). This is caused from the SELinux
> permissions checks on a process where it tries to validate the inherited
> files from fork() by traversing them through iterate_fd() (which
> traverse files under spin_lock) and call
> match_file(security/selinux/hooks.c) where the permission checks happen.
> This audit information is logged using dump_common_audit_data() where it
> calls d_path() to get the file path name. If the file check happen on
> the dmabuf's fd, then it ends up in ->dmabuffs_dname() and use mutex to
> access dmabuf->name. The flow will be like below:
> flush_unauthorized_files()
>   iterate_fd()
> spin_lock() --> Start of the atomic section.
>   match_file()
> file_has_perm()
>   avc_has_perm()
> avc_audit()
>   slow_avc_audit()
> common_lsm_audit()
>   dump_common_audit_data()
> audit_log_d_path()
>   d_path()
> dmabuffs_dname()
>   mutex_lock()--> Sleep while atomic.
>
> Call trace captured (on 4.19 kernels) is below:
> ___might_sleep+0x204/0x208
> __might_sleep+0x50/0x88
> __mutex_lock_common+0x5c/0x1068
> __mutex_lock_common+0x5c/0x1068
> mutex_lock_nested+0x40/0x50
> dmabuffs_dname+0xa0/0x170
> d_path+0x84/0x290
> audit_log_d_path+0x74/0x130
> common_lsm_audit+0x334/0x6e8
> slow_avc_audit+0xb8/0xf8
> avc_has_perm+0x154/0x218
> file_has_perm+0x70/0x180
> match_file+0x60/0x78
> iterate_fd+0x128/0x168
> selinux_bprm_committing_creds+0x178/0x248
> security_bprm_committing_creds+0x30/0x48
> install_exec_creds+0x1c/0x68
> load_elf_binary+0x3a4/0x14e0
> search_binary_handler+0xb0/0x1e0
>
> So, use spinlock to access dmabuf->name to avoid sleep-while-atomic.
Any objections to this change? This changes protection only for
dmabuf->name field, but I'd request either of you to review it,
please?
>
> Cc:  [5.3+]
> Signed-off-by: Charan Teja Reddy 
> ---
>  drivers/dma-buf/dma-buf.c | 13 +++--
>  include/linux/dma-buf.h   |  1 +
>  2 files changed, 8 insertions(+), 6 deletions(-)
>
> diff --git a/drivers/dma-buf/dma-buf.c b/drivers/dma-buf/dma-buf.c
> index 01ce125..2e0456c 100644
> --- a/drivers/dma-buf/dma-buf.c
> +++ b/drivers/dma-buf/dma-buf.c
> @@ -45,10 +45,10 @@ static char *dmabuffs_dname(struct dentry *dentry, char 
> *buffer, int buflen)
> size_t ret = 0;
>
> dmabuf = dentry->d_fsdata;
> -   dma_resv_lock(dmabuf->resv, NULL);
> +   spin_lock(>name_lock);
> if (dmabuf->name)
> ret = strlcpy(name, dmabuf->name, DMA_BUF_NAME_LEN);
> -   dma_resv_unlock(dmabuf->resv);
> +   spin_unlock(>name_lock);
>
> return dynamic_dname(dentry, buffer, buflen, "/%s:%s",
>  dentry->d_name.name, ret > 0 ? name : "");
> @@ -335,7 +335,7 @@ static long dma_buf_set_name(struct dma_buf *dmabuf, 
> const char __user *buf)
> if (IS_ERR(name))
> return PTR_ERR(name);
>
> -   dma_resv_lock(dmabuf->resv, NULL);
> +   spin_lock(>name_lock);
> if (!list_empty(>attachments)) {
> ret = -EBUSY;
> kfree(name);
> @@ -345,7 +345,7 @@ static long dma_buf_set_name(struct dma_buf *dmabuf, 
> const char __user *buf)
> dmabuf->name = name;
>
>  out_unlock:
> -   dma_resv_unlock(dmabuf->resv);
> +   spin_unlock(>name_lock);
> return ret;
>  }
>
> @@ -405,10 +405,10 @@ static void dma_buf_show_fdinfo(struct seq_file *m, 
> struct file *file)
> /* 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);
> -   dma_resv_lock(dmabuf->resv, NULL);
> +   spin_lock(>name_lock);
> if (dmabuf->name)
> seq_printf(m, "name:\t%s\n", dmabuf->name);
> -   dma_resv_unlock(dmabuf->resv);
> +   spin_unlock(>name_lock);
>  }
>
>  static const struct file_operations dma_buf_fops = {
> @@ -546,6 +546,7 @@ struct dma_buf *dma_buf_export(const struct 
> dma_buf_export_info *exp_info)
> dmabuf->size = exp_info->size;
> dmabuf->exp_name = exp_info->exp_name;
> dmabuf->owner = exp_info->owner;
> +   spin_lock_init(>name_lock);
> init_waitqueue_head(>poll);
> dmabuf->cb_excl.poll = dmabuf->cb_shared.poll = >poll;
> dmabuf->cb_excl.active = dmabuf->cb_shared.active = 0;
> diff --git a/include/linux/dma-buf.h b/include/linux/dma-buf.h
> index ab0c156..93108fd 100644
> --- a/include/linux/dma-buf.h
> +++ b/include/linux/dma-buf.h
> @@ -311,6 +311,7 @@ struct dma_buf {
> void *vmap_ptr;
> const char *exp_name;
> const char *name;
> +   spinlock_t name_lock;
> 

[PATCH v2] drm/tegra: Add zpos property for cursor planes

2020-06-16 Thread Thierry Reding
From: Thierry Reding 

As of commit 4dc55525b095 ("drm: plane: Verify that no or all planes
have a zpos property") a warning is emitted if there's a mix of planes
with and without a zpos property.

On Tegra, cursor planes are always composited on top of all other
planes, which is why they never had a zpos property attached to them.
However, since the composition order is fixed, this is trivial to
remedy by simply attaching an immutable zpos property to them.

Changes in v2:
- hardcode cursor plane zpos to 255 instead of 0 (Ville)

Reported-by: Jonathan Hunter 
Signed-off-by: Thierry Reding 
---
 drivers/gpu/drm/tegra/dc.c  | 9 +++--
 drivers/gpu/drm/tegra/hub.c | 2 +-
 2 files changed, 8 insertions(+), 3 deletions(-)

diff --git a/drivers/gpu/drm/tegra/dc.c b/drivers/gpu/drm/tegra/dc.c
index 83f31c6e891c..85408eed4685 100644
--- a/drivers/gpu/drm/tegra/dc.c
+++ b/drivers/gpu/drm/tegra/dc.c
@@ -787,7 +787,7 @@ static struct drm_plane *tegra_primary_plane_create(struct 
drm_device *drm,
}
 
drm_plane_helper_add(>base, _plane_helper_funcs);
-   drm_plane_create_zpos_property(>base, plane->index, 0, 255);
+   drm_plane_create_zpos_property(>base, plane->index, 0, 254);
 
err = drm_plane_create_rotation_property(>base,
 DRM_MODE_ROTATE_0,
@@ -957,6 +957,7 @@ static struct drm_plane 
*tegra_dc_cursor_plane_create(struct drm_device *drm,
}
 
drm_plane_helper_add(>base, _cursor_plane_helper_funcs);
+   drm_plane_create_zpos_immutable_property(>base, 255);
 
return >base;
 }
@@ -1074,7 +1075,11 @@ static struct drm_plane 
*tegra_dc_overlay_plane_create(struct drm_device *drm,
}
 
drm_plane_helper_add(>base, _plane_helper_funcs);
-   drm_plane_create_zpos_property(>base, plane->index, 0, 255);
+
+   if (!cursor)
+   drm_plane_create_zpos_property(>base, plane->index, 0, 
254);
+   else
+   drm_plane_create_zpos_immutable_property(>base, 255);
 
err = drm_plane_create_rotation_property(>base,
 DRM_MODE_ROTATE_0,
diff --git a/drivers/gpu/drm/tegra/hub.c b/drivers/gpu/drm/tegra/hub.c
index 22a03f7ffdc1..f8afc05bceb3 100644
--- a/drivers/gpu/drm/tegra/hub.c
+++ b/drivers/gpu/drm/tegra/hub.c
@@ -590,7 +590,7 @@ struct drm_plane *tegra_shared_plane_create(struct 
drm_device *drm,
}
 
drm_plane_helper_add(p, _shared_plane_helper_funcs);
-   drm_plane_create_zpos_property(p, 0, 0, 255);
+   drm_plane_create_zpos_property(p, index, 0, 254);
 
return p;
 }
-- 
2.24.1

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


Re: [PATCH] drm/kselftest: fix spellint typo in test-drm_mm.c

2020-06-16 Thread Daniel Vetter
On Mon, Jun 15, 2020 at 09:11:51PM +0900, Masanari Iida wrote:
> This patch fix a spelling typo in test-drm_mm.c
> 
> Signed-off-by: Masanari Iida 

Applied, thanks for your patch.
-Daniel

> ---
>  drivers/gpu/drm/selftests/test-drm_mm.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/drivers/gpu/drm/selftests/test-drm_mm.c 
> b/drivers/gpu/drm/selftests/test-drm_mm.c
> index 9aabe82dcd3a..31602134c515 100644
> --- a/drivers/gpu/drm/selftests/test-drm_mm.c
> +++ b/drivers/gpu/drm/selftests/test-drm_mm.c
> @@ -2359,7 +2359,7 @@ static int __init test_drm_mm_init(void)
>   while (!random_seed)
>   random_seed = get_random_int();
>  
> - pr_info("Testing DRM range manger (struct drm_mm), with 
> random_seed=0x%x max_iterations=%u max_prime=%u\n",
> + pr_info("Testing DRM range manager (struct drm_mm), with 
> random_seed=0x%x max_iterations=%u max_prime=%u\n",
>   random_seed, max_iterations, max_prime);
>   err = run_selftests(selftests, ARRAY_SIZE(selftests), NULL);
>  
> -- 
> 2.27.0.83.g0313f36c6ebe
> 

-- 
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: [PATCH 0/6] remove deprecated i2c_new_device API

2020-06-16 Thread Daniel Vetter
On Mon, Jun 15, 2020 at 09:58:09AM +0200, Wolfram Sang wrote:
> I want to remove the above API this cycle, and just a few patches have
> not made it into 5.8-rc1. They have been reviewed and most had been
> promised to get into linux-next, but well, things happen. So, I hope it
> is okay for everyone to collect them like this and push them via I2C for
> 5.8-rc2.

for the drm side of things:

Acked-by: Daniel Vetter 
> 
> One minor exception is the media documentation patch which I simply have
> missed so far, but it is trivial.
> 
> And then, finally, there is the removal of the old API as the final
> patch. Phew, that's been a long ride.
> 
> I am open for comments, of course.
> 
> Happy hacking,
> 
>Wolfram
> 
> 
> Wolfram Sang (6):
>   drm: encoder_slave: fix refcouting error for modules
>   drm: encoder_slave: use new I2C API
>   x86/platform/intel-mid: convert to use i2c_new_client_device()
>   video: backlight: tosa_lcd: convert to use i2c_new_client_device()
>   Documentation: media: convert to use i2c_new_client_device()
>   i2c: remove deprecated i2c_new_device API
> 
>  .../driver-api/media/v4l2-subdev.rst  |  2 +-
>  .../userspace-api/media/conf_nitpick.py   |  2 +-
>  arch/x86/platform/intel-mid/sfi.c |  4 +--
>  drivers/gpu/drm/drm_encoder_slave.c   | 15 ---
>  drivers/i2c/i2c-core-base.c   | 25 ---
>  drivers/video/backlight/tosa_lcd.c|  4 +--
>  include/linux/i2c.h   |  8 +++---
>  7 files changed, 14 insertions(+), 46 deletions(-)
> 
> -- 
> 2.27.0
> 
> ___
> 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: [PATCH] drm/shmem-helper: Fix obj->filp derefence

2020-06-16 Thread Thomas Zimmermann
Hi,

as discussed on IRC, we still need this patch.

Am 15.06.20 um 17:10 schrieb Daniel Vetter:
> I broke that in my refactoring:
> 
> commit 7d2cd72a9aa3df3604cafd169a2d4a525afb68ca
> Author: Daniel Vetter 
> Date:   Fri May 29 16:05:42 2020 +0200
> 
> drm/shmem-helpers: Simplify dma-buf importing
> 
> Reported-by: Thomas Zimmermann 
> Fixes: 7d2cd72a9aa3 ("drm/shmem-helpers: Simplify dma-buf importing")
> Cc: Boris Brezillon 
> Cc: Thomas Zimmermann 
> Cc: Gerd Hoffmann 
> Cc: Rob Herring 
> Cc: Noralf Trønnes 
> Signed-off-by: Daniel Vetter 
> ---
>  drivers/gpu/drm/drm_gem_shmem_helper.c | 20 +++-
>  1 file changed, 11 insertions(+), 9 deletions(-)
> 
> diff --git a/drivers/gpu/drm/drm_gem_shmem_helper.c 
> b/drivers/gpu/drm/drm_gem_shmem_helper.c
> index 0a7e3b664bc2..3e7ee407a17c 100644
> --- a/drivers/gpu/drm/drm_gem_shmem_helper.c
> +++ b/drivers/gpu/drm/drm_gem_shmem_helper.c
> @@ -70,15 +70,17 @@ __drm_gem_shmem_create(struct drm_device *dev, size_t 
> size, bool private)
>   mutex_init(>vmap_lock);
>   INIT_LIST_HEAD(>madv_list);
>  
> - /*
> -  * Our buffers are kept pinned, so allocating them
> -  * from the MOVABLE zone is a really bad idea, and
> -  * conflicts with CMA. See comments above new_inode()
> -  * why this is required _and_ expected if you're
> -  * going to pin these pages.
> -  */
> - mapping_set_gfp_mask(obj->filp->f_mapping, GFP_HIGHUSER |
> -  __GFP_RETRY_MAYFAIL | __GFP_NOWARN);
> + if (!private) {

I would test for (obj->filp) here, because it's what the branch depends
on. Your choice. In any case

Tested-by: Thomas Zimmermann 
Reviewed-by: Thomas Zimmermann 


> + /*
> +  * Our buffers are kept pinned, so allocating them
> +  * from the MOVABLE zone is a really bad idea, and
> +  * conflicts with CMA. See comments above new_inode()
> +  * why this is required _and_ expected if you're
> +  * going to pin these pages.
> +  */
> + mapping_set_gfp_mask(obj->filp->f_mapping, GFP_HIGHUSER |
> +  __GFP_RETRY_MAYFAIL | __GFP_NOWARN);
> + }
>  
>   return shmem;
>  
> 

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



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


Re: [PATCH] drm/bridge: fix reference count leaks due to pm_runtime_get_sync()

2020-06-16 Thread Daniel Vetter
On Sun, Jun 14, 2020 at 04:46:55PM +0300, Laurent Pinchart wrote:
> Hi Aditya,
> 
> (CC'ing Rafael)
> 
> Thank you for the patch.
> 
> On Sat, Jun 13, 2020 at 09:40:05PM -0500, Aditya Pakki wrote:
> > On calling pm_runtime_get_sync() the reference count of the device
> > is incremented. In case of failure, decrement the
> > reference count before returning the error.
> > 
> > Signed-off-by: Aditya Pakki 
> 
> I've seen lots of similar patches recently. Instead of mass-patching the
> drivers this way, shouldn't pm_runtime_get_sync() (and similar
> functions) decrease the refcount on their failure path ? That would
> require patching drivers that already handle this issue, but I believe
> that would cause less churn, and more importantly, avoid the issue once
> and for good for new code.

Yeah the current interface looks rather dodgy, generally drivers aren't
expected to clean up if the first function failed.

Rafael and Greg, what's your take on this here? See patch below and
Laurent's comment above.
-Daniel

> 
> > ---
> >  drivers/gpu/drm/bridge/cdns-dsi.c | 8 ++--
> >  1 file changed, 6 insertions(+), 2 deletions(-)
> > 
> > diff --git a/drivers/gpu/drm/bridge/cdns-dsi.c 
> > b/drivers/gpu/drm/bridge/cdns-dsi.c
> > index 69c3892caee5..583cb8547106 100644
> > --- a/drivers/gpu/drm/bridge/cdns-dsi.c
> > +++ b/drivers/gpu/drm/bridge/cdns-dsi.c
> > @@ -788,8 +788,10 @@ static void cdns_dsi_bridge_enable(struct drm_bridge 
> > *bridge)
> > u32 tmp, reg_wakeup, div;
> > int nlanes;
> >  
> > -   if (WARN_ON(pm_runtime_get_sync(dsi->base.dev) < 0))
> > +   if (WARN_ON(pm_runtime_get_sync(dsi->base.dev) < 0)) {
> > +   pm_runtime_put(dsi->base.dev);
> > return;
> > +   }
> >  
> > mode = >encoder->crtc->state->adjusted_mode;
> > nlanes = output->dev->lanes;
> > @@ -1028,8 +1030,10 @@ static ssize_t cdns_dsi_transfer(struct 
> > mipi_dsi_host *host,
> > int ret, i, tx_len, rx_len;
> >  
> > ret = pm_runtime_get_sync(host->dev);
> > -   if (ret < 0)
> > +   if (ret < 0) {
> > +   pm_runtime_put(host->dev);
> > return ret;
> > +   }
> >  
> > cdns_dsi_init_link(dsi);
> >  
> 
> -- 
> Regards,
> 
> Laurent Pinchart

-- 
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: [Linaro-mm-sig] [PATCH 04/18] dma-fence: prime lockdep annotations

2020-06-16 Thread Daniel Vetter
Hi Jason,

Somehow this got stuck somewhere in the mail queues, only popped up just
now ...

On Thu, Jun 11, 2020 at 11:15:15AM -0300, Jason Gunthorpe wrote:
> On Thu, Jun 11, 2020 at 10:34:30AM +0200, Daniel Vetter wrote:
> > > I still have my doubts about allowing fence waiting from within shrinkers.
> > > IMO ideally they should use a trywait approach, in order to allow memory
> > > allocation during command submission for drivers that
> > > publish fences before command submission. (Since early reservation object
> > > release requires that).
> > 
> > Yeah it is a bit annoying, e.g. for drm/scheduler I think we'll end up
> > with a mempool to make sure it can handle it's allocations.
> > 
> > > But since drivers are already waiting from within shrinkers and I take 
> > > your
> > > word for HMM requiring this,
> > 
> > Yeah the big trouble is HMM and mmu notifiers. That's the really awkward
> > one, the shrinker one is a lot less established.
> 
> I really question if HW that needs something like DMA fence should
> even be using mmu notifiers - the best use is HW that can fence the
> DMA directly without having to get involved with some command stream
> processing.
> 
> Or at the very least it should not be a generic DMA fence but a
> narrowed completion tied only into the same GPU driver's command
> completion processing which should be able to progress without
> blocking.

The problem with gpus is that these completions leak across the board like
mad. Both internally within memory managers (made a lot worse with p2p
direct access to vram), and through uapi.

Many gpus still have a very hard time preempting, so doing an overall
switch in drivers/gpu to a memory management model where that is required
is not a very realistic option.  And minimally you need either preempt
(still takes a while, but a lot faster generally than waiting for work to
complete) or hw faults (just a bunch of tlb flushes plus virtual indexed
caches, so just the caveat of that for a gpu, which has lots and big tlbs
and caches). So preventing the completion leaks within the kernel is I
think unrealistic, except if we just say "well sorry, run on windows,
mkay" for many gpu workloads. Or more realistic "well sorry, run on the
nvidia blob with nvidia hw".

The userspace side we can somewhat isolate, at least for pure compute
workloads. But the thing is drivers/gpu is a continum from tiny socs
(where dma_fence is a very nice model) to huge compute stuff (where it's
maybe not the nicest, but hey hw sucks so still neeeded). Doing full on
break in uapi somewhere in there is at least a bit awkward, e.g. some of
the media codec code on intel runs all the way from the smallest intel soc
to the big transcode servers.

So the current status quo is "total mess, every driver defines their own
rules". All I'm trying to do is some common rules here, do make this mess
slightly more manageable and overall reviewable and testable.

I have no illusions that this is fundamentally pretty horrible, and the
leftover wiggle room for writing memory manager is barely more than a
hairline. Just not seeing how other options are better.

> The intent of notifiers was never to endlessly block while vast
> amounts of SW does work.
> 
> Going around and switching everything in a GPU to GFP_ATOMIC seems
> like bad idea.

It's not everyone, or at least not everywhere, it's some fairly limited
cases. Also, even if we drop the mmu_notifier on the floor, then we're
stuck with shrinkers and GFP_NOFS. Still need a mempool of some sorts to
guarantee you get out of a bind, so not much better.

At least that's my current understanding of where we are across all
drivers.

> > I've pinged a bunch of armsoc gpu driver people and ask them how much this
> > hurts, so that we have a clear answer. On x86 I don't think we have much
> > of a choice on this, with userptr in amd and i915 and hmm work in nouveau
> > (but nouveau I think doesn't use dma_fence in there). 
> 
> Right, nor will RDMA ODP. 

Hm, what's the context here? I thought RDMA side you really don't want
dma_fence in mmu_notifiers, so not clear to me what you're agreeing on
here.
-Daniel
-- 
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: [PATCH] drm/shmem-helper: Only dma-buf imports are private obj

2020-06-16 Thread Thomas Zimmermann
Hi

Am 16.06.20 um 13:47 schrieb Daniel Vetter:
> I broke that in my refactoring:
> 
> commit 7d2cd72a9aa3df3604cafd169a2d4a525afb68ca
> Author: Daniel Vetter 
> Date:   Fri May 29 16:05:42 2020 +0200
> 
> drm/shmem-helpers: Simplify dma-buf importing
> 
> I'm not entirely sure of the history here, but I suspect that in one
> of the rebases or when applying the patch I moved the hunk from
> drm_gem_shmem_prime_import_sg_table(), where it should be, to
> drm_gem_shmem_create_with_handle(), which is totally wrong.
> 
> Remedy this.
> 
> Thanks for Thomas for the crucual hint in debugging this.
> 
> Reported-by: Thomas Zimmermann 
> Fixes: 7d2cd72a9aa3 ("drm/shmem-helpers: Simplify dma-buf importing")
> Cc: Boris Brezillon 
> Cc: Thomas Zimmermann 
> Cc: Gerd Hoffmann 
> Cc: Rob Herring 
> Cc: Noralf Trønnes 
> Signed-off-by: Daniel Vetter 

Tested-by: Thomas Zimmermann 
Reviewed-by: Thomas Zimmermann 

> ---
>  drivers/gpu/drm/drm_gem_shmem_helper.c | 4 ++--
>  1 file changed, 2 insertions(+), 2 deletions(-)
> 
> diff --git a/drivers/gpu/drm/drm_gem_shmem_helper.c 
> b/drivers/gpu/drm/drm_gem_shmem_helper.c
> index 0a7e3b664bc2..837e0840990c 100644
> --- a/drivers/gpu/drm/drm_gem_shmem_helper.c
> +++ b/drivers/gpu/drm/drm_gem_shmem_helper.c
> @@ -377,7 +377,7 @@ drm_gem_shmem_create_with_handle(struct drm_file 
> *file_priv,
>   struct drm_gem_shmem_object *shmem;
>   int ret;
>  
> - shmem = __drm_gem_shmem_create(dev, size, true);
> + shmem = drm_gem_shmem_create(dev, size);
>   if (IS_ERR(shmem))
>   return shmem;
>  
> @@ -730,7 +730,7 @@ drm_gem_shmem_prime_import_sg_table(struct drm_device 
> *dev,
>   size_t size = PAGE_ALIGN(attach->dmabuf->size);
>   struct drm_gem_shmem_object *shmem;
>  
> - shmem = drm_gem_shmem_create(dev, size);
> + shmem = __drm_gem_shmem_create(dev, size, true);
>   if (IS_ERR(shmem))
>   return ERR_CAST(shmem);
>  
> 

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



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


Re: [PATCH] drm/i915/display: fix missing null check on allocated dsb object

2020-06-16 Thread Dan Carpenter
On Tue, Jun 16, 2020 at 12:42:21PM +0100, Colin King wrote:
> From: Colin Ian King 
> 
> Currently there is no null check for a failed memory allocation
> on the dsb object and without this a null pointer dereference
> error can occur. Fix this by adding a null check.
> 
> Note: added a drm_err message in keeping with the error message style
> in the function.

Don't give in to peer pressure!  That's like being a lemming when Disney
film makers come to push you off the cliff to create the 1958 nature
film "White Wilderness".

regards,
dan carpenter

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


Re: [PATCH] drm/i915/display: fix missing null check on allocated dsb object

2020-06-16 Thread Colin Ian King
On 16/06/2020 12:54, Dan Carpenter wrote:
> On Tue, Jun 16, 2020 at 12:42:21PM +0100, Colin King wrote:
>> From: Colin Ian King 
>>
>> Currently there is no null check for a failed memory allocation
>> on the dsb object and without this a null pointer dereference
>> error can occur. Fix this by adding a null check.
>>
>> Note: added a drm_err message in keeping with the error message style
>> in the function.
> 
> Don't give in to peer pressure!  That's like being a lemming when Disney
> film makers come to push you off the cliff to create the 1958 nature
> film "White Wilderness".

:-)

> 
> regards,
> dan carpenter
> 

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


Re: [PATCH 57/59] drm/ast: Use managed pci functions

2020-06-16 Thread Daniel Vetter
On Thu, Jun 11, 2020 at 02:04:03PM +0200, Thomas Zimmermann wrote:
> Hi
> 
> Am 15.04.20 um 09:40 schrieb Daniel Vetter:
> > Allows us to remove a bit of cleanup code.
> > 
> > Signed-off-by: Daniel Vetter 
> > Cc: Dave Airlie 
> > Cc: Thomas Zimmermann 
> > Cc: Gerd Hoffmann 
> > Cc: Daniel Vetter 
> > Cc: Emil Velikov 
> > Cc: "Noralf Trønnes" 
> > Cc: Sam Ravnborg 
> > Cc: "Christian König" 
> > Cc: "Y.C. Chen" 
> 
> Reviewed-by: Thomas Zimmermann 
> 
> Thanks for answering my questions. Sorry for never getting back to it.

Nw, patch applied now, thanks for your review.
-Daniel

> 
> Best regards
> Thomas
> 
> > ---
> >  drivers/gpu/drm/ast/ast_drv.c  | 10 +++---
> >  drivers/gpu/drm/ast/ast_main.c |  3 ---
> >  2 files changed, 3 insertions(+), 10 deletions(-)
> > 
> > diff --git a/drivers/gpu/drm/ast/ast_drv.c b/drivers/gpu/drm/ast/ast_drv.c
> > index b7ba22dddcad..48a9cc4e080a 100644
> > --- a/drivers/gpu/drm/ast/ast_drv.c
> > +++ b/drivers/gpu/drm/ast/ast_drv.c
> > @@ -91,15 +91,13 @@ static int ast_pci_probe(struct pci_dev *pdev, const 
> > struct pci_device_id *ent)
> >  
> > ast_kick_out_firmware_fb(pdev);
> >  
> > -   ret = pci_enable_device(pdev);
> > +   ret = pcim_enable_device(pdev);
> > if (ret)
> > return ret;
> >  
> > dev = drm_dev_alloc(, >dev);
> > -   if (IS_ERR(dev)) {
> > -   ret = PTR_ERR(dev);
> > -   goto err_pci_disable_device;
> > -   }
> > +   if (IS_ERR(dev))
> > +   return  PTR_ERR(dev);
> >  
> > dev->pdev = pdev;
> > pci_set_drvdata(pdev, dev);
> > @@ -120,8 +118,6 @@ static int ast_pci_probe(struct pci_dev *pdev, const 
> > struct pci_device_id *ent)
> > ast_driver_unload(dev);
> >  err_drm_dev_put:
> > drm_dev_put(dev);
> > -err_pci_disable_device:
> > -   pci_disable_device(pdev);
> > return ret;
> >  
> >  }
> > diff --git a/drivers/gpu/drm/ast/ast_main.c b/drivers/gpu/drm/ast/ast_main.c
> > index e5398e3dabe7..1b35728ad871 100644
> > --- a/drivers/gpu/drm/ast/ast_main.c
> > +++ b/drivers/gpu/drm/ast/ast_main.c
> > @@ -531,8 +531,5 @@ void ast_driver_unload(struct drm_device *dev)
> > drm_mode_config_cleanup(dev);
> >  
> > ast_mm_fini(ast);
> > -   if (ast->ioregs != ast->regs + AST_IO_MM_OFFSET)
> > -   pci_iounmap(dev->pdev, ast->ioregs);
> > -   pci_iounmap(dev->pdev, ast->regs);
> > kfree(ast);
> >  }
> > 
> 
> -- 
> Thomas Zimmermann
> Graphics Driver Developer
> SUSE Software Solutions Germany GmbH
> Maxfeldstr. 5, 90409 Nürnberg, Germany
> (HRB 36809, AG Nürnberg)
> Geschäftsführer: Felix Imendörffer
> 




-- 
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


[PATCH] drm/shmem-helper: Only dma-buf imports are private obj

2020-06-16 Thread Daniel Vetter
I broke that in my refactoring:

commit 7d2cd72a9aa3df3604cafd169a2d4a525afb68ca
Author: Daniel Vetter 
Date:   Fri May 29 16:05:42 2020 +0200

drm/shmem-helpers: Simplify dma-buf importing

I'm not entirely sure of the history here, but I suspect that in one
of the rebases or when applying the patch I moved the hunk from
drm_gem_shmem_prime_import_sg_table(), where it should be, to
drm_gem_shmem_create_with_handle(), which is totally wrong.

Remedy this.

Thanks for Thomas for the crucual hint in debugging this.

Reported-by: Thomas Zimmermann 
Fixes: 7d2cd72a9aa3 ("drm/shmem-helpers: Simplify dma-buf importing")
Cc: Boris Brezillon 
Cc: Thomas Zimmermann 
Cc: Gerd Hoffmann 
Cc: Rob Herring 
Cc: Noralf Trønnes 
Signed-off-by: Daniel Vetter 
---
 drivers/gpu/drm/drm_gem_shmem_helper.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/drivers/gpu/drm/drm_gem_shmem_helper.c 
b/drivers/gpu/drm/drm_gem_shmem_helper.c
index 0a7e3b664bc2..837e0840990c 100644
--- a/drivers/gpu/drm/drm_gem_shmem_helper.c
+++ b/drivers/gpu/drm/drm_gem_shmem_helper.c
@@ -377,7 +377,7 @@ drm_gem_shmem_create_with_handle(struct drm_file *file_priv,
struct drm_gem_shmem_object *shmem;
int ret;
 
-   shmem = __drm_gem_shmem_create(dev, size, true);
+   shmem = drm_gem_shmem_create(dev, size);
if (IS_ERR(shmem))
return shmem;
 
@@ -730,7 +730,7 @@ drm_gem_shmem_prime_import_sg_table(struct drm_device *dev,
size_t size = PAGE_ALIGN(attach->dmabuf->size);
struct drm_gem_shmem_object *shmem;
 
-   shmem = drm_gem_shmem_create(dev, size);
+   shmem = __drm_gem_shmem_create(dev, size, true);
if (IS_ERR(shmem))
return ERR_CAST(shmem);
 
-- 
2.27.0

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


[PATCH] drm/i915/display: fix missing null check on allocated dsb object

2020-06-16 Thread Colin King
From: Colin Ian King 

Currently there is no null check for a failed memory allocation
on the dsb object and without this a null pointer dereference
error can occur. Fix this by adding a null check.

Note: added a drm_err message in keeping with the error message style
in the function.

Addresses-Coverity: ("Dereference null return")
Fixes: afeda4f3b1c8 ("drm/i915/dsb: Pre allocate and late cleanup of cmd 
buffer")
Signed-off-by: Colin Ian King 
---
 drivers/gpu/drm/i915/display/intel_dsb.c | 4 
 1 file changed, 4 insertions(+)

diff --git a/drivers/gpu/drm/i915/display/intel_dsb.c 
b/drivers/gpu/drm/i915/display/intel_dsb.c
index 24e6d63e2d47..566fa72427b3 100644
--- a/drivers/gpu/drm/i915/display/intel_dsb.c
+++ b/drivers/gpu/drm/i915/display/intel_dsb.c
@@ -271,6 +271,10 @@ void intel_dsb_prepare(struct intel_crtc_state *crtc_state)
return;
 
dsb = kmalloc(sizeof(*dsb), GFP_KERNEL);
+   if (!dsb) {
+   drm_err(>drm, "DSB object creation failed\n");
+   return;
+   }
 
wakeref = intel_runtime_pm_get(>runtime_pm);
 
-- 
2.27.0.rc0

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


Re: [PATCH] drm/shmem-helper: Fix obj->filp derefence

2020-06-16 Thread Thomas Zimmermann
Hi Daniel

Am 15.06.20 um 17:10 schrieb Daniel Vetter:
> I broke that in my refactoring:
> 
> commit 7d2cd72a9aa3df3604cafd169a2d4a525afb68ca
> Author: Daniel Vetter 
> Date:   Fri May 29 16:05:42 2020 +0200
> 
> drm/shmem-helpers: Simplify dma-buf importing
> 
> Reported-by: Thomas Zimmermann 
> Fixes: 7d2cd72a9aa3 ("drm/shmem-helpers: Simplify dma-buf importing")
> Cc: Boris Brezillon 
> Cc: Thomas Zimmermann 
> Cc: Gerd Hoffmann 
> Cc: Rob Herring 
> Cc: Noralf Trønnes 
> Signed-off-by: Daniel Vetter 
> ---
>  drivers/gpu/drm/drm_gem_shmem_helper.c | 20 +++-
>  1 file changed, 11 insertions(+), 9 deletions(-)
> 
> diff --git a/drivers/gpu/drm/drm_gem_shmem_helper.c 
> b/drivers/gpu/drm/drm_gem_shmem_helper.c
> index 0a7e3b664bc2..3e7ee407a17c 100644
> --- a/drivers/gpu/drm/drm_gem_shmem_helper.c
> +++ b/drivers/gpu/drm/drm_gem_shmem_helper.c
> @@ -70,15 +70,17 @@ __drm_gem_shmem_create(struct drm_device *dev, size_t 
> size, bool private)
>   mutex_init(>vmap_lock);
>   INIT_LIST_HEAD(>madv_list);
>  
> - /*
> -  * Our buffers are kept pinned, so allocating them
> -  * from the MOVABLE zone is a really bad idea, and
> -  * conflicts with CMA. See comments above new_inode()
> -  * why this is required _and_ expected if you're
> -  * going to pin these pages.
> -  */
> - mapping_set_gfp_mask(obj->filp->f_mapping, GFP_HIGHUSER |
> -  __GFP_RETRY_MAYFAIL | __GFP_NOWARN);
> + if (!private) {
> + /*
> +  * Our buffers are kept pinned, so allocating them
> +  * from the MOVABLE zone is a really bad idea, and
> +  * conflicts with CMA. See comments above new_inode()
> +  * why this is required _and_ expected if you're
> +  * going to pin these pages.
> +  */
> + mapping_set_gfp_mask(obj->filp->f_mapping, GFP_HIGHUSER |
> +  __GFP_RETRY_MAYFAIL | __GFP_NOWARN);
> + }

This bug is gone, but now I see

[5.577857] [ cut here ]
[5.577881] WARNING: CPU: 0 PID: 1 at drivers/gpu/drm/drm_gem.c:564
drm_gem_get_pages+0x190/0x1b0
[5.577883] Modules linked in:
[5.577891] CPU: 0 PID: 1 Comm: swapper/0 Not tainted 5.7.0-1-pae+ #40
[5.577893] Hardware name: MSI  MS-6380
   /MS-6380 , BIOS 07.00T

[5.577897] EIP: drm_gem_get_pages+0x190/0x1b0
[5.577904] Code: b7 ff 8d 45 b0 e8 30 63 b7 ff e8 6b d8 38 00 eb 9d
8d b4 26 00 00 00 00 66 90 89 fb eb 97 8d 74 26 00 bb f4 ff ff ff eb 8c
90 <0f> 0b bb ea ff ff ff eb 82 8d b4 26 00 00 00 00 0f 0b e9 95 fe ff
[5.577907] EAX: f24c0c00 EBX: f24c0c00 ECX: f3ae1900 EDX: 
[5.577909] ESI:  EDI:  EBP: f3941b50 ESP: f3941afc
[5.577912] DS: 007b ES: 007b FS: 00d8 GS: 00e0 SS: 0068 EFLAGS: 00010246
[5.577915] CR0: 80050033 CR2: b7f5c784 CR3: 1b6b8000 CR4: 06f0
[5.577918] Call Trace:
[5.577938]  ? _cond_resched+0x18/0x50
[5.577950]  drm_gem_shmem_get_pages+0x52/0xa0
[5.577955]  drm_gem_shmem_vmap+0xa1/0x160
[5.577963]  skms_simple_display_pipe_update+0x68/0xb0
[5.577973]  drm_simple_kms_plane_atomic_update+0x23/0x30
[5.577976]  drm_atomic_helper_commit_planes+0xba/0x220
[5.577981]  drm_atomic_helper_commit_tail+0x33/0x70
[5.577984]  commit_tail+0xe7/0x120
[5.577988]  drm_atomic_helper_commit+0x107/0x130
[5.577991]  ? drm_atomic_helper_setup_commit+0x5a0/0x5a0
[5.577995]  drm_atomic_commit+0x3a/0x50
[5.577999]  drm_client_modeset_commit_atomic+0x1ae/0x1e0
[5.578004]  drm_client_modeset_commit_locked+0x48/0x80
[5.578008]  drm_client_modeset_commit+0x20/0x40
[5.578012]  drm_fb_helper_restore_fbdev_mode_unlocked+0x44/0x90
[5.578015]  drm_fb_helper_set_par+0x2e/0x40
[5.578025]  fbcon_init+0x285/0x590
[5.578035]  visual_init+0xb9/0x120
[5.578040]  do_bind_con_driver.isra.0+0x18a/0x280
[5.578045]  do_take_over_console+0x2c/0x40
[5.578049]  do_fbcon_takeover+0x5f/0xd0
[5.578053]  fbcon_fb_registered+0xb7/0xe0
[5.578057]  do_register_framebuffer+0x1ae/0x2e0
[5.578062]  register_framebuffer+0x1c/0x30
[5.578065]  __drm_fb_helper_initial_config_and_unlock+0x96/0xd0
[5.578069]  drm_fbdev_client_hotplug+0x136/0x220
[5.578072]  drm_fbdev_generic_setup+0x9f/0x14a
[5.578076]  ? skms_device_create.constprop.0+0x9f/0xb0
[5.578079]  skms_probe+0x1b/0x20
[5.578083]  platform_drv_probe+0x47/0x90
[5.578092]  really_probe+0x2a9/0x3f0
[5.578096]  driver_probe_device+0xa9/0xf0
[5.578100]  ? _cond_resched+0x18/0x50
[5.578103]  device_driver_attach+0x99/0xa0
[5.578107]  __driver_attach+0x79/0x130
[5.578111]  ? device_driver_attach+0xa0/0xa0
[5.578114]  bus_for_each_dev+0x5b/0xa0
[5.578118]  driver_attach+0x19/0x20
[5.578122]  ? device_driver_attach+0xa0/0xa0
[5.578125]  

Re: [EXTERNAL] Re: [RFC PATCH 0/4] DirectX on Linux

2020-06-16 Thread Pavel Machek
Hi!

> Thanks for the discussion. I may not be able to immediately answer all of 
> your questions, but I'll do my best .
> 

Could you do something with your email settings? Because this is not how you 
should use
email on lkml. "[EXTERNAL]" in the subject, top-posting, unwrapped lines...

Thank you,
Pavel
-- 
(english) http://www.livejournal.com/~pavelmachek
(cesky, pictures) 
http://atrey.karlin.mff.cuni.cz/~pavel/picture/horses/blog.html
___
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel


Re: [RFC PATCH 0/4] DirectX on Linux

2020-06-16 Thread Pavel Machek
> > Having said that, I hit one stumbling block:
> > "Further, at this time there are no presentation integration. "
> >
> > If we upstream this driver as-is into some hyperv specific place, and
> > you decide to add presentation integration this is more than likely
> > going to mean you will want to interact with dma-bufs and dma-fences.
> > If the driver is hidden away in a hyperv place it's likely we won't
> > even notice that feature landing until it's too late.
> >
> > I would like to see a coherent plan for presentation support (not
> > code, just an architectural diagram), because I think when you
> > contemplate how that works it will change the picture of how this
> > driver looks and intergrates into the rest of the Linux graphics
> > ecosystem.
> >
> > As-is I'd rather this didn't land under my purview, since I don't see
> > the value this adds to the Linux ecosystem at all, and I think it's
> > important when putting a burden on upstream that you provide some
> > value.
> 
> I also have another concern from a legal standpoint I'd rather not
> review the ioctl part of this. I'd probably request under DRI
> developers abstain as well.
> 
> This is a Windows kernel API being smashed into a Linux driver. I don't want 
> to be 
> tainted by knowledge of an API that I've no idea of the legal status of 
> derived works. 
> (it this all covered patent wise under OIN?)

If you can't look onto it, perhaps it is not suitable to merge into kernel...?

What would be legal requirements so this is "safe to look at"? We should really
require submitter to meet them...

Pavel
-- 
(english) http://www.livejournal.com/~pavelmachek
(cesky, pictures) 
http://atrey.karlin.mff.cuni.cz/~pavel/picture/horses/blog.html
___
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel


Re: [RFC PATCH 0/4] DirectX on Linux

2020-06-16 Thread Pavel Machek
Hi!

> > The driver creates the /dev/dxg device, which can be opened by user mode
> > application and handles their ioctls. The IOCTL interface to the driver
> > is defined in dxgkmthk.h (Dxgkrnl Graphics Port Driver ioctl
> > definitions). The interface matches the D3DKMT interface on Windows.
> > Ioctls are implemented in ioctl.c.
> 
> Echoing what others said, you're not making a DRM driver. The driver should 
> live outside 
> of the DRM code.
> 

Actually, this sounds to me like "this should not be merged into linux kernel". 
I mean,
we already have DRM API on Linux. We don't want another one, do we?

And at the very least... this misses API docs for /dev/dxg. Code can't really 
be reviewed without that.

Best regards,

Pavel

-- 
(english) http://www.livejournal.com/~pavelmachek
(cesky, pictures) 
http://atrey.karlin.mff.cuni.cz/~pavel/picture/horses/blog.html
___
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel


Re: linux-next: build failure after merge of the drm-intel-fixes tree

2020-06-16 Thread Joonas Lahtinen
Quoting Stephen Rothwell (2020-06-16 02:39:12)
> Hi all,
> 
> After merging the drm-intel-fixes tree, today's linux-next build (x86_64
> allmodconfig) failed like this:
> 
> In file included from drivers/gpu/drm/i915/gt/intel_lrc.c:5972:
> drivers/gpu/drm/i915/gt/selftest_lrc.c: In function 
> 'live_timeslice_nopreempt':
> drivers/gpu/drm/i915/gt/selftest_lrc.c:1333:3: error: too few arguments to 
> function 'engine_heartbeat_disable'
>  1333 |   engine_heartbeat_disable(engine);
>   |   ^~~~
> drivers/gpu/drm/i915/gt/selftest_lrc.c:54:13: note: declared here
>54 | static void engine_heartbeat_disable(struct intel_engine_cs *engine,
>   | ^~~~
> drivers/gpu/drm/i915/gt/selftest_lrc.c:1402:3: error: too few arguments to 
> function 'engine_heartbeat_enable'
>  1402 |   engine_heartbeat_enable(engine);
>   |   ^~~
> drivers/gpu/drm/i915/gt/selftest_lrc.c:64:13: note: declared here
>64 | static void engine_heartbeat_enable(struct intel_engine_cs *engine,
>   | ^~~
> 
> Caused by commit
> 
>   04dc41776145 ("drm/i915/gt: Prevent timeslicing into unpreemptable 
> requests")
> 
> I have reverted that commit for today.

Thanks for reporting. I had my drm-intel-fixes build tree configured
without selftests. I've now corrected that and added a missing dependency
patch.

Regards, Joonas

> 
> -- 
> Cheers,
> Stephen Rothwell
___
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel


[PATCH] drm/amdgpu: Simplify IRQ vector request logic

2020-06-16 Thread Piotr Stankiewicz
pci_alloc_irq_vectors() will handle fallback from MSI-X to MSI
internally, if necessary. So remove checks which determine if we are
dealing with MSI or MSI-X and rely on pci_alloc_irq_vectors() to do the
right thing.

Signed-off-by: Piotr Stankiewicz 
Reviewed-by: Andy Shevchenko 
---
 drivers/gpu/drm/amd/amdgpu/amdgpu_irq.c | 11 +--
 1 file changed, 1 insertion(+), 10 deletions(-)

diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_irq.c 
b/drivers/gpu/drm/amd/amdgpu/amdgpu_irq.c
index 0cc4c67f95f7..2d68ad7d45d4 100644
--- a/drivers/gpu/drm/amd/amdgpu/amdgpu_irq.c
+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_irq.c
@@ -248,17 +248,8 @@ int amdgpu_irq_init(struct amdgpu_device *adev)
adev->irq.msi_enabled = false;
 
if (amdgpu_msi_ok(adev)) {
-   int nvec = pci_msix_vec_count(adev->pdev);
-   unsigned int flags;
-
-   if (nvec <= 0) {
-   flags = PCI_IRQ_MSI;
-   } else {
-   flags = PCI_IRQ_MSI | PCI_IRQ_MSIX;
-   }
/* we only need one vector */
-   nvec = pci_alloc_irq_vectors(adev->pdev, 1, 1, flags);
-   if (nvec > 0) {
+   if (pci_alloc_irq_vectors(adev->pdev, 1, 1, PCI_IRQ_MSI | 
PCI_IRQ_MSIX) > 0) {
adev->irq.msi_enabled = true;
dev_dbg(adev->dev, "using MSI/MSI-X.\n");
}
-- 
2.17.2

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


[PATCH][next] drm/i915: fix a couple of spelling mistakes in kernel parameter help text

2020-06-16 Thread Colin King
From: Colin Ian King 

There are a couple of spelling mistakes in kernel parameter help text,
namely "helpfull" and "paramters".  Fix them.

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

diff --git a/drivers/gpu/drm/i915/i915_params.c 
b/drivers/gpu/drm/i915/i915_params.c
index a7b61e6ec508..8d8db9ff0a48 100644
--- a/drivers/gpu/drm/i915/i915_params.c
+++ b/drivers/gpu/drm/i915/i915_params.c
@@ -99,8 +99,8 @@ i915_param_named_unsafe(enable_psr, int, 0400,
 
 i915_param_named(psr_safest_params, bool, 0400,
"Replace PSR VBT parameters by the safest and not optimal ones. This "
-   "is helpfull to detect if PSR issues are related to bad values set in "
-   " VBT. (0=use VBT paramters, 1=use safest parameters)");
+   "is helpful to detect if PSR issues are related to bad values set in "
+   " VBT. (0=use VBT parameters, 1=use safest parameters)");
 
 i915_param_named_unsafe(force_probe, charp, 0400,
"Force probe the driver for specified devices. "
-- 
2.27.0.rc0

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


Re: [PATCH] drivers/video/backlight: Use kobj_to_dev() instead

2020-06-16 Thread Lee Jones
On Mon, 15 Jun 2020, Wang Qing wrote:

> Use kobj_to_dev() instead of container_of()
> 
> Signed-off-by: Wang Qing 
> ---
>  drivers/video/backlight/lm3533_bl.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
>  mode change 100644 => 100755 drivers/video/backlight/lm3533_bl.c

I've fixed the broken subject line.

For future submissions, please use `git log --oneline -- `
to view the expected formatting for any given .

Applied, thanks.

> diff --git a/drivers/video/backlight/lm3533_bl.c 
> b/drivers/video/backlight/lm3533_bl.c
> index ee09d1b..0c7830f
> --- a/drivers/video/backlight/lm3533_bl.c
> +++ b/drivers/video/backlight/lm3533_bl.c
> @@ -235,7 +235,7 @@ static struct attribute *lm3533_bl_attributes[] = {
>  static umode_t lm3533_bl_attr_is_visible(struct kobject *kobj,
>struct attribute *attr, int n)
>  {
> - struct device *dev = container_of(kobj, struct device, kobj);
> + struct device *dev = kobj_to_dev(kobj);
>   struct lm3533_bl *bl = dev_get_drvdata(dev);
>   umode_t mode = attr->mode;
>  

-- 
Lee Jones [李琼斯]
Senior Technical Lead - Developer Services
Linaro.org │ Open source software for Arm SoCs
Follow Linaro: Facebook | Twitter | Blog
___
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel


[Bug 208205] [amdgpu] System hang / freeze

2020-06-16 Thread bugzilla-daemon
https://bugzilla.kernel.org/show_bug.cgi?id=208205

Automne von Einzbern (mar...@automne.me) changed:

   What|Removed |Added

URL||https://bugs.archlinux.org/
   ||task/66991
 CC||mar...@automne.me
 Regression|No  |Yes

-- 
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 v3 07/15] drm/amdgpu: Use PCI_IRQ_MSI_TYPES where appropriate

2020-06-16 Thread Stankiewicz, Piotr
> -Original Message-
> From: Alex Deucher 
> Sent: Tuesday, June 9, 2020 10:24 PM
> 
> On Tue, Jun 9, 2020 at 5:18 AM Piotr Stankiewicz
>  wrote:
> >
> > Seeing as there is shorthand available to use when asking for any type
> > of interrupt, or any type of message signalled interrupt, leverage it.
> >
> > Signed-off-by: Piotr Stankiewicz 
> > Reviewed-by: Andy Shevchenko 
> 
> Acked-by: Alex Deucher 
> 

Thanks. Adding PCI_IRQ_MSI_TYPES won't land upstream. But I'll send
a patch with the other simplifications, just now.
___
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel


[Bug 208205] New: [amdgpu] System hang / freeze

2020-06-16 Thread bugzilla-daemon
https://bugzilla.kernel.org/show_bug.cgi?id=208205

Bug ID: 208205
   Summary: [amdgpu] System hang / freeze
   Product: Drivers
   Version: 2.5
Kernel Version: 5.7.2
  Hardware: x86-64
OS: Linux
  Tree: Mainline
Status: NEW
  Severity: high
  Priority: P1
 Component: Video(DRI - non Intel)
  Assignee: drivers_video-...@kernel-bugs.osdl.org
  Reporter: mar...@automne.me
Regression: No

System hang at random moments even on headless servers (without any monitor or
X server installed).

Here some testimonies and kernels logs : https://bugs.archlinux.org/task/66991

No issue on 5.6.

-- 
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 00/17] spelling.txt: /decriptors/descriptors/

2020-06-16 Thread Martin K. Petersen
On Tue, 9 Jun 2020 13:45:53 +0100, Kieran Bingham wrote:

> I wouldn't normally go through spelling fixes, but I caught sight of
> this typo twice, and then foolishly grepped the tree for it, and saw how
> pervasive it was.
> 
> so here I am ... fixing a typo globally... but with an addition in
> scripts/spelling.txt so it shouldn't re-appear ;-)
> 
> [...]

Applied to 5.9/scsi-queue, thanks!

[06/17] scsi: Fix trivial spelling
https://git.kernel.org/mkp/scsi/c/0a19a725c0ed

-- 
Martin K. Petersen  Oracle Linux Engineering
___
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel


[PATCH 0/3] drm/bridge: ps8640: Make sure all needed is powered to get the EDID

2020-06-16 Thread Enric Balletbo i Serra
The first patch is not really related and can be picked alone. The
second and the third are to rework the power handling to get the EDID.
Basically, we need to make sure all the needed is powered to be able
to get the EDID. Before, we saw that getting the EDID failed as
explained in the third patch.

Note that this series depends on:

  https://lore.kernel.org/patchwork/project/lkml/list/?series=448525

to apply cleanly.


Enric Balletbo i Serra (3):
  drm/bridge: ps8640: Return an error for incorrect attach flags
  drm/bridge: ps8640: Print an error if VDO control fails
  drm/bridge: ps8640: Rework power state handling

 drivers/gpu/drm/bridge/parade-ps8640.c | 94 ++
 1 file changed, 82 insertions(+), 12 deletions(-)

-- 
2.27.0

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


Re: [PATCH v6 2/9] ARM: dts: stm32: Add pin map for ltdc & spi5 on stm32f429-disco board

2020-06-16 Thread dillon min
On Mon, Jun 15, 2020 at 5:45 PM Alexandre Torgue
 wrote:
>
> Hi Dillon
>
> On 5/27/20 9:27 AM, dillon.min...@gmail.com wrote:
> > From: dillon min 
> >
> > This patch adds the pin configuration for ltdc and spi5 controller
> > on stm32f429-disco board.
> >
> > Signed-off-by: dillon min 
> > ---
> >   arch/arm/boot/dts/stm32f4-pinctrl.dtsi | 67 
> > ++
> >   1 file changed, 67 insertions(+)
> >
> > diff --git a/arch/arm/boot/dts/stm32f4-pinctrl.dtsi 
> > b/arch/arm/boot/dts/stm32f4-pinctrl.dtsi
> > index 392fa143ce07..0eb107f968cd 100644
> > --- a/arch/arm/boot/dts/stm32f4-pinctrl.dtsi
> > +++ b/arch/arm/boot/dts/stm32f4-pinctrl.dtsi
> > @@ -316,6 +316,73 @@
> >   };
> >   };
> >
> > + ltdc_pins_f429_disco: ltdc-1 {
>
> Sorry I missed this issue during review. I changed ltdc_pins_f429_disco
> by ltdc_pins_b when I applied your patch.
Okay, thanks for reviewing.

Regrades,
Dillon,
>
>
> Regards
> alex
>
> > + pins {
> > + pinmux =  > AF14)>,
> > + /* LCD_HSYNC */
> > +   > AF14)>,
> > +  /* LCD_VSYNC */
> > +   > AF14)>,
> > +  /* LCD_CLK */
> > +   > AF14)>,
> > +  /* LCD_R2 */
> > +  ,
> > +  /* LCD_R3 */
> > +   > AF14)>,
> > +  /* LCD_R4 */
> > +   > AF14)>,
> > +  /* LCD_R5 */
> > +  ,
> > +  /* LCD_R6*/
> > +   > AF14)>,
> > +  /* LCD_R7 */
> > +   > AF14)>,
> > +  /* LCD_G2 */
> > +  ,
> > +  /* LCD_G3 */
> > +   > AF14)>,
> > +  /* LCD_G4 */
> > +   > AF14)>,
> > +  /* LCD_B2 */
> > +   > AF14)>,
> > +  /* LCD_B3*/
> > +   > AF14)>,
> > +  /* LCD_G5 */
> > +   > AF14)>,
> > +  /* LCD_G6 */
> > +   > AF14)>,
> > +  /* LCD_G7 */
> > +  ,
> > +  /* LCD_B4 */
> > +   > AF14)>,
> > +  /* LCD_B5 */
> > +   > AF14)>,
> > +  /* LCD_B6 */
> > +   > AF14)>,
> > +  /* LCD_B7 */
> > +   > AF14)>;
> > +  /* LCD_DE */
> > + slew-rate = <2>;
> > + };
> > + };
> > +
> > + spi5_pins: spi5-0 {
> > + pins1 {
> > + pinmux = ,
> > + /* SPI5_CLK */
> > +  ;
> > + /* SPI5_MOSI */
> > + bias-disable;
> > + drive-push-pull;
> > + slew-rate = <0>;
> > + };
> > + pins2 {
> > + pinmux = ;
> > + /* SPI5_MISO */
> > + bias-disable;
> > + };
> > + };
> > +
> >   dcmi_pins: dcmi-0 {
> >   pins {
> >   pinmux =  > AF13)>, /* DCMI_HSYNC */
> >
___
dri-devel mailing list

[RESEND PATCH v4 2/7] drm/bridge_connector: Set default status connected for eDP connectors

2020-06-16 Thread Enric Balletbo i Serra
In an eDP application, HPD is not required and on most bridge chips
useless. If HPD is not used, we need to set initial status as connected,
otherwise the connector created by the drm_bridge_connector API remains
in an unknown state.

Signed-off-by: Enric Balletbo i Serra 
Reviewed-by: Laurent Pinchart 
Acked-by: Sam Ravnborg 
---

Changes in v4: None
Changes in v3: None
Changes in v2: None

 drivers/gpu/drm/drm_bridge_connector.c | 1 +
 1 file changed, 1 insertion(+)

diff --git a/drivers/gpu/drm/drm_bridge_connector.c 
b/drivers/gpu/drm/drm_bridge_connector.c
index c6994fe673f31..a58cbde59c34a 100644
--- a/drivers/gpu/drm/drm_bridge_connector.c
+++ b/drivers/gpu/drm/drm_bridge_connector.c
@@ -187,6 +187,7 @@ drm_bridge_connector_detect(struct drm_connector 
*connector, bool force)
case DRM_MODE_CONNECTOR_DPI:
case DRM_MODE_CONNECTOR_LVDS:
case DRM_MODE_CONNECTOR_DSI:
+   case DRM_MODE_CONNECTOR_eDP:
status = connector_status_connected;
break;
default:
-- 
2.27.0

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


[RESEND PATCH v4 3/7] drm/mediatek: mtk_dsi: Rename bridge to next_bridge

2020-06-16 Thread Enric Balletbo i Serra
This is really a cosmetic change just to make a bit more readable the
code after convert the driver to drm_bridge. The bridge variable name
will be used by the encoder drm_bridge, and the chained bridge will be
named next_bridge.

Signed-off-by: Enric Balletbo i Serra 
Reviewed-by: Laurent Pinchart 
Acked-by: Sam Ravnborg 
Reviewed-by: Chun-Kuang Hu 
---

Changes in v4: None
Changes in v3:
- Replace s/bridge/next bridge/ for comment. (Laurent Pinchart)

Changes in v2: None

 drivers/gpu/drm/mediatek/mtk_dsi.c | 11 ++-
 1 file changed, 6 insertions(+), 5 deletions(-)

diff --git a/drivers/gpu/drm/mediatek/mtk_dsi.c 
b/drivers/gpu/drm/mediatek/mtk_dsi.c
index 270bf22c98feb..208f49bf14a01 100644
--- a/drivers/gpu/drm/mediatek/mtk_dsi.c
+++ b/drivers/gpu/drm/mediatek/mtk_dsi.c
@@ -183,7 +183,7 @@ struct mtk_dsi {
struct drm_encoder encoder;
struct drm_connector conn;
struct drm_panel *panel;
-   struct drm_bridge *bridge;
+   struct drm_bridge *next_bridge;
struct phy *phy;
 
void __iomem *regs;
@@ -894,9 +894,10 @@ static int mtk_dsi_create_conn_enc(struct drm_device *drm, 
struct mtk_dsi *dsi)
 */
dsi->encoder.possible_crtcs = 1;
 
-   /* If there's a bridge, attach to it and let it create the connector */
-   if (dsi->bridge) {
-   ret = drm_bridge_attach(>encoder, dsi->bridge, NULL, 0);
+   /* If there's a next bridge, attach to it and let it create the 
connector */
+   if (dsi->next_bridge) {
+   ret = drm_bridge_attach(>encoder, dsi->next_bridge, NULL,
+   0);
if (ret) {
DRM_ERROR("Failed to attach bridge to drm\n");
goto err_encoder_cleanup;
@@ -1177,7 +1178,7 @@ static int mtk_dsi_probe(struct platform_device *pdev)
}
 
ret = drm_of_find_panel_or_bridge(dev->of_node, 0, 0,
- >panel, >bridge);
+ >panel, >next_bridge);
if (ret)
goto err_unregister_host;
 
-- 
2.27.0

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


[PATCH 1/6] drm: encoder_slave: fix refcouting error for modules

2020-06-16 Thread Wolfram Sang
module_put() balances try_module_get(), not request_module(). Fix the
error path to match that.

Fixes: 2066facca4c7 ("drm/kms: slave encoder interface.")
Signed-off-by: Wolfram Sang 
Reviewed-by: Emil Velikov 
---

I'd like to push it via I2C for 5.8-rc2.

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

diff --git a/drivers/gpu/drm/drm_encoder_slave.c 
b/drivers/gpu/drm/drm_encoder_slave.c
index cf804389f5ec..d50a7884e69e 100644
--- a/drivers/gpu/drm/drm_encoder_slave.c
+++ b/drivers/gpu/drm/drm_encoder_slave.c
@@ -84,7 +84,7 @@ int drm_i2c_encoder_init(struct drm_device *dev,
 
err = encoder_drv->encoder_init(client, dev, encoder);
if (err)
-   goto fail_unregister;
+   goto fail_module_put;
 
if (info->platform_data)
encoder->slave_funcs->set_config(>base,
@@ -92,9 +92,10 @@ int drm_i2c_encoder_init(struct drm_device *dev,
 
return 0;
 
+fail_module_put:
+   module_put(module);
 fail_unregister:
i2c_unregister_device(client);
-   module_put(module);
 fail:
return err;
 }
-- 
2.27.0

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


Re: [PATCH 00/17] spelling.txt: /decriptors/descriptors/

2020-06-16 Thread Jason Gunthorpe
On Tue, Jun 09, 2020 at 01:45:53PM +0100, Kieran Bingham wrote:
> I wouldn't normally go through spelling fixes, but I caught sight of
> this typo twice, and then foolishly grepped the tree for it, and saw how
> pervasive it was.
> 
> so here I am ... fixing a typo globally... but with an addition in
> scripts/spelling.txt so it shouldn't re-appear ;-)
> 
> Cc: linux-arm-ker...@lists.infradead.org (moderated list:TI DAVINCI MACHINE 
> SUPPORT)
> Cc: linux-ker...@vger.kernel.org (open list)
> Cc: linux...@vger.kernel.org (open list:DEVICE FREQUENCY EVENT 
> (DEVFREQ-EVENT))
> Cc: linux-g...@vger.kernel.org (open list:GPIO SUBSYSTEM)
> Cc: dri-devel@lists.freedesktop.org (open list:DRM DRIVERS)
> Cc: linux-r...@vger.kernel.org (open list:HFI1 DRIVER)
> Cc: linux-in...@vger.kernel.org (open list:INPUT (KEYBOARD, MOUSE, JOYSTICK, 
> TOUCHSCREEN)...)
> Cc: linux-...@lists.infradead.org (open list:NAND FLASH SUBSYSTEM)
> Cc: net...@vger.kernel.org (open list:NETWORKING DRIVERS)
> Cc: ath...@lists.infradead.org (open list:QUALCOMM ATHEROS ATH10K WIRELESS 
> DRIVER)
> Cc: linux-wirel...@vger.kernel.org (open list:NETWORKING DRIVERS (WIRELESS))
> Cc: linux-s...@vger.kernel.org (open list:IBM Power Virtual FC Device Drivers)
> Cc: linuxppc-...@lists.ozlabs.org (open list:LINUX FOR POWERPC (32-BIT AND 
> 64-BIT))
> Cc: linux-...@vger.kernel.org (open list:USB SUBSYSTEM)
> Cc: virtualizat...@lists.linux-foundation.org (open list:VIRTIO CORE AND NET 
> DRIVERS)
> Cc: linux...@kvack.org (open list:MEMORY MANAGEMENT)
> 
> 
> Kieran Bingham (17):
>   arch: arm: mach-davinci: Fix trivial spelling
>   drivers: infiniband: Fix trivial spelling
>   drivers: infiniband: Fix trivial spelling

I took these two RDMA patches and merged them, thanks

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


[PATCH] drivers/video/backlight: Use kobj_to_dev() instead

2020-06-16 Thread Wang Qing
Use kobj_to_dev() instead of container_of()

Signed-off-by: Wang Qing 
---
 drivers/video/backlight/lm3533_bl.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)
 mode change 100644 => 100755 drivers/video/backlight/lm3533_bl.c

diff --git a/drivers/video/backlight/lm3533_bl.c 
b/drivers/video/backlight/lm3533_bl.c
index ee09d1b..0c7830f
--- a/drivers/video/backlight/lm3533_bl.c
+++ b/drivers/video/backlight/lm3533_bl.c
@@ -235,7 +235,7 @@ static struct attribute *lm3533_bl_attributes[] = {
 static umode_t lm3533_bl_attr_is_visible(struct kobject *kobj,
 struct attribute *attr, int n)
 {
-   struct device *dev = container_of(kobj, struct device, kobj);
+   struct device *dev = kobj_to_dev(kobj);
struct lm3533_bl *bl = dev_get_drvdata(dev);
umode_t mode = attr->mode;
 
-- 
2.7.4

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


[RESEND PATCH v4 7/7] drm/mediatek: mtk_dsi: Create connector for bridges

2020-06-16 Thread Enric Balletbo i Serra
Use the drm_bridge_connector helper to create a connector for pipelines
that use drm_bridge. This allows splitting connector operations across
multiple bridges when necessary, instead of having the last bridge in
the chain creating the connector and handling all connector operations
internally.

Signed-off-by: Enric Balletbo i Serra 
Acked-by: Sam Ravnborg 
---

Changes in v4: None
Changes in v3:
- Move the bridge.type line to the patch that adds drm_bridge support. (Laurent 
Pinchart)

Changes in v2: None

 drivers/gpu/drm/mediatek/mtk_dsi.c | 13 -
 1 file changed, 12 insertions(+), 1 deletion(-)

diff --git a/drivers/gpu/drm/mediatek/mtk_dsi.c 
b/drivers/gpu/drm/mediatek/mtk_dsi.c
index 4f3bd095c1eee..471fcafdf3488 100644
--- a/drivers/gpu/drm/mediatek/mtk_dsi.c
+++ b/drivers/gpu/drm/mediatek/mtk_dsi.c
@@ -17,6 +17,7 @@
 
 #include 
 #include 
+#include 
 #include 
 #include 
 #include 
@@ -183,6 +184,7 @@ struct mtk_dsi {
struct drm_encoder encoder;
struct drm_bridge bridge;
struct drm_bridge *next_bridge;
+   struct drm_connector *connector;
struct phy *phy;
 
void __iomem *regs;
@@ -977,10 +979,19 @@ static int mtk_dsi_encoder_init(struct drm_device *drm, 
struct mtk_dsi *dsi)
 */
dsi->encoder.possible_crtcs = 1;
 
-   ret = drm_bridge_attach(>encoder, >bridge, NULL, 0);
+   ret = drm_bridge_attach(>encoder, >bridge, NULL,
+   DRM_BRIDGE_ATTACH_NO_CONNECTOR);
if (ret)
goto err_cleanup_encoder;
 
+   dsi->connector = drm_bridge_connector_init(drm, >encoder);
+   if (IS_ERR(dsi->connector)) {
+   DRM_ERROR("Unable to create bridge connector\n");
+   ret = PTR_ERR(dsi->connector);
+   goto err_cleanup_encoder;
+   }
+   drm_connector_attach_encoder(dsi->connector, >encoder);
+
return 0;
 
 err_cleanup_encoder:
-- 
2.27.0

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


[RESEND PATCH v4 0/7] Convert mtk-dsi to drm_bridge API and get EDID for ps8640 bridge

2020-06-16 Thread Enric Balletbo i Serra
(This resend is to fix some trivial conflicts due the merge window)

The PS8640 dsi-to-eDP bridge driver is using the panel bridge API,
however, not all the components in the chain have been ported to the
drm_bridge API. Actually, when a panel is attached the default panel's mode
is used, but in some cases we can't get display up if mode getting from
eDP control EDID is not chosen.

This series address that problem, first implements the .get_edid()
callback in the PS8640 driver (which is not used until the conversion is
done) and then, converts the Mediatek DSI driver to use the drm_bridge
API.

As far as I know, we're the only users of the mediatek dsi driver in
mainline, so should be safe to switch to the new chain of drm_bridge API
unconditionally.

The patches has been tested on a Acer Chromebook R13 (Elm) running a
Chrome OS userspace and checking that the valid EDID mode reported by
the bridge is selected.

Changes in v4:
- Remove double call to drm_encoder_init(). (Chun-Kuang Hu)
- Cleanup the encoder in mtk_dsi_unbind(). (Chun-Kuang Hu)

Changes in v3:
- Replace s/bridge/next bridge/ for comment. (Laurent Pinchart)
- Add the bridge.type. (Laurent Pinchart)
- Use next_bridge field to store the panel bridge. (Laurent Pinchart)
- Add the bridge.type field. (Laurent Pinchart)
- This patch requires https://lkml.org/lkml/2020/4/16/2080 to work
  properly.
- Move the bridge.type line to the patch that adds drm_bridge support. (Laurent 
Pinchart)

Changes in v2:
- Do not set connector_type for panel here. (Sam Ravnborg)

Enric Balletbo i Serra (7):
  drm/bridge: ps8640: Get the EDID from eDP control
  drm/bridge_connector: Set default status connected for eDP connectors
  drm/mediatek: mtk_dsi: Rename bridge to next_bridge
  drm/mediatek: mtk_dsi: Convert to bridge driver
  drm/mediatek: mtk_dsi: Use simple encoder
  drm/mediatek: mtk_dsi: Use the drm_panel_bridge API
  drm/mediatek: mtk_dsi: Create connector for bridges

 drivers/gpu/drm/bridge/parade-ps8640.c |  12 ++
 drivers/gpu/drm/drm_bridge_connector.c |   1 +
 drivers/gpu/drm/mediatek/mtk_dsi.c | 269 -
 3 files changed, 97 insertions(+), 185 deletions(-)

-- 
2.27.0

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


[PATCH 4/6] video: backlight: tosa_lcd: convert to use i2c_new_client_device()

2020-06-16 Thread Wolfram Sang
Move away from the deprecated API and return the shiny new ERRPTR where
useful.

Signed-off-by: Wolfram Sang 
Reviewed-by: Daniel Thompson 
---

I'd like to push it via I2C for 5.8-rc2.

 drivers/video/backlight/tosa_lcd.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/drivers/video/backlight/tosa_lcd.c 
b/drivers/video/backlight/tosa_lcd.c
index e8ab583e5098..113116d3585c 100644
--- a/drivers/video/backlight/tosa_lcd.c
+++ b/drivers/video/backlight/tosa_lcd.c
@@ -107,7 +107,7 @@ static void tosa_lcd_tg_on(struct tosa_lcd_data *data)
/* TG LCD GVSS */
tosa_tg_send(spi, TG_PINICTL, 0x0);
 
-   if (!data->i2c) {
+   if (IS_ERR_OR_NULL(data->i2c)) {
/*
 * after the pannel is powered up the first time,
 * we can access the i2c bus so probe for the DAC
@@ -119,7 +119,7 @@ static void tosa_lcd_tg_on(struct tosa_lcd_data *data)
.addr   = DAC_BASE,
.platform_data = data->spi,
};
-   data->i2c = i2c_new_device(adap, );
+   data->i2c = i2c_new_client_device(adap, );
}
 }
 
-- 
2.27.0

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


[PATCH 0/6] remove deprecated i2c_new_device API

2020-06-16 Thread Wolfram Sang
I want to remove the above API this cycle, and just a few patches have
not made it into 5.8-rc1. They have been reviewed and most had been
promised to get into linux-next, but well, things happen. So, I hope it
is okay for everyone to collect them like this and push them via I2C for
5.8-rc2.

One minor exception is the media documentation patch which I simply have
missed so far, but it is trivial.

And then, finally, there is the removal of the old API as the final
patch. Phew, that's been a long ride.

I am open for comments, of course.

Happy hacking,

   Wolfram


Wolfram Sang (6):
  drm: encoder_slave: fix refcouting error for modules
  drm: encoder_slave: use new I2C API
  x86/platform/intel-mid: convert to use i2c_new_client_device()
  video: backlight: tosa_lcd: convert to use i2c_new_client_device()
  Documentation: media: convert to use i2c_new_client_device()
  i2c: remove deprecated i2c_new_device API

 .../driver-api/media/v4l2-subdev.rst  |  2 +-
 .../userspace-api/media/conf_nitpick.py   |  2 +-
 arch/x86/platform/intel-mid/sfi.c |  4 +--
 drivers/gpu/drm/drm_encoder_slave.c   | 15 ---
 drivers/i2c/i2c-core-base.c   | 25 ---
 drivers/video/backlight/tosa_lcd.c|  4 +--
 include/linux/i2c.h   |  8 +++---
 7 files changed, 14 insertions(+), 46 deletions(-)

-- 
2.27.0

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


[RESEND PATCH v4 1/7] drm/bridge: ps8640: Get the EDID from eDP control

2020-06-16 Thread Enric Balletbo i Serra
The PS8640 DSI-to-eDP bridge can retrieve the EDID, so implement the
.get_edid callback and set the flag to indicate the core to use it.

Signed-off-by: Enric Balletbo i Serra 
Reviewed-by: Laurent Pinchart 
Acked-by: Sam Ravnborg 
---

Changes in v4: None
Changes in v3: None
Changes in v2: None

 drivers/gpu/drm/bridge/parade-ps8640.c | 12 
 1 file changed, 12 insertions(+)

diff --git a/drivers/gpu/drm/bridge/parade-ps8640.c 
b/drivers/gpu/drm/bridge/parade-ps8640.c
index 4b099196afeba..13755d278db6d 100644
--- a/drivers/gpu/drm/bridge/parade-ps8640.c
+++ b/drivers/gpu/drm/bridge/parade-ps8640.c
@@ -242,8 +242,18 @@ static int ps8640_bridge_attach(struct drm_bridge *bridge,
return ret;
 }
 
+static struct edid *ps8640_bridge_get_edid(struct drm_bridge *bridge,
+  struct drm_connector *connector)
+{
+   struct ps8640 *ps_bridge = bridge_to_ps8640(bridge);
+
+   return drm_get_edid(connector,
+   ps_bridge->page[PAGE0_DP_CNTL]->adapter);
+}
+
 static const struct drm_bridge_funcs ps8640_bridge_funcs = {
.attach = ps8640_bridge_attach,
+   .get_edid = ps8640_bridge_get_edid,
.post_disable = ps8640_post_disable,
.pre_enable = ps8640_pre_enable,
 };
@@ -294,6 +304,8 @@ static int ps8640_probe(struct i2c_client *client)
 
ps_bridge->bridge.funcs = _bridge_funcs;
ps_bridge->bridge.of_node = dev->of_node;
+   ps_bridge->bridge.ops = DRM_BRIDGE_OP_EDID;
+   ps_bridge->bridge.type = DRM_MODE_CONNECTOR_eDP;
 
ps_bridge->page[PAGE0_DP_CNTL] = client;
 
-- 
2.27.0

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


Re: [PATCH v2] drm/etnaviv: fix ref count leak via pm_runtime_get_sync

2020-06-16 Thread Markus Elfring
…
> In case of failure, decrement the ref count before returning.

Can it be nicer to use the term “reference count” here?

Will the tag “Fixes” become helpful for the commit message?


…
> +++ b/drivers/gpu/drm/etnaviv/etnaviv_gpu.c
…
> @@ -1326,6 +1331,7 @@  struct dma_fence *etnaviv_gpu_submit(struct 
> etnaviv_gem_submit *submit)
>   ret = event_alloc(gpu, nr_events, event);
>   if (ret) {
>   DRM_ERROR("no free events\n");
> + pm_runtime_put_noidle(gpu->dev);
>   return NULL;
>   }

I suggest to move a bit of exception handling code to the end of
this function implementation so that it can be better reused after
the addition of a jump target like “put_runtime”.

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


[PATCH 1/2] drm/amdgpu/debugfs: fix memory leak when pm_runtime_get_sync failed

2020-06-16 Thread Chen Tao
Fix memory leak in amdgpu_debugfs_gpr_read not freeing data when
pm_runtime_get_sync failed.

Fixes: a9ffe2a983383 ("drm/amdgpu/debugfs: properly handle runtime pm")
Signed-off-by: Chen Tao 
---
 drivers/gpu/drm/amd/amdgpu/amdgpu_debugfs.c | 4 +++-
 1 file changed, 3 insertions(+), 1 deletion(-)

diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_debugfs.c 
b/drivers/gpu/drm/amd/amdgpu/amdgpu_debugfs.c
index 1a4894fa3693..bea8c36a53a4 100644
--- a/drivers/gpu/drm/amd/amdgpu/amdgpu_debugfs.c
+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_debugfs.c
@@ -862,8 +862,10 @@ static ssize_t amdgpu_debugfs_gpr_read(struct file *f, 
char __user *buf,
return -ENOMEM;
 
r = pm_runtime_get_sync(adev->ddev->dev);
-   if (r < 0)
+   if (r < 0) {
+   kfree(data);
return r;
+   }
 
r = amdgpu_virt_enable_access_debugfs(adev);
if (r < 0)
-- 
2.22.0

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


Re: [PATCH 26/38] vt: use newly defined CUR_* macros

2020-06-16 Thread Helge Deller
On 15.06.20 09:48, Jiri Slaby wrote:
> We defined macros for all the magic constants in the previous patch. So
> let us use the macro in the code now.
>
> No functional change intended.
>
> Signed-off-by: Jiri Slaby 
> Cc: Thomas Winischhofer 
> Cc: Bartlomiej Zolnierkiewicz 
> Cc: "James E.J. Bottomley" 
> Cc: Helge Deller 
> Cc: linux-...@vger.kernel.org
> Cc: dri-devel@lists.freedesktop.org
> Cc: linux-fb...@vger.kernel.org
> Cc: linux-par...@vger.kernel.org


Acked-by: Helge Deller 

Thanks!
Helge

> ---
>  drivers/tty/vt/vt.c | 22 +-
>  drivers/usb/misc/sisusbvga/sisusb_con.c |  2 +-
>  drivers/video/console/mdacon.c  |  2 +-
>  drivers/video/console/sticon.c  |  2 +-
>  drivers/video/console/vgacon.c  |  2 +-
>  drivers/video/fbdev/core/bitblit.c  |  2 +-
>  drivers/video/fbdev/core/fbcon.c|  2 +-
>  drivers/video/fbdev/core/fbcon_ccw.c|  2 +-
>  drivers/video/fbdev/core/fbcon_cw.c |  2 +-
>  drivers/video/fbdev/core/fbcon_ud.c |  2 +-
>  drivers/video/fbdev/core/tileblit.c |  2 +-
>  11 files changed, 23 insertions(+), 19 deletions(-)
>
> diff --git a/drivers/tty/vt/vt.c b/drivers/tty/vt/vt.c
> index af1ef717f416..2b9fc628f05b 100644
> --- a/drivers/tty/vt/vt.c
> +++ b/drivers/tty/vt/vt.c
> @@ -866,17 +866,18 @@ static void add_softcursor(struct vc_data *vc)
>   int i = scr_readw((u16 *) vc->vc_pos);
>   u32 type = vc->vc_cursor_type;
>
> - if (!(type & 0x10))
> + if (!(type & CUR_SW))
>   return;
>   if (softcursor_original != -1)
>   return;
>   softcursor_original = i;
> - i |= (type >> 8) & 0xff00;
> - i ^= type & 0xff00;
> - if ((type & 0x20) && (softcursor_original & 0x7000) == (i & 0x7000))
> - i ^= 0x7000;
> - if ((type & 0x40) && (i & 0x700) == ((i & 0x7000) >> 4))
> - i ^= 0x0700;
> + i |= CUR_SET(type);
> + i ^= CUR_CHANGE(type);
> + if ((type & CUR_ALWAYS_BG) &&
> + (softcursor_original & CUR_BG) == (i & CUR_BG))
> + i ^= CUR_BG;
> + if ((type & CUR_INVERT_FG_BG) && (i & CUR_FG) == ((i & CUR_BG) >> 4))
> + i ^= CUR_FG;
>   scr_writew(i, (u16 *)vc->vc_pos);
>   if (con_should_update(vc))
>   vc->vc_sw->con_putc(vc, i, vc->state.y, vc->state.x);
> @@ -910,7 +911,7 @@ static void set_cursor(struct vc_data *vc)
>   if (vc_is_sel(vc))
>   clear_selection();
>   add_softcursor(vc);
> - if ((vc->vc_cursor_type & 0x0f) != 1)
> + if (CUR_SIZE(vc->vc_cursor_type) != CUR_NONE)
>   vc->vc_sw->con_cursor(vc, CM_DRAW);
>   } else
>   hide_cursor(vc);
> @@ -2322,7 +2323,10 @@ static void do_con_trol(struct tty_struct *tty, struct 
> vc_data *vc, int c)
>   case 'c':
>   if (vc->vc_priv == EPdec) {
>   if (vc->vc_par[0])
> - vc->vc_cursor_type = vc->vc_par[0] | 
> (vc->vc_par[1] << 8) | (vc->vc_par[2] << 16);
> + vc->vc_cursor_type =
> + CUR_MAKE(vc->vc_par[0],
> +  vc->vc_par[1],
> +  vc->vc_par[2]);
>   else
>   vc->vc_cursor_type = cur_default;
>   return;
> diff --git a/drivers/usb/misc/sisusbvga/sisusb_con.c 
> b/drivers/usb/misc/sisusbvga/sisusb_con.c
> index 80657c49310a..1058eaba3084 100644
> --- a/drivers/usb/misc/sisusbvga/sisusb_con.c
> +++ b/drivers/usb/misc/sisusbvga/sisusb_con.c
> @@ -727,7 +727,7 @@ sisusbcon_cursor(struct vc_data *c, int mode)
>
>   baseline = c->vc_font.height - (c->vc_font.height < 10 ? 1 : 2);
>
> - switch (c->vc_cursor_type & 0x0f) {
> + switch (CUR_SIZE(c->vc_cursor_type)) {
>   case CUR_BLOCK: from = 1;
>   to   = c->vc_font.height;
>   break;
> diff --git a/drivers/video/console/mdacon.c b/drivers/video/console/mdacon.c
> index 00cb6245fbef..ef29b321967f 100644
> --- a/drivers/video/console/mdacon.c
> +++ b/drivers/video/console/mdacon.c
> @@ -492,7 +492,7 @@ static void mdacon_cursor(struct vc_data *c, int mode)
>
>   mda_set_cursor(c->state.y * mda_num_columns * 2 + c->state.x * 2);
>
> - switch (c->vc_cursor_type & 0x0f) {
> + switch (CUR_SIZE(c->vc_cursor_type)) {
>
>   case CUR_LOWER_THIRD:   mda_set_cursor_size(10, 13); break;
>   case CUR_LOWER_HALF:mda_set_cursor_size(7,  13); break;
> diff --git a/drivers/video/console/sticon.c b/drivers/video/console/sticon.c
> index bbcdfd312c36..21a5c280c8c9 100644
> --- a/drivers/video/console/sticon.c
> +++ b/drivers/video/console/sticon.c
> @@ -139,7 

[PATCH 2/6] drm: encoder_slave: use new I2C API

2020-06-16 Thread Wolfram Sang
i2c_new_client() is deprecated, use the replacement
i2c_new_client_device(). Also, we have a helper to check if a driver is
bound. Use it to simplify the code. Note that this changes the errno for
a failed device creation from ENOMEM to ENODEV. No callers currently
interpret this errno, though, so we use this condensed error check.

Signed-off-by: Wolfram Sang 
Reviewed-by: Emil Velikov 
---

I'd like to push it via I2C for 5.8-rc2.

 drivers/gpu/drm/drm_encoder_slave.c | 10 ++
 1 file changed, 2 insertions(+), 8 deletions(-)

diff --git a/drivers/gpu/drm/drm_encoder_slave.c 
b/drivers/gpu/drm/drm_encoder_slave.c
index d50a7884e69e..e464429d32df 100644
--- a/drivers/gpu/drm/drm_encoder_slave.c
+++ b/drivers/gpu/drm/drm_encoder_slave.c
@@ -61,13 +61,8 @@ int drm_i2c_encoder_init(struct drm_device *dev,
 
request_module("%s%s", I2C_MODULE_PREFIX, info->type);
 
-   client = i2c_new_device(adap, info);
-   if (!client) {
-   err = -ENOMEM;
-   goto fail;
-   }
-
-   if (!client->dev.driver) {
+   client = i2c_new_client_device(adap, info);
+   if (!i2c_client_has_driver(client)) {
err = -ENODEV;
goto fail_unregister;
}
@@ -96,7 +91,6 @@ int drm_i2c_encoder_init(struct drm_device *dev,
module_put(module);
 fail_unregister:
i2c_unregister_device(client);
-fail:
return err;
 }
 EXPORT_SYMBOL(drm_i2c_encoder_init);
-- 
2.27.0

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


Re: [PATCH 13/29] dt: fix broken links due to txt->yaml renames

2020-06-16 Thread Thomas Bogendoerfer
On Mon, Jun 15, 2020 at 08:46:52AM +0200, Mauro Carvalho Chehab wrote:
> There are some new broken doc links due to yaml renames
> at DT. Developers should really run:
> 
>   ./scripts/documentation-file-ref-check
> 
> in order to solve those issues while submitting patches.
> This tool can even fix most of the issues with:
> 
>   ./scripts/documentation-file-ref-check --fix
> 
> Signed-off-by: Mauro Carvalho Chehab 
> ---
>  Documentation/devicetree/bindings/display/bridge/sii902x.txt  | 2 +-
>  .../devicetree/bindings/display/rockchip/rockchip-drm.yaml| 2 +-
>  Documentation/devicetree/bindings/net/mediatek-bluetooth.txt  | 2 +-
>  Documentation/devicetree/bindings/sound/audio-graph-card.txt  | 2 +-
>  Documentation/devicetree/bindings/sound/st,sti-asoc-card.txt  | 2 +-
>  Documentation/mips/ingenic-tcu.rst| 2 +-
>  MAINTAINERS   | 4 ++--
>  7 files changed, 8 insertions(+), 8 deletions(-)

Acked-by: Thomas Bogendoerfer 

-- 
Crap can work. Given enough thrust pigs will fly, but it's not necessarily a
good idea.[ RFC1925, 2.3 ]
___
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel


  1   2   >