[Bug 108493] Unigine Heaven at 4K crashes amdgpu and causes a GPU hang
https://bugs.freedesktop.org/show_bug.cgi?id=108493 --- Comment #12 from Timur Kristóf --- By the way the voltage issue has already been reported against ROCm and is supposed to be already fixed. The details are here: https://github.com/RadeonOpenCompute/ROCm/issues/348 -- You are receiving this mail because: You are the assignee for the bug.___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [Intel-gfx] [PATCH v2 3/3] drm/i915: Add short HPD IRQ storm detection for non-MST systems
Hi Lyude, Thank you for the patch! Perhaps something to improve: [auto build test WARNING on drm-intel/for-linux-next] [also build test WARNING on v4.19 next-20181019] [if your patch is applied to the wrong git tree, please drop us a note to help improve the system] url: https://github.com/0day-ci/linux/commits/Lyude-Paul/drm-i915-HPD-IRQ-storm-detection-fixes/20181027-085424 base: git://anongit.freedesktop.org/drm-intel for-linux-next reproduce: make htmldocs All warnings (new ones prefixed by >>): include/net/mac80211.h:977: warning: Function parameter or member 'status.rates' not described in 'ieee80211_tx_info' include/net/mac80211.h:977: warning: Function parameter or member 'status.ack_signal' not described in 'ieee80211_tx_info' include/net/mac80211.h:977: warning: Function parameter or member 'status.ampdu_ack_len' not described in 'ieee80211_tx_info' include/net/mac80211.h:977: warning: Function parameter or member 'status.ampdu_len' not described in 'ieee80211_tx_info' include/net/mac80211.h:977: warning: Function parameter or member 'status.antenna' not described in 'ieee80211_tx_info' include/net/mac80211.h:977: warning: Function parameter or member 'status.tx_time' not described in 'ieee80211_tx_info' include/net/mac80211.h:977: warning: Function parameter or member 'status.is_valid_ack_signal' not described in 'ieee80211_tx_info' include/net/mac80211.h:977: warning: Function parameter or member 'status.status_driver_data' not described in 'ieee80211_tx_info' include/net/mac80211.h:977: warning: Function parameter or member 'driver_rates' not described in 'ieee80211_tx_info' include/net/mac80211.h:977: warning: Function parameter or member 'pad' not described in 'ieee80211_tx_info' include/net/mac80211.h:977: warning: Function parameter or member 'rate_driver_data' not described in 'ieee80211_tx_info' net/mac80211/sta_info.h:588: warning: Function parameter or member 'rx_stats_avg' not described in 'sta_info' net/mac80211/sta_info.h:588: warning: Function parameter or member 'rx_stats_avg.signal' not described in 'sta_info' net/mac80211/sta_info.h:588: warning: Function parameter or member 'rx_stats_avg.chain_signal' not described in 'sta_info' net/mac80211/sta_info.h:588: warning: Function parameter or member 'status_stats.filtered' not described in 'sta_info' net/mac80211/sta_info.h:588: warning: Function parameter or member 'status_stats.retry_failed' not described in 'sta_info' net/mac80211/sta_info.h:588: warning: Function parameter or member 'status_stats.retry_count' not described in 'sta_info' net/mac80211/sta_info.h:588: warning: Function parameter or member 'status_stats.lost_packets' not described in 'sta_info' net/mac80211/sta_info.h:588: warning: Function parameter or member 'status_stats.last_tdls_pkt_time' not described in 'sta_info' net/mac80211/sta_info.h:588: warning: Function parameter or member 'status_stats.msdu_retries' not described in 'sta_info' net/mac80211/sta_info.h:588: warning: Function parameter or member 'status_stats.msdu_failed' not described in 'sta_info' net/mac80211/sta_info.h:588: warning: Function parameter or member 'status_stats.last_ack' not described in 'sta_info' net/mac80211/sta_info.h:588: warning: Function parameter or member 'status_stats.last_ack_signal' not described in 'sta_info' net/mac80211/sta_info.h:588: warning: Function parameter or member 'status_stats.ack_signal_filled' not described in 'sta_info' net/mac80211/sta_info.h:588: warning: Function parameter or member 'status_stats.avg_ack_signal' not described in 'sta_info' net/mac80211/sta_info.h:588: warning: Function parameter or member 'tx_stats.packets' not described in 'sta_info' net/mac80211/sta_info.h:588: warning: Function parameter or member 'tx_stats.bytes' not described in 'sta_info' net/mac80211/sta_info.h:588: warning: Function parameter or member 'tx_stats.last_rate' not described in 'sta_info' net/mac80211/sta_info.h:588: warning: Function parameter or member 'tx_stats.msdu' not described in 'sta_info' include/linux/dma-buf.h:304: warning: Function parameter or member 'cb_excl.cb' not described in 'dma_buf' include/linux/dma-buf.h:304: warning: Function parameter or member 'cb_excl.poll' not described in 'dma_buf' include/linux/dma-buf.h:304: warning: Function parameter or member 'cb_excl.active' not described in 'dma_buf' include/linux/dma-buf.h:304: warning: Function parameter or member 'cb_shared.cb' not described in 'dma_buf' include/linux/dma-buf.h:304: warning: Function parameter or member 'cb_shared.poll' not described in 'dma_buf' include/linux/dma-buf.h:304: warning: Function parameter or member 'cb_shared.active' not described in 'dma_buf' include/linux/dma-fence-array.h:54: warning: Function parameter or member 'work' not described in 'dma_fence_array' include/linux/gpio/driver.h:142: warning: Function
[Bug 108493] Unigine Heaven at 4K crashes amdgpu and causes a GPU hang
https://bugs.freedesktop.org/show_bug.cgi?id=108493 --- Comment #11 from Timur Kristóf --- Created attachment 142228 --> https://bugs.freedesktop.org/attachment.cgi?id=142228=edit Content of /sys/class/drm/card0/device/pp_table -- You are receiving this mail because: You are the assignee for the bug.___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
[Bug 108493] Unigine Heaven at 4K crashes amdgpu and causes a GPU hang
https://bugs.freedesktop.org/show_bug.cgi?id=108493 --- Comment #10 from Timur Kristóf --- Created attachment 142227 --> https://bugs.freedesktop.org/attachment.cgi?id=142227=edit Content of /sys/kernel/debug/dri/0/amdgpu_vbios -- You are receiving this mail because: You are the assignee for the bug.___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
[Bug 108493] Unigine Heaven at 4K crashes amdgpu and causes a GPU hang
https://bugs.freedesktop.org/show_bug.cgi?id=108493 --- Comment #9 from Timur Kristóf --- I think I discovered a possible reason for this issue. If you look at the DDEBUG dumps, it says in several places: "This slot was corrupted in GPU memory". So I began to suspect something was wrong with the VRAM. After looking around a bit, I found that the amdgpu driver does not honor the voltage settings from the VBIOS, and sets the memory to use lower voltages instead. So basically the driver undervolts the VRAM without me asking to do so. I guess this might be considered a feature for some people. However, when I manually edit pp_od_clk_voltage to increase the OD_MCLK voltages, then the card begins to work in a stable manner and the GPU hang is gone. (Or at the very least I haven't seen a hang yet, whereas previously it used to hang in less than a minute.) In my case, the VBIOS wants to set the MCLK voltages to 1000 mV at all frequencies, while amdgpu sets them to 750 mv, 800 mV, and 900mV. And it turns out that 900 mV is just too low for my card at 1750 MHz. [root@timur-xps ~]# cat /sys/class/drm/card0/device/pp_od_clk_voltage OD_SCLK: 0:300MHz750mV 1:588MHz765mV 2:952MHz900mV 3: 1041MHz975mV 4: 1106MHz 1031mV 5: 1168MHz 1093mV 6: 1209MHz 1143mV 7: 1244MHz 1150mV OD_MCLK: 0:300MHz750mV 1: 1000MHz800mV 2: 1750MHz900mV OD_RANGE: SCLK: 300MHz 2000MHz MCLK: 300MHz 2250MHz VDDC: 750mV1150mV [root@timur-xps ~]# cat /sys/kernel/debug/dri/0/amdgpu_vbios > mybios.rom [root@timur-xps ~]# pbec -i mybios.rom -s -r MEMORY_CLOCK [DEFAULT] ATOM_MCLK_ENTRY Array Entry: 0 Frequency: 300 MHz. Voltage:. 1000 MV Entry: 1 Frequency: 1000 MHz. Voltage:. 1000 MV Entry: 2 Frequency: 1750 MHz. Voltage:. 1000 MV Here is some info about the VBIOS: [root@timur-xps ~]# cat /sys/class/drm/card0/device/subsystem_device 0xe343 [root@timur-xps ~]# cat /sys/class/drm/card0/device/subsystem_vendor 0x1da2 [root@timur-xps ~]# cat /sys/class/drm/card0/device/vbios_version 113-D00034-S07 -- You are receiving this mail because: You are the assignee for the bug.___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
[Bug 107518] polaris powerplay init fails: There must be 1 or more PCIE levels defined in PPTable
https://bugs.freedesktop.org/show_bug.cgi?id=107518 --- Comment #13 from Timothy Pearson --- The patch originally at https://cgit.freedesktop.org/~agd5f/linux/commit/?h=amd-staging-drm-next=8242308cc3c4419832126ab78ca409ce7110ab33 is no longer available: > Bad commit reference: 8242308cc3c4419832126ab78ca409ce7110ab33 Is an equivalent now in mainline? I'd like to try it out on one of our POWER9 boxes. Thanks! -- You are receiving this mail because: You are the assignee for the bug.___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
[radeon-alex:amd-staging-drm-next 709/711] drivers/gpu/drm/amd/amdgpu/../powerplay/smumgr/smu8_smumgr.c:67:11: error: expected identifier or '(' before '=' token
tree: git://people.freedesktop.org/~agd5f/linux.git amd-staging-drm-next
head: af1ad4786a34698ea5bc7a3e6fb833646b22c71d
commit: 97a7445714f44d2a978237281af297b299770f16 [709/711] drm/amdgpu: fix
reporting of failed msg sent to SMU
config: i386-allmodconfig (attached as .config)
compiler: gcc-7 (Debian 7.3.0-1) 7.3.0
reproduce:
git checkout 97a7445714f44d2a978237281af297b299770f16
# save the attached .config to linux build tree
make ARCH=i386
Note: the radeon-alex/amd-staging-drm-next HEAD
af1ad4786a34698ea5bc7a3e6fb833646b22c71d builds fine.
It only hurts bisectibility.
All errors (new ones prefixed by >>):
drivers/gpu/drm/amd/amdgpu/../powerplay/smumgr/smu8_smumgr.c: In function
'smu8_send_msg_to_smc_async':
>> drivers/gpu/drm/amd/amdgpu/../powerplay/smumgr/smu8_smumgr.c:67:11: error:
>> expected identifier or '(' before '=' token
uint32_t = val;
^
>> drivers/gpu/drm/amd/amdgpu/../powerplay/smumgr/smu8_smumgr.c:76:3: error:
>> 'val' undeclared (first use in this function); did you mean 'dal'?
val = cgs_read_register(hwmgr->device,
^~~
dal
drivers/gpu/drm/amd/amdgpu/../powerplay/smumgr/smu8_smumgr.c:76:3: note:
each undeclared identifier is reported only once for each function it appears in
vim +67 drivers/gpu/drm/amd/amdgpu/../powerplay/smumgr/smu8_smumgr.c
63
64 static int smu8_send_msg_to_smc_async(struct pp_hwmgr *hwmgr, uint16_t
msg)
65 {
66 int result = 0;
> 67 uint32_t = val;
68
69 if (hwmgr == NULL || hwmgr->device == NULL)
70 return -EINVAL;
71
72 result = PHM_WAIT_FIELD_UNEQUAL(hwmgr,
73 SMU_MP1_SRBM2P_RESP_0, CONTENT,
0);
74 if (result != 0) {
75 /* Read the last message to SMU, to report actual cause
*/
> 76 val = cgs_read_register(hwmgr->device,
77 mmSMU_MP1_SRBM2P_MSG_0);
78 pr_err("smu8_send_msg_to_smc_async (0x%04x) failed\n",
msg);
79 pr_err("SMU still servicing msg (0x%04x)\n", val);
80 return result;
81 }
82
83 cgs_write_register(hwmgr->device, mmSMU_MP1_SRBM2P_RESP_0, 0);
84 cgs_write_register(hwmgr->device, mmSMU_MP1_SRBM2P_MSG_0, msg);
85
86 return 0;
87 }
88
---
0-DAY kernel test infrastructureOpen Source Technology Center
https://lists.01.org/pipermail/kbuild-all Intel Corporation
.config.gz
Description: application/gzip
___
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel
[Bug 201439] Amdgpu: general protection fault at dce110_vblank_set in display resume
https://bugzilla.kernel.org/show_bug.cgi?id=201439 fin4...@hotmail.com changed: What|Removed |Added Status|NEW |RESOLVED Resolution|--- |CODE_FIX --- Comment #5 from fin4...@hotmail.com --- The latest wip kernel fixes this bug. -- 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 108573] Cannot set pstate values with under-voltage values for Vega M
https://bugs.freedesktop.org/show_bug.cgi?id=108573 Bug ID: 108573 Summary: Cannot set pstate values with under-voltage values for Vega M Product: DRI Version: unspecified Hardware: x86-64 (AMD64) OS: Linux (All) Status: NEW Severity: normal Priority: medium Component: DRM/AMDgpu Assignee: dri-devel@lists.freedesktop.org Reporter: rstr...@gmail.com There are some folks on the Windows side of the fence that have had great success under-volting their Vega M chips to improve thermals (and because TDP is shared between the CPU and GPU on Kaby Lake G, also their overall performance). Following information from the amdgpu kernel documentation I applied the following kernel boot parameters: amdgpu.runpm=0 amdgpu.ppfeaturemask=0x Note: I needed the runpm=0 so that the Vega M is active at all times, otherwise it powers down and I'm unable to make any changes to the pstates using the sysfs API. Then doing the following I was able to see the current pstates: cat /sys/bus/pci/drivers/amdgpu/:01:00.0/pp_od_clk_voltage OD_SCLK: 0:225MHz750mV 1:400MHz750mV 2:535MHz750mV 3:715MHz750mV 4:850MHz750mV 5:960MHz750mV 6:985MHz750mV 7: 1011MHz750mV OD_MCLK: 0:300MHz750mV 1:500MHz750mV 2:700MHz750mV OD_RANGE: SCLK: 225MHz 1011MHz MCLK: 300MHz700MHz VDDC: 750mV 750mV Then this to support updating the pstates: echo "manual" > /sys/bus/pci/drivers/amdgpu/:01:00.0/power_dpm_force_performance_level Then I could push in new pstates values, provided the voltage was *exactly* 750mV. I could set a higher clock speed for a pstate, but I was unable to make any changes to the voltage. echo "s 0 226 750" > /sys/bus/pci/drivers/amdgpu/:01:00.0/pp_od_clk_voltage works and increases the MHz from 225 -> 226 for pstate 0, but echo "s 0 226 749" > /sys/bus/pci/drivers/amdgpu/:01:00.0/pp_od_clk_voltage -bash: echo: write error: Invalid argument Checking dmesg I see that the voltage range is constrained between 750 and 750. [ 4362.487021] amdgpu: [powerplay] OD voltage is out of range [750 - 750] mV Would it be possible to add support for under-volting Vega M using the sysfs API? I also noticed that it's not possible to set any pstate higher than 1011 MHz, perhaps that's a harder sell, but it might also be nice to be able to overclock the Vega M a little! P.S. I tried finding these values in the kernel source code, but I was extremely confused about the way they are defined. I figured worse case scenario I could manually modify these limits and recompile the kernel. Thanks! Rob -- You are receiving this mail because: You are the assignee for the bug.___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
[Bug 108096] [amd-staging-drm-next] SDDM screen corruption (not usable) with RX580, amdgpu, dc=1 (of course), regression - [bisected]
https://bugs.freedesktop.org/show_bug.cgi?id=108096 --- Comment #17 from Dieter Nützel --- (In reply to Andrey Grodzovsky from comment #16) > Question is do you have " winsys/amdgpu: pass the BO list via the CS ioctl > on DRM >= 3.27.0" commit in your MESA tree ? I am not clear on that. 461a864316 winsys/amdgpu: pass the BO list via the CS ioctl on DRM >= 3.27.0 commit 461a864316d5b70ea99c9e1dba7d71973af2aacc Author: Marek Olšák Date: Thu Jul 12 00:50:52 2018 -0400 winsys/amdgpu: pass the BO list via the CS ioctl on DRM >= 3.27.0 Any other ideas? Thank you Andrey for looking into, it! -- You are receiving this mail because: You are the assignee for the bug.___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
[PATCH v2 0/3] drm/i915: HPD IRQ storm detection fixes
IMPORTANT - As a note: I have not had the customer who this second patch is for test this yet, I'm sending this ahead of time to make sure this is something that isn't too crazy for upstream to accept. I'm not planning on pushing this after review until I've verified this actually fixes their problems. This series contains a fix for a problem which is very difficult to reproduce under normal circumstances without specialized testing hardware, along with a fix that seems to be required for some especially rebellious GM45 laptops. Lyude Paul (3): drm/i915: Fix possible race in intel_dp_add_mst_connector() drm/i915: Fix NULL deref when re-enabling HPD IRQs on systems with MST drm/i915: Add short HPD IRQ storm detection for non-MST systems drivers/gpu/drm/i915/i915_debugfs.c | 74 drivers/gpu/drm/i915/i915_drv.h | 5 +- drivers/gpu/drm/i915/i915_irq.c | 7 +++ drivers/gpu/drm/i915/intel_dp_mst.c | 8 +-- drivers/gpu/drm/i915/intel_hotplug.c | 51 +++ 5 files changed, 120 insertions(+), 25 deletions(-) -- 2.17.2 ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
[PATCH v2 2/3] drm/i915: Fix NULL deref when re-enabling HPD IRQs on systems with MST
Turns out that if you trigger an HPD storm on a system that has an MST
topology connected to it, you'll end up causing the kernel to eventually
hit a NULL deref:
[ 332.339041] BUG: unable to handle kernel NULL pointer dereference at
00ec
[ 332.340906] PGD 0 P4D 0
[ 332.342750] Oops: [#1] SMP PTI
[ 332.344579] CPU: 2 PID: 25 Comm: kworker/2:0 Kdump: loaded Tainted: G
O 4.18.0-rc3short-hpd-storm+ #2
[ 332.346453] Hardware name: LENOVO 20BWS1KY00/20BWS1KY00, BIOS JBET71WW (1.35
) 09/14/2018
[ 332.348361] Workqueue: events intel_hpd_irq_storm_reenable_work [i915]
[ 332.350301] RIP: 0010:intel_hpd_irq_storm_reenable_work.cold.3+0x2f/0x86
[i915]
[ 332.352213] Code: 00 00 ba e8 00 00 00 48 c7 c6 c0 aa 5f a0 48 c7 c7 d0 73
62 a0 4c 89 c1 4c 89 04 24 e8 7f f5 af e0 4c 8b 04 24 44 89 f8 29 e8 <41> 39 80
ec 00 00 00 0f 85 43 13 fc ff 41 0f b6 86 b8 04 00 00 41
[ 332.354286] RSP: 0018:c9147e48 EFLAGS: 00010006
[ 332.356344] RAX: 0005 RBX: 8802c226c9d4 RCX: 0006
[ 332.358404] RDX: RSI: 0082 RDI: 88032dc95570
[ 332.360466] RBP: 0005 R08: R09: 88031b3dc840
[ 332.362528] R10: R11: 00031a069602 R12: 8802c226ca20
[ 332.364575] R13: 8802c2268000 R14: 880310661000 R15: 000a
[ 332.366615] FS: () GS:88032dc8()
knlGS:
[ 332.368658] CS: 0010 DS: ES: CR0: 80050033
[ 332.370690] CR2: 00ec CR3: 0200a003 CR4: 003606e0
[ 332.372724] DR0: DR1: DR2:
[ 332.374773] DR3: DR6: fffe0ff0 DR7: 0400
[ 332.376798] Call Trace:
[ 332.378809] process_one_work+0x1a1/0x350
[ 332.380806] worker_thread+0x30/0x380
[ 332.382777] ? wq_update_unbound_numa+0x10/0x10
[ 332.384772] kthread+0x112/0x130
[ 332.386740] ? kthread_create_worker_on_cpu+0x70/0x70
[ 332.388706] ret_from_fork+0x35/0x40
[ 332.390651] Modules linked in: i915(O) vfat fat joydev btusb btrtl btbcm
btintel bluetooth ecdh_generic iTCO_wdt wmi_bmof i2c_algo_bit drm_kms_helper
intel_rapl syscopyarea sysfillrect x86_pkg_temp_thermal sysimgblt coretemp
fb_sys_fops crc32_pclmul drm psmouse pcspkr mei_me mei i2c_i801 lpc_ich
mfd_core i2c_core tpm_tis tpm_tis_core thinkpad_acpi wmi tpm rfkill video
crc32c_intel serio_raw ehci_pci xhci_pci ehci_hcd xhci_hcd [last unloaded: i915]
[ 332.394963] CR2: 00ec
This appears to be due to the fact that with an MST topology, not all
intel_connector structs will have ->encoder set. So, fix this by
skipping connectors without encoders in
intel_hpd_irq_storm_reenable_work().
For those wondering, this bug was found on accident while simulating HPD
storms using a Chamelium connected to a ThinkPad T450s (Broadwell).
Changes since v1:
- Check intel_connector->mst_port instead of intel_connector->encoder
Signed-off-by: Lyude Paul
Cc: Ville Syrjälä
---
drivers/gpu/drm/i915/intel_hotplug.c | 4 +++-
1 file changed, 3 insertions(+), 1 deletion(-)
diff --git a/drivers/gpu/drm/i915/intel_hotplug.c
b/drivers/gpu/drm/i915/intel_hotplug.c
index 648a13c6043c..8326900a311e 100644
--- a/drivers/gpu/drm/i915/intel_hotplug.c
+++ b/drivers/gpu/drm/i915/intel_hotplug.c
@@ -228,7 +228,9 @@ static void intel_hpd_irq_storm_reenable_work(struct
work_struct *work)
drm_for_each_connector_iter(connector, _iter) {
struct intel_connector *intel_connector =
to_intel_connector(connector);
- if (intel_connector->encoder->hpd_pin == pin) {
+ /* Don't check MST ports, they don't have pins */
+ if (!intel_connector->mst_port &&
+ intel_connector->encoder->hpd_pin == pin) {
if (connector->polled !=
intel_connector->polled)
DRM_DEBUG_DRIVER("Reenabling HPD on
connector %s\n",
connector->name);
--
2.17.2
___
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel
[PATCH v2 3/3] drm/i915: Add short HPD IRQ storm detection for non-MST systems
Unfortunately, it seems that the HPD IRQ storm problem from the early
days of Intel GPUs was never entirely solved, only mostly. Within the
last couple of days, I got a bug report from one of our customers who
had been having issues with their machine suddenly booting up very
slowly after having updated. The amount of time it took to boot went
from around 30 seconds, to over 6 minutes consistently.
After some investigation, I discovered that i915 was reporting massive
amounts of short HPD IRQ spam on this system from the DisplayPort port,
despite there not being anything actually connected. The symptoms would
start with one "long" HPD IRQ being detected at boot:
[1.891398] [drm:intel_get_hpd_pins [i915]] hotplug event received, stat
0x0044, dig 0x0044, pins 0x00a0
[1.891436] [drm:intel_hpd_irq_handler [i915]] digital hpd port B - long
[1.891472] [drm:intel_hpd_irq_handler [i915]] Received HPD interrupt on PIN
5 - cnt: 0
[1.891508] [drm:intel_hpd_irq_handler [i915]] digital hpd port D - long
[1.891544] [drm:intel_hpd_irq_handler [i915]] Received HPD interrupt on PIN
7 - cnt: 0
[1.891592] [drm:intel_dp_hpd_pulse [i915]] got hpd irq on port B - long
[1.891628] [drm:intel_dp_hpd_pulse [i915]] got hpd irq on port D - long
…
followed by constant short IRQs afterwards:
[1.895091] [drm:intel_encoder_hotplug [i915]] [CONNECTOR:66:DP-1] status
updated from unknown to disconnected
[1.895129] [drm:i915_hotplug_work_func [i915]] Connector DP-3 (pin 7)
received hotplug event.
[1.895165] [drm:intel_dp_detect [i915]] [CONNECTOR:72:DP-3]
[1.895275] [drm:intel_get_hpd_pins [i915]] hotplug event received, stat
0x0020, dig 0x0020, pins 0x0080
[1.895312] [drm:intel_hpd_irq_handler [i915]] digital hpd port D - short
[1.895762] [drm:intel_get_hpd_pins [i915]] hotplug event received, stat
0x0020, dig 0x0020, pins 0x0080
[1.895799] [drm:intel_hpd_irq_handler [i915]] digital hpd port D - short
[1.896239] [drm:intel_dp_aux_xfer [i915]] dp_aux_ch timeout status
0x71450085
[1.896293] [drm:intel_get_hpd_pins [i915]] hotplug event received, stat
0x0020, dig 0x0020, pins 0x0080
[1.896330] [drm:intel_hpd_irq_handler [i915]] digital hpd port D - short
[1.896781] [drm:intel_get_hpd_pins [i915]] hotplug event received, stat
0x0020, dig 0x0020, pins 0x0080
[1.896817] [drm:intel_hpd_irq_handler [i915]] digital hpd port D - short
[1.897275] [drm:intel_get_hpd_pins [i915]] hotplug event received, stat
0x0020, dig 0x0020, pins 0x0080
The customer's system in question has a GM45 GPU, which is apparently
well known for hotplugging storms.
So, workaround this impressively broken hardware by changing the default
HPD storm threshold from 5 to 50. Then, make long IRQs count for 10, and
short IRQs count for 1. This makes it so that 5 long IRQs will trigger
an HPD storm, and on systems with short HPD storm detection 50 short
IRQs will trigger an HPD storm. 50 short IRQs amounts to 100ms of
constant pulsing, which seems like a good middleground between being too
sensitive and not being sensitive enough (which would cause visible
stutters in userspace every time a storm occurs).
And just to be extra safe: we don't enable this by default on systems
with MST support. There's too high of a chance of MST support triggering
storm detection, and systems that are new enough to support MST are a
lot less likely to have issues with IRQ storms anyway.
As a note: this patch was tested using a ThinkPad T450s and a Chamelium
to simulate the short IRQ storms.
Changes since v1:
- Don't use two separate thresholds, just make long IRQs count for 10
each and short IRQs count for 1. This simplifies the code a bit
- Ville Syrjälä
Signed-off-by: Lyude Paul
Cc: Ville Syrjälä
---
drivers/gpu/drm/i915/i915_debugfs.c | 74
drivers/gpu/drm/i915/i915_drv.h | 5 +-
drivers/gpu/drm/i915/i915_irq.c | 7 +++
drivers/gpu/drm/i915/intel_hotplug.c | 47 +++---
4 files changed, 113 insertions(+), 20 deletions(-)
diff --git a/drivers/gpu/drm/i915/i915_debugfs.c
b/drivers/gpu/drm/i915/i915_debugfs.c
index b4744a68cd88..1595b8565875 100644
--- a/drivers/gpu/drm/i915/i915_debugfs.c
+++ b/drivers/gpu/drm/i915/i915_debugfs.c
@@ -4641,6 +4641,79 @@ static const struct file_operations
i915_hpd_storm_ctl_fops = {
.write = i915_hpd_storm_ctl_write
};
+static int i915_hpd_short_storm_ctl_show(struct seq_file *m, void *data)
+{
+ struct drm_i915_private *dev_priv = m->private;
+
+ seq_printf(m, "Enabled: %s\n",
+ yesno(dev_priv->hotplug.hpd_short_storm_enabled));
+
+ return 0;
+}
+
+static int
+i915_hpd_short_storm_ctl_open(struct inode *inode, struct file *file)
+{
+ return single_open(file, i915_hpd_short_storm_ctl_show,
+ inode->i_private);
+}
+
+static ssize_t
[PATCH v2 1/3] drm/i915: Fix possible race in intel_dp_add_mst_connector()
This hasn't caused any issues yet that I'm aware of, but as Ville
Syrjälä pointed out - we need to make sure that
intel_connector->mst_port is set before initializing MST connectors,
since in theory we could potentially check intel_connector->mst_port in
i915_hpd_poll_init_work() after registering the connector but before
having written it's value.
Signed-off-by: Lyude Paul
Cc: Ville Syrjälä
---
drivers/gpu/drm/i915/intel_dp_mst.c | 8
1 file changed, 4 insertions(+), 4 deletions(-)
diff --git a/drivers/gpu/drm/i915/intel_dp_mst.c
b/drivers/gpu/drm/i915/intel_dp_mst.c
index 77920f1a3da1..026d6365cff1 100644
--- a/drivers/gpu/drm/i915/intel_dp_mst.c
+++ b/drivers/gpu/drm/i915/intel_dp_mst.c
@@ -448,6 +448,10 @@ static struct drm_connector
*intel_dp_add_mst_connector(struct drm_dp_mst_topolo
if (!intel_connector)
return NULL;
+ intel_connector->get_hw_state = intel_dp_mst_get_hw_state;
+ intel_connector->mst_port = intel_dp;
+ intel_connector->port = port;
+
connector = _connector->base;
ret = drm_connector_init(dev, connector, _dp_mst_connector_funcs,
DRM_MODE_CONNECTOR_DisplayPort);
@@ -458,10 +462,6 @@ static struct drm_connector
*intel_dp_add_mst_connector(struct drm_dp_mst_topolo
drm_connector_helper_add(connector,
_dp_mst_connector_helper_funcs);
- intel_connector->get_hw_state = intel_dp_mst_get_hw_state;
- intel_connector->mst_port = intel_dp;
- intel_connector->port = port;
-
for_each_pipe(dev_priv, pipe) {
struct drm_encoder *enc =
_dp->mst_encoders[pipe]->base.base;
--
2.17.2
___
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [PATCH v5 5/5] drm/tinydrm: Switch from CMA to shmem buffers
Den 17.10.2018 15.04, skrev Noralf Trønnes:
This move makes tinydrm useful for more drivers. tinydrm doesn't need
continuous memory, but at the time it was convenient to use the CMA
library. The spi core can do dma on is_vmalloc() addresses making this
possible.
Cc: David Lechner
Signed-off-by: Noralf Trønnes
Acked-by: David Lechner
Tested-by: David Lechner
---
David,
FYI This series is scratched.
See the shmem helper patch thread for details.
Noralf.
drivers/gpu/drm/tinydrm/Kconfig| 2 +-
drivers/gpu/drm/tinydrm/core/tinydrm-core.c| 92 +++---
drivers/gpu/drm/tinydrm/core/tinydrm-helpers.c | 5 ++
drivers/gpu/drm/tinydrm/ili9225.c | 14 ++--
drivers/gpu/drm/tinydrm/ili9341.c | 6 +-
drivers/gpu/drm/tinydrm/mi0283qt.c | 6 +-
drivers/gpu/drm/tinydrm/mipi-dbi.c | 38 ---
drivers/gpu/drm/tinydrm/repaper.c | 24 +++
drivers/gpu/drm/tinydrm/st7586.c | 15 +++--
drivers/gpu/drm/tinydrm/st7735r.c | 6 +-
include/drm/tinydrm/tinydrm.h | 36 +++---
11 files changed, 91 insertions(+), 153 deletions(-)
diff --git a/drivers/gpu/drm/tinydrm/Kconfig b/drivers/gpu/drm/tinydrm/Kconfig
index 16f4b5c91f1b..aa0cabba5ace 100644
--- a/drivers/gpu/drm/tinydrm/Kconfig
+++ b/drivers/gpu/drm/tinydrm/Kconfig
@@ -2,7 +2,7 @@ menuconfig DRM_TINYDRM
tristate "Support for simple displays"
depends on DRM
select DRM_KMS_HELPER
- select DRM_KMS_CMA_HELPER
+ select DRM_GEM_SHMEM_HELPER
help
Choose this option if you have a tinydrm supported display.
If M is selected the module will be called tinydrm.
diff --git a/drivers/gpu/drm/tinydrm/core/tinydrm-core.c
b/drivers/gpu/drm/tinydrm/core/tinydrm-core.c
index 255341ee4eb9..38ba361d1af2 100644
--- a/drivers/gpu/drm/tinydrm/core/tinydrm-core.c
+++ b/drivers/gpu/drm/tinydrm/core/tinydrm-core.c
@@ -12,6 +12,7 @@
#include
#include
#include
+#include
#include
#include
#include
@@ -23,7 +24,7 @@
*
* It is based on _simple_display_pipe coupled with a _connector which
* has only one fixed _display_mode. The framebuffers are backed by the
- * cma helper and have support for framebuffer flushing (dirty).
+ * shmem buffers and have support for framebuffer flushing (dirty).
* fbdev support is also included.
*
*/
@@ -37,84 +38,41 @@
*/
/**
- * tinydrm_gem_cma_prime_import_sg_table - Produce a CMA GEM object from
- * another driver's scatter/gather table of pinned pages
- * @drm: DRM device to import into
- * @attach: DMA-BUF attachment
- * @sgt: Scatter/gather table of pinned pages
+ * tinydrm_fb_destroy - Destroy framebuffer
+ * @fb: Framebuffer
*
- * This function imports a scatter/gather table exported via DMA-BUF by
- * another driver using drm_gem_cma_prime_import_sg_table(). It sets the
- * kernel virtual address on the CMA object. Drivers should use this as their
- * _driver->gem_prime_import_sg_table callback if they need the virtual
- * address. tinydrm_gem_cma_free_object() should be used in combination with
- * this function.
- *
- * Returns:
- * A pointer to a newly created GEM object or an ERR_PTR-encoded negative
- * error code on failure.
+ * This function unmaps the virtual address on the backing buffer and destroys
the framebuffer.
+ * Drivers should use this as their _framebuffer_funcs->destroy callback.
*/
-struct drm_gem_object *
-tinydrm_gem_cma_prime_import_sg_table(struct drm_device *drm,
- struct dma_buf_attachment *attach,
- struct sg_table *sgt)
+void tinydrm_fb_destroy(struct drm_framebuffer *fb)
{
- struct drm_gem_cma_object *cma_obj;
- struct drm_gem_object *obj;
- void *vaddr;
+ struct drm_gem_object *gem = drm_gem_fb_get_obj(fb, 0);
+ struct drm_gem_shmem_object *shmem = to_drm_gem_shmem_obj(gem);
- vaddr = dma_buf_vmap(attach->dmabuf);
- if (!vaddr) {
- DRM_ERROR("Failed to vmap PRIME buffer\n");
- return ERR_PTR(-ENOMEM);
- }
-
- obj = drm_gem_cma_prime_import_sg_table(drm, attach, sgt);
- if (IS_ERR(obj)) {
- dma_buf_vunmap(attach->dmabuf, vaddr);
- return obj;
- }
-
- cma_obj = to_drm_gem_cma_obj(obj);
- cma_obj->vaddr = vaddr;
-
- return obj;
+ drm_gem_vunmap(gem, shmem->vaddr);
+ drm_gem_fb_destroy(fb);
}
-EXPORT_SYMBOL(tinydrm_gem_cma_prime_import_sg_table);
-
-/**
- * tinydrm_gem_cma_free_object - Free resources associated with a CMA GEM
- * object
- * @gem_obj: GEM object to free
- *
- * This function frees the backing memory of the CMA GEM object, cleans up the
- * GEM object state and frees the memory used to store the object itself using
- * drm_gem_cma_free_object(). It also handles PRIME buffers
Re: [PATCH 0/3] drm: tinydrm driver for adafruit PiTFT 3.5" touchscreen
Den 26.10.2018 21.16, skrev Noralf Trønnes: Den 26.10.2018 04.30, skrev Eric Anholt: Noralf Trønnes writes: Den 25.10.2018 18.29, skrev Eric Anholt: Eric Anholt writes: I was going to start working on making the vc4 driver work with tinydrm panels, but it turned out tinydrm didn't have the panel I had previously bought. So, last night I ported the fbtft staging driver over to DRM. It seems to work (with DT at https://github.com/anholt/linux/commits/drm-misc-next-hx8357d) -- fbdev works great including rotated, and so does modetest. However, when X11 comes up at 16bpp, I get: https://photos.app.goo.gl/8tuhzPFFoDGamEfk8 If I have tinydrm set a preferred bpp of 24, X looks great. Noralf, any ideas? Also, with these patches and the format modifier patch I just sent, mesa with vc4 is now working with this driver on this branch: https://gitlab.freedesktop.org/anholt/mesa/commits/kmsro Ah, nice to see this happening! Getting hw rendering was one of the advantages I saw DRM could provide over fbdev on these displays. Little did I know how complicated graphics was outside fbdev, so I was unable to realise this myself. The current solution to get hw rendering is to have a userspace process that continously copies the framebuffer: https://github.com/tasanakorn/rpi-fbcp It's used by some of the small DIY handheld game consoles that run emulators which requires hw rendering. Now I wonder how we can improve performance of the SPI updates. At what SPI speed are you running? The datasheet for most of these display controllers list the max speed as 10MHz, but almost all of them can go faster. Some are reported going as high as 70-80MHz. That's for the pixel data transfer, not the commands. tinydrm/mipi-dbi.c sends commands at 10MHz and pixels at full speed (mipi_dbi_spi_cmd_max_speed()). Most panels I have run at 32MHz or 48MHz. I copied the DT from the adafruit tree, which has it at 32mhz. System performance seems to be limited by the copy and format conversion I think -- in particular, I wonder if we shouldn't be doing our dirty copies in our own workqueue. I haven't managed to get any really good profiling data yet, though. glxgears at 128x128 is nice and smooth, and at 480x320 it's 6fps. That's not filling 32mhz of SPI. On the other hand, I would have expected the uncached reads for the 4-to-2 swapped conversion to be able to go faster than 3.5mb/sec. If it's the uncached reads, we could at least use NEON for the copy to cached, and probably even do the whole conversion in NEON with a bit more thought. Another option: use a vc4 RCL to do RGBA to RGB565, since that will be less pressure on the bus. But then, I suppose I should just figure out what's going on that makes X11 at RGBA break, and fix that so we don't even have to do that conversion. I keep hoping there's some way we could feed output from the DISPSLAVE HVS register directly to the SPI master -- FIFO32 gets us close (two 16-bit pixels packed next to each other, leftmost in the lower 2 bytes), but the need for byte swapping (as opposed to R/B swapping) I think makes it impossible. I just did some speed tests on a 320x240 display at 3 different speeds. I also tried with byteswapping disabled. Only full updates will benefit from passing the buffer straight through to SPI. This is because partial updates are copied to a transfer buffer anyways to minimize SPI transfer time. No need to transfer things that haven't changed and a memory copy is far cheaper than a SPI transfer. SPI at 48MHz: pi@pi2835:~$ od -An -vtu4 --endian=big /sys/bus/spi/devices/spi0.0/of_node/spi-max-frequency 4800 pi@pi2835:~$ ./libdrm/tests/modetest/modetest -M mi0283qt -s 28:320x240@XR24 -v setting mode 320x240-0Hz@XR24 on connectors 28, crtc 30 freq: 24.87Hz freq: 24.79Hz pi@pi2835:~$ ./libdrm/tests/modetest/modetest -M mi0283qt -s 28:320x240@RG16 -v setting mode 320x240-0Hz@RG16 on connectors 28, crtc 30 freq: 26.33Hz freq: 26.30Hz disable byteswapping: pi@pi2835:~$ ./libdrm/tests/modetest/modetest -M mi0283qt -s 28:320x240@RG16 -v setting mode 320x240-0Hz@RG16 on connectors 28, crtc 30 freq: 28.40Hz freq: 28.43Hz SPI at 64MHz (seems to work): pi@pi2835:~$ od -An -vtu4 --endian=big /sys/bus/spi/devices/spi0.0/of_node/spi-max-frequency 6400 pi@pi2835:~$ ./libdrm/tests/modetest/modetest -M mi0283qt -s 28:320x240@XR24 -v setting mode 320x240-0Hz@XR24 on connectors 28, crtc 30 freq: 32.74Hz freq: 32.69Hz pi@pi2835:~$ ./libdrm/tests/modetest/modetest -M mi0283qt -s 28:320x240@RG16 -v setting mode 320x240-0Hz@RG16 on connectors 28, crtc 30 freq: 35.44Hz freq: 35.19Hz disabled byteswapping: pi@pi2835:~$ ./libdrm/tests/modetest/modetest -M mi0283qt -s 28:320x240@RG16 -v setting mode 320x240-0Hz@RG16 on connectors 28, crtc 30 freq: 39.29Hz freq: 39.11Hz SPI at 128MHz (not at all as garbled as I expected): pi@pi2835:~$ od -An -vtu4 --endian=big /sys/bus/spi/devices/spi0.0/of_node/spi-max-frequency
Re: [PATCH v4 3/4] drm: Document variable refresh properties
On Fri, Oct 26, 2018 at 08:06:11PM +, Kazlauskas, Nicholas wrote: > On 10/26/18 3:13 PM, Manasi Navare wrote: > > On Fri, Oct 26, 2018 at 05:34:15PM +, Kazlauskas, Nicholas wrote: > >> On 10/26/18 10:53 AM, Ville Syrjälä wrote: > >>> On Fri, Oct 26, 2018 at 02:49:31PM +, Kazlauskas, Nicholas wrote: > On 10/26/18 7:37 AM, Pekka Paalanen wrote: > > On Thu, 11 Oct 2018 12:39:41 -0400 > > Nicholas Kazlauskas wrote: > > > >> These include the drm_connector 'vrr_capable' and the drm_crtc > >> 'vrr_enabled' properties. > >> > >> Signed-off-by: Nicholas Kazlauskas > >> --- > >> Documentation/gpu/drm-kms.rst | 7 +++ > >> drivers/gpu/drm/drm_connector.c | 22 ++ > >> 2 files changed, 29 insertions(+) > >> > >> diff --git a/Documentation/gpu/drm-kms.rst > >> b/Documentation/gpu/drm-kms.rst > >> index 4b1501b4835b..8da2a178cf85 100644 > >> --- a/Documentation/gpu/drm-kms.rst > >> +++ b/Documentation/gpu/drm-kms.rst > >> @@ -575,6 +575,13 @@ Explicit Fencing Properties > >> .. kernel-doc:: drivers/gpu/drm/drm_atomic_uapi.c > >>:doc: explicit fencing properties > >> > >> + > >> +Variable Refresh Properties > >> +--- > >> + > >> +.. kernel-doc:: drivers/gpu/drm/drm_connector.c > >> + :doc: Variable refresh properties > >> + > >> Existing KMS Properties > >> --- > >> > >> diff --git a/drivers/gpu/drm/drm_connector.c > >> b/drivers/gpu/drm/drm_connector.c > >> index f0deeb7298d0..2a12853ca917 100644 > >> --- a/drivers/gpu/drm/drm_connector.c > >> +++ b/drivers/gpu/drm/drm_connector.c > >> @@ -1254,6 +1254,28 @@ int > >> drm_mode_create_scaling_mode_property(struct drm_device *dev) > >> } > >> EXPORT_SYMBOL(drm_mode_create_scaling_mode_property); > >> > >> +/** > >> + * DOC: Variable refresh properties > >> + * > >> + * Variable refresh rate control is supported via properties on the > >> + * _connector and _crtc objects. > >> + * > >> + * "vrr_capable": > >> + *Optional _connector boolean property that drivers should > >> attach > >> + *with drm_connector_attach_vrr_capable_property() on connectors > >> that > >> + *could support variable refresh rates. Drivers should update the > >> + *property value by calling > >> drm_connector_set_vrr_capable_property(). > >> + * > >> + *Absence of the property should indicate absence of support. > >> + * > >> + * "vrr_enabled": > >> + *Default _crtc boolean property that notifies the driver > >> that the > >> + *variable refresh rate adjustment should be enabled for the CRTC. > > > > Hi, > > > > where is the documentation that explains how drivers must implement > > "variable refresh rate adjustment"? > > > > What should and could userspace expect to get if it flicks this switch? > > > > I also think the kernel documentation should include a description of > > what VRR actually is and how it conceptually works as far as userspace > > is concerned. > > > > That is, the kernel documentation should describe what this thing does, > > so that we avoid every driver implementing a different thing. For > > example, one driver could prevent the luminance flickering itself by > > tuning the timings while another driver might not do that, which means > > that an application tested on the former driver will look just fine > > while it is unbearable to watch on the latter. > > > > > > Thanks, > > pq > > I felt it was best to leave this more on the vague side to not impose > restrictions yet on what a driver must do. > > If you think it's worth defining what the "baseline" expectation is for > userspace, I would probably describe it as "utilizing the monitor's > variable refresh rate to reduce stuttering or tearing that can occur > during flipping". This is currently what the amdgpu driver has enabled > > > > I would also mention that without VRR, the display engine processes the > > flips > > independently of the rendering speed which might result into stuttering or > > tearing. > > > > Might also be worth giving a quick example with numbers in the > > documentation. > > Something like: For Eg if the rendering speed is 40Hz, without VRR, display > > engine > > will flip at constant 60Hz, which means that same frame will be displayed > > twice which > > will be observed as stutter/repetition. > > With VRR enabled, the vertical front porch will be extended and the flip > > will > > be processed when its ready or at max blanking time resulting a smooth > > transition without repetition. > > Good points about mentioning the problems it solves
[Bug 201285] Kernel oops in amdgpu with Ryzen5 2400G
https://bugzilla.kernel.org/show_bug.cgi?id=201285 --- Comment #7 from vl...@aresgate.net --- Created attachment 279173 --> https://bugzilla.kernel.org/attachment.cgi?id=279173=edit debug_logs_new -- 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 201285] Kernel oops in amdgpu with Ryzen5 2400G
https://bugzilla.kernel.org/show_bug.cgi?id=201285 --- Comment #6 from vl...@aresgate.net --- Created attachment 279171 --> https://bugzilla.kernel.org/attachment.cgi?id=279171=edit debug logs -- 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 201285] Kernel oops in amdgpu with Ryzen5 2400G
https://bugzilla.kernel.org/show_bug.cgi?id=201285 oyvi...@everdot.org changed: What|Removed |Added CC||oyvi...@everdot.org --- Comment #5 from oyvi...@everdot.org --- Created attachment 279169 --> https://bugzilla.kernel.org/attachment.cgi?id=279169=edit dmesg output kernel 4.19 I see the same [9.749492] Call Trace: [9.749569] ? _cond_resched+0x15/0x30 [9.749702] dcn10_create_resource_pool+0x781/0x9d0 [amdgpu] [9.749849] ? dal_aux_engine_dce110_create+0x39/0x80 [amdgpu] and so on message on each boot. I also get another error after some uptime. -- 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
[PATCH v2 2/4] drm/dp_mst: Start tracking per-port VCPI allocations
There has been a TODO waiting for quite a long time in
drm_dp_mst_topology.c:
/* We cannot rely on port->vcpi.num_slots to update
* topology_state->avail_slots as the port may not exist if the parent
* branch device was unplugged. This should be fixed by tracking
* per-port slot allocation in drm_dp_mst_topology_state instead of
* depending on the caller to tell us how many slots to release.
*/
That's not the only reason we should fix this: forcing the driver to
track the VCPI allocations throughout a state's atomic check is
error prone, because it means that extra care has to be taken with the
order that drm_dp_atomic_find_vcpi_slots() and
drm_dp_atomic_release_vcpi_slots() are called in in order to ensure
idempotency. Currently the only driver actually using these helpers,
i915, doesn't even do this correctly: multiple ->best_encoder() checks
with i915's current implementation would not be idempotent and would
over-allocate VCPI slots, something I learned trying to implement
fallback retraining in MST.
So: simplify this whole mess, and teach drm_dp_atomic_find_vcpi_slots()
and drm_dp_atomic_release_vcpi_slots() to track the VCPI allocations for
each port. This allows us to ensure idempotency without having to rely
on the driver as much. Additionally: the driver doesn't need to do any
kind of VCPI slot tracking anymore if it doesn't need it for it's own
internal state.
Additionally; this adds a new drm_dp_mst_atomic_check() helper which
must be used by atomic drivers to perform validity checks for the new
VCPI allocations incurred by a state.
Also: update the documentation and make it more obvious that these
/must/ be called by /all/ atomic drivers supporting MST.
Changes since v1:
- Don't use the now-removed ->atomic_check() for private objects hook,
just give drivers a function to call themselves
Signed-off-by: Lyude Paul
Cc: Daniel Vetter
---
drivers/gpu/drm/drm_dp_mst_topology.c | 190 +-
drivers/gpu/drm/i915/intel_display.c | 8 ++
drivers/gpu/drm/i915/intel_dp_mst.c | 31 +++--
include/drm/drm_dp_mst_helper.h | 11 +-
4 files changed, 192 insertions(+), 48 deletions(-)
diff --git a/drivers/gpu/drm/drm_dp_mst_topology.c
b/drivers/gpu/drm/drm_dp_mst_topology.c
index 8c3cfac437f4..dcfab7536914 100644
--- a/drivers/gpu/drm/drm_dp_mst_topology.c
+++ b/drivers/gpu/drm/drm_dp_mst_topology.c
@@ -2614,21 +2614,33 @@ static int drm_dp_init_vcpi(struct
drm_dp_mst_topology_mgr *mgr,
}
/**
- * drm_dp_atomic_find_vcpi_slots() - Find and add vcpi slots to the state
+ * drm_dp_atomic_find_vcpi_slots() - Find and add VCPI slots to the state
* @state: global atomic state
* @mgr: MST topology manager for the port
* @port: port to find vcpi slots for
* @pbn: bandwidth required for the mode in PBN
*
+ * Allocates VCPI slots to @port, replacing any previous VCPI allocations it
+ * may have had. Any atomic drivers which support MST must call this function
+ * in their atomic_check() handlers to change the current VCPI allocation for
+ * the new state. After the ->atomic_check() hooks of the driver and all other
+ * mode objects in the state have been called, DRM will check the final VCPI
+ * allocations to ensure that they will fit into the available bandwidth on
+ * the topology.
+ *
+ * See also: drm_dp_atomic_release_vcpi_slots()
+ *
* RETURNS:
- * Total slots in the atomic state assigned for this port or error
+ * Total slots in the atomic state assigned for this port, or a negative error
+ * code if the port no longer exists
*/
int drm_dp_atomic_find_vcpi_slots(struct drm_atomic_state *state,
struct drm_dp_mst_topology_mgr *mgr,
struct drm_dp_mst_port *port, int pbn)
{
struct drm_dp_mst_topology_state *topology_state;
- int req_slots;
+ struct drm_dp_vcpi_allocation *pos, *vcpi = NULL;
+ int prev_slots, req_slots, ret;
topology_state = drm_atomic_get_mst_topology_state(state, mgr);
if (IS_ERR(topology_state))
@@ -2637,20 +2649,41 @@ int drm_dp_atomic_find_vcpi_slots(struct
drm_atomic_state *state,
port = drm_dp_get_validated_port_ref(mgr, port);
if (port == NULL)
return -EINVAL;
- req_slots = DIV_ROUND_UP(pbn, mgr->pbn_div);
- DRM_DEBUG_KMS("vcpi slots req=%d, avail=%d\n",
- req_slots, topology_state->avail_slots);
- if (req_slots > topology_state->avail_slots) {
- drm_dp_put_port(port);
- return -ENOSPC;
+ /* Find the current allocation for this port, if any */
+ list_for_each_entry(pos, _state->vcpis, next) {
+ if (pos->port == port) {
+ vcpi = pos;
+ prev_slots = vcpi->vcpi;
+ break;
+ }
}
+ if (!vcpi)
+ prev_slots = 0;
+
+ req_slots =
[PATCH v2 1/4] drm/dp_mst: Add some atomic state iterator macros
Signed-off-by: Lyude Paul
Cc: Daniel Vetter
---
include/drm/drm_dp_mst_helper.h | 77 +
1 file changed, 77 insertions(+)
diff --git a/include/drm/drm_dp_mst_helper.h b/include/drm/drm_dp_mst_helper.h
index 59f005b419cf..3faceb66f5cb 100644
--- a/include/drm/drm_dp_mst_helper.h
+++ b/include/drm/drm_dp_mst_helper.h
@@ -628,4 +628,81 @@ int drm_dp_atomic_release_vcpi_slots(struct
drm_atomic_state *state,
int drm_dp_send_power_updown_phy(struct drm_dp_mst_topology_mgr *mgr,
struct drm_dp_mst_port *port, bool power_up);
+extern const struct drm_private_state_funcs drm_dp_mst_topology_state_funcs;
+
+static inline bool
+__drm_dp_mst_state_iter_get(struct drm_atomic_state *state,
+ struct drm_dp_mst_topology_mgr **mgr,
+ struct drm_dp_mst_topology_state **old_state,
+ struct drm_dp_mst_topology_state **new_state,
+ int i)
+{
+ struct __drm_private_objs_state *objs_state = >private_objs[i];
+
+ if (objs_state->ptr->funcs != _dp_mst_topology_state_funcs)
+ return false;
+
+ *mgr = to_dp_mst_topology_mgr(objs_state->ptr);
+ if (old_state)
+ *old_state = to_dp_mst_topology_state(objs_state->old_state);
+ if (new_state)
+ *new_state = to_dp_mst_topology_state(objs_state->new_state);
+
+ return true;
+}
+
+/**
+ * for_each_oldnew_mst_mgr_in_state - iterate over all DP MST topology
+ * managers in an atomic update
+ * @__state: drm_atomic_state pointer
+ * @mgr: drm_dp_mst_topology_mgr iteration cursor
+ * @old_state: drm_dp_mst_topology_state iteration cursor for the old
+ * state
+ * @new_state: drm_dp_mst_topology_state iteration cursor for the new
+ * state
+ * @__i: int iteration cursor, for macro-internal use
+ *
+ * This iterates over all DRM DP MST topology managers in an atomic update,
+ * tracking both old and new state. This is useful in places where the state
+ * delta needs to be considered, for example in atomic check functions.
+ */
+#define for_each_oldnew_mst_mgr_in_state(__state, mgr, old_state, new_state,
__i) \
+ for ((__i) = 0; (__i) < (__state)->num_private_objs; (__i)++) \
+ for_each_if(__drm_dp_mst_state_iter_get((__state), &(mgr),
&(old_state), &(new_state), (__i)))
+
+/**
+ * for_each_old_mst_mgr_in_state - iterate over all DP MST topology managers
+ * in an atomic update
+ * @__state: drm_atomic_state pointer
+ * @mgr: drm_dp_mst_topology_mgr iteration cursor
+ * @old_state: drm_dp_mst_topology_state iteration cursor for the old
+ * state
+ * @__i: int iteration cursor, for macro-internal use
+ *
+ * This iterates over all DRM DP MST topology managers in an atomic update,
+ * tracking only the old state. This is useful in disable functions, where we
+ * need the old state the hardware is still in.
+ */
+#define for_each_old_mst_mgr_in_state(__state, mgr, old_state, __i) \
+ for ((__i) = 0; (__i) < (__state)->num_private_objs; (__i)++) \
+ for_each_if(__drm_dp_mst_state_iter_get((__state), &(mgr),
&(old_state), NULL, (__i)))
+
+/**
+ * for_each_new_mst_mgr_in_state - iterate over all DP MST topology managers
+ * in an atomic update
+ * @__state: drm_atomic_state pointer
+ * @mgr: drm_dp_mst_topology_mgr iteration cursor
+ * @new_state: drm_dp_mst_topology_state iteration cursor for the new
+ * state
+ * @__i: int iteration cursor, for macro-internal use
+ *
+ * This iterates over all DRM DP MST topology managers in an atomic update,
+ * tracking only the new state. This is useful in enable functions, where we
+ * need the new state the hardware should be in when the atomic commit
+ * operation has completed.
+ */
+#define for_each_new_mst_mgr_in_state(__state, mgr, new_state, __i) \
+ for ((__i) = 0; (__i) < (__state)->num_private_objs; (__i)++) \
+ for_each_if(__drm_dp_mst_state_iter_get((__state), &(mgr),
NULL, &(new_state), (__i)))
+
#endif
--
2.17.2
___
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel
[PATCH v2 4/4] drm/nouveau: Use atomic VCPI helpers for MST
Currently, nouveau uses the yolo method of setting up MST displays: it
uses the old VCPI helpers (drm_dp_find_vcpi_slots()) for computing the
display configuration. These helpers don't take care to make sure they
take a reference to the mstb port that they're checking, and
additionally don't actually check whether or not the topology still has
enough bandwidth to provide the VCPI tokens required.
So, drop usage of the old helpers and move entirely over to the atomic
helpers.
Signed-off-by: Lyude Paul
Cc: Daniel Vetter
---
drivers/gpu/drm/nouveau/dispnv50/disp.c | 54 +
1 file changed, 47 insertions(+), 7 deletions(-)
diff --git a/drivers/gpu/drm/nouveau/dispnv50/disp.c
b/drivers/gpu/drm/nouveau/dispnv50/disp.c
index 6cbbae3f438b..37503319e5b1 100644
--- a/drivers/gpu/drm/nouveau/dispnv50/disp.c
+++ b/drivers/gpu/drm/nouveau/dispnv50/disp.c
@@ -747,16 +747,22 @@ nv50_msto_atomic_check(struct drm_encoder *encoder,
struct drm_crtc_state *crtc_state,
struct drm_connector_state *conn_state)
{
- struct nv50_mstc *mstc = nv50_mstc(conn_state->connector);
+ struct drm_atomic_state *state = crtc_state->state;
+ struct drm_connector *connector = conn_state->connector;
+ struct nv50_mstc *mstc = nv50_mstc(connector);
struct nv50_mstm *mstm = mstc->mstm;
- int bpp = conn_state->connector->display_info.bpc * 3;
+ int bpp = connector->display_info.bpc * 3;
int slots;
- mstc->pbn = drm_dp_calc_pbn_mode(crtc_state->adjusted_mode.clock, bpp);
-
- slots = drm_dp_find_vcpi_slots(>mgr, mstc->pbn);
- if (slots < 0)
- return slots;
+ mstc->pbn = drm_dp_calc_pbn_mode(crtc_state->adjusted_mode.clock,
+bpp);
+ /* Zombies don't need VCPI */
+ if (!drm_connector_is_unregistered(connector)) {
+ slots = drm_dp_atomic_find_vcpi_slots(state, >mgr,
+ mstc->port, mstc->pbn);
+ if (slots < 0)
+ return slots;
+ }
return nv50_outp_atomic_check_view(encoder, crtc_state, conn_state,
mstc->native);
@@ -920,12 +926,38 @@ nv50_mstc_get_modes(struct drm_connector *connector)
return ret;
}
+static int
+nv50_mstc_atomic_check(struct drm_connector *connector,
+ struct drm_connector_state *new_conn_state)
+{
+ struct drm_atomic_state *state = new_conn_state->state;
+ struct nv50_mstc *mstc = nv50_mstc(connector);
+ struct drm_dp_mst_topology_mgr *mgr = >mstm->mgr;
+ struct drm_connector_state *old_conn_state;
+ struct drm_crtc *old_crtc;
+
+ old_conn_state = drm_atomic_get_old_connector_state(state, connector);
+ old_crtc = old_conn_state->crtc;
+
+ /* We only need to release VCPI here if we're going from having a CRTC
+* on this connector, to not having one
+*/
+ if (!old_crtc || new_conn_state->crtc)
+ return 0;
+
+ /* Release the previous VCPI allocation since the encoder's
+* atomic_check() won't be called
+*/
+ return drm_dp_atomic_release_vcpi_slots(state, mgr, mstc->port);
+}
+
static const struct drm_connector_helper_funcs
nv50_mstc_help = {
.get_modes = nv50_mstc_get_modes,
.mode_valid = nv50_mstc_mode_valid,
.best_encoder = nv50_mstc_best_encoder,
.atomic_best_encoder = nv50_mstc_atomic_best_encoder,
+ .atomic_check = nv50_mstc_atomic_check,
};
static enum drm_connector_status
@@ -2084,6 +2116,8 @@ nv50_disp_atomic_check(struct drm_device *dev, struct
drm_atomic_state *state)
struct drm_connector *connector;
struct drm_crtc_state *new_crtc_state;
struct drm_crtc *crtc;
+ struct drm_dp_mst_topology_mgr *mgr;
+ struct drm_dp_mst_topology_state *mst_state;
int ret, i;
/* We need to handle colour management on a per-plane basis. */
@@ -2109,6 +2143,12 @@ nv50_disp_atomic_check(struct drm_device *dev, struct
drm_atomic_state *state)
return ret;
}
+ for_each_new_mst_mgr_in_state(state, mgr, mst_state, i) {
+ ret = drm_dp_mst_atomic_check(mst_state);
+ if (ret)
+ return ret;
+ }
+
return 0;
}
--
2.17.2
___
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel
[PATCH v2 3/4] drm/dp_mst: Check payload count in drm_dp_mst_atomic_check()
It occurred to me that we never actually check this! So let's start
doing that.
Signed-off-by: Lyude Paul
Cc: Daniel Vetter
---
drivers/gpu/drm/drm_dp_mst_topology.c | 8 +++-
1 file changed, 7 insertions(+), 1 deletion(-)
diff --git a/drivers/gpu/drm/drm_dp_mst_topology.c
b/drivers/gpu/drm/drm_dp_mst_topology.c
index dcfab7536914..8bb03700e199 100644
--- a/drivers/gpu/drm/drm_dp_mst_topology.c
+++ b/drivers/gpu/drm/drm_dp_mst_topology.c
@@ -3238,7 +3238,7 @@ int drm_dp_mst_atomic_check(struct
drm_dp_mst_topology_state *state)
{
struct drm_dp_mst_topology_mgr *mgr = state->mgr;
struct drm_dp_vcpi_allocation *pos;
- int avail_slots = 63;
+ int avail_slots = 63, payload_count = 0;
list_for_each_entry(pos, >vcpis, next) {
DRM_DEBUG_ATOMIC("[MST PORT:%p] requires %d vcpi slots\n",
@@ -3251,6 +3251,12 @@ int drm_dp_mst_atomic_check(struct
drm_dp_mst_topology_state *state)
avail_slots + pos->vcpi);
return -ENOSPC;
}
+
+ if (++payload_count > mgr->max_payloads) {
+ DRM_DEBUG_ATOMIC("[MST MGR:%p] state %p has too many
payloads (max=%d)\n",
+mgr, state, mgr->max_payloads);
+ return -EINVAL;
+ }
}
DRM_DEBUG_ATOMIC("[MST MGR:%p] state %p vcpi avail=%d used=%d\n",
mgr, state, avail_slots, 63 - avail_slots);
--
2.17.2
___
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel
[PATCH v2 0/4] drm/dp_mst: Improve VCPI helpers, use in nouveau
This patchset does some cleaning up of the atomic VCPI helpers for MST, and converts nouveau over to using them. I would have included amdgpu in this patch as well, but at the moment moving them over to the atomic helpers is nontrivial. Cc: Daniel Vetter Lyude Paul (4): drm/dp_mst: Add some atomic state iterator macros drm/dp_mst: Start tracking per-port VCPI allocations drm/dp_mst: Check payload count in drm_dp_mst_atomic_check() drm/nouveau: Use atomic VCPI helpers for MST drivers/gpu/drm/drm_dp_mst_topology.c | 196 drivers/gpu/drm/i915/intel_display.c| 8 + drivers/gpu/drm/i915/intel_dp_mst.c | 31 ++-- drivers/gpu/drm/nouveau/dispnv50/disp.c | 54 ++- include/drm/drm_dp_mst_helper.h | 88 ++- 5 files changed, 322 insertions(+), 55 deletions(-) -- 2.17.2 ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
[Bug 108096] [amd-staging-drm-next] SDDM screen corruption (not usable) with RX580, amdgpu, dc=1 (of course), regression - [bisected]
https://bugs.freedesktop.org/show_bug.cgi?id=108096 --- Comment #16 from Andrey Grodzovsky --- Question is do you have " winsys/amdgpu: pass the BO list via the CS ioctl on DRM >= 3.27.0" commit in your MESA tree ? I am not clear on that. -- You are receiving this mail because: You are the assignee for the bug.___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [Intel-gfx] [PATCH v3 2/4] drm/i915: Eliminate the horrendous format check code
On Fri, 2018-03-09 at 17:14 +0200, Ville Syrjala wrote:
> From: Ville Syrjälä
>
> Replace the messy framebuffer format/modifier validation code
> with a single call to drm_any_plane_has_format(). The code was
> extremely annoying to maintain as you had to have a lot of platform
> checks for different formats. The new code requires zero maintenance.
>
> v2: Nuke the modifier checks as well since the core does that too now
> v3: Call drm_any_plane_has_format() from the driver code
>
> Signed-off-by: Ville Syrjälä
Patch looks good to me, but does not apply cleanly now.
> ---
> drivers/gpu/drm/i915/intel_display.c | 90
>
> 1 file changed, 8 insertions(+), 82 deletions(-)
>
> diff --git a/drivers/gpu/drm/i915/intel_display.c
> b/drivers/gpu/drm/i915/intel_display.c
> index 2933ad38094f..7f06fa83d894 100644
> --- a/drivers/gpu/drm/i915/intel_display.c
> +++ b/drivers/gpu/drm/i915/intel_display.c
> @@ -13989,7 +13989,6 @@ static int intel_framebuffer_init(struct
> intel_framebuffer *intel_fb,
> {
> struct drm_i915_private *dev_priv = to_i915(obj->base.dev);
> struct drm_framebuffer *fb = _fb->base;
> - struct drm_format_name_buf format_name;
> u32 pitch_limit;
> unsigned int tiling, stride;
> int ret = -EINVAL;
> @@ -14020,33 +14019,14 @@ static int intel_framebuffer_init(struct
> intel_framebuffer *intel_fb,
> }
> }
>
> - /* Passed in modifier sanity checking. */
> - switch (mode_cmd->modifier[0]) {
> - case I915_FORMAT_MOD_Y_TILED_CCS:
> - case I915_FORMAT_MOD_Yf_TILED_CCS:
> - switch (mode_cmd->pixel_format) {
> - case DRM_FORMAT_XBGR:
> - case DRM_FORMAT_ABGR:
> - case DRM_FORMAT_XRGB:
> - case DRM_FORMAT_ARGB:
> - break;
> - default:
> - DRM_DEBUG_KMS("RC supported only with RGB
> formats\n");
> - goto err;
> - }
> - /* fall through */
> - case I915_FORMAT_MOD_Y_TILED:
> - case I915_FORMAT_MOD_Yf_TILED:
> - if (INTEL_GEN(dev_priv) < 9) {
> - DRM_DEBUG_KMS("Unsupported tiling 0x%llx!\n",
> - mode_cmd->modifier[0]);
> - goto err;
> - }
> - case DRM_FORMAT_MOD_LINEAR:
> - case I915_FORMAT_MOD_X_TILED:
> - break;
> - default:
> - DRM_DEBUG_KMS("Unsupported fb modifier 0x%llx!\n",
> + if (!drm_any_plane_has_format(_priv->drm,
> + mode_cmd->pixel_format,
> + mode_cmd->modifier[0])) {
> + struct drm_format_name_buf format_name;
> +
> + DRM_DEBUG_KMS("unsupported pixel format %s / modifier
> 0x%llx\n",
> + drm_get_format_name(mode_cmd-
> >pixel_format,
> + _name),
> mode_cmd->modifier[0]);
> goto err;
> }
> @@ -14081,60 +14061,6 @@ static int intel_framebuffer_init(struct
> intel_framebuffer *intel_fb,
> goto err;
> }
>
> - /* Reject formats not supported by any plane early. */
> - switch (mode_cmd->pixel_format) {
> - case DRM_FORMAT_C8:
> - case DRM_FORMAT_RGB565:
> - case DRM_FORMAT_XRGB:
> - case DRM_FORMAT_ARGB:
> - break;
> - case DRM_FORMAT_XRGB1555:
> - if (INTEL_GEN(dev_priv) > 3) {
> - DRM_DEBUG_KMS("unsupported pixel format: %s\n",
> - drm_get_format_name(mode_cmd-
> >pixel_format, _name));
> - goto err;
> - }
> - break;
> - case DRM_FORMAT_ABGR:
> - if (!IS_VALLEYVIEW(dev_priv) &&
> !IS_CHERRYVIEW(dev_priv) &&
> - INTEL_GEN(dev_priv) < 9) {
> - DRM_DEBUG_KMS("unsupported pixel format: %s\n",
> - drm_get_format_name(mode_cmd-
> >pixel_format, _name));
> - goto err;
> - }
> - break;
> - case DRM_FORMAT_XBGR:
> - case DRM_FORMAT_XRGB2101010:
> - case DRM_FORMAT_XBGR2101010:
> - if (INTEL_GEN(dev_priv) < 4) {
> - DRM_DEBUG_KMS("unsupported pixel format: %s\n",
> - drm_get_format_name(mode_cmd-
> >pixel_format, _name));
> - goto err;
> - }
> - break;
> - case DRM_FORMAT_ABGR2101010:
> - if (!IS_VALLEYVIEW(dev_priv) &&
> !IS_CHERRYVIEW(dev_priv)) {
> - DRM_DEBUG_KMS("unsupported pixel format: %s\n",
> - drm_get_format_name(mode_cmd-
> >pixel_format, _name));
> - goto err;
> - }
> - break;
> - case DRM_FORMAT_YUYV:
> -
Re: [PATCH v4 3/4] drm: Document variable refresh properties
On 10/26/18 3:13 PM, Manasi Navare wrote: > On Fri, Oct 26, 2018 at 05:34:15PM +, Kazlauskas, Nicholas wrote: >> On 10/26/18 10:53 AM, Ville Syrjälä wrote: >>> On Fri, Oct 26, 2018 at 02:49:31PM +, Kazlauskas, Nicholas wrote: On 10/26/18 7:37 AM, Pekka Paalanen wrote: > On Thu, 11 Oct 2018 12:39:41 -0400 > Nicholas Kazlauskas wrote: > >> These include the drm_connector 'vrr_capable' and the drm_crtc >> 'vrr_enabled' properties. >> >> Signed-off-by: Nicholas Kazlauskas >> --- >> Documentation/gpu/drm-kms.rst | 7 +++ >> drivers/gpu/drm/drm_connector.c | 22 ++ >> 2 files changed, 29 insertions(+) >> >> diff --git a/Documentation/gpu/drm-kms.rst >> b/Documentation/gpu/drm-kms.rst >> index 4b1501b4835b..8da2a178cf85 100644 >> --- a/Documentation/gpu/drm-kms.rst >> +++ b/Documentation/gpu/drm-kms.rst >> @@ -575,6 +575,13 @@ Explicit Fencing Properties >> .. kernel-doc:: drivers/gpu/drm/drm_atomic_uapi.c >>:doc: explicit fencing properties >> >> + >> +Variable Refresh Properties >> +--- >> + >> +.. kernel-doc:: drivers/gpu/drm/drm_connector.c >> + :doc: Variable refresh properties >> + >> Existing KMS Properties >> --- >> >> diff --git a/drivers/gpu/drm/drm_connector.c >> b/drivers/gpu/drm/drm_connector.c >> index f0deeb7298d0..2a12853ca917 100644 >> --- a/drivers/gpu/drm/drm_connector.c >> +++ b/drivers/gpu/drm/drm_connector.c >> @@ -1254,6 +1254,28 @@ int drm_mode_create_scaling_mode_property(struct >> drm_device *dev) >> } >> EXPORT_SYMBOL(drm_mode_create_scaling_mode_property); >> >> +/** >> + * DOC: Variable refresh properties >> + * >> + * Variable refresh rate control is supported via properties on the >> + * _connector and _crtc objects. >> + * >> + * "vrr_capable": >> + * Optional _connector boolean property that drivers should >> attach >> + * with drm_connector_attach_vrr_capable_property() on connectors >> that >> + * could support variable refresh rates. Drivers should update the >> + * property value by calling >> drm_connector_set_vrr_capable_property(). >> + * >> + * Absence of the property should indicate absence of support. >> + * >> + * "vrr_enabled": >> + * Default _crtc boolean property that notifies the driver >> that the >> + * variable refresh rate adjustment should be enabled for the CRTC. > > Hi, > > where is the documentation that explains how drivers must implement > "variable refresh rate adjustment"? > > What should and could userspace expect to get if it flicks this switch? > > I also think the kernel documentation should include a description of > what VRR actually is and how it conceptually works as far as userspace > is concerned. > > That is, the kernel documentation should describe what this thing does, > so that we avoid every driver implementing a different thing. For > example, one driver could prevent the luminance flickering itself by > tuning the timings while another driver might not do that, which means > that an application tested on the former driver will look just fine > while it is unbearable to watch on the latter. > > > Thanks, > pq I felt it was best to leave this more on the vague side to not impose restrictions yet on what a driver must do. If you think it's worth defining what the "baseline" expectation is for userspace, I would probably describe it as "utilizing the monitor's variable refresh rate to reduce stuttering or tearing that can occur during flipping". This is currently what the amdgpu driver has enabled > > I would also mention that without VRR, the display engine processes the flips > independently of the rendering speed which might result into stuttering or > tearing. > > Might also be worth giving a quick example with numbers in the documentation. > Something like: For Eg if the rendering speed is 40Hz, without VRR, display > engine > will flip at constant 60Hz, which means that same frame will be displayed > twice which > will be observed as stutter/repetition. > With VRR enabled, the vertical front porch will be extended and the flip will > be processed when its ready or at max blanking time resulting a smooth > transition without repetition. Good points about mentioning the problems it solves in the documentation. > for support. The implementation also isn't much more complex than just passing the variable refresh range to the hardware. I wouldn't want every driver to be forced to implement some sort of luminance flickering by default. It's not noticeable on
Re: [Intel-gfx] [PATCH v3 1/4] drm: Add drm_any_plane_has_format()
On Fri, 2018-03-09 at 17:14 +0200, Ville Syrjala wrote:
> From: Ville Syrjälä
>
> Add a function to check whether there is at least one plane that
> supports a specific format and modifier combination. Drivers can
> use this to reject unsupported formats/modifiers in .fb_create().
>
> v2: Accept anyformat if the driver doesn't do planes (Eric)
> s/planes_have_format/any_plane_has_format/ (Eric)
> Check the modifier as well since we already have a function
> that does both
> v3: Don't do the check in the core since we may not know the
> modifier yet, instead export the function and let drivers
> call it themselves
>
> Cc: Eric Anholt
> Signed-off-by: Ville Syrjälä
I ended up writing a similar patch for i915. Having this in the core
seems better and patch still applies cleanly.
Reviewed-by: Dhinakaran Pandiyan
> ---
> drivers/gpu/drm/drm_plane.c | 23 +++
> include/drm/drm_mode_config.h | 6 ++
> include/drm/drm_plane.h | 2 ++
> 3 files changed, 31 insertions(+)
>
> diff --git a/drivers/gpu/drm/drm_plane.c
> b/drivers/gpu/drm/drm_plane.c
> index a5d1fc7e8a37..3b2d6f8d889d 100644
> --- a/drivers/gpu/drm/drm_plane.c
> +++ b/drivers/gpu/drm/drm_plane.c
> @@ -578,6 +578,29 @@ int drm_plane_check_pixel_format(struct
> drm_plane *plane,
> return 0;
> }
>
> +/**
> + * drm_any_plane_has_format - Check whether any plane supports this
> format and modifier combination
> + * @dev: DRM device
> + * @format: pixel format (DRM_FORMAT_*)
> + * @modifier: data layout modifier
> + *
> + * Returns:
> + * Whether at least one plane supports the specified format and
> modifier combination.
> + */
> +bool drm_any_plane_has_format(struct drm_device *dev,
> + u32 format, u64 modifier)
> +{
> + struct drm_plane *plane;
> +
> + drm_for_each_plane(plane, dev) {
> + if (drm_plane_check_pixel_format(plane, format,
> modifier) == 0)
> + return true;
> + }
> +
> + return false;
> +}
> +EXPORT_SYMBOL(drm_any_plane_has_format);
> +
> /*
> * __setplane_internal - setplane handler for internal callers
> *
> diff --git a/include/drm/drm_mode_config.h
> b/include/drm/drm_mode_config.h
> index 7569f22ffef6..9b894de9a75d 100644
> --- a/include/drm/drm_mode_config.h
> +++ b/include/drm/drm_mode_config.h
> @@ -52,6 +52,12 @@ struct drm_mode_config_funcs {
>* requested metadata, but most of that is left to the driver.
> See
>* drm_mode_fb_cmd2 for details.
>*
> + * To validate the pixel format and modifier drivers can use
> + * drm_any_plane_has_format() to make sure at least one plane
> supports
> + * the requested values. Note that the driver must first
> determine the
> + * actual modifier used if the request doesn't have it
> specified,
> + * ie. when (@mode_cmd->flags & DRM_MODE_FB_MODIFIERS) == 0.
> + *
>* If the parameters are deemed valid and the backing storage
> objects in
>* the underlying memory manager all exist, then the driver
> allocates
>* a new _framebuffer structure, subclassed to contain
> diff --git a/include/drm/drm_plane.h b/include/drm/drm_plane.h
> index f7bf4a48b1c3..930e8fdd90f8 100644
> --- a/include/drm/drm_plane.h
> +++ b/include/drm/drm_plane.h
> @@ -683,5 +683,7 @@ static inline struct drm_plane
> *drm_plane_find(struct drm_device *dev,
> #define drm_for_each_plane(plane, dev) \
> list_for_each_entry(plane, &(dev)->mode_config.plane_list,
> head)
>
> +bool drm_any_plane_has_format(struct drm_device *dev,
> + u32 format, u64 modifier);
>
> #endif
___
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel
[PATCH 6/6] drm/qxl: Use 'unsigned int' instead of 'bool'
Use 'unsigned int' with bitfield instead of 'bool' to avoid alignment
issues and remove checkpatch.pl check:
CHECK: Avoid using bool structure members because of possible alignment
issues
Signed-off-by: Shayenne da Luz Moura
---
drivers/gpu/drm/qxl/qxl_drv.h | 10 +-
1 file changed, 5 insertions(+), 5 deletions(-)
diff --git a/drivers/gpu/drm/qxl/qxl_drv.h b/drivers/gpu/drm/qxl/qxl_drv.h
index d78bcb95df3e..14d3fa855708 100644
--- a/drivers/gpu/drm/qxl/qxl_drv.h
+++ b/drivers/gpu/drm/qxl/qxl_drv.h
@@ -88,10 +88,10 @@ struct qxl_bo {
/* Constant after initialization */
struct drm_gem_object gem_base;
- bool is_primary; /* is this now a primary surface */
- bool is_dumb;
+ unsigned int is_primary:1; /* is this now a primary surface */
+ unsigned int is_dumb:1;
struct qxl_bo *shadow;
- bool hw_surf_alloc;
+ unsigned int hw_surf_alloc:1;
struct qxl_surface surf;
uint32_t surface_id;
struct qxl_release *surf_create;
@@ -128,7 +128,7 @@ struct qxl_output {
struct qxl_mman {
struct ttm_bo_global_refbo_global_ref;
struct drm_global_reference mem_global_ref;
- boolmem_global_referenced;
+ unsigned int mem_global_referenced:1;
struct ttm_bo_devicebdev;
};
@@ -229,7 +229,7 @@ struct qxl_device {
struct qxl_ram_header *ram_header;
- bool primary_created;
+ unsigned int primary_created:1;
struct qxl_memslot *mem_slots;
uint8_t n_mem_slots;
--
2.19.1
___
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel
[PATCH 5/6] drm/qxl: Add space before open parentheses
Add space to remove checkpath.pl error:
ERROR: space required before the open parenthesis '('
Signed-off-by: Shayenne da Luz Moura
---
drivers/gpu/drm/qxl/qxl_display.c | 4 ++--
1 file changed, 2 insertions(+), 2 deletions(-)
diff --git a/drivers/gpu/drm/qxl/qxl_display.c
b/drivers/gpu/drm/qxl/qxl_display.c
index 5b00e0f26de1..2ce9a8dcec84 100644
--- a/drivers/gpu/drm/qxl/qxl_display.c
+++ b/drivers/gpu/drm/qxl/qxl_display.c
@@ -940,8 +940,8 @@ static enum drm_mode_status qxl_conn_mode_valid(struct
drm_connector *connector,
/* TODO: is this called for user defined modes? (xrandr --add-mode)
* TODO: check that the mode fits in the framebuffer */
- if(qdev->monitors_config_width == mode->hdisplay &&
- qdev->monitors_config_height == mode->vdisplay)
+ if (qdev->monitors_config_width == mode->hdisplay &&
+ qdev->monitors_config_height == mode->vdisplay)
return MODE_OK;
for (i = 0; i < ARRAY_SIZE(common_modes); i++) {
--
2.19.1
___
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel
[PATCH 4/6] drm/qxl: Use 'unsigned int' instead of 'usigned'
Use 'usigned int' instead of 'usigned' to remove the checkpath.pl warning:
WARNING: Prefer 'unsigned int' to bare use of 'unsigned'
Signed-off-by: Shayenne da Luz Moura
---
drivers/gpu/drm/qxl/qxl_cmd.c | 2 +-
drivers/gpu/drm/qxl/qxl_debugfs.c | 4 ++--
drivers/gpu/drm/qxl/qxl_display.c | 12 ++--
drivers/gpu/drm/qxl/qxl_draw.c| 8
drivers/gpu/drm/qxl/qxl_drv.h | 18 +-
drivers/gpu/drm/qxl/qxl_fb.c | 4 ++--
drivers/gpu/drm/qxl/qxl_image.c | 2 +-
drivers/gpu/drm/qxl/qxl_object.c | 2 +-
drivers/gpu/drm/qxl/qxl_ttm.c | 10 +-
9 files changed, 31 insertions(+), 31 deletions(-)
diff --git a/drivers/gpu/drm/qxl/qxl_cmd.c b/drivers/gpu/drm/qxl/qxl_cmd.c
index 6ec4b84a6bd4..dffc5093ff16 100644
--- a/drivers/gpu/drm/qxl/qxl_cmd.c
+++ b/drivers/gpu/drm/qxl/qxl_cmd.c
@@ -376,7 +376,7 @@ void qxl_io_destroy_primary(struct qxl_device *qdev)
}
void qxl_io_create_primary(struct qxl_device *qdev,
- unsigned offset, struct qxl_bo *bo)
+ unsigned int offset, struct qxl_bo *bo)
{
struct qxl_surface_create *create;
diff --git a/drivers/gpu/drm/qxl/qxl_debugfs.c
b/drivers/gpu/drm/qxl/qxl_debugfs.c
index 8c35b3433f00..118422549828 100644
--- a/drivers/gpu/drm/qxl/qxl_debugfs.c
+++ b/drivers/gpu/drm/qxl/qxl_debugfs.c
@@ -101,9 +101,9 @@ qxl_debugfs_init(struct drm_minor *minor)
int qxl_debugfs_add_files(struct qxl_device *qdev,
struct drm_info_list *files,
- unsigned nfiles)
+ unsigned int nfiles)
{
- unsigned i;
+ unsigned int i;
for (i = 0; i < qdev->debugfs_count; i++) {
if (qdev->debugfs[i].files == files) {
diff --git a/drivers/gpu/drm/qxl/qxl_display.c
b/drivers/gpu/drm/qxl/qxl_display.c
index f88dbfa4656a..5b00e0f26de1 100644
--- a/drivers/gpu/drm/qxl/qxl_display.c
+++ b/drivers/gpu/drm/qxl/qxl_display.c
@@ -253,8 +253,8 @@ static struct mode_size {
};
static int qxl_add_common_modes(struct drm_connector *connector,
-unsigned pwidth,
-unsigned pheight)
+unsigned int pwidth,
+unsigned int pheight)
{
struct drm_device *dev = connector->dev;
struct drm_display_mode *mode = NULL;
@@ -393,9 +393,9 @@ static const struct drm_crtc_funcs qxl_crtc_funcs = {
static int qxl_framebuffer_surface_dirty(struct drm_framebuffer *fb,
struct drm_file *file_priv,
-unsigned flags, unsigned color,
+unsigned int flags, unsigned int color,
struct drm_clip_rect *clips,
-unsigned num_clips)
+unsigned int num_clips)
{
/* TODO: vmwgfx where this was cribbed from had locking. Why? */
struct qxl_device *qdev = fb->dev->dev_private;
@@ -919,8 +919,8 @@ static int qdev_crtc_init(struct drm_device *dev, int
crtc_id)
static int qxl_conn_get_modes(struct drm_connector *connector)
{
- unsigned pwidth = 1024;
- unsigned pheight = 768;
+ unsigned int pwidth = 1024;
+ unsigned int pheight = 768;
int ret = 0;
ret = qxl_add_monitors_config_modes(connector, , );
diff --git a/drivers/gpu/drm/qxl/qxl_draw.c b/drivers/gpu/drm/qxl/qxl_draw.c
index a41d48eb1374..c34e45662965 100644
--- a/drivers/gpu/drm/qxl/qxl_draw.c
+++ b/drivers/gpu/drm/qxl/qxl_draw.c
@@ -25,7 +25,7 @@
static int alloc_clips(struct qxl_device *qdev,
struct qxl_release *release,
- unsigned num_clips,
+ unsigned int num_clips,
struct qxl_bo **clips_bo)
{
int size = sizeof(struct qxl_clip_rects) + sizeof(struct qxl_rect) *
num_clips;
@@ -37,7 +37,7 @@ static int alloc_clips(struct qxl_device *qdev,
* the qxl_clip_rects. This is *not* the same as the memory allocated
* on the device, it is offset to qxl_clip_rects.chunk.data */
static struct qxl_rect *drawable_set_clipping(struct qxl_device *qdev,
- unsigned num_clips,
+ unsigned int num_clips,
struct qxl_bo *clips_bo)
{
struct qxl_clip_rects *dev_clips;
@@ -266,9 +266,9 @@ void qxl_draw_opaque_fb(const struct qxl_fb_image
*qxl_fb_image,
void qxl_draw_dirty_fb(struct qxl_device *qdev,
struct drm_framebuffer *fb,
struct qxl_bo *bo,
- unsigned flags, unsigned color,
+ unsigned int flags, unsigned int color,
struct drm_clip_rect *clips,
- unsigned num_clips, int inc)
+
[PATCH 3/6] drm/qxl: Remove exceding whiteline
Remove extra whiteline to clean the checkpatch.pl check:
CHECK: Please don't use multiple blank lines
Signed-off-by: Shayenne da Luz Moura
---
drivers/gpu/drm/qxl/qxl_cmd.c | 1 -
drivers/gpu/drm/qxl/qxl_debugfs.c | 1 -
drivers/gpu/drm/qxl/qxl_dev.h | 1 -
drivers/gpu/drm/qxl/qxl_display.c | 1 -
drivers/gpu/drm/qxl/qxl_draw.c| 1 -
drivers/gpu/drm/qxl/qxl_drv.h | 3 ---
drivers/gpu/drm/qxl/qxl_kms.c | 1 -
drivers/gpu/drm/qxl/qxl_object.c | 2 --
drivers/gpu/drm/qxl/qxl_prime.c | 1 -
drivers/gpu/drm/qxl/qxl_release.c | 1 -
drivers/gpu/drm/qxl/qxl_ttm.c | 2 --
11 files changed, 15 deletions(-)
diff --git a/drivers/gpu/drm/qxl/qxl_cmd.c b/drivers/gpu/drm/qxl/qxl_cmd.c
index 25ec8e6544ee..6ec4b84a6bd4 100644
--- a/drivers/gpu/drm/qxl/qxl_cmd.c
+++ b/drivers/gpu/drm/qxl/qxl_cmd.c
@@ -369,7 +369,6 @@ void qxl_io_flush_surfaces(struct qxl_device *qdev)
wait_for_io_cmd(qdev, 0, QXL_IO_FLUSH_SURFACES_ASYNC);
}
-
void qxl_io_destroy_primary(struct qxl_device *qdev)
{
wait_for_io_cmd(qdev, 0, QXL_IO_DESTROY_PRIMARY_ASYNC);
diff --git a/drivers/gpu/drm/qxl/qxl_debugfs.c
b/drivers/gpu/drm/qxl/qxl_debugfs.c
index 15c84068d3fb..8c35b3433f00 100644
--- a/drivers/gpu/drm/qxl/qxl_debugfs.c
+++ b/drivers/gpu/drm/qxl/qxl_debugfs.c
@@ -34,7 +34,6 @@
#include "qxl_drv.h"
#include "qxl_object.h"
-
#if defined(CONFIG_DEBUG_FS)
static int
qxl_debugfs_irq_received(struct seq_file *m, void *data)
diff --git a/drivers/gpu/drm/qxl/qxl_dev.h b/drivers/gpu/drm/qxl/qxl_dev.h
index 94c5aec71920..a0ee41632d7e 100644
--- a/drivers/gpu/drm/qxl/qxl_dev.h
+++ b/drivers/gpu/drm/qxl/qxl_dev.h
@@ -28,7 +28,6 @@
OF THIS SOFTWARE, EVEN IF ADVISED OF THE POSSIBILITY OF SUCH DAMAGE.
*/
-
#ifndef H_QXL_DEV
#define H_QXL_DEV
diff --git a/drivers/gpu/drm/qxl/qxl_display.c
b/drivers/gpu/drm/qxl/qxl_display.c
index b7421dcdeeb6..f88dbfa4656a 100644
--- a/drivers/gpu/drm/qxl/qxl_display.c
+++ b/drivers/gpu/drm/qxl/qxl_display.c
@@ -960,7 +960,6 @@ static struct drm_encoder *qxl_best_encoder(struct
drm_connector *connector)
return _output->enc;
}
-
static const struct drm_encoder_helper_funcs qxl_enc_helper_funcs = {
};
diff --git a/drivers/gpu/drm/qxl/qxl_draw.c b/drivers/gpu/drm/qxl/qxl_draw.c
index ed08e9ec4827..a41d48eb1374 100644
--- a/drivers/gpu/drm/qxl/qxl_draw.c
+++ b/drivers/gpu/drm/qxl/qxl_draw.c
@@ -342,7 +342,6 @@ void qxl_draw_dirty_fb(struct qxl_device *qdev,
if (ret)
goto out_release_backoff;
-
ret = qxl_image_init(qdev, release, dimage, surface_base,
left, top, width, height, depth, stride);
qxl_bo_kunmap(bo);
diff --git a/drivers/gpu/drm/qxl/qxl_drv.h b/drivers/gpu/drm/qxl/qxl_drv.h
index 8ff70a7281a7..4b90f9bd7280 100644
--- a/drivers/gpu/drm/qxl/qxl_drv.h
+++ b/drivers/gpu/drm/qxl/qxl_drv.h
@@ -23,7 +23,6 @@
* Alon Levy
*/
-
#ifndef QXL_DRV_H
#define QXL_DRV_H
@@ -133,7 +132,6 @@ struct qxl_mman {
struct ttm_bo_devicebdev;
};
-
struct qxl_memslot {
uint8_t generation;
uint64_tstart_phys_addr;
@@ -372,7 +370,6 @@ int qxl_mode_dumb_mmap(struct drm_file *filp,
struct drm_device *dev,
uint32_t handle, uint64_t *offset_p);
-
/* qxl ttm */
int qxl_ttm_init(struct qxl_device *qdev);
void qxl_ttm_fini(struct qxl_device *qdev);
diff --git a/drivers/gpu/drm/qxl/qxl_kms.c b/drivers/gpu/drm/qxl/qxl_kms.c
index d1ba0ab1be82..f6975d7c7d10 100644
--- a/drivers/gpu/drm/qxl/qxl_kms.c
+++ b/drivers/gpu/drm/qxl/qxl_kms.c
@@ -285,7 +285,6 @@ int qxl_device_init(struct qxl_device *qdev,
(unsigned long)qdev->surfaceram_base,
(unsigned long)qdev->surfaceram_size);
-
INIT_WORK(>gc_work, qxl_gc_work);
return 0;
diff --git a/drivers/gpu/drm/qxl/qxl_object.c b/drivers/gpu/drm/qxl/qxl_object.c
index e1f2404b8f6f..18e67903b01b 100644
--- a/drivers/gpu/drm/qxl/qxl_object.c
+++ b/drivers/gpu/drm/qxl/qxl_object.c
@@ -74,7 +74,6 @@ void qxl_ttm_placement_from_domain(struct qxl_bo *qbo, u32
domain, bool pinned)
}
}
-
int qxl_bo_create(struct qxl_device *qdev,
unsigned long size, bool kernel, bool pinned, u32 domain,
struct qxl_surface *surf,
@@ -266,7 +265,6 @@ static int __qxl_bo_unpin(struct qxl_bo *bo)
return r;
}
-
/*
* Reserve the BO before pinning the object. If the BO was reserved
* beforehand, use the internal version directly __qxl_bo_pin.
diff --git a/drivers/gpu/drm/qxl/qxl_prime.c b/drivers/gpu/drm/qxl/qxl_prime.c
index 9f029dda1f07..a55dece118b2 100644
--- a/drivers/gpu/drm/qxl/qxl_prime.c
+++ b/drivers/gpu/drm/qxl/qxl_prime.c
@@ -38,7 +38,6 @@ void qxl_gem_prime_unpin(struct drm_gem_object *obj)
WARN_ONCE(1, "not implemented");
}
-
struct sg_table *qxl_gem_prime_get_sg_table(struct drm_gem_object *obj)
{
WARN_ONCE(1, "not
[PATCH 2/6] drm/qxl: Add line after variable declarations
Add whiteline after variable declarations to remove the checkpath.pl
warning:
WARNING: Missing a blank line after declarations
Signed-off-by: Shayenne da Luz Moura
---
drivers/gpu/drm/qxl/qxl_cmd.c | 4
drivers/gpu/drm/qxl/qxl_display.c | 2 ++
drivers/gpu/drm/qxl/qxl_draw.c| 2 ++
drivers/gpu/drm/qxl/qxl_dumb.c| 1 +
drivers/gpu/drm/qxl/qxl_image.c | 2 ++
drivers/gpu/drm/qxl/qxl_ioctl.c | 2 ++
drivers/gpu/drm/qxl/qxl_kms.c | 1 +
drivers/gpu/drm/qxl/qxl_object.c | 1 +
drivers/gpu/drm/qxl/qxl_object.h | 2 ++
9 files changed, 17 insertions(+)
diff --git a/drivers/gpu/drm/qxl/qxl_cmd.c b/drivers/gpu/drm/qxl/qxl_cmd.c
index 208af9f37914..25ec8e6544ee 100644
--- a/drivers/gpu/drm/qxl/qxl_cmd.c
+++ b/drivers/gpu/drm/qxl/qxl_cmd.c
@@ -84,6 +84,7 @@ static int qxl_check_header(struct qxl_ring *ring)
int ret;
struct qxl_ring_header *header = &(ring->ring->header);
unsigned long flags;
+
spin_lock_irqsave(>lock, flags);
ret = header->prod - header->cons < header->num_items;
if (ret == 0)
@@ -97,6 +98,7 @@ int qxl_check_idle(struct qxl_ring *ring)
int ret;
struct qxl_ring_header *header = &(ring->ring->header);
unsigned long flags;
+
spin_lock_irqsave(>lock, flags);
ret = header->prod == header->cons;
spin_unlock_irqrestore(>lock, flags);
@@ -110,6 +112,7 @@ int qxl_ring_push(struct qxl_ring *ring,
uint8_t *elt;
int idx, ret;
unsigned long flags;
+
spin_lock_irqsave(>lock, flags);
if (header->prod - header->cons == header->num_items) {
header->notify_on_cons = header->cons + 1;
@@ -156,6 +159,7 @@ static bool qxl_ring_pop(struct qxl_ring *ring,
volatile uint8_t *ring_elt;
int idx;
unsigned long flags;
+
spin_lock_irqsave(>lock, flags);
if (header->cons == header->prod) {
header->notify_on_prod = header->cons + 1;
diff --git a/drivers/gpu/drm/qxl/qxl_display.c
b/drivers/gpu/drm/qxl/qxl_display.c
index 87d16a0ce01e..b7421dcdeeb6 100644
--- a/drivers/gpu/drm/qxl/qxl_display.c
+++ b/drivers/gpu/drm/qxl/qxl_display.c
@@ -259,6 +259,7 @@ static int qxl_add_common_modes(struct drm_connector
*connector,
struct drm_device *dev = connector->dev;
struct drm_display_mode *mode = NULL;
int i;
+
for (i = 0; i < ARRAY_SIZE(common_modes); i++) {
mode = drm_cvt_mode(dev, common_modes[i].w, common_modes[i].h,
60, false, false, false);
@@ -315,6 +316,7 @@ static void qxl_crtc_update_monitors_config(struct drm_crtc
*crtc,
oldcount = qdev->monitors_config->count;
if (crtc->state->active) {
struct drm_display_mode *mode = >mode;
+
head.width = mode->hdisplay;
head.height = mode->vdisplay;
head.x = crtc->x;
diff --git a/drivers/gpu/drm/qxl/qxl_draw.c b/drivers/gpu/drm/qxl/qxl_draw.c
index cc5b32e749ce..ed08e9ec4827 100644
--- a/drivers/gpu/drm/qxl/qxl_draw.c
+++ b/drivers/gpu/drm/qxl/qxl_draw.c
@@ -168,6 +168,7 @@ void qxl_draw_opaque_fb(const struct qxl_fb_image
*qxl_fb_image,
int ret;
struct qxl_drm_image *dimage;
struct qxl_bo *palette_bo = NULL;
+
if (stride == 0)
stride = depth * width / 8;
@@ -214,6 +215,7 @@ void qxl_draw_opaque_fb(const struct qxl_fb_image
*qxl_fb_image,
if (depth == 1) {
void *ptr;
+
ret = qxl_palette_create_1bit(palette_bo, release,
qxl_fb_image);
ptr = qxl_bo_kmap_atomic_page(qdev, dimage->bo, 0);
diff --git a/drivers/gpu/drm/qxl/qxl_dumb.c b/drivers/gpu/drm/qxl/qxl_dumb.c
index 089e5fcf80e0..e3765739c396 100644
--- a/drivers/gpu/drm/qxl/qxl_dumb.c
+++ b/drivers/gpu/drm/qxl/qxl_dumb.c
@@ -38,6 +38,7 @@ int qxl_mode_dumb_create(struct drm_file *file_priv,
int r;
struct qxl_surface surf;
uint32_t pitch, format;
+
pitch = args->width * ((args->bpp + 1) / 8);
args->size = pitch * args->height;
args->size = ALIGN(args->size, PAGE_SIZE);
diff --git a/drivers/gpu/drm/qxl/qxl_image.c b/drivers/gpu/drm/qxl/qxl_image.c
index 7fbcc35e8ad3..13b9a18ccde5 100644
--- a/drivers/gpu/drm/qxl/qxl_image.c
+++ b/drivers/gpu/drm/qxl/qxl_image.c
@@ -136,6 +136,7 @@ qxl_image_init_helper(struct qxl_device *qdev,
int remain;
int page;
int size;
+
if (stride == linesize && chunk_stride == stride) {
remain = linesize * height;
page = 0;
@@ -163,6 +164,7 @@ qxl_image_init_helper(struct qxl_device *qdev,
}
} else {
unsigned page_base, page_offset, out_offset;
+
for (i = 0 ; i < height ; ++i) {
i_data = (void *)data + i * stride;
[PATCH 1/6] drm/qxl: Remove trailing whitespace
Remove extra tab and space to clean the checkpath.pl error. ERROR: trailing whitespace Signed-off-by: Shayenne da Luz Moura --- drivers/gpu/drm/qxl/qxl_dumb.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/drivers/gpu/drm/qxl/qxl_dumb.c b/drivers/gpu/drm/qxl/qxl_dumb.c index c666b89eed5d..089e5fcf80e0 100644 --- a/drivers/gpu/drm/qxl/qxl_dumb.c +++ b/drivers/gpu/drm/qxl/qxl_dumb.c @@ -52,7 +52,7 @@ int qxl_mode_dumb_create(struct drm_file *file_priv, default: return -EINVAL; } - + surf.width = args->width; surf.height = args->height; surf.stride = pitch; -- 2.19.1 ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
[PATCH 0/6] drm/qxl: Remove checkpatch issues
This series cleans the following checkpatch.pl issues:
ERROR: trailing whitespace
WARNING: Missing a blank line after declarations
CHECK: Please don't use multiple blank lines
WARNING: Prefer 'unsigned int' to bare use of 'unsigned'
ERROR: space required before the open parenthesis '('
CHECK: Avoid using bool structure members because of possible alignment issues
Shayenne da Luz Moura (6):
drm/qxl: Remove trailing whitespace
drm/qxl: Add line after variable declarations
drm/qxl: Remove exceding whiteline
drm/qxl: Use 'unsigned int' instead of 'usigned'
drm/qxl: Add space before open parentheses
drm/qxl: Use 'unsigned int' instead of bool
drivers/gpu/drm/qxl/qxl_cmd.c | 7 +--
drivers/gpu/drm/qxl/qxl_debugfs.c | 5 ++---
drivers/gpu/drm/qxl/qxl_dev.h | 1 -
drivers/gpu/drm/qxl/qxl_display.c | 19 ++-
drivers/gpu/drm/qxl/qxl_draw.c| 11 ++-
drivers/gpu/drm/qxl/qxl_drv.h | 31 ++-
drivers/gpu/drm/qxl/qxl_dumb.c| 3 ++-
drivers/gpu/drm/qxl/qxl_fb.c | 4 ++--
drivers/gpu/drm/qxl/qxl_image.c | 4 +++-
drivers/gpu/drm/qxl/qxl_ioctl.c | 2 ++
drivers/gpu/drm/qxl/qxl_kms.c | 2 +-
drivers/gpu/drm/qxl/qxl_object.c | 5 ++---
drivers/gpu/drm/qxl/qxl_object.h | 2 ++
drivers/gpu/drm/qxl/qxl_prime.c | 1 -
drivers/gpu/drm/qxl/qxl_release.c | 1 -
drivers/gpu/drm/qxl/qxl_ttm.c | 12 +---
16 files changed, 56 insertions(+), 54 deletions(-)
--
2.19.1
___
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [PATCH 0/3] drm: tinydrm driver for adafruit PiTFT 3.5" touchscreen
Den 26.10.2018 04.30, skrev Eric Anholt: Noralf Trønnes writes: Den 25.10.2018 18.29, skrev Eric Anholt: Eric Anholt writes: I was going to start working on making the vc4 driver work with tinydrm panels, but it turned out tinydrm didn't have the panel I had previously bought. So, last night I ported the fbtft staging driver over to DRM. It seems to work (with DT at https://github.com/anholt/linux/commits/drm-misc-next-hx8357d) -- fbdev works great including rotated, and so does modetest. However, when X11 comes up at 16bpp, I get: https://photos.app.goo.gl/8tuhzPFFoDGamEfk8 If I have tinydrm set a preferred bpp of 24, X looks great. Noralf, any ideas? Also, with these patches and the format modifier patch I just sent, mesa with vc4 is now working with this driver on this branch: https://gitlab.freedesktop.org/anholt/mesa/commits/kmsro Ah, nice to see this happening! Getting hw rendering was one of the advantages I saw DRM could provide over fbdev on these displays. Little did I know how complicated graphics was outside fbdev, so I was unable to realise this myself. The current solution to get hw rendering is to have a userspace process that continously copies the framebuffer: https://github.com/tasanakorn/rpi-fbcp It's used by some of the small DIY handheld game consoles that run emulators which requires hw rendering. Now I wonder how we can improve performance of the SPI updates. At what SPI speed are you running? The datasheet for most of these display controllers list the max speed as 10MHz, but almost all of them can go faster. Some are reported going as high as 70-80MHz. That's for the pixel data transfer, not the commands. tinydrm/mipi-dbi.c sends commands at 10MHz and pixels at full speed (mipi_dbi_spi_cmd_max_speed()). Most panels I have run at 32MHz or 48MHz. I copied the DT from the adafruit tree, which has it at 32mhz. System performance seems to be limited by the copy and format conversion I think -- in particular, I wonder if we shouldn't be doing our dirty copies in our own workqueue. I haven't managed to get any really good profiling data yet, though. glxgears at 128x128 is nice and smooth, and at 480x320 it's 6fps. That's not filling 32mhz of SPI. On the other hand, I would have expected the uncached reads for the 4-to-2 swapped conversion to be able to go faster than 3.5mb/sec. If it's the uncached reads, we could at least use NEON for the copy to cached, and probably even do the whole conversion in NEON with a bit more thought. Another option: use a vc4 RCL to do RGBA to RGB565, since that will be less pressure on the bus. But then, I suppose I should just figure out what's going on that makes X11 at RGBA break, and fix that so we don't even have to do that conversion. I keep hoping there's some way we could feed output from the DISPSLAVE HVS register directly to the SPI master -- FIFO32 gets us close (two 16-bit pixels packed next to each other, leftmost in the lower 2 bytes), but the need for byte swapping (as opposed to R/B swapping) I think makes it impossible. I just did some speed tests on a 320x240 display at 3 different speeds. I also tried with byteswapping disabled. Only full updates will benefit from passing the buffer straight through to SPI. This is because partial updates are copied to a transfer buffer anyways to minimize SPI transfer time. No need to transfer things that haven't changed and a memory copy is far cheaper than a SPI transfer. SPI at 48MHz: pi@pi2835:~$ od -An -vtu4 --endian=big /sys/bus/spi/devices/spi0.0/of_node/spi-max-frequency 4800 pi@pi2835:~$ ./libdrm/tests/modetest/modetest -M mi0283qt -s 28:320x240@XR24 -v setting mode 320x240-0Hz@XR24 on connectors 28, crtc 30 freq: 24.87Hz freq: 24.79Hz pi@pi2835:~$ ./libdrm/tests/modetest/modetest -M mi0283qt -s 28:320x240@RG16 -v setting mode 320x240-0Hz@RG16 on connectors 28, crtc 30 freq: 26.33Hz freq: 26.30Hz disable byteswapping: pi@pi2835:~$ ./libdrm/tests/modetest/modetest -M mi0283qt -s 28:320x240@RG16 -v setting mode 320x240-0Hz@RG16 on connectors 28, crtc 30 freq: 28.40Hz freq: 28.43Hz SPI at 64MHz (seems to work): pi@pi2835:~$ od -An -vtu4 --endian=big /sys/bus/spi/devices/spi0.0/of_node/spi-max-frequency 6400 pi@pi2835:~$ ./libdrm/tests/modetest/modetest -M mi0283qt -s 28:320x240@XR24 -v setting mode 320x240-0Hz@XR24 on connectors 28, crtc 30 freq: 32.74Hz freq: 32.69Hz pi@pi2835:~$ ./libdrm/tests/modetest/modetest -M mi0283qt -s 28:320x240@RG16 -v setting mode 320x240-0Hz@RG16 on connectors 28, crtc 30 freq: 35.44Hz freq: 35.19Hz disabled byteswapping: pi@pi2835:~$ ./libdrm/tests/modetest/modetest -M mi0283qt -s 28:320x240@RG16 -v setting mode 320x240-0Hz@RG16 on connectors 28, crtc 30 freq: 39.29Hz freq: 39.11Hz SPI at 128MHz (not at all as garbled as I expected): pi@pi2835:~$ od -An -vtu4 --endian=big /sys/bus/spi/devices/spi0.0/of_node/spi-max-frequency 12800 pi@pi2835:~$
Re: [PATCH v4 3/4] drm: Document variable refresh properties
On Fri, Oct 26, 2018 at 05:34:15PM +, Kazlauskas, Nicholas wrote: > On 10/26/18 10:53 AM, Ville Syrjälä wrote: > > On Fri, Oct 26, 2018 at 02:49:31PM +, Kazlauskas, Nicholas wrote: > >> On 10/26/18 7:37 AM, Pekka Paalanen wrote: > >>> On Thu, 11 Oct 2018 12:39:41 -0400 > >>> Nicholas Kazlauskas wrote: > >>> > These include the drm_connector 'vrr_capable' and the drm_crtc > 'vrr_enabled' properties. > > Signed-off-by: Nicholas Kazlauskas > --- > Documentation/gpu/drm-kms.rst | 7 +++ > drivers/gpu/drm/drm_connector.c | 22 ++ > 2 files changed, 29 insertions(+) > > diff --git a/Documentation/gpu/drm-kms.rst > b/Documentation/gpu/drm-kms.rst > index 4b1501b4835b..8da2a178cf85 100644 > --- a/Documentation/gpu/drm-kms.rst > +++ b/Documentation/gpu/drm-kms.rst > @@ -575,6 +575,13 @@ Explicit Fencing Properties > .. kernel-doc:: drivers/gpu/drm/drm_atomic_uapi.c > :doc: explicit fencing properties > > + > +Variable Refresh Properties > +--- > + > +.. kernel-doc:: drivers/gpu/drm/drm_connector.c > + :doc: Variable refresh properties > + > Existing KMS Properties > --- > > diff --git a/drivers/gpu/drm/drm_connector.c > b/drivers/gpu/drm/drm_connector.c > index f0deeb7298d0..2a12853ca917 100644 > --- a/drivers/gpu/drm/drm_connector.c > +++ b/drivers/gpu/drm/drm_connector.c > @@ -1254,6 +1254,28 @@ int drm_mode_create_scaling_mode_property(struct > drm_device *dev) > } > EXPORT_SYMBOL(drm_mode_create_scaling_mode_property); > > +/** > + * DOC: Variable refresh properties > + * > + * Variable refresh rate control is supported via properties on the > + * _connector and _crtc objects. > + * > + * "vrr_capable": > + * Optional _connector boolean property that drivers should > attach > + * with drm_connector_attach_vrr_capable_property() on connectors > that > + * could support variable refresh rates. Drivers should update the > + * property value by calling > drm_connector_set_vrr_capable_property(). > + * > + * Absence of the property should indicate absence of support. > + * > + * "vrr_enabled": > + * Default _crtc boolean property that notifies the driver > that the > + * variable refresh rate adjustment should be enabled for the CRTC. > >>> > >>> Hi, > >>> > >>> where is the documentation that explains how drivers must implement > >>> "variable refresh rate adjustment"? > >>> > >>> What should and could userspace expect to get if it flicks this switch? > >>> > >>> I also think the kernel documentation should include a description of > >>> what VRR actually is and how it conceptually works as far as userspace > >>> is concerned. > >>> > >>> That is, the kernel documentation should describe what this thing does, > >>> so that we avoid every driver implementing a different thing. For > >>> example, one driver could prevent the luminance flickering itself by > >>> tuning the timings while another driver might not do that, which means > >>> that an application tested on the former driver will look just fine > >>> while it is unbearable to watch on the latter. > >>> > >>> > >>> Thanks, > >>> pq > >> > >> I felt it was best to leave this more on the vague side to not impose > >> restrictions yet on what a driver must do. > >> > >> If you think it's worth defining what the "baseline" expectation is for > >> userspace, I would probably describe it as "utilizing the monitor's > >> variable refresh rate to reduce stuttering or tearing that can occur > >> during flipping". This is currently what the amdgpu driver has enabled I would also mention that without VRR, the display engine processes the flips independently of the rendering speed which might result into stuttering or tearing. Might also be worth giving a quick example with numbers in the documentation. Something like: For Eg if the rendering speed is 40Hz, without VRR, display engine will flip at constant 60Hz, which means that same frame will be displayed twice which will be observed as stutter/repetition. With VRR enabled, the vertical front porch will be extended and the flip will be processed when its ready or at max blanking time resulting a smooth transition without repetition. > >> for support. The implementation also isn't much more complex than just > >> passing the variable refresh range to the hardware. > >> > >> I wouldn't want every driver to be forced to implement some sort of > >> luminance flickering by default. It's not noticeable on many panels and > >> any tuning would inherently add latency to the output. It would probably > >> be better left as a new property or a driver specific feature.
[Bug 201285] Kernel oops in amdgpu with Ryzen5 2400G
https://bugzilla.kernel.org/show_bug.cgi?id=201285 --- Comment #4 from vl...@aresgate.net --- Forgot to mention system locks up as well at random after new 4.19.0 kernel -- 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 201285] Kernel oops in amdgpu with Ryzen5 2400G
https://bugzilla.kernel.org/show_bug.cgi?id=201285 vl...@aresgate.net changed: What|Removed |Added CC||vl...@aresgate.net --- Comment #3 from vl...@aresgate.net --- also seeing this on my AMD Ryzen 5 2400G with Radeon Vega Graphics with kernel 4.19.0 -- 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/v3d: Fix compilation error on kbuilt test robot.
Am 26.10.18 um 16:43 schrieb Andrey Grodzovsky:
Signed-off-by: Andrey Grodzovsky
Reviewed-by: Christian König
---
drivers/gpu/drm/v3d/v3d_sched.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/drivers/gpu/drm/v3d/v3d_sched.c b/drivers/gpu/drm/v3d/v3d_sched.c
index 273d0fb..80b641f 100644
--- a/drivers/gpu/drm/v3d/v3d_sched.c
+++ b/drivers/gpu/drm/v3d/v3d_sched.c
@@ -222,7 +222,7 @@ v3d_sched_init(struct v3d_dev *v3d)
_sched_ops,
hw_jobs_limit, job_hang_limit,
msecs_to_jiffies(hang_limit_ms),
-"v3d_render", true);
+"v3d_render");
if (ret) {
dev_err(v3d->dev, "Failed to create render scheduler: %d.",
ret);
___
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel
[PATCH] gpu: drm/lease: fix spelling mistake, EACCESS -> EACCES
From: Colin Ian King Trivial fix to a spelling mistake of the error access name EACCESS, rename to EACCES Signed-off-by: Colin Ian King --- drivers/gpu/drm/drm_lease.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/drivers/gpu/drm/drm_lease.c b/drivers/gpu/drm/drm_lease.c index 24a177ea5417..9ab88db8fad0 100644 --- a/drivers/gpu/drm/drm_lease.c +++ b/drivers/gpu/drm/drm_lease.c @@ -195,7 +195,7 @@ EXPORT_SYMBOL(drm_lease_filter_crtcs); * make sure all of the desired objects can be leased, atomically * leasing them to the new drmmaster. * - * ERR_PTR(-EACCESS) some other master holds the title to any object + * ERR_PTR(-EACCES)some other master holds the title to any object * ERR_PTR(-ENOENT)some object is not a valid DRM object for this device * ERR_PTR(-EBUSY) some other lessee holds title to this object * ERR_PTR(-EEXIST)same object specified more than once in the provided list -- 2.19.1 ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [PATCH v4 3/4] drm: Document variable refresh properties
On Fri, Oct 26, 2018 at 05:34:15PM +, Kazlauskas, Nicholas wrote: > On 10/26/18 10:53 AM, Ville Syrjälä wrote: > > On Fri, Oct 26, 2018 at 02:49:31PM +, Kazlauskas, Nicholas wrote: > >> On 10/26/18 7:37 AM, Pekka Paalanen wrote: > >>> On Thu, 11 Oct 2018 12:39:41 -0400 > >>> Nicholas Kazlauskas wrote: > >>> > These include the drm_connector 'vrr_capable' and the drm_crtc > 'vrr_enabled' properties. > > Signed-off-by: Nicholas Kazlauskas > --- > Documentation/gpu/drm-kms.rst | 7 +++ > drivers/gpu/drm/drm_connector.c | 22 ++ > 2 files changed, 29 insertions(+) > > diff --git a/Documentation/gpu/drm-kms.rst > b/Documentation/gpu/drm-kms.rst > index 4b1501b4835b..8da2a178cf85 100644 > --- a/Documentation/gpu/drm-kms.rst > +++ b/Documentation/gpu/drm-kms.rst > @@ -575,6 +575,13 @@ Explicit Fencing Properties > .. kernel-doc:: drivers/gpu/drm/drm_atomic_uapi.c > :doc: explicit fencing properties > > + > +Variable Refresh Properties > +--- > + > +.. kernel-doc:: drivers/gpu/drm/drm_connector.c > + :doc: Variable refresh properties > + > Existing KMS Properties > --- > > diff --git a/drivers/gpu/drm/drm_connector.c > b/drivers/gpu/drm/drm_connector.c > index f0deeb7298d0..2a12853ca917 100644 > --- a/drivers/gpu/drm/drm_connector.c > +++ b/drivers/gpu/drm/drm_connector.c > @@ -1254,6 +1254,28 @@ int drm_mode_create_scaling_mode_property(struct > drm_device *dev) > } > EXPORT_SYMBOL(drm_mode_create_scaling_mode_property); > > +/** > + * DOC: Variable refresh properties > + * > + * Variable refresh rate control is supported via properties on the > + * _connector and _crtc objects. > + * > + * "vrr_capable": > + * Optional _connector boolean property that drivers should > attach > + * with drm_connector_attach_vrr_capable_property() on connectors > that > + * could support variable refresh rates. Drivers should update the > + * property value by calling > drm_connector_set_vrr_capable_property(). > + * > + * Absence of the property should indicate absence of support. > + * > + * "vrr_enabled": > + * Default _crtc boolean property that notifies the driver > that the > + * variable refresh rate adjustment should be enabled for the CRTC. > >>> > >>> Hi, > >>> > >>> where is the documentation that explains how drivers must implement > >>> "variable refresh rate adjustment"? > >>> > >>> What should and could userspace expect to get if it flicks this switch? > >>> > >>> I also think the kernel documentation should include a description of > >>> what VRR actually is and how it conceptually works as far as userspace > >>> is concerned. > >>> > >>> That is, the kernel documentation should describe what this thing does, > >>> so that we avoid every driver implementing a different thing. For > >>> example, one driver could prevent the luminance flickering itself by > >>> tuning the timings while another driver might not do that, which means > >>> that an application tested on the former driver will look just fine > >>> while it is unbearable to watch on the latter. > >>> > >>> > >>> Thanks, > >>> pq > >> > >> I felt it was best to leave this more on the vague side to not impose > >> restrictions yet on what a driver must do. > >> > >> If you think it's worth defining what the "baseline" expectation is for > >> userspace, I would probably describe it as "utilizing the monitor's > >> variable refresh rate to reduce stuttering or tearing that can occur > >> during flipping". This is currently what the amdgpu driver has enabled > >> for support. The implementation also isn't much more complex than just > >> passing the variable refresh range to the hardware. > >> > >> I wouldn't want every driver to be forced to implement some sort of > >> luminance flickering by default. It's not noticeable on many panels and > >> any tuning would inherently add latency to the output. It would probably > >> be better left as a new property or a driver specific feature. > >> > >> In general I would imagine that most future VRR features would end up as > >> new properties. Anything that's purely software could be implemented as > >> a drm helper that every driver can use. I think the target presentation > >> timestamp feature is a good example for that. > > > > Speaking of timestamps. What is the expected behaviour of vblank > > timestamps when vrr is enabled? > > > > When vrr is enabled the duration of the vertical front porch will be > extended until flip or timeout occurs. The vblank timestamp will vary > based on duration of the vertical front porch. The min/max duration for > the front
Re: [PATCH 2/2] drm/i915: Add short HPD IRQ storm detection for non-MST systems
On Fri, 2018-10-26 at 20:52 +0300, Ville Syrjälä wrote:
> On Thu, Oct 25, 2018 at 06:26:57PM -0400, Lyude Paul wrote:
> > Unfortunately, it seems that the HPD IRQ storm problem from the early
> > days of Intel GPUs was never entirely solved, only mostly. Within the
> > last couple of days, I got a bug report from one of our customers who
> > had been having issues with their machine suddenly booting up very
> > slowly after having updated. The amount of time it took to boot went
> > from around 30 seconds, to over 6 minutes consistently.
> >
> > After some investigation, I discovered that i915 was reporting massive
> > amounts of short HPD IRQ spam on this system from the DisplayPort port,
> > despite there not being anything actually connected. The symptoms would
> > start with one "long" HPD IRQ being detected at boot:
> >
> > [1.891398] [drm:intel_get_hpd_pins [i915]] hotplug event received,
> > stat 0x0044, dig 0x0044, pins 0x00a0
> > [1.891436] [drm:intel_hpd_irq_handler [i915]] digital hpd port B -
> > long
> > [1.891472] [drm:intel_hpd_irq_handler [i915]] Received HPD interrupt
> > on PIN 5 - cnt: 0
> > [1.891508] [drm:intel_hpd_irq_handler [i915]] digital hpd port D -
> > long
> > [1.891544] [drm:intel_hpd_irq_handler [i915]] Received HPD interrupt
> > on PIN 7 - cnt: 0
> > [1.891592] [drm:intel_dp_hpd_pulse [i915]] got hpd irq on port B -
> > long
> > [1.891628] [drm:intel_dp_hpd_pulse [i915]] got hpd irq on port D -
> > long
> > …
> >
> > followed by constant short IRQs afterwards:
> >
> > [1.895091] [drm:intel_encoder_hotplug [i915]] [CONNECTOR:66:DP-1]
> > status updated from unknown to disconnected
> > [1.895129] [drm:i915_hotplug_work_func [i915]] Connector DP-3 (pin 7)
> > received hotplug event.
> > [1.895165] [drm:intel_dp_detect [i915]] [CONNECTOR:72:DP-3]
> > [1.895275] [drm:intel_get_hpd_pins [i915]] hotplug event received,
> > stat 0x0020, dig 0x0020, pins 0x0080
> > [1.895312] [drm:intel_hpd_irq_handler [i915]] digital hpd port D -
> > short
> > [1.895762] [drm:intel_get_hpd_pins [i915]] hotplug event received,
> > stat 0x0020, dig 0x0020, pins 0x0080
> > [1.895799] [drm:intel_hpd_irq_handler [i915]] digital hpd port D -
> > short
> > [1.896239] [drm:intel_dp_aux_xfer [i915]] dp_aux_ch timeout status
> > 0x71450085
> > [1.896293] [drm:intel_get_hpd_pins [i915]] hotplug event received,
> > stat 0x0020, dig 0x0020, pins 0x0080
> > [1.896330] [drm:intel_hpd_irq_handler [i915]] digital hpd port D -
> > short
> > [1.896781] [drm:intel_get_hpd_pins [i915]] hotplug event received,
> > stat 0x0020, dig 0x0020, pins 0x0080
> > [1.896817] [drm:intel_hpd_irq_handler [i915]] digital hpd port D -
> > short
> > [1.897275] [drm:intel_get_hpd_pins [i915]] hotplug event received,
> > stat 0x0020, dig 0x0020, pins 0x0080
> >
> > The customer's system in question has a GM45 GPU, which is apparently
> > well known for hotplugging storms.
> >
> > So, workaround this impressively broken hardware by detecting short IRQ
> > storms using a separate threshold and count, one that is much higher
> > then the threshold for long IRQs. We default to a threshold of 50 short
> > pulses within the timespan of a second, which should amount to about
> > 100ms of constant pulsing. This should be a good middle ground between
> > avoiding detecting false IRQ storms, while still catching short IRQ
> > storms quickly enough to minimize the amount of time we'll stutter every
> > time hotplugging gets re-enabled and immediately disabled by another
> > short IRQ storm.
> >
> > And just to be extra safe: we don't enable this by default on systems
> > with MST support. There's too high of a chance of MST support triggering
> > storm detection, and systems that are new enough to support MST are a
> > lot less likely to have issues with IRQ storms anyway.
> >
> > As a note: this patch was tested using a ThinkPad T450s and a Chamelium
> > to simulate the short IRQ storms.
> >
> > Signed-off-by: Lyude Paul
> > Cc: Ville Syrjälä
> > ---
> > drivers/gpu/drm/i915/i915_debugfs.c | 89 +++-
> > drivers/gpu/drm/i915/i915_drv.h | 15 +++--
> > drivers/gpu/drm/i915/i915_irq.c | 14 -
> > drivers/gpu/drm/i915/intel_hotplug.c | 84 --
> > 4 files changed, 150 insertions(+), 52 deletions(-)
> >
> > diff --git a/drivers/gpu/drm/i915/i915_debugfs.c
> > b/drivers/gpu/drm/i915/i915_debugfs.c
> > index b4744a68cd88..84e89fbd55fb 100644
> > --- a/drivers/gpu/drm/i915/i915_debugfs.c
> > +++ b/drivers/gpu/drm/i915/i915_debugfs.c
> > @@ -4566,21 +4566,24 @@ static const struct file_operations
> > i915_forcewake_fops = {
> > .release = i915_forcewake_release,
> > };
> >
> > -static int i915_hpd_storm_ctl_show(struct seq_file *m, void *data)
> > +static int i915_hpd_storm_show(struct seq_file *m, bool long_hpd)
> > {
> >
Re: [PATCH 2/2] drm/i915: Add short HPD IRQ storm detection for non-MST systems
On Thu, Oct 25, 2018 at 06:26:57PM -0400, Lyude Paul wrote:
> Unfortunately, it seems that the HPD IRQ storm problem from the early
> days of Intel GPUs was never entirely solved, only mostly. Within the
> last couple of days, I got a bug report from one of our customers who
> had been having issues with their machine suddenly booting up very
> slowly after having updated. The amount of time it took to boot went
> from around 30 seconds, to over 6 minutes consistently.
>
> After some investigation, I discovered that i915 was reporting massive
> amounts of short HPD IRQ spam on this system from the DisplayPort port,
> despite there not being anything actually connected. The symptoms would
> start with one "long" HPD IRQ being detected at boot:
>
> [1.891398] [drm:intel_get_hpd_pins [i915]] hotplug event received, stat
> 0x0044, dig 0x0044, pins 0x00a0
> [1.891436] [drm:intel_hpd_irq_handler [i915]] digital hpd port B - long
> [1.891472] [drm:intel_hpd_irq_handler [i915]] Received HPD interrupt on
> PIN 5 - cnt: 0
> [1.891508] [drm:intel_hpd_irq_handler [i915]] digital hpd port D - long
> [1.891544] [drm:intel_hpd_irq_handler [i915]] Received HPD interrupt on
> PIN 7 - cnt: 0
> [1.891592] [drm:intel_dp_hpd_pulse [i915]] got hpd irq on port B - long
> [1.891628] [drm:intel_dp_hpd_pulse [i915]] got hpd irq on port D - long
> …
>
> followed by constant short IRQs afterwards:
>
> [1.895091] [drm:intel_encoder_hotplug [i915]] [CONNECTOR:66:DP-1] status
> updated from unknown to disconnected
> [1.895129] [drm:i915_hotplug_work_func [i915]] Connector DP-3 (pin 7)
> received hotplug event.
> [1.895165] [drm:intel_dp_detect [i915]] [CONNECTOR:72:DP-3]
> [1.895275] [drm:intel_get_hpd_pins [i915]] hotplug event received, stat
> 0x0020, dig 0x0020, pins 0x0080
> [1.895312] [drm:intel_hpd_irq_handler [i915]] digital hpd port D - short
> [1.895762] [drm:intel_get_hpd_pins [i915]] hotplug event received, stat
> 0x0020, dig 0x0020, pins 0x0080
> [1.895799] [drm:intel_hpd_irq_handler [i915]] digital hpd port D - short
> [1.896239] [drm:intel_dp_aux_xfer [i915]] dp_aux_ch timeout status
> 0x71450085
> [1.896293] [drm:intel_get_hpd_pins [i915]] hotplug event received, stat
> 0x0020, dig 0x0020, pins 0x0080
> [1.896330] [drm:intel_hpd_irq_handler [i915]] digital hpd port D - short
> [1.896781] [drm:intel_get_hpd_pins [i915]] hotplug event received, stat
> 0x0020, dig 0x0020, pins 0x0080
> [1.896817] [drm:intel_hpd_irq_handler [i915]] digital hpd port D - short
> [1.897275] [drm:intel_get_hpd_pins [i915]] hotplug event received, stat
> 0x0020, dig 0x0020, pins 0x0080
>
> The customer's system in question has a GM45 GPU, which is apparently
> well known for hotplugging storms.
>
> So, workaround this impressively broken hardware by detecting short IRQ
> storms using a separate threshold and count, one that is much higher
> then the threshold for long IRQs. We default to a threshold of 50 short
> pulses within the timespan of a second, which should amount to about
> 100ms of constant pulsing. This should be a good middle ground between
> avoiding detecting false IRQ storms, while still catching short IRQ
> storms quickly enough to minimize the amount of time we'll stutter every
> time hotplugging gets re-enabled and immediately disabled by another
> short IRQ storm.
>
> And just to be extra safe: we don't enable this by default on systems
> with MST support. There's too high of a chance of MST support triggering
> storm detection, and systems that are new enough to support MST are a
> lot less likely to have issues with IRQ storms anyway.
>
> As a note: this patch was tested using a ThinkPad T450s and a Chamelium
> to simulate the short IRQ storms.
>
> Signed-off-by: Lyude Paul
> Cc: Ville Syrjälä
> ---
> drivers/gpu/drm/i915/i915_debugfs.c | 89 +++-
> drivers/gpu/drm/i915/i915_drv.h | 15 +++--
> drivers/gpu/drm/i915/i915_irq.c | 14 -
> drivers/gpu/drm/i915/intel_hotplug.c | 84 --
> 4 files changed, 150 insertions(+), 52 deletions(-)
>
> diff --git a/drivers/gpu/drm/i915/i915_debugfs.c
> b/drivers/gpu/drm/i915/i915_debugfs.c
> index b4744a68cd88..84e89fbd55fb 100644
> --- a/drivers/gpu/drm/i915/i915_debugfs.c
> +++ b/drivers/gpu/drm/i915/i915_debugfs.c
> @@ -4566,21 +4566,24 @@ static const struct file_operations
> i915_forcewake_fops = {
> .release = i915_forcewake_release,
> };
>
> -static int i915_hpd_storm_ctl_show(struct seq_file *m, void *data)
> +static int i915_hpd_storm_show(struct seq_file *m, bool long_hpd)
> {
> struct drm_i915_private *dev_priv = m->private;
> struct i915_hotplug *hotplug = _priv->hotplug;
> + const unsigned int threshold = long_hpd ?
> + hotplug->long_hpd_storm_threshold :
> +
Re: [PATCH 1/2] drm/i915: Fix NULL deref when re-enabling HPD IRQs on systems with MST
On Thu, Oct 25, 2018 at 06:26:56PM -0400, Lyude Paul wrote:
> Turns out that if you trigger an HPD storm on a system that has an MST
> topology connected to it, you'll end up causing the kernel to eventually
> hit a NULL deref:
>
> [ 332.339041] BUG: unable to handle kernel NULL pointer dereference at
> 00ec
> [ 332.340906] PGD 0 P4D 0
> [ 332.342750] Oops: [#1] SMP PTI
> [ 332.344579] CPU: 2 PID: 25 Comm: kworker/2:0 Kdump: loaded Tainted: G
> O 4.18.0-rc3short-hpd-storm+ #2
> [ 332.346453] Hardware name: LENOVO 20BWS1KY00/20BWS1KY00, BIOS JBET71WW
> (1.35 ) 09/14/2018
> [ 332.348361] Workqueue: events intel_hpd_irq_storm_reenable_work [i915]
> [ 332.350301] RIP: 0010:intel_hpd_irq_storm_reenable_work.cold.3+0x2f/0x86
> [i915]
> [ 332.352213] Code: 00 00 ba e8 00 00 00 48 c7 c6 c0 aa 5f a0 48 c7 c7 d0 73
> 62 a0 4c 89 c1 4c 89 04 24 e8 7f f5 af e0 4c 8b 04 24 44 89 f8 29 e8 <41> 39
> 80 ec 00 00 00 0f 85 43 13 fc ff 41 0f b6 86 b8 04 00 00 41
> [ 332.354286] RSP: 0018:c9147e48 EFLAGS: 00010006
> [ 332.356344] RAX: 0005 RBX: 8802c226c9d4 RCX:
> 0006
> [ 332.358404] RDX: RSI: 0082 RDI:
> 88032dc95570
> [ 332.360466] RBP: 0005 R08: R09:
> 88031b3dc840
> [ 332.362528] R10: R11: 00031a069602 R12:
> 8802c226ca20
> [ 332.364575] R13: 8802c2268000 R14: 880310661000 R15:
> 000a
> [ 332.366615] FS: () GS:88032dc8()
> knlGS:
> [ 332.368658] CS: 0010 DS: ES: CR0: 80050033
> [ 332.370690] CR2: 00ec CR3: 0200a003 CR4:
> 003606e0
> [ 332.372724] DR0: DR1: DR2:
>
> [ 332.374773] DR3: DR6: fffe0ff0 DR7:
> 0400
> [ 332.376798] Call Trace:
> [ 332.378809] process_one_work+0x1a1/0x350
> [ 332.380806] worker_thread+0x30/0x380
> [ 332.382777] ? wq_update_unbound_numa+0x10/0x10
> [ 332.384772] kthread+0x112/0x130
> [ 332.386740] ? kthread_create_worker_on_cpu+0x70/0x70
> [ 332.388706] ret_from_fork+0x35/0x40
> [ 332.390651] Modules linked in: i915(O) vfat fat joydev btusb btrtl btbcm
> btintel bluetooth ecdh_generic iTCO_wdt wmi_bmof i2c_algo_bit drm_kms_helper
> intel_rapl syscopyarea sysfillrect x86_pkg_temp_thermal sysimgblt coretemp
> fb_sys_fops crc32_pclmul drm psmouse pcspkr mei_me mei i2c_i801 lpc_ich
> mfd_core i2c_core tpm_tis tpm_tis_core thinkpad_acpi wmi tpm rfkill video
> crc32c_intel serio_raw ehci_pci xhci_pci ehci_hcd xhci_hcd [last unloaded:
> i915]
> [ 332.394963] CR2: 00ec
>
> This appears to be due to the fact that with an MST topology, not all
> intel_connector structs will have ->encoder set. So, fix this by
> skipping connectors without encoders in
> intel_hpd_irq_storm_reenable_work().
>
> For those wondering, this bug was found on accident while simulating HPD
> storms using a Chamelium connected to a ThinkPad T450s (Broadwell).
>
> Signed-off-by: Lyude Paul
> Cc: Ville Syrjälä
> ---
> drivers/gpu/drm/i915/intel_hotplug.c | 3 ++-
> 1 file changed, 2 insertions(+), 1 deletion(-)
>
> diff --git a/drivers/gpu/drm/i915/intel_hotplug.c
> b/drivers/gpu/drm/i915/intel_hotplug.c
> index 648a13c6043c..7d21aac10d16 100644
> --- a/drivers/gpu/drm/i915/intel_hotplug.c
> +++ b/drivers/gpu/drm/i915/intel_hotplug.c
> @@ -228,7 +228,8 @@ static void intel_hpd_irq_storm_reenable_work(struct
> work_struct *work)
> drm_for_each_connector_iter(connector, _iter) {
> struct intel_connector *intel_connector =
> to_intel_connector(connector);
>
> - if (intel_connector->encoder->hpd_pin == pin) {
> + if (intel_connector->encoder &&
> + intel_connector->encoder->hpd_pin == pin) {
Hmm. Yeah, with MST that assignment happens at enable time. I guess we
currently don't clear that pointer ever, so this seems safe. If we start
to clear it this becomes racy.
Maybe it would be better to just skip MST connectors entirely? Ah,
i915_hpd_poll_init_work() already used ->mst_port for just that. So
probabloy better follow the same example here.
Hmm. That seems a bit racy as well though. We'd need to
reorder intel_dp_add_mst_connector() to assign ->mst_port
before connector_init() to prevent that race.
> if (connector->polled !=
> intel_connector->polled)
> DRM_DEBUG_DRIVER("Reenabling HPD on
> connector %s\n",
>connector->name);
> --
> 2.17.2
--
Ville Syrjälä
Intel
___
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [PATCH v4 3/4] drm: Document variable refresh properties
On 10/26/18 10:53 AM, Ville Syrjälä wrote: > On Fri, Oct 26, 2018 at 02:49:31PM +, Kazlauskas, Nicholas wrote: >> On 10/26/18 7:37 AM, Pekka Paalanen wrote: >>> On Thu, 11 Oct 2018 12:39:41 -0400 >>> Nicholas Kazlauskas wrote: >>> These include the drm_connector 'vrr_capable' and the drm_crtc 'vrr_enabled' properties. Signed-off-by: Nicholas Kazlauskas --- Documentation/gpu/drm-kms.rst | 7 +++ drivers/gpu/drm/drm_connector.c | 22 ++ 2 files changed, 29 insertions(+) diff --git a/Documentation/gpu/drm-kms.rst b/Documentation/gpu/drm-kms.rst index 4b1501b4835b..8da2a178cf85 100644 --- a/Documentation/gpu/drm-kms.rst +++ b/Documentation/gpu/drm-kms.rst @@ -575,6 +575,13 @@ Explicit Fencing Properties .. kernel-doc:: drivers/gpu/drm/drm_atomic_uapi.c :doc: explicit fencing properties + +Variable Refresh Properties +--- + +.. kernel-doc:: drivers/gpu/drm/drm_connector.c + :doc: Variable refresh properties + Existing KMS Properties --- diff --git a/drivers/gpu/drm/drm_connector.c b/drivers/gpu/drm/drm_connector.c index f0deeb7298d0..2a12853ca917 100644 --- a/drivers/gpu/drm/drm_connector.c +++ b/drivers/gpu/drm/drm_connector.c @@ -1254,6 +1254,28 @@ int drm_mode_create_scaling_mode_property(struct drm_device *dev) } EXPORT_SYMBOL(drm_mode_create_scaling_mode_property); +/** + * DOC: Variable refresh properties + * + * Variable refresh rate control is supported via properties on the + * _connector and _crtc objects. + * + * "vrr_capable": + *Optional _connector boolean property that drivers should attach + *with drm_connector_attach_vrr_capable_property() on connectors that + *could support variable refresh rates. Drivers should update the + *property value by calling drm_connector_set_vrr_capable_property(). + * + *Absence of the property should indicate absence of support. + * + * "vrr_enabled": + *Default _crtc boolean property that notifies the driver that the + *variable refresh rate adjustment should be enabled for the CRTC. >>> >>> Hi, >>> >>> where is the documentation that explains how drivers must implement >>> "variable refresh rate adjustment"? >>> >>> What should and could userspace expect to get if it flicks this switch? >>> >>> I also think the kernel documentation should include a description of >>> what VRR actually is and how it conceptually works as far as userspace >>> is concerned. >>> >>> That is, the kernel documentation should describe what this thing does, >>> so that we avoid every driver implementing a different thing. For >>> example, one driver could prevent the luminance flickering itself by >>> tuning the timings while another driver might not do that, which means >>> that an application tested on the former driver will look just fine >>> while it is unbearable to watch on the latter. >>> >>> >>> Thanks, >>> pq >> >> I felt it was best to leave this more on the vague side to not impose >> restrictions yet on what a driver must do. >> >> If you think it's worth defining what the "baseline" expectation is for >> userspace, I would probably describe it as "utilizing the monitor's >> variable refresh rate to reduce stuttering or tearing that can occur >> during flipping". This is currently what the amdgpu driver has enabled >> for support. The implementation also isn't much more complex than just >> passing the variable refresh range to the hardware. >> >> I wouldn't want every driver to be forced to implement some sort of >> luminance flickering by default. It's not noticeable on many panels and >> any tuning would inherently add latency to the output. It would probably >> be better left as a new property or a driver specific feature. >> >> In general I would imagine that most future VRR features would end up as >> new properties. Anything that's purely software could be implemented as >> a drm helper that every driver can use. I think the target presentation >> timestamp feature is a good example for that. > > Speaking of timestamps. What is the expected behaviour of vblank > timestamps when vrr is enabled? > When vrr is enabled the duration of the vertical front porch will be extended until flip or timeout occurs. The vblank timestamp will vary based on duration of the vertical front porch. The min/max duration for the front porch can be specified by the driver via the min/max range. No changes to vblank timestamping handling should be necessary to accommodate variable refresh rate. I think it's probably best to update the documentation for vrr_enable with some of the specifics I described above. That should help
[Bug 108570] AMDGPU DC not working on PCI Express 2.0?
https://bugs.freedesktop.org/show_bug.cgi?id=108570 Alex Deucher changed: What|Removed |Added Status|NEW |RESOLVED Resolution|--- |NOTABUG --- Comment #4 from Alex Deucher --- (In reply to TestMode11 from comment #0) > On RX560 I get the following in dmesg > > [8.426625] kfd kfd: skipped device 1002:67ef, PCI rejects atomics That message is from kfd. It's for ROCm. Unrelated to DC > > Then in Weston start log I do not see "DRM supports atomic modesetting". > > Does this mean that DC is not active? Do AMDGPU DC and atomic modesetting > only work with PCI Express 3.0? No, DC has nothing to do with pci atomics. DC is enabled: [8.690697] [drm] Display Core initialized with v3.2.02! -- You are receiving this mail because: You are the assignee for the bug.___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
[Bug 108521] RX 580 as eGPU amdgpu: gpu post error!
https://bugs.freedesktop.org/show_bug.cgi?id=108521 --- Comment #27 from Robert Strube --- Thanks for the response. I think it was just a coincidence that the eGPU started working with acpi=off. Taking a closer look at the issue it really does appear to be a BIOS problem that prevents the proper PCI resource allocation to one of the TB PCI bridges. In fact when I took a closer look at the dmesg with acpi=off I still see the resource issues present. I've opened up an official bug report here with the kernel ACPI BIOS team here: https://bugzilla.kernel.org/show_bug.cgi?id=201527 I realize the issue should really be solved by the manufacturer, but perhaps the kernel devs can create a work around and/or have more direct lines of communication with the Dell engineers. Thank you both for your suggestions and comments. Rob -- You are receiving this mail because: You are the assignee for the bug.___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
[Bug 108562] Radeon R5 M330 freezes when dpm enabled
https://bugs.freedesktop.org/show_bug.cgi?id=108562 --- Comment #4 from Slava --- this is xorg log after normal shutdown, not after gpu hung. I will try paste some logs after gpu hung bit later -- You are receiving this mail because: You are the assignee for the bug.___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
[Bug 108570] AMDGPU DC not working on PCI Express 2.0?
https://bugs.freedesktop.org/show_bug.cgi?id=108570 --- Comment #3 from testmod...@protonmail.com --- Not seeing any DC success or error messages - though not clear how these would look. -- You are receiving this mail because: You are the assignee for the bug.___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
[Bug 108570] AMDGPU DC not working on PCI Express 2.0?
https://bugs.freedesktop.org/show_bug.cgi?id=108570 --- Comment #2 from testmod...@protonmail.com --- Created attachment 142223 --> https://bugs.freedesktop.org/attachment.cgi?id=142223=edit dmesg-amd -- You are receiving this mail because: You are the assignee for the bug.___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
[Bug 108562] Radeon R5 M330 freezes when dpm enabled
https://bugs.freedesktop.org/show_bug.cgi?id=108562 --- Comment #3 from Slava --- Created attachment 14 --> https://bugs.freedesktop.org/attachment.cgi?id=14=edit xorg log this is xorg log after normal shutdown, not after gpu hung -- You are receiving this mail because: You are the assignee for the bug.___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
[Bug 108562] Radeon R5 M330 freezes when dpm enabled
https://bugs.freedesktop.org/show_bug.cgi?id=108562 --- Comment #2 from Slava --- Created attachment 142221 --> https://bugs.freedesktop.org/attachment.cgi?id=142221=edit dmesg -- You are receiving this mail because: You are the assignee for the bug.___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
[Bug 108570] AMDGPU DC not working on PCI Express 2.0?
https://bugs.freedesktop.org/show_bug.cgi?id=108570 --- Comment #1 from John Bridgman --- I don't believe there is any connection between "atomic" modesetting and atomic operations in PCIE 3.0. Do you see other DC-related messages ? Might be helpful to post your full dmesg output. -- You are receiving this mail because: You are the assignee for the bug.___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
[Bug 108570] AMDGPU DC not working on PCI Express 2.0?
https://bugs.freedesktop.org/show_bug.cgi?id=108570 testmod...@protonmail.com changed: What|Removed |Added Summary|AMDGPU DC Not workin on PCI |AMDGPU DC not working on |Express 2.0?|PCI Express 2.0? -- You are receiving this mail because: You are the assignee for the bug.___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
[Bug 108570] AMDGPU DC Not workin on PCI Express 2.0?
https://bugs.freedesktop.org/show_bug.cgi?id=108570 Bug ID: 108570 Summary: AMDGPU DC Not workin on PCI Express 2.0? Product: DRI Version: XOrg git Hardware: Other OS: All Status: NEW Severity: normal Priority: medium Component: DRM/AMDgpu Assignee: dri-devel@lists.freedesktop.org Reporter: testmod...@protonmail.com On RX560 I get the following in dmesg [8.426625] kfd kfd: skipped device 1002:67ef, PCI rejects atomics Then in Weston start log I do not see "DRM supports atomic modesetting". Does this mean that DC is not active? Do AMDGPU DC and atomic modesetting only work with PCI Express 3.0? -- You are receiving this mail because: You are the assignee for the bug.___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [PATCH] drm/vgem: create a render node for vgem
Hi Emil,
Thank you for the patch! Yet something to improve:
[auto build test ERROR on sof-driver-fuweitax/master]
[also build test ERROR on v4.19 next-20181019]
[if your patch is applied to the wrong git tree, please drop us a note to help
improve the system]
url:
https://github.com/0day-ci/linux/commits/Emil-Velikov/drm-vgem-create-a-render-node-for-vgem/20181026-233734
base: https://github.com/fuweitax/linux master
config: i386-randconfig-x077-201842 (attached as .config)
compiler: gcc-7 (Debian 7.3.0-1) 7.3.0
reproduce:
# save the attached .config to linux build tree
make ARCH=i386
All errors (new ones prefixed by >>):
>> drivers/gpu/drm/vgem/vgem_drv.c:436:21: error: expected '}' before ';' token
DRIVER_RENDER;
^
drivers/gpu/drm/vgem/vgem_drv.c:424:13: warning: 'vgem_release' defined but
not used [-Wunused-function]
static void vgem_release(struct drm_device *dev)
^~~~
drivers/gpu/drm/vgem/vgem_drv.c:401:12: warning: 'vgem_prime_mmap' defined
but not used [-Wunused-function]
static int vgem_prime_mmap(struct drm_gem_object *obj,
^~~
drivers/gpu/drm/vgem/vgem_drv.c:393:13: warning: 'vgem_prime_vunmap' defined
but not used [-Wunused-function]
static void vgem_prime_vunmap(struct drm_gem_object *obj, void *vaddr)
^
drivers/gpu/drm/vgem/vgem_drv.c:380:14: warning: 'vgem_prime_vmap' defined
but not used [-Wunused-function]
static void *vgem_prime_vmap(struct drm_gem_object *obj)
^~~
drivers/gpu/drm/vgem/vgem_drv.c:355:31: warning:
'vgem_prime_import_sg_table' defined but not used [-Wunused-function]
static struct drm_gem_object *vgem_prime_import_sg_table(struct drm_device
*dev,
^~
drivers/gpu/drm/vgem/vgem_drv.c:347:31: warning: 'vgem_prime_import' defined
but not used [-Wunused-function]
static struct drm_gem_object* vgem_prime_import(struct drm_device *dev,
^
drivers/gpu/drm/vgem/vgem_drv.c:340:25: warning: 'vgem_prime_get_sg_table'
defined but not used [-Wunused-function]
static struct sg_table *vgem_prime_get_sg_table(struct drm_gem_object *obj)
^~~
drivers/gpu/drm/vgem/vgem_drv.c:333:13: warning: 'vgem_prime_unpin' defined
but not used [-Wunused-function]
static void vgem_prime_unpin(struct drm_gem_object *obj)
^~~~
drivers/gpu/drm/vgem/vgem_drv.c:315:12: warning: 'vgem_prime_pin' defined
but not used [-Wunused-function]
static int vgem_prime_pin(struct drm_gem_object *obj)
^~
drivers/gpu/drm/vgem/vgem_drv.c:253:30: warning: 'vgem_ioctls' defined but
not used [-Wunused-variable]
static struct drm_ioctl_desc vgem_ioctls[] = {
^~~
drivers/gpu/drm/vgem/vgem_drv.c:227:12: warning: 'vgem_gem_dumb_map' defined
but not used [-Wunused-function]
static int vgem_gem_dumb_map(struct drm_file *file, struct drm_device *dev,
^
drivers/gpu/drm/vgem/vgem_drv.c:204:12: warning: 'vgem_gem_dumb_create'
defined but not used [-Wunused-function]
static int vgem_gem_dumb_create(struct drm_file *file, struct drm_device
*dev,
^~~~
drivers/gpu/drm/vgem/vgem_drv.c:145:13: warning: 'vgem_postclose' defined
but not used [-Wunused-function]
static void vgem_postclose(struct drm_device *dev, struct drm_file *file)
^~
drivers/gpu/drm/vgem/vgem_drv.c:125:12: warning: 'vgem_open' defined but not
used [-Wunused-function]
static int vgem_open(struct drm_device *dev, struct drm_file *file)
^
drivers/gpu/drm/vgem/vgem_drv.c:50:13: warning: 'vgem_gem_free_object'
defined but not used [-Wunused-function]
static void vgem_gem_free_object(struct drm_gem_object *obj)
^~~~
vim +436 drivers/gpu/drm/vgem/vgem_drv.c
433
434 static struct drm_driver vgem_driver = {
435 .driver_features= DRIVER_GEM | DRIVER_PRIME |
> 436DRIVER_RENDER;
437 .release= vgem_release,
438 .open = vgem_open,
439 .postclose = vgem_postclose,
440 .gem_free_object_unlocked = vgem_gem_free_object,
441 .gem_vm_ops = _gem_vm_ops,
442 .ioctls = vgem_ioctls,
443 .num_ioctls = ARRAY_SIZE(vgem_ioctls),
444 .fops = _driver_fops,
445
446 .dumb_create= vgem_gem_dumb_create,
447 .
[radeon-alex:amd-staging-drm-next 3/3] drivers/staging/vboxvideo/vbox_ttm.c:204:9: error: undefined identifier 'vbox_ttm_global_release'
tree: git://people.freedesktop.org/~agd5f/linux.git amd-staging-drm-next
head: 4cab470e01108ad0a7a74c6a30d83e7e8e60aa9e
commit: 4cab470e01108ad0a7a74c6a30d83e7e8e60aa9e [3/3] drm/amdgpu/amdkfd: clean
up mmhub and gfxhub includes
config: x86_64-allmodconfig (attached as .config)
compiler: gcc-7 (Debian 7.3.0-1) 7.3.0
reproduce:
git checkout 4cab470e01108ad0a7a74c6a30d83e7e8e60aa9e
# save the attached .config to linux build tree
make ARCH=x86_64
All errors (new ones prefixed by >>):
>> drivers/staging/vboxvideo/vbox_ttm.c:204:9: error: undefined identifier
>> 'vbox_ttm_global_release'
drivers/staging/vboxvideo/vbox_ttm.c:218:9: error: undefined identifier
'vbox_ttm_global_release'
In file included from drivers/staging/vboxvideo/vbox_ttm.c:30:0:
drivers/staging/vboxvideo/vbox_drv.h:98:28: error: field 'bo_global_ref' has
incomplete type
struct ttm_bo_global_ref bo_global_ref;
^
drivers/staging/vboxvideo/vbox_ttm.c: In function 'vbox_mm_init':
drivers/staging/vboxvideo/vbox_ttm.c:204:2: error: implicit declaration of
function 'vbox_ttm_global_release'; did you mean 'ttm_mem_global_release'?
[-Werror=implicit-function-declaration]
vbox_ttm_global_release(vbox);
^~~
ttm_mem_global_release
cc1: some warnings being treated as errors
--
>> drivers/gpu/drm/v3d/v3d_sched.c:221:29: error: too many arguments for
>> function drm_sched_init
drivers/gpu/drm/v3d/v3d_sched.c: In function 'v3d_sched_init':
drivers/gpu/drm/v3d/v3d_sched.c:221:8: error: too many arguments to function
'drm_sched_init'
ret = drm_sched_init(>queue[V3D_RENDER].sched,
^~
In file included from drivers/gpu/drm/v3d/v3d_drv.h:9:0,
from drivers/gpu/drm/v3d/v3d_sched.c:23:
include/drm/gpu_scheduler.h:290:5: note: declared here
int drm_sched_init(struct drm_gpu_scheduler *sched,
^~
vim +/vbox_ttm_global_release +204 drivers/staging/vboxvideo/vbox_ttm.c
dd55d44f Hans de Goede 2017-07-06 168
dd55d44f Hans de Goede 2017-07-06 169 int vbox_mm_init(struct vbox_private
*vbox)
dd55d44f Hans de Goede 2017-07-06 170 {
dd55d44f Hans de Goede 2017-07-06 171 int ret;
dd55d44f Hans de Goede 2017-07-06 172 struct drm_device *dev =
vbox->dev;
dd55d44f Hans de Goede 2017-07-06 173 struct ttm_bo_device *bdev =
>ttm.bdev;
dd55d44f Hans de Goede 2017-07-06 174
dd55d44f Hans de Goede 2017-07-06 175 ret =
ttm_bo_device_init(>ttm.bdev,
dd55d44f Hans de Goede 2017-07-06 176
_bo_driver,
dd55d44f Hans de Goede 2017-07-06 177
dev->anon_inode->i_mapping,
dd55d44f Hans de Goede 2017-07-06 178
DRM_FILE_PAGE_OFFSET, true);
dd55d44f Hans de Goede 2017-07-06 179 if (ret) {
dd55d44f Hans de Goede 2017-07-06 180 DRM_ERROR("Error
initialising bo driver; %d\n", ret);
dd55d44f Hans de Goede 2017-07-06 181 goto
err_ttm_global_release;
dd55d44f Hans de Goede 2017-07-06 182 }
dd55d44f Hans de Goede 2017-07-06 183
dd55d44f Hans de Goede 2017-07-06 184 ret = ttm_bo_init_mm(bdev,
TTM_PL_VRAM,
dd55d44f Hans de Goede 2017-07-06 185
vbox->available_vram_size >> PAGE_SHIFT);
dd55d44f Hans de Goede 2017-07-06 186 if (ret) {
dd55d44f Hans de Goede 2017-07-06 187 DRM_ERROR("Failed ttm
VRAM init: %d\n", ret);
dd55d44f Hans de Goede 2017-07-06 188 goto err_device_release;
dd55d44f Hans de Goede 2017-07-06 189 }
dd55d44f Hans de Goede 2017-07-06 190
dd55d44f Hans de Goede 2017-07-06 191 #ifdef DRM_MTRR_WC
dd55d44f Hans de Goede 2017-07-06 192 vbox->fb_mtrr =
drm_mtrr_add(pci_resource_start(dev->pdev, 0),
dd55d44f Hans de Goede 2017-07-06 193
pci_resource_len(dev->pdev, 0),
dd55d44f Hans de Goede 2017-07-06 194
DRM_MTRR_WC);
dd55d44f Hans de Goede 2017-07-06 195 #else
dd55d44f Hans de Goede 2017-07-06 196 vbox->fb_mtrr =
arch_phys_wc_add(pci_resource_start(dev->pdev, 0),
dd55d44f Hans de Goede 2017-07-06 197
pci_resource_len(dev->pdev, 0));
dd55d44f Hans de Goede 2017-07-06 198 #endif
dd55d44f Hans de Goede 2017-07-06 199 return 0;
dd55d44f Hans de Goede 2017-07-06 200
dd55d44f Hans de Goede 2017-07-06 201 err_device_release:
dd55d44f Hans de Goede 2017-07-06 202
ttm_bo_device_release(>ttm.bdev);
dd55d44f Hans de Goede 2017-07-06 203 err_ttm_global_release:
dd55d44f Hans de Goede 2017-07-06 @204 vbox_ttm_global_release(vbox);
dd55d44f Hans de Goede 2017-07-06 205 return ret;
dd55d44f Hans de Goede 2017-07-06 206 }
dd55d44f Hans de Goede 2017-07-06 207
:: The
Re: [PATCH] RFC: Make igts for cross-driver stuff mandatory?
On Fri, Oct 19, 2018 at 4:51 AM Daniel Vetter wrote: > > Hi all, > > This is just to collect feedback on this idea, and see whether the > overall dri-devel community stands on all this. I think the past few > cross-vendor uapi extensions all came with igts attached, and > personally I think there's lots of value in having them: A > cross-vendor interface isn't useful if every driver implements it > slightly differently. > > I think there's 2 questions here: > > - Do we want to make such testcases mandatory? > > - If yes, are we there yet, or is there something crucially missing > still? > > And of course there's a bunch of details to figure out. Like we > probably want to also recommend the selftests/unit-tests in > drivers/gpu/drm/selftest, since fairly often that's a much more > effective approach to checking the details than from userspace. > > Feedback and thoughts very much appreciated. As long as we can fix up any cross platform issues, this seems reasonable to me. Alex > > Cheers, Daniel > --- > Documentation/gpu/drm-uapi.rst | 7 +++ > 1 file changed, 7 insertions(+) > > diff --git a/Documentation/gpu/drm-uapi.rst b/Documentation/gpu/drm-uapi.rst > index 4b4bf2c5eac5..91cf6e4b6303 100644 > --- a/Documentation/gpu/drm-uapi.rst > +++ b/Documentation/gpu/drm-uapi.rst > @@ -238,6 +238,13 @@ DRM specific patterns. Note that ENOTTY has the slightly > unintuitive meaning of > Testing and validation > == > > +Testing Requirements for userspace API > +-- > + > +New cross-driver userspace interface extensions, like new IOCTL, new KMS > +properties, new files in sysfs or anything else that constitutes an API > change > +need to have driver-agnostic testcases in IGT for that feature. > + > Validating changes with IGT > --- > > -- > 2.19.1 > > ___ > dri-devel mailing list > dri-devel@lists.freedesktop.org > https://lists.freedesktop.org/mailman/listinfo/dri-devel ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [RFC PATCH] drm/vc4: Add a load tracker to prevent HVS underflow errors
On Fri, Oct 26, 2018 at 04:52:33PM +0200, Boris Brezillon wrote: > On Fri, 26 Oct 2018 16:26:03 +0200 > Daniel Vetter wrote: > > > On Fri, Oct 26, 2018 at 3:57 PM Boris Brezillon > > wrote: > > > > > > On Fri, 26 Oct 2018 15:30:26 +0200 > > > Daniel Vetter wrote: > > > > > > > On Thu, Oct 25, 2018 at 11:41:14AM +0200, Boris Brezillon wrote: > > > > > On Thu, 25 Oct 2018 11:33:14 +0200 > > > > > Daniel Vetter wrote: > > > > > > > > > > > On Thu, Oct 25, 2018 at 10:09:31AM +0200, Boris Brezillon wrote: > > > > > > > On Tue, 23 Oct 2018 15:44:43 +0200 > > > > > > > Daniel Vetter wrote: > > > > > > > > > > > > > > > On Tue, Oct 23, 2018 at 09:55:08AM +0200, Boris Brezillon > > > > > > > > wrote: > > > > > > > > > Hi Daniel, > > > > > > > > > > > > > > > > > > On Tue, 16 Oct 2018 14:57:43 +0200 > > > > > > > > > Daniel Vetter wrote: > > > > > > > > > > > > > > > > > > > On Tue, Oct 16, 2018 at 11:40:45AM +0200, Boris Brezillon > > > > > > > > > > wrote: > > > > > > > > > > > The HVS block is supposed to fill the pixelvalve FIFOs > > > > > > > > > > > fast enough to > > > > > > > > > > > meet the requested framerate. The problem is, the HVS and > > > > > > > > > > > memory bus > > > > > > > > > > > bandwidths are limited, and if we don't take these > > > > > > > > > > > limitations into > > > > > > > > > > > account we might end up with HVS underflow errors. > > > > > > > > > > > > > > > > > > > > > > This patch is trying to model the per-plane HVS and > > > > > > > > > > > memory bus bandwidth > > > > > > > > > > > consumption and take a decision at atomic_check() time > > > > > > > > > > > whether the > > > > > > > > > > > estimated load will fit in the HVS and membus budget. > > > > > > > > > > > > > > > > > > > > > > Note that we take an extra margin on the memory bus > > > > > > > > > > > consumption to let > > > > > > > > > > > the system run smoothly when other blocks are doing heavy > > > > > > > > > > > use of the > > > > > > > > > > > memory bus. Same goes for the HVS limit, except the > > > > > > > > > > > margin is smaller in > > > > > > > > > > > this case, since the HVS is not used by external > > > > > > > > > > > components. > > > > > > > > > > > > > > > > > > > > > > Signed-off-by: Boris Brezillon > > > > > > > > > > > > > > > > > > > > > > --- > > > > > > > > > > > This logic has been validated using a simple shell script > > > > > > > > > > > and > > > > > > > > > > > some instrumentation in the VC4 driver: > > > > > > > > > > > > > > > > > > > > > > - capture underflow errors at the HVS level and expose a > > > > > > > > > > > debugfs file > > > > > > > > > > > reporting those errors > > > > > > > > > > > - add debugfs files to expose when atomic_check fails > > > > > > > > > > > because of the > > > > > > > > > > > HVS or membus load limitation or when it fails for > > > > > > > > > > > other reasons > > > > > > > > > > > > > > > > > > > > > > The branch containing those modification is available > > > > > > > > > > > here [1], and the > > > > > > > > > > > script (which is internally using modetest) is here [2] > > > > > > > > > > > (please note > > > > > > > > > > > that I'm bad at writing shell scripts :-)). > > > > > > > > > > > > > > > > > > > > > > Note that those modification tend to over-estimate the > > > > > > > > > > > load, and thus > > > > > > > > > > > reject setups that might have previously worked, so we > > > > > > > > > > > might want to > > > > > > > > > > > adjust the limits to avoid that. > > > > > > > > > > > > > > > > > > > > > > [1]https://github.com/bbrezillon/linux/tree/vc4/hvs-bandwidth-eval > > > > > > > > > > > [2]https://github.com/bbrezillon/vc4-hvs-bandwidth-test > > > > > > > > > > > > > > > > > > > > Any interest in using igt to test this stuff? We have at > > > > > > > > > > least a bunch of > > > > > > > > > > tests already in there that try all kinds of plane setups. > > > > > > > > > > And we use > > > > > > > > > > those to hunt for underruns on i915 hw. > > > > > > > > > > > > > > > > > > > > Wrt underrun reporting: On i915 we just dump them into > > > > > > > > > > dmesg at the error > > > > > > > > > > level, using DRM_ERROR, > > > > > > > > > > > > > > > > > > Are you masking the underrun interrupt after it's been > > > > > > > > > reported? If we > > > > > > > > > don't do that on VC4 we just end up flooding the kernel-log > > > > > > > > > buffer until > > > > > > > > > someone comes and update the config. > > > > > > > > > > > > > > > > Yeah we do that too. Rule is that a full modeset will clear any > > > > > > > > underrun > > > > > > > > masking (so tests need to make sure they start with a modeset, > > > > > > > > or it'll be > > > > > > > > for nothing). > > > > > > > > > > > > > > > > > > > > > > > > > > > plus a tracepoint. See e.g. > > > > > > > > > > intel_pch_fifo_underrun_irq_handler(). If there's interest > > > > > > > > > > we could > > > > > > > > > > perhaps extract
Re: [PATCH v2 5/6] dt-bindings: drm/panel: simple: Innolux TV123WAM is actually P120ZDG-BF1
Sean, On Fri, Oct 26, 2018 at 7:47 AM Sean Paul wrote: > > I didn't see your v2 when I replied to the v1 patch, so for the record, > > > > Reviewed-by: Sean Paul > > > > Also note to whoever applies this to -misc, v1 also had > > > > Reviewed-by: Abhinav Kumar > > And I just realized that patches 5 & 6 were swapped in v2. So nevermind this, > Doug's v2 tags are correct. > > Dear coffee, please kick in soon. Sorry about that--I should have mentioned it in the notes. I realized that the bindings were supposed to land before the code so I swapped them. Thanks for your review! -Doug ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
[Bug 199139] System freezes (kernel, amdgpu NULL pointer dereference) when video enters powersafe state
https://bugzilla.kernel.org/show_bug.cgi?id=199139 --- Comment #19 from Eduard (wirch.edu...@gmail.com) --- I figured out it is not true. After a longer period of turned off screen (lightDM automatically turns it off if you lock the station), the hangs appeared again. So I'm back again on 4.14. :( -- 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 v4 3/4] drm: Document variable refresh properties
On Fri, Oct 26, 2018 at 02:49:31PM +, Kazlauskas, Nicholas wrote: > On 10/26/18 7:37 AM, Pekka Paalanen wrote: > > On Thu, 11 Oct 2018 12:39:41 -0400 > > Nicholas Kazlauskas wrote: > > > >> These include the drm_connector 'vrr_capable' and the drm_crtc > >> 'vrr_enabled' properties. > >> > >> Signed-off-by: Nicholas Kazlauskas > >> --- > >> Documentation/gpu/drm-kms.rst | 7 +++ > >> drivers/gpu/drm/drm_connector.c | 22 ++ > >> 2 files changed, 29 insertions(+) > >> > >> diff --git a/Documentation/gpu/drm-kms.rst b/Documentation/gpu/drm-kms.rst > >> index 4b1501b4835b..8da2a178cf85 100644 > >> --- a/Documentation/gpu/drm-kms.rst > >> +++ b/Documentation/gpu/drm-kms.rst > >> @@ -575,6 +575,13 @@ Explicit Fencing Properties > >> .. kernel-doc:: drivers/gpu/drm/drm_atomic_uapi.c > >> :doc: explicit fencing properties > >> > >> + > >> +Variable Refresh Properties > >> +--- > >> + > >> +.. kernel-doc:: drivers/gpu/drm/drm_connector.c > >> + :doc: Variable refresh properties > >> + > >> Existing KMS Properties > >> --- > >> > >> diff --git a/drivers/gpu/drm/drm_connector.c > >> b/drivers/gpu/drm/drm_connector.c > >> index f0deeb7298d0..2a12853ca917 100644 > >> --- a/drivers/gpu/drm/drm_connector.c > >> +++ b/drivers/gpu/drm/drm_connector.c > >> @@ -1254,6 +1254,28 @@ int drm_mode_create_scaling_mode_property(struct > >> drm_device *dev) > >> } > >> EXPORT_SYMBOL(drm_mode_create_scaling_mode_property); > >> > >> +/** > >> + * DOC: Variable refresh properties > >> + * > >> + * Variable refresh rate control is supported via properties on the > >> + * _connector and _crtc objects. > >> + * > >> + * "vrr_capable": > >> + *Optional _connector boolean property that drivers should > >> attach > >> + *with drm_connector_attach_vrr_capable_property() on connectors > >> that > >> + *could support variable refresh rates. Drivers should update the > >> + *property value by calling > >> drm_connector_set_vrr_capable_property(). > >> + * > >> + *Absence of the property should indicate absence of support. > >> + * > >> + * "vrr_enabled": > >> + *Default _crtc boolean property that notifies the driver > >> that the > >> + *variable refresh rate adjustment should be enabled for the CRTC. > > > > Hi, > > > > where is the documentation that explains how drivers must implement > > "variable refresh rate adjustment"? > > > > What should and could userspace expect to get if it flicks this switch? > > > > I also think the kernel documentation should include a description of > > what VRR actually is and how it conceptually works as far as userspace > > is concerned. > > > > That is, the kernel documentation should describe what this thing does, > > so that we avoid every driver implementing a different thing. For > > example, one driver could prevent the luminance flickering itself by > > tuning the timings while another driver might not do that, which means > > that an application tested on the former driver will look just fine > > while it is unbearable to watch on the latter. > > > > > > Thanks, > > pq > > I felt it was best to leave this more on the vague side to not impose > restrictions yet on what a driver must do. > > If you think it's worth defining what the "baseline" expectation is for > userspace, I would probably describe it as "utilizing the monitor's > variable refresh rate to reduce stuttering or tearing that can occur > during flipping". This is currently what the amdgpu driver has enabled > for support. The implementation also isn't much more complex than just > passing the variable refresh range to the hardware. > > I wouldn't want every driver to be forced to implement some sort of > luminance flickering by default. It's not noticeable on many panels and > any tuning would inherently add latency to the output. It would probably > be better left as a new property or a driver specific feature. > > In general I would imagine that most future VRR features would end up as > new properties. Anything that's purely software could be implemented as > a drm helper that every driver can use. I think the target presentation > timestamp feature is a good example for that. Speaking of timestamps. What is the expected behaviour of vblank timestamps when vrr is enabled? -- Ville Syrjälä Intel ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [PATCH] drm/v3d: Fix compilation error on kbuilt test robot.
On Fri, Oct 26, 2018 at 10:43 AM Andrey Grodzovsky
wrote:
>
> Signed-off-by: Andrey Grodzovsky
Reviewed-by: Alex Deucher
> ---
> drivers/gpu/drm/v3d/v3d_sched.c | 2 +-
> 1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/drivers/gpu/drm/v3d/v3d_sched.c b/drivers/gpu/drm/v3d/v3d_sched.c
> index 273d0fb..80b641f 100644
> --- a/drivers/gpu/drm/v3d/v3d_sched.c
> +++ b/drivers/gpu/drm/v3d/v3d_sched.c
> @@ -222,7 +222,7 @@ v3d_sched_init(struct v3d_dev *v3d)
> _sched_ops,
> hw_jobs_limit, job_hang_limit,
> msecs_to_jiffies(hang_limit_ms),
> -"v3d_render", true);
> +"v3d_render");
> if (ret) {
> dev_err(v3d->dev, "Failed to create render scheduler: %d.",
> ret);
> --
> 2.7.4
>
> ___
> dri-devel mailing list
> dri-devel@lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/dri-devel
___
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [RFC PATCH] drm/vc4: Add a load tracker to prevent HVS underflow errors
On Fri, 26 Oct 2018 16:26:03 +0200 Daniel Vetter wrote: > On Fri, Oct 26, 2018 at 3:57 PM Boris Brezillon > wrote: > > > > On Fri, 26 Oct 2018 15:30:26 +0200 > > Daniel Vetter wrote: > > > > > On Thu, Oct 25, 2018 at 11:41:14AM +0200, Boris Brezillon wrote: > > > > On Thu, 25 Oct 2018 11:33:14 +0200 > > > > Daniel Vetter wrote: > > > > > > > > > On Thu, Oct 25, 2018 at 10:09:31AM +0200, Boris Brezillon wrote: > > > > > > On Tue, 23 Oct 2018 15:44:43 +0200 > > > > > > Daniel Vetter wrote: > > > > > > > > > > > > > On Tue, Oct 23, 2018 at 09:55:08AM +0200, Boris Brezillon wrote: > > > > > > > > Hi Daniel, > > > > > > > > > > > > > > > > On Tue, 16 Oct 2018 14:57:43 +0200 > > > > > > > > Daniel Vetter wrote: > > > > > > > > > > > > > > > > > On Tue, Oct 16, 2018 at 11:40:45AM +0200, Boris Brezillon > > > > > > > > > wrote: > > > > > > > > > > The HVS block is supposed to fill the pixelvalve FIFOs fast > > > > > > > > > > enough to > > > > > > > > > > meet the requested framerate. The problem is, the HVS and > > > > > > > > > > memory bus > > > > > > > > > > bandwidths are limited, and if we don't take these > > > > > > > > > > limitations into > > > > > > > > > > account we might end up with HVS underflow errors. > > > > > > > > > > > > > > > > > > > > This patch is trying to model the per-plane HVS and memory > > > > > > > > > > bus bandwidth > > > > > > > > > > consumption and take a decision at atomic_check() time > > > > > > > > > > whether the > > > > > > > > > > estimated load will fit in the HVS and membus budget. > > > > > > > > > > > > > > > > > > > > Note that we take an extra margin on the memory bus > > > > > > > > > > consumption to let > > > > > > > > > > the system run smoothly when other blocks are doing heavy > > > > > > > > > > use of the > > > > > > > > > > memory bus. Same goes for the HVS limit, except the margin > > > > > > > > > > is smaller in > > > > > > > > > > this case, since the HVS is not used by external components. > > > > > > > > > > > > > > > > > > > > Signed-off-by: Boris Brezillon > > > > > > > > > > --- > > > > > > > > > > This logic has been validated using a simple shell script > > > > > > > > > > and > > > > > > > > > > some instrumentation in the VC4 driver: > > > > > > > > > > > > > > > > > > > > - capture underflow errors at the HVS level and expose a > > > > > > > > > > debugfs file > > > > > > > > > > reporting those errors > > > > > > > > > > - add debugfs files to expose when atomic_check fails > > > > > > > > > > because of the > > > > > > > > > > HVS or membus load limitation or when it fails for other > > > > > > > > > > reasons > > > > > > > > > > > > > > > > > > > > The branch containing those modification is available here > > > > > > > > > > [1], and the > > > > > > > > > > script (which is internally using modetest) is here [2] > > > > > > > > > > (please note > > > > > > > > > > that I'm bad at writing shell scripts :-)). > > > > > > > > > > > > > > > > > > > > Note that those modification tend to over-estimate the > > > > > > > > > > load, and thus > > > > > > > > > > reject setups that might have previously worked, so we > > > > > > > > > > might want to > > > > > > > > > > adjust the limits to avoid that. > > > > > > > > > > > > > > > > > > > > [1]https://github.com/bbrezillon/linux/tree/vc4/hvs-bandwidth-eval > > > > > > > > > > [2]https://github.com/bbrezillon/vc4-hvs-bandwidth-test > > > > > > > > > > > > > > > > > > Any interest in using igt to test this stuff? We have at > > > > > > > > > least a bunch of > > > > > > > > > tests already in there that try all kinds of plane setups. > > > > > > > > > And we use > > > > > > > > > those to hunt for underruns on i915 hw. > > > > > > > > > > > > > > > > > > Wrt underrun reporting: On i915 we just dump them into dmesg > > > > > > > > > at the error > > > > > > > > > level, using DRM_ERROR, > > > > > > > > > > > > > > > > Are you masking the underrun interrupt after it's been > > > > > > > > reported? If we > > > > > > > > don't do that on VC4 we just end up flooding the kernel-log > > > > > > > > buffer until > > > > > > > > someone comes and update the config. > > > > > > > > > > > > > > Yeah we do that too. Rule is that a full modeset will clear any > > > > > > > underrun > > > > > > > masking (so tests need to make sure they start with a modeset, or > > > > > > > it'll be > > > > > > > for nothing). > > > > > > > > > > > > > > > > > > > > > > > > plus a tracepoint. See e.g. > > > > > > > > > intel_pch_fifo_underrun_irq_handler(). If there's interest we > > > > > > > > > could > > > > > > > > > perhaps extract this into something common, similar to what > > > > > > > > > was done with > > > > > > > > > crc support already. > > > > > > > > > > > > > > > > > > > > > > > > > I'm not a big fan of hardcoded trace points in general (because > > > > > > > > of the > > > > > > > > whole "it's part of the stable ABI" thing), and in this case,
Re: [PATCH v4 3/4] drm: Document variable refresh properties
On 10/26/18 7:37 AM, Pekka Paalanen wrote: > On Thu, 11 Oct 2018 12:39:41 -0400 > Nicholas Kazlauskas wrote: > >> These include the drm_connector 'vrr_capable' and the drm_crtc >> 'vrr_enabled' properties. >> >> Signed-off-by: Nicholas Kazlauskas >> --- >> Documentation/gpu/drm-kms.rst | 7 +++ >> drivers/gpu/drm/drm_connector.c | 22 ++ >> 2 files changed, 29 insertions(+) >> >> diff --git a/Documentation/gpu/drm-kms.rst b/Documentation/gpu/drm-kms.rst >> index 4b1501b4835b..8da2a178cf85 100644 >> --- a/Documentation/gpu/drm-kms.rst >> +++ b/Documentation/gpu/drm-kms.rst >> @@ -575,6 +575,13 @@ Explicit Fencing Properties >> .. kernel-doc:: drivers/gpu/drm/drm_atomic_uapi.c >> :doc: explicit fencing properties >> >> + >> +Variable Refresh Properties >> +--- >> + >> +.. kernel-doc:: drivers/gpu/drm/drm_connector.c >> + :doc: Variable refresh properties >> + >> Existing KMS Properties >> --- >> >> diff --git a/drivers/gpu/drm/drm_connector.c >> b/drivers/gpu/drm/drm_connector.c >> index f0deeb7298d0..2a12853ca917 100644 >> --- a/drivers/gpu/drm/drm_connector.c >> +++ b/drivers/gpu/drm/drm_connector.c >> @@ -1254,6 +1254,28 @@ int drm_mode_create_scaling_mode_property(struct >> drm_device *dev) >> } >> EXPORT_SYMBOL(drm_mode_create_scaling_mode_property); >> >> +/** >> + * DOC: Variable refresh properties >> + * >> + * Variable refresh rate control is supported via properties on the >> + * _connector and _crtc objects. >> + * >> + * "vrr_capable": >> + * Optional _connector boolean property that drivers should attach >> + * with drm_connector_attach_vrr_capable_property() on connectors that >> + * could support variable refresh rates. Drivers should update the >> + * property value by calling drm_connector_set_vrr_capable_property(). >> + * >> + * Absence of the property should indicate absence of support. >> + * >> + * "vrr_enabled": >> + * Default _crtc boolean property that notifies the driver that the >> + * variable refresh rate adjustment should be enabled for the CRTC. > > Hi, > > where is the documentation that explains how drivers must implement > "variable refresh rate adjustment"? > > What should and could userspace expect to get if it flicks this switch? > > I also think the kernel documentation should include a description of > what VRR actually is and how it conceptually works as far as userspace > is concerned. > > That is, the kernel documentation should describe what this thing does, > so that we avoid every driver implementing a different thing. For > example, one driver could prevent the luminance flickering itself by > tuning the timings while another driver might not do that, which means > that an application tested on the former driver will look just fine > while it is unbearable to watch on the latter. > > > Thanks, > pq I felt it was best to leave this more on the vague side to not impose restrictions yet on what a driver must do. If you think it's worth defining what the "baseline" expectation is for userspace, I would probably describe it as "utilizing the monitor's variable refresh rate to reduce stuttering or tearing that can occur during flipping". This is currently what the amdgpu driver has enabled for support. The implementation also isn't much more complex than just passing the variable refresh range to the hardware. I wouldn't want every driver to be forced to implement some sort of luminance flickering by default. It's not noticeable on many panels and any tuning would inherently add latency to the output. It would probably be better left as a new property or a driver specific feature. In general I would imagine that most future VRR features would end up as new properties. Anything that's purely software could be implemented as a drm helper that every driver can use. I think the target presentation timestamp feature is a good example for that. > >> + * >> + * Support for variable refresh rate will depend on the "vrr_capable" >> + * property exposed on the _connector object. >> + */ >> + >> /** >>* drm_connector_attach_vrr_capable_property - creates the >>* vrr_capable property > Nicholas Kazlauskas ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [PATCH v2 5/6] dt-bindings: drm/panel: simple: Innolux TV123WAM is actually P120ZDG-BF1
On Fri, Oct 26, 2018 at 10:43:56AM -0400, Sean Paul wrote:
> On Thu, Oct 25, 2018 at 03:21:33PM -0700, Douglas Anderson wrote:
> > As far as I can tell the bindings that were added in commit
> > 9c04400f7ea6 ("dt-bindings: drm/panel: Document Innolux TV123WAM panel
> > bindings") weren't actually for Innolux TV123WAM but were actually for
> > Innolux P120ZDG-BF1.
> >
> > As far as I can tell the Innolux TV123WAM isn't a real panel and but
> > it's a mosh between the TI TV123WAM and the Innolux P120ZDG-BF1.
> > Let's unmosh.
> >
> > Here's my evidence:
> >
> > * Searching for TV123WAM on the Internet turns up a TI panel. While
> > it's possible that an Innolux panel has the same model number as the
> > TI Panel, it seems a little doubtful. Looking up the datasheet from
> > the TI Panel shows that it's 1920 x 1280 and 259.2 mm x 172.8 mm.
> >
> > * As far as I know, the patch adding the Innolux Panel was supposed to
> > be for the board that's sitting in front of me as I type this
> > (support for that board is not yet upstream). On the back of that
> > panel I see Innolux P120ZDZ-EZ1 rev B1.
> >
> > * Someone pointed me at a datasheet that's supposed to be for the
> > panel in front of me (sorry, I can't share the datasheet). That
> > datasheet has the string "p120zdg-bf1"
> >
> > * If I search for "P120ZDG-BF1" on the Internet I get hits for panels
> > that are 2160x1440. They don't have datasheets, but the fact that
> > the resolution matches is a good sign.
> >
> > While we doing the rename, also mention that no-hpd can be used with
> > this panel. See the previous patch in this series ("drm/panel:
> > simple: Add "no-hpd" delay for Innolux TV123WAM").
> >
> > Fixes: 9c04400f7ea6 ("dt-bindings: drm/panel: Document Innolux TV123WAM
> > panel bindings")
> > Signed-off-by: Douglas Anderson
> > Reviewed-by: Rob Herring
>
> I didn't see your v2 when I replied to the v1 patch, so for the record,
>
> Reviewed-by: Sean Paul
>
> Also note to whoever applies this to -misc, v1 also had
>
> Reviewed-by: Abhinav Kumar
And I just realized that patches 5 & 6 were swapped in v2. So nevermind this,
Doug's v2 tags are correct.
Dear coffee, please kick in soon.
Sean
>
>
> Sean
>
> > Cc: Sandeep Panda
> > ---
> >
> > Changes in v2: None
> >
> > .../{innolux,tv123wam.txt => innolux,p120zdg-bf1.txt} | 8 +---
> > 1 file changed, 5 insertions(+), 3 deletions(-)
> > rename
> > Documentation/devicetree/bindings/display/panel/{innolux,tv123wam.txt =>
> > innolux,p120zdg-bf1.txt} (70%)
> >
> > diff --git
> > a/Documentation/devicetree/bindings/display/panel/innolux,tv123wam.txt
> > b/Documentation/devicetree/bindings/display/panel/innolux,p120zdg-bf1.txt
> > similarity index 70%
> > rename from
> > Documentation/devicetree/bindings/display/panel/innolux,tv123wam.txt
> > rename to
> > Documentation/devicetree/bindings/display/panel/innolux,p120zdg-bf1.txt
> > index a9b35265fa13..513f03466aba 100644
> > --- a/Documentation/devicetree/bindings/display/panel/innolux,tv123wam.txt
> > +++
> > b/Documentation/devicetree/bindings/display/panel/innolux,p120zdg-bf1.txt
> > @@ -1,20 +1,22 @@
> > -Innolux TV123WAM 12.3 inch eDP 2K display panel
> > +Innolux P120ZDG-BF1 12.02 inch eDP 2K display panel
> >
> > This binding is compatible with the simple-panel binding, which is
> > specified
> > in simple-panel.txt in this directory.
> >
> > Required properties:
> > -- compatible: should be "innolux,tv123wam"
> > +- compatible: should be "innolux,p120zdg-bf1"
> > - power-supply: regulator to provide the supply voltage
> >
> > Optional properties:
> > - enable-gpios: GPIO pin to enable or disable the panel
> > - backlight: phandle of the backlight device attached to the panel
> > +- no-hpd: If HPD isn't hooked up; add this property.
> >
> > Example:
> > panel_edp: panel-edp {
> > - compatible = "innolux,tv123wam";
> > + compatible = "innolux,p120zdg-bf1";
> > enable-gpios = < 31 GPIO_ACTIVE_LOW>;
> > power-supply = <_l2>;
> > backlight = <>;
> > + no-hpd;
> > };
> > --
> > 2.19.1.568.g152ad8e336-goog
> >
>
> --
> Sean Paul, Software Engineer, Google / Chromium OS
--
Sean Paul, Software Engineer, Google / Chromium OS
___
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [PATCH v2 6/6] drm/panel: simple: Innolux TV123WAM is actually P120ZDG-BF1
On Thu, Oct 25, 2018 at 03:21:34PM -0700, Douglas Anderson wrote:
> As far as I can tell the panel that was added in commit da50bd4258db
> ("drm/panel: simple: Add Innolux TV123WAM panel driver support")
> wasn't actually an Innolux TV123WAM but was actually an Innolux
> P120ZDG-BF1.
>
> As far as I can tell the Innolux TV123WAM isn't a real panel and but
> it's a mosh between the TI TV123WAM and the Innolux P120ZDG-BF1.
> Let's unmosh.
>
> Here's my evidence:
>
> * Searching for TV123WAM on the Internet turns up a TI panel. While
> it's possible that an Innolux panel has the same model number as the
> TI Panel, it seems a little doubtful. Looking up the datasheet from
> the TI Panel shows that it's 1920 x 1280 and 259.2 mm x 172.8 mm.
>
> * As far as I know, the patch adding the Innolux Panel was supposed to
> be for the board that's sitting in front of me as I type this
> (support for that board is not yet upstream). On the back of that
> panel I see Innolux P120ZDZ-EZ1 rev B1.
>
> * Someone pointed me at a datasheet that's supposed to be for the
> panel in front of me (sorry, I can't share the datasheet). That
> datasheet has the string "p120zdg-bf1"
>
> * If I search for "P120ZDG-BF1" on the Internet I get hits for panels
> that are 2160x1440. They don't have datasheets, but the fact that
> the resolution matches is a good sign.
>
> In any case, let's update the name and also the physical size to match
> the correct panel.
>
> Fixes: da50bd4258db ("drm/panel: simple: Add Innolux TV123WAM panel driver
> support")
> Signed-off-by: Douglas Anderson
> Reviewed-by: Abhinav Kumar
> Cc: Sandeep Panda
Reviewed-by: Sean Paul
> ---
>
> Changes in v2: None
>
> drivers/gpu/drm/panel/panel-simple.c | 14 +++---
> 1 file changed, 7 insertions(+), 7 deletions(-)
>
> diff --git a/drivers/gpu/drm/panel/panel-simple.c
> b/drivers/gpu/drm/panel/panel-simple.c
> index 88592f9a0018..a04ffb3b2174 100644
> --- a/drivers/gpu/drm/panel/panel-simple.c
> +++ b/drivers/gpu/drm/panel/panel-simple.c
> @@ -1373,7 +1373,7 @@ static const struct panel_desc innolux_n156bge_l21 = {
> },
> };
>
> -static const struct drm_display_mode innolux_tv123wam_mode = {
> +static const struct drm_display_mode innolux_p120zdg_bf1_mode = {
> .clock = 206016,
> .hdisplay = 2160,
> .hsync_start = 2160 + 48,
> @@ -1387,13 +1387,13 @@ static const struct drm_display_mode
> innolux_tv123wam_mode = {
> .flags = DRM_MODE_FLAG_PHSYNC | DRM_MODE_FLAG_PVSYNC,
> };
>
> -static const struct panel_desc innolux_tv123wam = {
> - .modes = _tv123wam_mode,
> +static const struct panel_desc innolux_p120zdg_bf1 = {
> + .modes = _p120zdg_bf1_mode,
> .num_modes = 1,
> .bpc = 8,
> .size = {
> - .width = 259,
> - .height = 173,
> + .width = 254,
> + .height = 169,
> },
> .delay = {
> .hpd_absent_delay = 200,
> @@ -2456,8 +2456,8 @@ static const struct of_device_id platform_of_match[] = {
> .compatible = "innolux,n156bge-l21",
> .data = _n156bge_l21,
> }, {
> - .compatible = "innolux,tv123wam",
> - .data = _tv123wam,
> + .compatible = "innolux,p120zdg-bf1",
> + .data = _p120zdg_bf1,
> }, {
> .compatible = "innolux,zj070na-01p",
> .data = _zj070na_01p,
> --
> 2.19.1.568.g152ad8e336-goog
>
--
Sean Paul, Software Engineer, Google / Chromium OS
___
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [PATCH v2 5/6] dt-bindings: drm/panel: simple: Innolux TV123WAM is actually P120ZDG-BF1
On Thu, Oct 25, 2018 at 03:21:33PM -0700, Douglas Anderson wrote:
> As far as I can tell the bindings that were added in commit
> 9c04400f7ea6 ("dt-bindings: drm/panel: Document Innolux TV123WAM panel
> bindings") weren't actually for Innolux TV123WAM but were actually for
> Innolux P120ZDG-BF1.
>
> As far as I can tell the Innolux TV123WAM isn't a real panel and but
> it's a mosh between the TI TV123WAM and the Innolux P120ZDG-BF1.
> Let's unmosh.
>
> Here's my evidence:
>
> * Searching for TV123WAM on the Internet turns up a TI panel. While
> it's possible that an Innolux panel has the same model number as the
> TI Panel, it seems a little doubtful. Looking up the datasheet from
> the TI Panel shows that it's 1920 x 1280 and 259.2 mm x 172.8 mm.
>
> * As far as I know, the patch adding the Innolux Panel was supposed to
> be for the board that's sitting in front of me as I type this
> (support for that board is not yet upstream). On the back of that
> panel I see Innolux P120ZDZ-EZ1 rev B1.
>
> * Someone pointed me at a datasheet that's supposed to be for the
> panel in front of me (sorry, I can't share the datasheet). That
> datasheet has the string "p120zdg-bf1"
>
> * If I search for "P120ZDG-BF1" on the Internet I get hits for panels
> that are 2160x1440. They don't have datasheets, but the fact that
> the resolution matches is a good sign.
>
> While we doing the rename, also mention that no-hpd can be used with
> this panel. See the previous patch in this series ("drm/panel:
> simple: Add "no-hpd" delay for Innolux TV123WAM").
>
> Fixes: 9c04400f7ea6 ("dt-bindings: drm/panel: Document Innolux TV123WAM panel
> bindings")
> Signed-off-by: Douglas Anderson
> Reviewed-by: Rob Herring
I didn't see your v2 when I replied to the v1 patch, so for the record,
Reviewed-by: Sean Paul
Also note to whoever applies this to -misc, v1 also had
Reviewed-by: Abhinav Kumar
Sean
> Cc: Sandeep Panda
> ---
>
> Changes in v2: None
>
> .../{innolux,tv123wam.txt => innolux,p120zdg-bf1.txt} | 8 +---
> 1 file changed, 5 insertions(+), 3 deletions(-)
> rename Documentation/devicetree/bindings/display/panel/{innolux,tv123wam.txt
> => innolux,p120zdg-bf1.txt} (70%)
>
> diff --git
> a/Documentation/devicetree/bindings/display/panel/innolux,tv123wam.txt
> b/Documentation/devicetree/bindings/display/panel/innolux,p120zdg-bf1.txt
> similarity index 70%
> rename from
> Documentation/devicetree/bindings/display/panel/innolux,tv123wam.txt
> rename to
> Documentation/devicetree/bindings/display/panel/innolux,p120zdg-bf1.txt
> index a9b35265fa13..513f03466aba 100644
> --- a/Documentation/devicetree/bindings/display/panel/innolux,tv123wam.txt
> +++ b/Documentation/devicetree/bindings/display/panel/innolux,p120zdg-bf1.txt
> @@ -1,20 +1,22 @@
> -Innolux TV123WAM 12.3 inch eDP 2K display panel
> +Innolux P120ZDG-BF1 12.02 inch eDP 2K display panel
>
> This binding is compatible with the simple-panel binding, which is specified
> in simple-panel.txt in this directory.
>
> Required properties:
> -- compatible: should be "innolux,tv123wam"
> +- compatible: should be "innolux,p120zdg-bf1"
> - power-supply: regulator to provide the supply voltage
>
> Optional properties:
> - enable-gpios: GPIO pin to enable or disable the panel
> - backlight: phandle of the backlight device attached to the panel
> +- no-hpd: If HPD isn't hooked up; add this property.
>
> Example:
> panel_edp: panel-edp {
> - compatible = "innolux,tv123wam";
> + compatible = "innolux,p120zdg-bf1";
> enable-gpios = < 31 GPIO_ACTIVE_LOW>;
> power-supply = <_l2>;
> backlight = <>;
> + no-hpd;
> };
> --
> 2.19.1.568.g152ad8e336-goog
>
--
Sean Paul, Software Engineer, Google / Chromium OS
___
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel
[PATCH] drm/v3d: Fix compilation error on kbuilt test robot.
Signed-off-by: Andrey Grodzovsky
---
drivers/gpu/drm/v3d/v3d_sched.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/drivers/gpu/drm/v3d/v3d_sched.c b/drivers/gpu/drm/v3d/v3d_sched.c
index 273d0fb..80b641f 100644
--- a/drivers/gpu/drm/v3d/v3d_sched.c
+++ b/drivers/gpu/drm/v3d/v3d_sched.c
@@ -222,7 +222,7 @@ v3d_sched_init(struct v3d_dev *v3d)
_sched_ops,
hw_jobs_limit, job_hang_limit,
msecs_to_jiffies(hang_limit_ms),
-"v3d_render", true);
+"v3d_render");
if (ret) {
dev_err(v3d->dev, "Failed to create render scheduler: %d.",
ret);
--
2.7.4
___
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel
[radeon-alex:amd-staging-drm-next 696/705] drivers/staging/vboxvideo/vbox_drv.h:98:28: error: field 'bo_global_ref' has incomplete type
tree: git://people.freedesktop.org/~agd5f/linux.git amd-staging-drm-next
head: 4cab470e01108ad0a7a74c6a30d83e7e8e60aa9e
commit: 852c20fdce7c6702c4d073f2c238c0859344bcb9 [696/705] drm/ttm: initialize
globals during device init
config: i386-allmodconfig (attached as .config)
compiler: gcc-7 (Debian 7.3.0-1) 7.3.0
reproduce:
git checkout 852c20fdce7c6702c4d073f2c238c0859344bcb9
# save the attached .config to linux build tree
make ARCH=i386
All errors (new ones prefixed by >>):
In file included from drivers/staging/vboxvideo/hgsmi_base.c:23:0:
>> drivers/staging/vboxvideo/vbox_drv.h:98:28: error: field 'bo_global_ref' has
>> incomplete type
struct ttm_bo_global_ref bo_global_ref;
^
--
In file included from drivers/staging/vboxvideo/vbox_ttm.c:30:0:
>> drivers/staging/vboxvideo/vbox_drv.h:98:28: error: field 'bo_global_ref' has
>> incomplete type
struct ttm_bo_global_ref bo_global_ref;
^
drivers/staging/vboxvideo/vbox_ttm.c: In function 'vbox_mm_init':
>> drivers/staging/vboxvideo/vbox_ttm.c:204:2: error: implicit declaration of
>> function 'vbox_ttm_global_release'; did you mean 'ttm_mem_global_release'?
>> [-Werror=implicit-function-declaration]
vbox_ttm_global_release(vbox);
^~~
ttm_mem_global_release
cc1: some warnings being treated as errors
vim +/bo_global_ref +98 drivers/staging/vboxvideo/vbox_drv.h
dd55d44f Hans de Goede 2017-07-06 76
dd55d44f Hans de Goede 2017-07-06 77 struct vbox_private {
dd55d44f Hans de Goede 2017-07-06 78 struct drm_device *dev;
dd55d44f Hans de Goede 2017-07-06 79
dd55d44f Hans de Goede 2017-07-06 80 u8 __iomem *guest_heap;
dd55d44f Hans de Goede 2017-07-06 81 u8 __iomem *vbva_buffers;
dd55d44f Hans de Goede 2017-07-06 82 struct gen_pool *guest_pool;
dd55d44f Hans de Goede 2017-07-06 83 struct vbva_buf_ctx *vbva_info;
dd55d44f Hans de Goede 2017-07-06 84 bool any_pitch;
dd55d44f Hans de Goede 2017-07-06 85 u32 num_crtcs;
dd55d44f Hans de Goede 2017-07-06 86 /** Amount of available VRAM,
including space used for buffers. */
dd55d44f Hans de Goede 2017-07-06 87 u32 full_vram_size;
dd55d44f Hans de Goede 2017-07-06 88 /** Amount of available VRAM,
not including space used for buffers. */
dd55d44f Hans de Goede 2017-07-06 89 u32 available_vram_size;
dd55d44f Hans de Goede 2017-07-06 90 /** Array of structures for
receiving mode hints. */
dd55d44f Hans de Goede 2017-07-06 91 struct vbva_modehint
*last_mode_hints;
dd55d44f Hans de Goede 2017-07-06 92
dd55d44f Hans de Goede 2017-07-06 93 struct vbox_fbdev *fbdev;
dd55d44f Hans de Goede 2017-07-06 94
dd55d44f Hans de Goede 2017-07-06 95 int fb_mtrr;
dd55d44f Hans de Goede 2017-07-06 96
dd55d44f Hans de Goede 2017-07-06 97 struct {
dd55d44f Hans de Goede 2017-07-06 @98 struct
ttm_bo_global_ref bo_global_ref;
dd55d44f Hans de Goede 2017-07-06 99 struct ttm_bo_device
bdev;
dd55d44f Hans de Goede 2017-07-06 100 } ttm;
dd55d44f Hans de Goede 2017-07-06 101
dd55d44f Hans de Goede 2017-07-06 102 struct mutex hw_mutex; /*
protects modeset and accel/vbva accesses */
dd55d44f Hans de Goede 2017-07-06 103 /**
dd55d44f Hans de Goede 2017-07-06 104 * We decide whether or not
user-space supports display hot-plug
dd55d44f Hans de Goede 2017-07-06 105 * depending on whether they
react to a hot-plug event after the initial
dd55d44f Hans de Goede 2017-07-06 106 * mode query.
dd55d44f Hans de Goede 2017-07-06 107 */
dd55d44f Hans de Goede 2017-07-06 108 bool initial_mode_queried;
dd55d44f Hans de Goede 2017-07-06 109 struct work_struct hotplug_work;
dd55d44f Hans de Goede 2017-07-06 110 u32 input_mapping_width;
dd55d44f Hans de Goede 2017-07-06 111 u32 input_mapping_height;
dd55d44f Hans de Goede 2017-07-06 112 /**
dd55d44f Hans de Goede 2017-07-06 113 * Is user-space using an
X.Org-style layout of one large frame-buffer
dd55d44f Hans de Goede 2017-07-06 114 * encompassing all screen ones
or is the fbdev console active?
dd55d44f Hans de Goede 2017-07-06 115 */
dd55d44f Hans de Goede 2017-07-06 116 bool single_framebuffer;
dd55d44f Hans de Goede 2017-07-06 117 u32 cursor_width;
dd55d44f Hans de Goede 2017-07-06 118 u32 cursor_height;
dd55d44f Hans de Goede 2017-07-06 119 u32 cursor_hot_x;
dd55d44f Hans de Goede 2017-07-06 120 u32 cursor_hot_y;
dd55d44f Hans de Goede 2017-07-06 121 size_t cursor_data_size;
dd55d44f Hans de Goede 2017-07-06 122 u8
cursor_data[CURSOR_DATA_SIZE];
dd55d44f Hans de Goede
Re: [PATCH v2 3/6] drm/panel: simple: Add "no-hpd" delay for Innolux TV123WAM
On Thu, Oct 25, 2018 at 03:21:31PM -0700, Douglas Anderson wrote:
> If the HPD signal isn't hooked up to this panel we need a 200 ms
> delay. In the datasheet this is shown as the maximum time that HPD
> will take to be asserted after power is given to the panel.
>
> Signed-off-by: Douglas Anderson
Reviewed-by: Sean Paul
> ---
>
> Changes in v2:
> - Use "hpd_absent_delay" property instead of a bool + prepare delay
>
> drivers/gpu/drm/panel/panel-simple.c | 1 +
> 1 file changed, 1 insertion(+)
>
> diff --git a/drivers/gpu/drm/panel/panel-simple.c
> b/drivers/gpu/drm/panel/panel-simple.c
> index 687fd087b9fc..88592f9a0018 100644
> --- a/drivers/gpu/drm/panel/panel-simple.c
> +++ b/drivers/gpu/drm/panel/panel-simple.c
> @@ -1396,6 +1396,7 @@ static const struct panel_desc innolux_tv123wam = {
> .height = 173,
> },
> .delay = {
> + .hpd_absent_delay = 200,
> .unprepare = 500,
> },
> };
> --
> 2.19.1.568.g152ad8e336-goog
>
--
Sean Paul, Software Engineer, Google / Chromium OS
___
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [igt-dev] [PATCH] RFC: Make igts for cross-driver stuff mandatory?
On Fri, Oct 26, 2018 at 4:32 AM Daniel Vetter wrote: > > On Fri, Oct 26, 2018 at 5:50 AM Zhou, David(ChunMing) > wrote: > > > > Make igt for cross-driver, I think you should rename it first, not an intel > > specific. NO company wants their employee working on other company stuff. > > You can rename it to DGT(drm graphics test), and published following > > libdrm, or directly merge to libdrm, then everyone can use it and develop > > it in same page, which is only my personal opinion. > > We renamed it ot IGT gpu tools, that was even enough for ARM folks. > If this is seriously what AMD expects before considering, I'm not sure > what to say. > > Alex, Christian, is this the official AMD stance that you can't touch > stuff because of the letter i? We don't have any restrictions. Alex > -Daniel > > > > Regards, > > David > > > > > -Original Message- > > > From: dri-devel On Behalf Of > > > Eric > > > Anholt > > > Sent: Friday, October 26, 2018 12:36 AM > > > To: Sean Paul ; Daniel Vetter > > > Cc: IGT development ; Intel Graphics > > > Development ; DRI Development > > de...@lists.freedesktop.org>; amd-...@lists.freedesktop.org > > > Subject: Re: [igt-dev] [PATCH] RFC: Make igts for cross-driver stuff > > > mandatory? > > > > > > Sean Paul writes: > > > > > > > On Fri, Oct 19, 2018 at 10:50:49AM +0200, Daniel Vetter wrote: > > > >> Hi all, > > > >> > > > >> This is just to collect feedback on this idea, and see whether the > > > >> overall dri-devel community stands on all this. I think the past few > > > >> cross-vendor uapi extensions all came with igts attached, and > > > >> personally I think there's lots of value in having them: A > > > >> cross-vendor interface isn't useful if every driver implements it > > > >> slightly differently. > > > >> > > > >> I think there's 2 questions here: > > > >> > > > >> - Do we want to make such testcases mandatory? > > > >> > > > > > > > > Yes, more testing == better code. > > > > > > > > > > > >> - If yes, are we there yet, or is there something crucially missing > > > >> still? > > > > > > > > In my experience, no. Last week while trying to replicate an intel-gfx > > > > CI failure, I tried compiling igt for one of my (intel) chromebooks. > > > > It seems like cross-compilation (or, in my case, just specifying > > > > prefix/ld_library_path/sbin_path) is broken on igt. If we want to > > > > impose restrictions across the entire subsystem, we need to make sure > > > > that everyone can build and deploy igt easily. > > > > > > > > I managed to hack around everything and get it working, but I still > > > > haven't tried switching out the toolchain. Once we have some GitLab CI > > > > to validate cross-compilation, then we can consider making IGT > > > > mandatory. > > > > > > > > It's possible that I'm just a meson n00b and didn't use the right > > > > incantation, so maybe it already works, but then we need better > > > documentation. > > > > > > > > I've pasted my horrible hacks below, I also didn't have libunwind, so > > > > removed its usage. > > > > > > I've also had to cut out libunwind for cross-compiling on many occasions. > > > Worst library. > > > > -- > Daniel Vetter > Software Engineer, Intel Corporation > +41 (0) 79 365 57 48 - http://blog.ffwll.ch > ___ > amd-gfx mailing list > amd-...@lists.freedesktop.org > https://lists.freedesktop.org/mailman/listinfo/amd-gfx ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [PATCH v2 2/6] drm/panel: simple: Support panels with HPD where HPD isn't connected
On Thu, Oct 25, 2018 at 03:21:30PM -0700, Douglas Anderson wrote:
> Some eDP panels that are designed to be always connected to a board
> use their HPD signal to signal that they've finished powering on and
> they're ready to be talked to.
>
> However, for various reasons it's possible that the HPD signal from
> the panel isn't actually hooked up. In the case where the HPD isn't
> hooked up you can look at the timing diagram on the panel datasheet
> and insert a delay for the maximum amount of time that the HPD might
> take to come up.
>
> Let's add support in simple-panel for this concept.
>
> At the moment we will co-opt the existing "prepare" delay to keep
> track of the delay and we'll use a boolean to specify that a given
> panel should only apply the delay if the "no-hpd" property was
> specified.
>
> Signed-off-by: Douglas Anderson
Reviewed-by: Sean Paul
> ---
>
> Changes in v2:
> - Use "hpd_absent_delay" property instead of a bool + prepare delay
>
> drivers/gpu/drm/panel/panel-simple.c | 14 --
> 1 file changed, 12 insertions(+), 2 deletions(-)
>
> diff --git a/drivers/gpu/drm/panel/panel-simple.c
> b/drivers/gpu/drm/panel/panel-simple.c
> index 97964f7f2ace..687fd087b9fc 100644
> --- a/drivers/gpu/drm/panel/panel-simple.c
> +++ b/drivers/gpu/drm/panel/panel-simple.c
> @@ -56,6 +56,8 @@ struct panel_desc {
> /**
>* @prepare: the time (in milliseconds) that it takes for the panel to
>* become ready and start receiving video data
> + * @hpd_absent_delay: Add this to the prepare delay if we know Hot
> + *Plug Detect isn't used.
>* @enable: the time (in milliseconds) that it takes for the panel to
>* display the first valid frame after starting to receive
>* video data
> @@ -66,6 +68,7 @@ struct panel_desc {
>*/
> struct {
> unsigned int prepare;
> + unsigned int hpd_absent_delay;
> unsigned int enable;
> unsigned int disable;
> unsigned int unprepare;
> @@ -79,6 +82,7 @@ struct panel_simple {
> struct drm_panel base;
> bool prepared;
> bool enabled;
> + bool no_hpd;
>
> const struct panel_desc *desc;
>
> @@ -202,6 +206,7 @@ static int panel_simple_unprepare(struct drm_panel *panel)
> static int panel_simple_prepare(struct drm_panel *panel)
> {
> struct panel_simple *p = to_panel_simple(panel);
> + unsigned int delay;
> int err;
>
> if (p->prepared)
> @@ -215,8 +220,11 @@ static int panel_simple_prepare(struct drm_panel *panel)
>
> gpiod_set_value_cansleep(p->enable_gpio, 1);
>
> - if (p->desc->delay.prepare)
> - msleep(p->desc->delay.prepare);
> + delay = p->desc->delay.prepare;
> + if (p->no_hpd)
> + delay += p->desc->delay.hpd_absent_delay;
> + if (delay)
> + msleep(delay);
>
> p->prepared = true;
>
> @@ -305,6 +313,8 @@ static int panel_simple_probe(struct device *dev, const
> struct panel_desc *desc)
> panel->prepared = false;
> panel->desc = desc;
>
> + panel->no_hpd = of_property_read_bool(dev->of_node, "no-hpd");
> +
> panel->supply = devm_regulator_get(dev, "power");
> if (IS_ERR(panel->supply))
> return PTR_ERR(panel->supply);
> --
> 2.19.1.568.g152ad8e336-goog
>
--
Sean Paul, Software Engineer, Google / Chromium OS
___
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [PATCH] drm/vgem: create a render node for vgem
Quoting Daniel Vetter (2018-10-26 14:40:36) > On Fri, Oct 26, 2018 at 01:06:47PM +0100, Emil Velikov wrote: > > From: Emil Velikov > > > > VGEM doesn't do anything modeset specific, so in a way exposing a > > primary node is 'wrong'. At the same time, we extensively use if for > > creating dumb buffers, fences, prime fd <> handle imports/exports. What problem are you solving? Now if you suggest a way to only expose a render node and not a master node, that might be more interesting. -Chris ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [PATCH 5/6] drm/panel: simple: Innolux TV123WAM is actually P120ZDG-BF1
On Thu, Oct 25, 2018 at 03:24:58PM -0700, Doug Anderson wrote:
> Hi,
>
> On Thu, Oct 25, 2018 at 11:13 AM Sean Paul wrote:
> >
> > On Mon, Oct 22, 2018 at 01:46:38PM -0700, Douglas Anderson wrote:
> > > As far as I can tell the panel that was added in commit da50bd4258db
> > > ("drm/panel: simple: Add Innolux TV123WAM panel driver support")
> > > wasn't actually an Innolux TV123WAM but was actually an Innolux
> > > P120ZDG-BF1.
> > >
> > > As far as I can tell the Innolux TV123WAM isn't a real panel and but
> > > it's a mosh between the TI TV123WAM and the Innolux P120ZDG-BF1.
> > > Let's unmosh.
> > >
> > > Here's my evidence:
> > >
> > > * Searching for TV123WAM on the Internet turns up a TI panel. While
> > > it's possible that an Innolux panel has the same model number as the
> > > TI Panel, it seems a little doubtful. Looking up the datasheet from
> > > the TI Panel shows that it's 1920 x 1280 and 259.2 mm x 172.8 mm.
> > >
> > > * As far as I know, the patch adding the Innolux Panel was supposed to
> > > be for the board that's sitting in front of me as I type this
> > > (support for that board is not yet upstream). On the back of that
> > > panel I see Innolux P120ZDZ-EZ1 rev B1.
> > >
> > > * Someone pointed me at a datasheet that's supposed to be for the
> > > panel in front of me (sorry, I can't share the datasheet). That
> > > datasheet has the string "p120zdg-bf1"
> > >
> > > * If I search for "P120ZDG-BF1" on the Internet I get hits for panels
> > > that are 2160x1440. They don't have datasheets, but the fact that
> > > the resolution matches is a good sign.
> > >
> > > In any case, let's update the name and also the physical size to match
> > > the correct panel.
> > >
> > > Fixes: da50bd4258db ("drm/panel: simple: Add Innolux TV123WAM panel
> > > driver support")
> > > Signed-off-by: Douglas Anderson
> > > Cc: Sandeep Panda
> > > ---
> > >
> > > drivers/gpu/drm/panel/panel-simple.c | 14 +++---
> > > 1 file changed, 7 insertions(+), 7 deletions(-)
> > >
> > > diff --git a/drivers/gpu/drm/panel/panel-simple.c
> > > b/drivers/gpu/drm/panel/panel-simple.c
> > > index 937e97490c30..7ee1abc5d81b 100644
> > > --- a/drivers/gpu/drm/panel/panel-simple.c
> > > +++ b/drivers/gpu/drm/panel/panel-simple.c
> > > @@ -1370,7 +1370,7 @@ static const struct panel_desc innolux_n156bge_l21
> > > = {
> > > },
> > > };
> > >
> > > -static const struct drm_display_mode innolux_tv123wam_mode = {
> > > +static const struct drm_display_mode innolux_p120zdg_bf1_mode = {
> > > .clock = 206016,
> > > .hdisplay = 2160,
> > > .hsync_start = 2160 + 48,
> > > @@ -1384,13 +1384,13 @@ static const struct drm_display_mode
> > > innolux_tv123wam_mode = {
> > > .flags = DRM_MODE_FLAG_PHSYNC | DRM_MODE_FLAG_PVSYNC,
> > > };
> > >
> > > -static const struct panel_desc innolux_tv123wam = {
> > > - .modes = _tv123wam_mode,
> > > +static const struct panel_desc innolux_p120zdg_bf1 = {
> > > + .modes = _p120zdg_bf1_mode,
> > > .num_modes = 1,
> > > .bpc = 8,
> > > .size = {
> > > - .width = 259,
> > > - .height = 173,
> > > + .width = 254,
> > > + .height = 169,
> > > },
> > > .delay = {
> > > .prepare = 200,
> > > @@ -2454,8 +2454,8 @@ static const struct of_device_id
> > > platform_of_match[] = {
> > > .compatible = "innolux,n156bge-l21",
> > > .data = _n156bge_l21,
> > > }, {
> > > - .compatible = "innolux,tv123wam",
> >
> > I think we should update the struct, but we might want to keep this around.
> > Given the tv123wam panel is TI, we're likely not going to have a collision
> > on
> > innolux,...
> >
> > That said, I'll defer to robh on this one, I'm not sure if changing names is
> > cool once the bindings have hit mainline.
>
> Rob gave the bindings patch a Reviewed-by tag, so I'm assuming he's
> cool with it. v2 still doesn't keep the "innolux,tv123wam" around.
> If you disagree then let me know and I'll do a v3.
I happily defer to Rob on all things dt. So,
Reviewed-by: Sean Paul
>
> -Doug
--
Sean Paul, Software Engineer, Google / Chromium OS
___
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel
[Bug 108562] Radeon R5 M330 freezes when dpm enabled
https://bugs.freedesktop.org/show_bug.cgi?id=108562 --- Comment #1 from Alex Deucher --- Please attach your xorg log and dmesg output. -- You are receiving this mail because: You are the assignee for the bug.___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [PATCH v5 2/4] drm: Add vrr_enabled property to drm CRTC
On Mon, 15 Oct 2018 12:06:52 +0200 Michel Dänzer wrote: > On 2018-10-15 11:47 a.m., Christian König wrote: > > Am 15.10.2018 um 11:40 schrieb Michel Dänzer: > >> On 2018-10-13 7:38 p.m., Christian König wrote: > >>> Am 12.10.2018 um 18:44 schrieb Nicholas Kazlauskas: > This patch introduces the 'vrr_enabled' CRTC property to allow > dynamic control over variable refresh rate support for a CRTC. > > This property should be treated like a content hint to the driver - > if the hardware or driver is not capable of driving variable refresh > timings then this is not considered an error. > > Capability for variable refresh rate support should be determined > by querying the vrr_capable drm connector property. > > It is worth noting that while the property is intended for atomic use > it isn't filtered from legacy userspace queries. This allows for Xorg > userspace drivers to implement support. > >>> I'm honestly still not convinced that a per CRTC property is actually > >>> the right approach. > >>> > >>> What we should rather do instead is to implement a target timestamp for > >>> the page flip. > >> You'd have to be more specific about how the latter could completely > >> replace the former. I don't see how it could. > > > > Each flip request send by an application gets a timestamp when the flip > > should be displayed. > > > > If I'm not completely mistaken we already have something like that in > > both DRI2 as well as DRI3. > > Certainly not DRI2, but for now we're not enabling VRR with that anyway. > > While the X11 Present extension specifies PresentOptionUST for this, > support for it isn't implemented yet in xserver (as in setting the > option has no effect, the X server interprets the target value as an MSC > anyway). > > So this couldn't work before the next major release of xserver, which > based on recent history could be at least about one year away. Hi > In contrast, the CRTC property based solution for the gaming use-case > can work even with older xserver versions. This is probably the heaviest reason. Coming up with a KMS UABI for target timestamps could get complicated. Do you need a flip queue deeper than one? Do you need to be able to cancel flips? > > So as far as I can see we only need to add an extra flag that those > > information are trust worth in the context of VRR as well. > > > > If we don't set this flag we always get the always working fallback > > behavior, e.g. VRR is disabled and we have a fixed refresh rate. > > > > If we set this flag and the timestamp is in the past we get the VRR > > behavior to display the next frame as soon as possible. > > > > If we set this flag and specific a specific timestamp then we get the > > VRR behavior to display the frame as close as possible to the specified > > timestamp. > > Apart from the above, another issue is that this would give direct > control to the client on whether or not VRR should be used. But we want > to allow the user to disable VRR even if a client wants to use it, via > an RandR output property. This requires that the Xorg driver can control > whether or not VRR can actually be used, via the CRTC property added by > this patch. It would not imply direct control to clients. The target timestamps go through the X server, the X server can mangle them or remove them before calling KMS any way it wants. The X server can invent a RandR property to disable/enable VRR. One would need a video-DDX update the very minimum to start passing the timestamps to the kernel, so there is no way VRR would be enabled unwanted. Thanks, pq pgpIMvjgnI1uP.pgp Description: OpenPGP digital signature ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [RFC PATCH] drm/vc4: Add a load tracker to prevent HVS underflow errors
On Fri, Oct 26, 2018 at 3:57 PM Boris Brezillon wrote: > > On Fri, 26 Oct 2018 15:30:26 +0200 > Daniel Vetter wrote: > > > On Thu, Oct 25, 2018 at 11:41:14AM +0200, Boris Brezillon wrote: > > > On Thu, 25 Oct 2018 11:33:14 +0200 > > > Daniel Vetter wrote: > > > > > > > On Thu, Oct 25, 2018 at 10:09:31AM +0200, Boris Brezillon wrote: > > > > > On Tue, 23 Oct 2018 15:44:43 +0200 > > > > > Daniel Vetter wrote: > > > > > > > > > > > On Tue, Oct 23, 2018 at 09:55:08AM +0200, Boris Brezillon wrote: > > > > > > > Hi Daniel, > > > > > > > > > > > > > > On Tue, 16 Oct 2018 14:57:43 +0200 > > > > > > > Daniel Vetter wrote: > > > > > > > > > > > > > > > On Tue, Oct 16, 2018 at 11:40:45AM +0200, Boris Brezillon wrote: > > > > > > > > > The HVS block is supposed to fill the pixelvalve FIFOs fast > > > > > > > > > enough to > > > > > > > > > meet the requested framerate. The problem is, the HVS and > > > > > > > > > memory bus > > > > > > > > > bandwidths are limited, and if we don't take these > > > > > > > > > limitations into > > > > > > > > > account we might end up with HVS underflow errors. > > > > > > > > > > > > > > > > > > This patch is trying to model the per-plane HVS and memory > > > > > > > > > bus bandwidth > > > > > > > > > consumption and take a decision at atomic_check() time > > > > > > > > > whether the > > > > > > > > > estimated load will fit in the HVS and membus budget. > > > > > > > > > > > > > > > > > > Note that we take an extra margin on the memory bus > > > > > > > > > consumption to let > > > > > > > > > the system run smoothly when other blocks are doing heavy use > > > > > > > > > of the > > > > > > > > > memory bus. Same goes for the HVS limit, except the margin is > > > > > > > > > smaller in > > > > > > > > > this case, since the HVS is not used by external components. > > > > > > > > > > > > > > > > > > Signed-off-by: Boris Brezillon > > > > > > > > > --- > > > > > > > > > This logic has been validated using a simple shell script and > > > > > > > > > some instrumentation in the VC4 driver: > > > > > > > > > > > > > > > > > > - capture underflow errors at the HVS level and expose a > > > > > > > > > debugfs file > > > > > > > > > reporting those errors > > > > > > > > > - add debugfs files to expose when atomic_check fails because > > > > > > > > > of the > > > > > > > > > HVS or membus load limitation or when it fails for other > > > > > > > > > reasons > > > > > > > > > > > > > > > > > > The branch containing those modification is available here > > > > > > > > > [1], and the > > > > > > > > > script (which is internally using modetest) is here [2] > > > > > > > > > (please note > > > > > > > > > that I'm bad at writing shell scripts :-)). > > > > > > > > > > > > > > > > > > Note that those modification tend to over-estimate the load, > > > > > > > > > and thus > > > > > > > > > reject setups that might have previously worked, so we might > > > > > > > > > want to > > > > > > > > > adjust the limits to avoid that. > > > > > > > > > > > > > > > > > > [1]https://github.com/bbrezillon/linux/tree/vc4/hvs-bandwidth-eval > > > > > > > > > [2]https://github.com/bbrezillon/vc4-hvs-bandwidth-test > > > > > > > > > > > > > > > > Any interest in using igt to test this stuff? We have at least > > > > > > > > a bunch of > > > > > > > > tests already in there that try all kinds of plane setups. And > > > > > > > > we use > > > > > > > > those to hunt for underruns on i915 hw. > > > > > > > > > > > > > > > > Wrt underrun reporting: On i915 we just dump them into dmesg at > > > > > > > > the error > > > > > > > > level, using DRM_ERROR, > > > > > > > > > > > > > > Are you masking the underrun interrupt after it's been reported? > > > > > > > If we > > > > > > > don't do that on VC4 we just end up flooding the kernel-log > > > > > > > buffer until > > > > > > > someone comes and update the config. > > > > > > > > > > > > Yeah we do that too. Rule is that a full modeset will clear any > > > > > > underrun > > > > > > masking (so tests need to make sure they start with a modeset, or > > > > > > it'll be > > > > > > for nothing). > > > > > > > > > > > > > > > > > > > > > plus a tracepoint. See e.g. > > > > > > > > intel_pch_fifo_underrun_irq_handler(). If there's interest we > > > > > > > > could > > > > > > > > perhaps extract this into something common, similar to what was > > > > > > > > done with > > > > > > > > crc support already. > > > > > > > > > > > > > > > > > > > > > > I'm not a big fan of hardcoded trace points in general (because > > > > > > > of the > > > > > > > whole "it's part of the stable ABI" thing), and in this case, > > > > > > > making the > > > > > > > tracepoint generic sounds even more risky to me. Indeed, how can > > > > > > > we know > > > > > > > about all the HW specific bits one might want to expose. For > > > > > > > instance, > > > > > > > I see the intel underrun tracepoint exposes a struct with a frame > > > > > > > and > > > > >
Re: [PATCH v2 1/3] drm/atomic: Add a generic infrastructure to track underrun errors
On Fri, Oct 26, 2018 at 4:13 PM Boris Brezillon wrote: > > On Fri, 26 Oct 2018 15:36:15 +0200 > Daniel Vetter wrote: > > > On Fri, Oct 26, 2018 at 02:36:56PM +0200, Boris Brezillon wrote: > > > Hi Daniel, > > > > > > On Fri, 26 Oct 2018 12:33:37 +0200 > > > Daniel Vetter wrote: > > > > > > > On Thu, Oct 25, 2018 at 02:45:44PM +0200, Boris Brezillon wrote: > > > > > Display controllers usually provide a lot features like overlay > > > > > planes, > > > > > hardware scalers, hardware rotations, ... > > > > > Most of the time those features work just fine when enabled > > > > > independently, but activating all of them at the same time tend to > > > > > show other limitations like the limited memory bus or scaler > > > > > bandwidth. > > > > > > > > > > This sort of bandwidth estimation is hard to get right, and we > > > > > sometimes fail to predict that a specific workload will not pass, > > > > > which will most likely result in underrun errors somewhere in the > > > > > display pipeline (most of the time at the CRTC level). > > > > > > > > > > This path aims at making underrun error tracking generic and exposing > > > > > underrun errors to userspace through a debugfs file. > > > > > > > > > > This way, CI tools like IGT, can try to enable a bunch of features and > > > > > if such underrun errors are reported after the settings have been > > > > > accepted and applied by the KMS infrastructure. When that happens it > > > > > means the load tracking algorithm needs some tweaking/fixing. > > > > > > > > > > Signed-off-by: Boris Brezillon > > > > > --- > > > > > Changes in v2: > > > > > - New patch > > > > > > > > Hm, not what I thought off. This is definitely not going to be reusable > > > > for i915, the only other driver with underrun reporting, since we have > > > > tons of underrun sources (not just one). And we need to mask/unmask them > > > > individually. > > > > > > I agree it's not perfect, but I did it this way so that hopefully we'll > > > be able to use drm_atomic_helper_commit_tail() at some point instead of > > > having our own implementation. > > > > I expect every non-trivial driver to need to overwrite commit_tail. That's > > the entire point for having created it. Exceptions are just the very, very > > simple ones without anything fancy. > > Again, just my opinion, but doing that means some drivers are left > behind when new stuff are added to the generic helpers, and sometimes > those new features might be of interest to the drivers that are not > directly using those generic helpers, except they usually don't even > know about it until one comes at looking at the generic helper again > and notice the additions. > Maybe I'm not following the dri-devel ML close enough, and I'm probably > the one to blame here, but I see a real benefit in having the maximum > number of drivers using the generic helpers. To be honest, with a few > modifications, I think the VC4 driver could entirely rely on those > generic helpers. commit_tail commits the sw state to hw. Your hw will not magically gain any new features, so there's really not much point in automatically adding stuff to the overall hw commit flow. Some exceptoins apply, and those get reviewed when the core change goes in. Yes I spend ridiculous amounts of my time reading everyone else's drivers. And I really don't want to make commit_tail 100% applicapable to everyone, that just makes reviewing drivers harder (since the flow isn't obvious by simply looking at commit_tail, instead you have to hunt down feature flags and other stuff that control things). This holds in general btw, and it's part of the no-midlayer design paradigm. No huge blobs that work for everyone, instead make things modular. But if you have a concrete example where we added something, and a driver should have used it but didn't, we'll need to look into that. -Daniel > > > If there's a way to add pre/post_commit() hooks (maybe they already > > > exist?) at various levels of the display pipeline that get called even > > > when the associated state has *not* changed, then I might be able to > > > place my mask/unmask/clear operations there. I really need that to > > > properly unmask the underrun interrupt, otherwise underruns might not be > > > detected after the first modeset if nothing changed in sub-parts of the > > > pipeline. > > > > Just overwrite commit_tail. That's why it exists. > > And that's what we do already, but I was hoping that someday we would > be able to switch to the generic helper... > > > > > > > It's also midlayer mistake, since you're stuff your callbacks into the > > > > drm_mode_config_funcs core structure (which should only be used for uapi > > > > interfaces, not helper stuff). > > > > > > Okay. > > > > > > > > > > > What I had in mind is just a simple function which spews into dmesg (so > > > > that i915 could use it) and increments the underrun counter in debugfs. > > > > Everything else needs to be handled in drivers I think. E.g. > >
Re: [PATCH v2 1/3] drm/atomic: Add a generic infrastructure to track underrun errors
On Fri, 26 Oct 2018 15:36:15 +0200 Daniel Vetter wrote: > On Fri, Oct 26, 2018 at 02:36:56PM +0200, Boris Brezillon wrote: > > Hi Daniel, > > > > On Fri, 26 Oct 2018 12:33:37 +0200 > > Daniel Vetter wrote: > > > > > On Thu, Oct 25, 2018 at 02:45:44PM +0200, Boris Brezillon wrote: > > > > Display controllers usually provide a lot features like overlay planes, > > > > hardware scalers, hardware rotations, ... > > > > Most of the time those features work just fine when enabled > > > > independently, but activating all of them at the same time tend to > > > > show other limitations like the limited memory bus or scaler > > > > bandwidth. > > > > > > > > This sort of bandwidth estimation is hard to get right, and we > > > > sometimes fail to predict that a specific workload will not pass, > > > > which will most likely result in underrun errors somewhere in the > > > > display pipeline (most of the time at the CRTC level). > > > > > > > > This path aims at making underrun error tracking generic and exposing > > > > underrun errors to userspace through a debugfs file. > > > > > > > > This way, CI tools like IGT, can try to enable a bunch of features and > > > > if such underrun errors are reported after the settings have been > > > > accepted and applied by the KMS infrastructure. When that happens it > > > > means the load tracking algorithm needs some tweaking/fixing. > > > > > > > > Signed-off-by: Boris Brezillon > > > > --- > > > > Changes in v2: > > > > - New patch > > > > > > Hm, not what I thought off. This is definitely not going to be reusable > > > for i915, the only other driver with underrun reporting, since we have > > > tons of underrun sources (not just one). And we need to mask/unmask them > > > individually. > > > > I agree it's not perfect, but I did it this way so that hopefully we'll > > be able to use drm_atomic_helper_commit_tail() at some point instead of > > having our own implementation. > > I expect every non-trivial driver to need to overwrite commit_tail. That's > the entire point for having created it. Exceptions are just the very, very > simple ones without anything fancy. Again, just my opinion, but doing that means some drivers are left behind when new stuff are added to the generic helpers, and sometimes those new features might be of interest to the drivers that are not directly using those generic helpers, except they usually don't even know about it until one comes at looking at the generic helper again and notice the additions. Maybe I'm not following the dri-devel ML close enough, and I'm probably the one to blame here, but I see a real benefit in having the maximum number of drivers using the generic helpers. To be honest, with a few modifications, I think the VC4 driver could entirely rely on those generic helpers. > > > If there's a way to add pre/post_commit() hooks (maybe they already > > exist?) at various levels of the display pipeline that get called even > > when the associated state has *not* changed, then I might be able to > > place my mask/unmask/clear operations there. I really need that to > > properly unmask the underrun interrupt, otherwise underruns might not be > > detected after the first modeset if nothing changed in sub-parts of the > > pipeline. > > Just overwrite commit_tail. That's why it exists. And that's what we do already, but I was hoping that someday we would be able to switch to the generic helper... > > > > It's also midlayer mistake, since you're stuff your callbacks into the > > > drm_mode_config_funcs core structure (which should only be used for uapi > > > interfaces, not helper stuff). > > > > Okay. > > > > > > > > What I had in mind is just a simple function which spews into dmesg (so > > > that i915 could use it) and increments the underrun counter in debugfs. > > > Everything else needs to be handled in drivers I think. E.g. > > > > > > drm_mode_config_report_underrun(struct drm_device *dev, > > > const char *source); > > > > > > and then rolling it out for i915 too (otherwise there's really no point in > > > the common infrastructure). > > > > Modifying the i915 driver was a bit premature since I didn't have your > > feedback on the general approach. And given your review, I'm glad I > > didn't start this conversion :-P. > > > > Anyway, I was not convinced having a generic infrastructure to report > > underrun errors was needed in the first place. The fact that these > > errors are very HW-specific, and that data attached to such events > > might be useful to understand what actually happened makes me think we > > don't really need this generic underrun reporting helper. > > > > Also note that IGT tests are likely to be HW specific too, because the > > workload needed to trigger underrun errors is likely to depend on the > > platform you're running on. And if you already have to write your own > > tests, I don't see clear benefit in exposing underrun errors >
[PATCH] fbdev: make FB_BACKLIGHT a tristate
BACKLIGHT_CLASS_DEVICE is already tristate, but a dependency
FB_BACKLIGHT prevents it from being built as a module. There
doesn't seem to be any particularly good reason for this, so
switch FB_BACKLIGHT over to tristate.
Signed-off-by: Rob Clark
Tested-by: Arnd Bergmann
---
v2: remove IS_ENABLED() from UABI headers. Userspace doesn't
know the kernel config, so just remove the ifdef guard
drivers/video/fbdev/Kconfig| 2 +-
drivers/video/fbdev/core/fbsysfs.c | 8
include/linux/fb.h | 2 +-
include/uapi/linux/fb.h| 2 --
4 files changed, 6 insertions(+), 8 deletions(-)
diff --git a/drivers/video/fbdev/Kconfig b/drivers/video/fbdev/Kconfig
index 591a13a59787..146ab2c347f8 100644
--- a/drivers/video/fbdev/Kconfig
+++ b/drivers/video/fbdev/Kconfig
@@ -198,7 +198,7 @@ config FB_MACMODES
default n
config FB_BACKLIGHT
- bool
+ tristate
depends on FB
select BACKLIGHT_LCD_SUPPORT
select BACKLIGHT_CLASS_DEVICE
diff --git a/drivers/video/fbdev/core/fbsysfs.c
b/drivers/video/fbdev/core/fbsysfs.c
index e31a182b42bf..44cca39f2b51 100644
--- a/drivers/video/fbdev/core/fbsysfs.c
+++ b/drivers/video/fbdev/core/fbsysfs.c
@@ -60,7 +60,7 @@ struct fb_info *framebuffer_alloc(size_t size, struct device
*dev)
info->device = dev;
info->fbcon_rotate_hint = -1;
-#ifdef CONFIG_FB_BACKLIGHT
+#if IS_ENABLED(CONFIG_FB_BACKLIGHT)
mutex_init(>bl_curve_mutex);
#endif
@@ -429,7 +429,7 @@ static ssize_t show_fbstate(struct device *device,
return snprintf(buf, PAGE_SIZE, "%d\n", fb_info->state);
}
-#ifdef CONFIG_FB_BACKLIGHT
+#if IS_ENABLED(CONFIG_FB_BACKLIGHT)
static ssize_t store_bl_curve(struct device *device,
struct device_attribute *attr,
const char *buf, size_t count)
@@ -510,7 +510,7 @@ static struct device_attribute device_attrs[] = {
__ATTR(stride, S_IRUGO, show_stride, NULL),
__ATTR(rotate, S_IRUGO|S_IWUSR, show_rotate, store_rotate),
__ATTR(state, S_IRUGO|S_IWUSR, show_fbstate, store_fbstate),
-#ifdef CONFIG_FB_BACKLIGHT
+#if IS_ENABLED(CONFIG_FB_BACKLIGHT)
__ATTR(bl_curve, S_IRUGO|S_IWUSR, show_bl_curve, store_bl_curve),
#endif
};
@@ -551,7 +551,7 @@ void fb_cleanup_device(struct fb_info *fb_info)
}
}
-#ifdef CONFIG_FB_BACKLIGHT
+#if IS_ENABLED(CONFIG_FB_BACKLIGHT)
/* This function generates a linear backlight curve
*
* 0: off
diff --git a/include/linux/fb.h b/include/linux/fb.h
index a3cab6dc9b44..7cdd31a69719 100644
--- a/include/linux/fb.h
+++ b/include/linux/fb.h
@@ -485,7 +485,7 @@ struct fb_info {
struct list_head modelist; /* mode list */
struct fb_videomode *mode; /* current mode */
-#ifdef CONFIG_FB_BACKLIGHT
+#if IS_ENABLED(CONFIG_FB_BACKLIGHT)
/* assigned backlight device */
/* set before framebuffer registration,
remove after unregister */
diff --git a/include/uapi/linux/fb.h b/include/uapi/linux/fb.h
index 6cd9b198b7c6..b6aac7ee1f67 100644
--- a/include/uapi/linux/fb.h
+++ b/include/uapi/linux/fb.h
@@ -393,11 +393,9 @@ struct fb_cursor {
struct fb_image image; /* Cursor image */
};
-#ifdef CONFIG_FB_BACKLIGHT
/* Settings for the generic backlight code */
#define FB_BACKLIGHT_LEVELS128
#define FB_BACKLIGHT_MAX 0xFF
-#endif
#endif /* _UAPI_LINUX_FB_H */
--
2.17.2
___
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [RFC PATCH] drm/vc4: Add a load tracker to prevent HVS underflow errors
On Fri, 26 Oct 2018 15:30:26 +0200 Daniel Vetter wrote: > On Thu, Oct 25, 2018 at 11:41:14AM +0200, Boris Brezillon wrote: > > On Thu, 25 Oct 2018 11:33:14 +0200 > > Daniel Vetter wrote: > > > > > On Thu, Oct 25, 2018 at 10:09:31AM +0200, Boris Brezillon wrote: > > > > On Tue, 23 Oct 2018 15:44:43 +0200 > > > > Daniel Vetter wrote: > > > > > > > > > On Tue, Oct 23, 2018 at 09:55:08AM +0200, Boris Brezillon wrote: > > > > > > Hi Daniel, > > > > > > > > > > > > On Tue, 16 Oct 2018 14:57:43 +0200 > > > > > > Daniel Vetter wrote: > > > > > > > > > > > > > On Tue, Oct 16, 2018 at 11:40:45AM +0200, Boris Brezillon wrote: > > > > > > > > > > > > > > > The HVS block is supposed to fill the pixelvalve FIFOs fast > > > > > > > > enough to > > > > > > > > meet the requested framerate. The problem is, the HVS and > > > > > > > > memory bus > > > > > > > > bandwidths are limited, and if we don't take these limitations > > > > > > > > into > > > > > > > > account we might end up with HVS underflow errors. > > > > > > > > > > > > > > > > This patch is trying to model the per-plane HVS and memory bus > > > > > > > > bandwidth > > > > > > > > consumption and take a decision at atomic_check() time whether > > > > > > > > the > > > > > > > > estimated load will fit in the HVS and membus budget. > > > > > > > > > > > > > > > > Note that we take an extra margin on the memory bus consumption > > > > > > > > to let > > > > > > > > the system run smoothly when other blocks are doing heavy use > > > > > > > > of the > > > > > > > > memory bus. Same goes for the HVS limit, except the margin is > > > > > > > > smaller in > > > > > > > > this case, since the HVS is not used by external components. > > > > > > > > > > > > > > > > Signed-off-by: Boris Brezillon > > > > > > > > --- > > > > > > > > This logic has been validated using a simple shell script and > > > > > > > > some instrumentation in the VC4 driver: > > > > > > > > > > > > > > > > - capture underflow errors at the HVS level and expose a > > > > > > > > debugfs file > > > > > > > > reporting those errors > > > > > > > > - add debugfs files to expose when atomic_check fails because > > > > > > > > of the > > > > > > > > HVS or membus load limitation or when it fails for other > > > > > > > > reasons > > > > > > > > > > > > > > > > The branch containing those modification is available here [1], > > > > > > > > and the > > > > > > > > script (which is internally using modetest) is here [2] (please > > > > > > > > note > > > > > > > > that I'm bad at writing shell scripts :-)). > > > > > > > > > > > > > > > > Note that those modification tend to over-estimate the load, > > > > > > > > and thus > > > > > > > > reject setups that might have previously worked, so we might > > > > > > > > want to > > > > > > > > adjust the limits to avoid that. > > > > > > > > > > > > > > > > [1]https://github.com/bbrezillon/linux/tree/vc4/hvs-bandwidth-eval > > > > > > > > [2]https://github.com/bbrezillon/vc4-hvs-bandwidth-test > > > > > > > > > > > > > > Any interest in using igt to test this stuff? We have at least a > > > > > > > bunch of > > > > > > > tests already in there that try all kinds of plane setups. And we > > > > > > > use > > > > > > > those to hunt for underruns on i915 hw. > > > > > > > > > > > > > > Wrt underrun reporting: On i915 we just dump them into dmesg at > > > > > > > the error > > > > > > > level, using DRM_ERROR, > > > > > > > > > > > > Are you masking the underrun interrupt after it's been reported? If > > > > > > we > > > > > > don't do that on VC4 we just end up flooding the kernel-log buffer > > > > > > until > > > > > > someone comes and update the config. > > > > > > > > > > Yeah we do that too. Rule is that a full modeset will clear any > > > > > underrun > > > > > masking (so tests need to make sure they start with a modeset, or > > > > > it'll be > > > > > for nothing). > > > > > > > > > > > > > > > > > > plus a tracepoint. See e.g. > > > > > > > intel_pch_fifo_underrun_irq_handler(). If there's interest we > > > > > > > could > > > > > > > perhaps extract this into something common, similar to what was > > > > > > > done with > > > > > > > crc support already. > > > > > > > > > > > > > > > > > > > I'm not a big fan of hardcoded trace points in general (because of > > > > > > the > > > > > > whole "it's part of the stable ABI" thing), and in this case, > > > > > > making the > > > > > > tracepoint generic sounds even more risky to me. Indeed, how can we > > > > > > know > > > > > > about all the HW specific bits one might want to expose. For > > > > > > instance, > > > > > > I see the intel underrun tracepoint exposes a struct with a frame > > > > > > and > > > > > > scanline field, and AFAICT, we don't have such information in the > > > > > > VC4 > > > > > > case. > > > > > > > > > > > > Any opinion on that? > > > > > > > > > > It's
[radeon-alex:drm-next-4.21-wip 83/87] drivers/gpu/drm/v3d/v3d_sched.c:221:8: error: too many arguments to function 'drm_sched_init'
tree: git://people.freedesktop.org/~agd5f/linux.git drm-next-4.21-wip
head: 0f117df308faec4f1eecc9a36b3a836181063483
commit: 0544d53c4e7baac87a8fc81a9aae001be0e034ae [83/87] drm/sched: Add boolean
to mark if sched is ready to work v4
config: xtensa-allyesconfig (attached as .config)
compiler: xtensa-linux-gcc (GCC) 8.1.0
reproduce:
wget
https://raw.githubusercontent.com/intel/lkp-tests/master/sbin/make.cross -O
~/bin/make.cross
chmod +x ~/bin/make.cross
git checkout 0544d53c4e7baac87a8fc81a9aae001be0e034ae
# save the attached .config to linux build tree
GCC_VERSION=8.1.0 make.cross ARCH=xtensa
All errors (new ones prefixed by >>):
drivers/gpu/drm/v3d/v3d_sched.c: In function 'v3d_sched_init':
>> drivers/gpu/drm/v3d/v3d_sched.c:221:8: error: too many arguments to function
>> 'drm_sched_init'
ret = drm_sched_init(>queue[V3D_RENDER].sched,
^~
In file included from drivers/gpu/drm/v3d/v3d_drv.h:9,
from drivers/gpu/drm/v3d/v3d_sched.c:23:
include/drm/gpu_scheduler.h:290:5: note: declared here
int drm_sched_init(struct drm_gpu_scheduler *sched,
^~
vim +/drm_sched_init +221 drivers/gpu/drm/v3d/v3d_sched.c
57692c94 Eric Anholt 2018-04-30 202
57692c94 Eric Anholt 2018-04-30 203 int
57692c94 Eric Anholt 2018-04-30 204 v3d_sched_init(struct v3d_dev *v3d)
57692c94 Eric Anholt 2018-04-30 205 {
57692c94 Eric Anholt 2018-04-30 206 int hw_jobs_limit = 1;
57692c94 Eric Anholt 2018-04-30 207 int job_hang_limit = 0;
57692c94 Eric Anholt 2018-04-30 208 int hang_limit_ms = 500;
57692c94 Eric Anholt 2018-04-30 209 int ret;
57692c94 Eric Anholt 2018-04-30 210
57692c94 Eric Anholt 2018-04-30 211 ret =
drm_sched_init(>queue[V3D_BIN].sched,
57692c94 Eric Anholt 2018-04-30 212
_sched_ops,
57692c94 Eric Anholt 2018-04-30 213
hw_jobs_limit, job_hang_limit,
57692c94 Eric Anholt 2018-04-30 214
msecs_to_jiffies(hang_limit_ms),
57692c94 Eric Anholt 2018-04-30 215 "v3d_bin");
57692c94 Eric Anholt 2018-04-30 216 if (ret) {
57692c94 Eric Anholt 2018-04-30 217 dev_err(v3d->dev,
"Failed to create bin scheduler: %d.", ret);
57692c94 Eric Anholt 2018-04-30 218 return ret;
57692c94 Eric Anholt 2018-04-30 219 }
57692c94 Eric Anholt 2018-04-30 220
57692c94 Eric Anholt 2018-04-30 @221 ret =
drm_sched_init(>queue[V3D_RENDER].sched,
57692c94 Eric Anholt 2018-04-30 222
_sched_ops,
57692c94 Eric Anholt 2018-04-30 223
hw_jobs_limit, job_hang_limit,
57692c94 Eric Anholt 2018-04-30 224
msecs_to_jiffies(hang_limit_ms),
0544d53c Andrey Grodzovsky 2018-10-18 225
"v3d_render", true);
57692c94 Eric Anholt 2018-04-30 226 if (ret) {
57692c94 Eric Anholt 2018-04-30 227 dev_err(v3d->dev,
"Failed to create render scheduler: %d.",
57692c94 Eric Anholt 2018-04-30 228 ret);
57692c94 Eric Anholt 2018-04-30 229
drm_sched_fini(>queue[V3D_BIN].sched);
57692c94 Eric Anholt 2018-04-30 230 return ret;
57692c94 Eric Anholt 2018-04-30 231 }
57692c94 Eric Anholt 2018-04-30 232
57692c94 Eric Anholt 2018-04-30 233 return 0;
57692c94 Eric Anholt 2018-04-30 234 }
57692c94 Eric Anholt 2018-04-30 235
:: The code at line 221 was first introduced by commit
:: 57692c94dcbe99a1e009a3da13fb3443562c drm/v3d: Introduce a new DRM
driver for Broadcom V3D V3.x+
:: TO: Eric Anholt
:: CC: Eric Anholt
---
0-DAY kernel test infrastructureOpen Source Technology Center
https://lists.01.org/pipermail/kbuild-all Intel Corporation
.config.gz
Description: application/gzip
___
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [PATCH] drm/vgem: create a render node for vgem
On Fri, Oct 26, 2018 at 01:06:47PM +0100, Emil Velikov wrote:
> From: Emil Velikov
>
> VGEM doesn't do anything modeset specific, so in a way exposing a
> primary node is 'wrong'. At the same time, we extensively use if for
> creating dumb buffers, fences, prime fd <> handle imports/exports.
>
> To the point that we explicitly annotate the vgem fence ioctls as
> DRM_RENDER_ALLOW and have an IGT test which opens the render node.
>
> close(drm_open_driver_render(DRIVER_VGEM))
Huh, I guess that test doesn't pass?
> Better late than never, let's flip the switch.
>
> Cc: David Airlie
> Cc: Daniel Vetter
> Signed-off-by: Emil Velikov
Acked-by: Daniel Vetter
> ---
> drivers/gpu/drm/vgem/vgem_drv.c | 3 ++-
> 1 file changed, 2 insertions(+), 1 deletion(-)
>
> diff --git a/drivers/gpu/drm/vgem/vgem_drv.c b/drivers/gpu/drm/vgem/vgem_drv.c
> index f1f7ab9dcdbf..f1d1d9e2c82e 100644
> --- a/drivers/gpu/drm/vgem/vgem_drv.c
> +++ b/drivers/gpu/drm/vgem/vgem_drv.c
> @@ -431,7 +431,8 @@ static void vgem_release(struct drm_device *dev)
> }
>
> static struct drm_driver vgem_driver = {
> - .driver_features= DRIVER_GEM | DRIVER_PRIME,
> + .driver_features= DRIVER_GEM | DRIVER_PRIME |
> + DRIVER_RENDER;
> .release= vgem_release,
> .open = vgem_open,
> .postclose = vgem_postclose,
> --
> 2.19.1
>
--
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: [Freedreno] [PATCH 3/3] drm/msm: Use Hardware counters for perf profiling
Added Rob to this thread.
On 10/17/2018 8:05 PM, Jordan Crouse wrote:
On Wed, Oct 17, 2018 at 06:34:01PM +0530, Sharat Masetty wrote:
This patch attempts to make use of the hardware counters for GPU busy %
estimation when possible and skip using the software counters as it also
accounts for software side delays. This should help give more accurate
representation of the GPU workload.
I would leave this more to Rob as this particular file makes more sense for
freedreno than it does for us but I think in general if you want to do this
then we should do use the hardware for all targets and get rid of the
mix of the old and the new.
Thanks for the comments Jordan. Yes, I prefer the same too, but the only
limiting factor for me is that I don't have a way to test changes for
targets such as a3xx and a4xx, and I also do not know if there is anyone
actually using this currently.
Hi Rob,
Can you please share your comments on this? Is it okay to remove
software counters functionality?
Sharat
Signed-off-by: Sharat Masetty
---
drivers/gpu/drm/msm/msm_gpu.c | 30 ++
drivers/gpu/drm/msm/msm_gpu.h | 5 +++--
drivers/gpu/drm/msm/msm_perf.c | 10 +-
3 files changed, 34 insertions(+), 11 deletions(-)
diff --git a/drivers/gpu/drm/msm/msm_gpu.c b/drivers/gpu/drm/msm/msm_gpu.c
index e9b5426..a896541 100644
--- a/drivers/gpu/drm/msm/msm_gpu.c
+++ b/drivers/gpu/drm/msm/msm_gpu.c
@@ -592,6 +592,9 @@ static void update_sw_cntrs(struct msm_gpu *gpu)
uint32_t elapsed;
unsigned long flags;
+ if (gpu->funcs->gpu_busy)
+ return;
Like here - there isn't any reason to not have a gpu_busy for each target
so then this code could mostly go away.
spin_lock_irqsave(>perf_lock, flags);
if (!gpu->perfcntr_active)
goto out;
@@ -620,6 +623,7 @@ void msm_gpu_perfcntr_start(struct msm_gpu *gpu)
/* we could dynamically enable/disable perfcntr registers too.. */
gpu->last_sample.active = msm_gpu_active(gpu);
gpu->last_sample.time = ktime_get();
+ gpu->last_sample.busy_cycles = 0;
gpu->activetime = gpu->totaltime = 0;
gpu->perfcntr_active = true;
update_hw_cntrs(gpu, 0, NULL);
@@ -632,9 +636,22 @@ void msm_gpu_perfcntr_stop(struct msm_gpu *gpu)
pm_runtime_put_sync(>pdev->dev);
}
+static void msm_gpu_hw_sample(struct msm_gpu *gpu, uint64_t *activetime,
+ uint64_t *totaltime)
+{
+ ktime_t time;
+
+ *activetime = gpu->funcs->gpu_busy(gpu,
+ >last_sample.busy_cycles);
+
+ time = ktime_get();
+ *totaltime = ktime_us_delta(time, gpu->last_sample.time);
+ gpu->last_sample.time = time;
+}
This can all be done inline in the sample function below.
/* returns -errno or # of cntrs sampled */
-int msm_gpu_perfcntr_sample(struct msm_gpu *gpu, uint32_t *activetime,
- uint32_t *totaltime, uint32_t ncntrs, uint32_t *cntrs)
+int msm_gpu_perfcntr_sample(struct msm_gpu *gpu, uint64_t *activetime,
+ uint64_t *totaltime, uint32_t ncntrs, uint32_t *cntrs)
This might be an good change (though if our activetime or totaltime ever
goes over 32 bits we've got ourselves a problem) - but it should be in a
separate patch.
{
unsigned long flags;
int ret;
@@ -646,13 +663,18 @@ int msm_gpu_perfcntr_sample(struct msm_gpu *gpu, uint32_t
*activetime,
goto out;
}
+ ret = update_hw_cntrs(gpu, ncntrs, cntrs);
+
+ if (gpu->funcs->gpu_busy) {
+ msm_gpu_hw_sample(gpu, activetime, totaltime);
+ goto out;
+ }
+
*activetime = gpu->activetime;
*totaltime = gpu->totaltime;
gpu->activetime = gpu->totaltime = 0;
- ret = update_hw_cntrs(gpu, ncntrs, cntrs);
-
out:
spin_unlock_irqrestore(>perf_lock, flags);
diff --git a/drivers/gpu/drm/msm/msm_gpu.h b/drivers/gpu/drm/msm/msm_gpu.h
index 0ff23ca..7dc775f 100644
--- a/drivers/gpu/drm/msm/msm_gpu.h
+++ b/drivers/gpu/drm/msm/msm_gpu.h
@@ -90,6 +90,7 @@ struct msm_gpu {
struct {
bool active;
ktime_t time;
+ u64 busy_cycles;
} last_sample;
uint32_t totaltime, activetime;/* sw counters */
uint32_t last_cntrs[5];/* hw counters */
@@ -275,8 +276,8 @@ static inline void gpu_write64(struct msm_gpu *gpu, u32 lo,
u32 hi, u64 val)
void msm_gpu_perfcntr_start(struct msm_gpu *gpu);
void msm_gpu_perfcntr_stop(struct msm_gpu *gpu);
-int msm_gpu_perfcntr_sample(struct msm_gpu *gpu, uint32_t *activetime,
- uint32_t *totaltime, uint32_t ncntrs, uint32_t *cntrs);
+int msm_gpu_perfcntr_sample(struct msm_gpu *gpu, uint64_t *activetime,
+ uint64_t *totaltime, uint32_t ncntrs, uint32_t *cntrs);
void msm_gpu_retire(struct msm_gpu *gpu);
void msm_gpu_submit(struct msm_gpu *gpu, struct msm_gem_submit *submit,
diff --git
Re: [PATCH v2 1/3] drm/atomic: Add a generic infrastructure to track underrun errors
On Fri, Oct 26, 2018 at 02:36:56PM +0200, Boris Brezillon wrote: > Hi Daniel, > > On Fri, 26 Oct 2018 12:33:37 +0200 > Daniel Vetter wrote: > > > On Thu, Oct 25, 2018 at 02:45:44PM +0200, Boris Brezillon wrote: > > > Display controllers usually provide a lot features like overlay planes, > > > hardware scalers, hardware rotations, ... > > > Most of the time those features work just fine when enabled > > > independently, but activating all of them at the same time tend to > > > show other limitations like the limited memory bus or scaler > > > bandwidth. > > > > > > This sort of bandwidth estimation is hard to get right, and we > > > sometimes fail to predict that a specific workload will not pass, > > > which will most likely result in underrun errors somewhere in the > > > display pipeline (most of the time at the CRTC level). > > > > > > This path aims at making underrun error tracking generic and exposing > > > underrun errors to userspace through a debugfs file. > > > > > > This way, CI tools like IGT, can try to enable a bunch of features and > > > if such underrun errors are reported after the settings have been > > > accepted and applied by the KMS infrastructure. When that happens it > > > means the load tracking algorithm needs some tweaking/fixing. > > > > > > Signed-off-by: Boris Brezillon > > > --- > > > Changes in v2: > > > - New patch > > > > Hm, not what I thought off. This is definitely not going to be reusable > > for i915, the only other driver with underrun reporting, since we have > > tons of underrun sources (not just one). And we need to mask/unmask them > > individually. > > I agree it's not perfect, but I did it this way so that hopefully we'll > be able to use drm_atomic_helper_commit_tail() at some point instead of > having our own implementation. I expect every non-trivial driver to need to overwrite commit_tail. That's the entire point for having created it. Exceptions are just the very, very simple ones without anything fancy. > If there's a way to add pre/post_commit() hooks (maybe they already > exist?) at various levels of the display pipeline that get called even > when the associated state has *not* changed, then I might be able to > place my mask/unmask/clear operations there. I really need that to > properly unmask the underrun interrupt, otherwise underruns might not be > detected after the first modeset if nothing changed in sub-parts of the > pipeline. Just overwrite commit_tail. That's why it exists. > > It's also midlayer mistake, since you're stuff your callbacks into the > > drm_mode_config_funcs core structure (which should only be used for uapi > > interfaces, not helper stuff). > > Okay. > > > > > What I had in mind is just a simple function which spews into dmesg (so > > that i915 could use it) and increments the underrun counter in debugfs. > > Everything else needs to be handled in drivers I think. E.g. > > > > drm_mode_config_report_underrun(struct drm_device *dev, > > const char *source); > > > > and then rolling it out for i915 too (otherwise there's really no point in > > the common infrastructure). > > Modifying the i915 driver was a bit premature since I didn't have your > feedback on the general approach. And given your review, I'm glad I > didn't start this conversion :-P. > > Anyway, I was not convinced having a generic infrastructure to report > underrun errors was needed in the first place. The fact that these > errors are very HW-specific, and that data attached to such events > might be useful to understand what actually happened makes me think we > don't really need this generic underrun reporting helper. > > Also note that IGT tests are likely to be HW specific too, because the > workload needed to trigger underrun errors is likely to depend on the > platform you're running on. And if you already have to write your own > tests, I don't see clear benefit in exposing underrun errors > generically. See my other reply in the earlier thread: That's why we're just dumping errors into dmesg, and catching any such thing in our runners. We do not have any specific underrun tests, we treat the entire igt test suite of kms tests as our underrun test suite. Extending that with more nasty corner cases is obviously great, and will benefit everyone. But I don't think you want an igt testcase that looks specifically for underruns. We do use crc to hunt for specific issues, but again that's portable between drivers. > Anyway, this is just my opinion, and if you think we actually need the > drm_mode_config_report_underrun() function, I'll implement it. I do think having some standard in how these errors are reported would be useful, even if it's just a special DRM_UNDERRUN_ERROR() macro. -Daniel -- Daniel Vetter Software Engineer, Intel Corporation http://blog.ffwll.ch ___ dri-devel mailing list dri-devel@lists.freedesktop.org
[Bug 108563] Kernel hangs up when no amdgpu.dc=0 (if DC not disabled) kernel parameter supplied and when GCN 1.1 and GCN 1.2/1.4 (very likely) device will be handled by AMDGPU
https://bugs.freedesktop.org/show_bug.cgi?id=108563 --- Comment #1 from fin4...@hotmail.com --- It is just a debug trace that you can delete: [ 18.371385] WARNING: CPU: 1 PID: 289 at drivers/gpu/drm/drm_plane.c:182 WARN_ON(drm_drv_uses_atomic_modeset(dev) && (!funcs->atomic_destroy_state || !funcs->atomic_duplicate_state)); -- You are receiving this mail because: You are the assignee for the bug.___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [RFC PATCH] drm/vc4: Add a load tracker to prevent HVS underflow errors
On Thu, Oct 25, 2018 at 11:41:14AM +0200, Boris Brezillon wrote: > On Thu, 25 Oct 2018 11:33:14 +0200 > Daniel Vetter wrote: > > > On Thu, Oct 25, 2018 at 10:09:31AM +0200, Boris Brezillon wrote: > > > On Tue, 23 Oct 2018 15:44:43 +0200 > > > Daniel Vetter wrote: > > > > > > > On Tue, Oct 23, 2018 at 09:55:08AM +0200, Boris Brezillon wrote: > > > > > Hi Daniel, > > > > > > > > > > On Tue, 16 Oct 2018 14:57:43 +0200 > > > > > Daniel Vetter wrote: > > > > > > > > > > > On Tue, Oct 16, 2018 at 11:40:45AM +0200, Boris Brezillon wrote: > > > > > > > The HVS block is supposed to fill the pixelvalve FIFOs fast > > > > > > > enough to > > > > > > > meet the requested framerate. The problem is, the HVS and memory > > > > > > > bus > > > > > > > bandwidths are limited, and if we don't take these limitations > > > > > > > into > > > > > > > account we might end up with HVS underflow errors. > > > > > > > > > > > > > > This patch is trying to model the per-plane HVS and memory bus > > > > > > > bandwidth > > > > > > > consumption and take a decision at atomic_check() time whether the > > > > > > > estimated load will fit in the HVS and membus budget. > > > > > > > > > > > > > > Note that we take an extra margin on the memory bus consumption > > > > > > > to let > > > > > > > the system run smoothly when other blocks are doing heavy use of > > > > > > > the > > > > > > > memory bus. Same goes for the HVS limit, except the margin is > > > > > > > smaller in > > > > > > > this case, since the HVS is not used by external components. > > > > > > > > > > > > > > Signed-off-by: Boris Brezillon > > > > > > > --- > > > > > > > This logic has been validated using a simple shell script and > > > > > > > some instrumentation in the VC4 driver: > > > > > > > > > > > > > > - capture underflow errors at the HVS level and expose a debugfs > > > > > > > file > > > > > > > reporting those errors > > > > > > > - add debugfs files to expose when atomic_check fails because of > > > > > > > the > > > > > > > HVS or membus load limitation or when it fails for other reasons > > > > > > > > > > > > > > The branch containing those modification is available here [1], > > > > > > > and the > > > > > > > script (which is internally using modetest) is here [2] (please > > > > > > > note > > > > > > > that I'm bad at writing shell scripts :-)). > > > > > > > > > > > > > > Note that those modification tend to over-estimate the load, and > > > > > > > thus > > > > > > > reject setups that might have previously worked, so we might want > > > > > > > to > > > > > > > adjust the limits to avoid that. > > > > > > > > > > > > > > [1]https://github.com/bbrezillon/linux/tree/vc4/hvs-bandwidth-eval > > > > > > > [2]https://github.com/bbrezillon/vc4-hvs-bandwidth-test > > > > > > > > > > > > Any interest in using igt to test this stuff? We have at least a > > > > > > bunch of > > > > > > tests already in there that try all kinds of plane setups. And we > > > > > > use > > > > > > those to hunt for underruns on i915 hw. > > > > > > > > > > > > Wrt underrun reporting: On i915 we just dump them into dmesg at the > > > > > > error > > > > > > level, using DRM_ERROR, > > > > > > > > > > Are you masking the underrun interrupt after it's been reported? If we > > > > > don't do that on VC4 we just end up flooding the kernel-log buffer > > > > > until > > > > > someone comes and update the config. > > > > > > > > Yeah we do that too. Rule is that a full modeset will clear any underrun > > > > masking (so tests need to make sure they start with a modeset, or it'll > > > > be > > > > for nothing). > > > > > > > > > > > > > > > plus a tracepoint. See e.g. > > > > > > intel_pch_fifo_underrun_irq_handler(). If there's interest we could > > > > > > perhaps extract this into something common, similar to what was > > > > > > done with > > > > > > crc support already. > > > > > > > > > > > > > > > > I'm not a big fan of hardcoded trace points in general (because of the > > > > > whole "it's part of the stable ABI" thing), and in this case, making > > > > > the > > > > > tracepoint generic sounds even more risky to me. Indeed, how can we > > > > > know > > > > > about all the HW specific bits one might want to expose. For instance, > > > > > I see the intel underrun tracepoint exposes a struct with a frame and > > > > > scanline field, and AFAICT, we don't have such information in the VC4 > > > > > case. > > > > > > > > > > Any opinion on that? > > > > > > > > It's only abi if you're unlucky. If it's just for debugging and > > > > validation, you can change it again. Tbh, no idea why we even have these > > > > tracepoints, they're fairly useless imo. CI only relies upon the dmesg > > > > output. Maybe run git blame and ask the original author, we can probably > > > > update them to suit our needs. > > > > > > Okay, I think I'll go for a generic debugfs entry that returns true > > >
Re: [PATCH v2 1/3] drm/atomic: Add a generic infrastructure to track underrun errors
Hi Daniel, On Fri, 26 Oct 2018 12:33:37 +0200 Daniel Vetter wrote: > On Thu, Oct 25, 2018 at 02:45:44PM +0200, Boris Brezillon wrote: > > Display controllers usually provide a lot features like overlay planes, > > hardware scalers, hardware rotations, ... > > Most of the time those features work just fine when enabled > > independently, but activating all of them at the same time tend to > > show other limitations like the limited memory bus or scaler > > bandwidth. > > > > This sort of bandwidth estimation is hard to get right, and we > > sometimes fail to predict that a specific workload will not pass, > > which will most likely result in underrun errors somewhere in the > > display pipeline (most of the time at the CRTC level). > > > > This path aims at making underrun error tracking generic and exposing > > underrun errors to userspace through a debugfs file. > > > > This way, CI tools like IGT, can try to enable a bunch of features and > > if such underrun errors are reported after the settings have been > > accepted and applied by the KMS infrastructure. When that happens it > > means the load tracking algorithm needs some tweaking/fixing. > > > > Signed-off-by: Boris Brezillon > > --- > > Changes in v2: > > - New patch > > Hm, not what I thought off. This is definitely not going to be reusable > for i915, the only other driver with underrun reporting, since we have > tons of underrun sources (not just one). And we need to mask/unmask them > individually. I agree it's not perfect, but I did it this way so that hopefully we'll be able to use drm_atomic_helper_commit_tail() at some point instead of having our own implementation. If there's a way to add pre/post_commit() hooks (maybe they already exist?) at various levels of the display pipeline that get called even when the associated state has *not* changed, then I might be able to place my mask/unmask/clear operations there. I really need that to properly unmask the underrun interrupt, otherwise underruns might not be detected after the first modeset if nothing changed in sub-parts of the pipeline. > > It's also midlayer mistake, since you're stuff your callbacks into the > drm_mode_config_funcs core structure (which should only be used for uapi > interfaces, not helper stuff). Okay. > > What I had in mind is just a simple function which spews into dmesg (so > that i915 could use it) and increments the underrun counter in debugfs. > Everything else needs to be handled in drivers I think. E.g. > > drm_mode_config_report_underrun(struct drm_device *dev, > const char *source); > > and then rolling it out for i915 too (otherwise there's really no point in > the common infrastructure). Modifying the i915 driver was a bit premature since I didn't have your feedback on the general approach. And given your review, I'm glad I didn't start this conversion :-P. Anyway, I was not convinced having a generic infrastructure to report underrun errors was needed in the first place. The fact that these errors are very HW-specific, and that data attached to such events might be useful to understand what actually happened makes me think we don't really need this generic underrun reporting helper. Also note that IGT tests are likely to be HW specific too, because the workload needed to trigger underrun errors is likely to depend on the platform you're running on. And if you already have to write your own tests, I don't see clear benefit in exposing underrun errors generically. Anyway, this is just my opinion, and if you think we actually need the drm_mode_config_report_underrun() function, I'll implement it. Regards, Boris ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [PATCH] drm/syncobj: Avoid kmalloc(GFP_KERNEL) under spinlock
Am 26.10.18 um 10:28 schrieb zhoucm1:
> Thanks, Could you help to submit to drm-misc again?
Done.
Christian.
>
> -David
>
>
> On 2018年10月26日 15:43, Christian König wrote:
>> Am 26.10.18 um 08:20 schrieb Chunming Zhou:
>>> drivers/gpu/drm/drm_syncobj.c:202:4-14: ERROR: function
>>> drm_syncobj_find_signal_pt_for_point called on line 390 inside lock
>>> on line 389 but uses GFP_KERNEL
>>>
>>> Find functions that refer to GFP_KERNEL but are called with locks
>>> held.
>>>
>>> Generated by: scripts/coccinelle/locks/call_kern.cocci
>>>
>>> v2:
>>> syncobj->timeline still needs protect.
>>>
>>> v3:
>>> use a global signaled fence instead of re-allocation.
>>>
>>> v4:
>>> Don't need moving lock.
>>> Don't expose func.
>>>
>>> v5:
>>> rename func and directly return.
>>>
>>> Tested by: syncobj_wait and ./deqp-vk -n dEQP-VK.*semaphore* with
>>> lock debug kernel options enabled.
>>>
>>> Signed-off-by: Chunming Zhou
>>> Cc: Maarten Lankhorst
>>> Cc: intel-...@lists.freedesktop.org
>>> Cc: Christian König
>>> Cc: Chris Wilson
>>> CC: Julia Lawall
>>> Reviewed-by: Chris Wilson
>>
>> Reviewed-by: Christian König
>>
>>> ---
>>> drivers/gpu/drm/drm_syncobj.c | 36
>>> ++-
>>> 1 file changed, 19 insertions(+), 17 deletions(-)
>>>
>>> diff --git a/drivers/gpu/drm/drm_syncobj.c
>>> b/drivers/gpu/drm/drm_syncobj.c
>>> index b7eaa603f368..d1c6f21c72b5 100644
>>> --- a/drivers/gpu/drm/drm_syncobj.c
>>> +++ b/drivers/gpu/drm/drm_syncobj.c
>>> @@ -80,6 +80,23 @@ struct drm_syncobj_signal_pt {
>>> struct list_head list;
>>> };
>>> +static DEFINE_SPINLOCK(signaled_fence_lock);
>>> +static struct dma_fence signaled_fence;
>>> +
>>> +static struct dma_fence *drm_syncobj_get_stub_fence(void)
>>> +{
>>> + spin_lock(_fence_lock);
>>> + if (!signaled_fence.ops) {
>>> + dma_fence_init(_fence,
>>> + _syncobj_stub_fence_ops,
>>> + _fence_lock,
>>> + 0, 0);
>>> + dma_fence_signal_locked(_fence);
>>> + }
>>> + spin_unlock(_fence_lock);
>>> +
>>> + return dma_fence_get(_fence);
>>> +}
>>> /**
>>> * drm_syncobj_find - lookup and reference a sync object.
>>> * @file_private: drm file private pointer
>>> @@ -113,23 +130,8 @@ static struct dma_fence
>>> struct drm_syncobj_signal_pt *signal_pt;
>>> if ((syncobj->type == DRM_SYNCOBJ_TYPE_TIMELINE) &&
>>> - (point <= syncobj->timeline)) {
>>> - struct drm_syncobj_stub_fence *fence =
>>> - kzalloc(sizeof(struct drm_syncobj_stub_fence),
>>> - GFP_KERNEL);
>>> -
>>> - if (!fence)
>>> - return NULL;
>>> - spin_lock_init(>lock);
>>> - dma_fence_init(>base,
>>> - _syncobj_stub_fence_ops,
>>> - >lock,
>>> - syncobj->timeline_context,
>>> - point);
>>> -
>>> - dma_fence_signal(>base);
>>> - return >base;
>>> - }
>>> + (point <= syncobj->timeline))
>>> + return drm_syncobj_get_stub_fence();
>>> list_for_each_entry(signal_pt, >signal_pt_list,
>>> list) {
>>> if (point > signal_pt->value)
>>
>
___
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel
[PATCH] drm/vgem: create a render node for vgem
From: Emil Velikov
VGEM doesn't do anything modeset specific, so in a way exposing a
primary node is 'wrong'. At the same time, we extensively use if for
creating dumb buffers, fences, prime fd <> handle imports/exports.
To the point that we explicitly annotate the vgem fence ioctls as
DRM_RENDER_ALLOW and have an IGT test which opens the render node.
close(drm_open_driver_render(DRIVER_VGEM))
Better late than never, let's flip the switch.
Cc: David Airlie
Cc: Daniel Vetter
Signed-off-by: Emil Velikov
---
drivers/gpu/drm/vgem/vgem_drv.c | 3 ++-
1 file changed, 2 insertions(+), 1 deletion(-)
diff --git a/drivers/gpu/drm/vgem/vgem_drv.c b/drivers/gpu/drm/vgem/vgem_drv.c
index f1f7ab9dcdbf..f1d1d9e2c82e 100644
--- a/drivers/gpu/drm/vgem/vgem_drv.c
+++ b/drivers/gpu/drm/vgem/vgem_drv.c
@@ -431,7 +431,8 @@ static void vgem_release(struct drm_device *dev)
}
static struct drm_driver vgem_driver = {
- .driver_features= DRIVER_GEM | DRIVER_PRIME,
+ .driver_features= DRIVER_GEM | DRIVER_PRIME |
+ DRIVER_RENDER;
.release= vgem_release,
.open = vgem_open,
.postclose = vgem_postclose,
--
2.19.1
___
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel
[Bug 108096] [amd-staging-drm-next] SDDM screen corruption (not usable) with RX580, amdgpu, dc=1 (of course), regression - [bisected]
https://bugs.freedesktop.org/show_bug.cgi?id=108096 --- Comment #15 from Dieter Nützel --- (In reply to Michel Dänzer from comment #14) > Which Git commit of Mesa are you using? Any local patches on top? Every, since ever, as always (even, since _before_ Aug 22, 2018) ...;-) But kidding aside, _currently_ #0ff1ccca25 (with merged branch from Marek for testing purposes) 04ba4eae68 (HEAD -> ext_gpu_shader4) Merge branch 'ext_gpu_shader4' of git://people.freedesktop.org/~mareko/mesa into ext_gpu_shader4 0ff1ccca25 (origin/master, origin/HEAD, master) radv: call nir_link_xfb_varyings() Has it something to do with the DRM version? DRM 3.26.0 (4.18) vs. DRM 3.27.0 (4.19)? [-] diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_drv.c b/drivers/gpu/drm/amd/amdgpu/amdgpu_drv.c index 06aede1..529500c 100644 --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_drv.c +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_drv.c @@ -69,9 +69,10 @@ * - 3.24.0 - Add high priority compute support for gfx9 * - 3.25.0 - Add support for sensor query info (stable pstate sclk/mclk). * - 3.26.0 - GFX9: Process AMDGPU_IB_FLAG_TC_WB_NOT_INVALIDATE. + * - 3.27.0 - Add new chunk to to AMDGPU_CS to enable BO_LIST creation. */ #define KMS_DRIVER_MAJOR 3 -#define KMS_DRIVER_MINOR 26 +#define KMS_DRIVER_MINOR 27 #define KMS_DRIVER_PATCHLEVEL 0 [-] But this commit is _in_ since author Andrey Grodzovsky2018-07-06 14:16:54 -0400 committer Alex Deucher 2018-07-16 15:29:47 -0500 and I had it running (on stable and amd-staging-drm-next, daily), even with AMD testing code (Huang Rui ray.huang at amd.com), Aug 16, 2018. https://lists.freedesktop.org/archives/amd-gfx/2018-August/025411.html Do you need more logs? With which kernel parameter? System _is_ running, but with unusable gfx/dri screen. -- You are receiving this mail because: You are the assignee for the bug.___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
[Bug 108563] Kernel hangs up when no amdgpu.dc=0 (if DC not disabled) kernel parameter supplied and when GCN 1.1 and GCN 1.2/1.4 (very likely) device will be handled by AMDGPU
https://bugs.freedesktop.org/show_bug.cgi?id=108563 Bug ID: 108563 Summary: Kernel hangs up when no amdgpu.dc=0 (if DC not disabled) kernel parameter supplied and when GCN 1.1 and GCN 1.2/1.4 (very likely) device will be handled by AMDGPU Product: DRI Version: unspecified Hardware: x86-64 (AMD64) OS: Linux (All) Status: NEW Severity: normal Priority: medium Component: DRM/AMDgpu Assignee: dri-devel@lists.freedesktop.org Reporter: mats...@interia.pl In my Arch Linux installation, I compiled and installed new fresh kernel 4.19. I have non-standard hardware configuration where are: Radeon R7 360 (Bonaire), Radeon RX 480 (Ellesmere) and Radeon RX VEGA 56 (GFX900) graphics card. If Radeon R7 360 was handled by Radeon driver (not AMDGPU) then system has been started properly without that errors. I solved temporarily trouble by supplying kernel parameter 'amdgpu.dc=0' (just disable DC). I have connetected monitor to Intel IGP (in the motherboard DVI port) and I am not using any Radeon graphics card to display screen. After run and loading AMDGPU driver module kernel print errors (if all Radeon graphics cards are handled by AMDGPU driver and if no amdgpu.dc=0 parameter): [ 18.371385] WARNING: CPU: 1 PID: 289 at drivers/gpu/drm/drm_plane.c:182 drm_universal_plane_init+0x55b/0x5f0 [drm] [ 18.371386] Modules linked in: mousedev input_leds joydev led_class amdkfd intel_rapl x86_pkg_temp_thermal intel_powerclamp amd_iommu_v2 coretemp amdgpu(+) kvm_intel crct10dif_pclmul crc32_pclmul ghash_clmulni_intel pcbc aesni_intel aes_x86_64 radeon crypto_simd cryptd gpio_ich iTCO_wdt iTCO_vendor_support glue_helper intel_cstate chash gpu_sched snd_hda_codec_realtek ttm pcspkr intel_uncore snd_hda_codec_generic evdev intel_rapl_perf i2c_i801 snd_hda_codec_hdmi alx mei_me mei mdio ie31200_edac lpc_ich mac_hid snd_hda_intel snd_hda_codec snd_hda_core rtc_cmos snd_hwdep snd_pcm snd_timer snd soundcore pcc_cpufreq sg crypto_user ip_tables x_tables ext4 crc32c_generic crc16 mbcache jbd2 fscrypto sr_mod sd_mod cdrom uas usb_storage hid_generic ata_generic pata_acpi usbhid hid ata_piix xhci_pci libata [ 18.371446] ehci_pci xhci_hcd ehci_hcd crc32c_intel scsi_mod usbcore usb_common i915 kvmgt vfio_mdev mdev vfio_iommu_type1 vfio kvm irqbypass intel_gtt i2c_algo_bit cec rc_core drm_kms_helper syscopyarea sysfillrect sysimgblt fb_sys_fops drm agpgart [ 18.371470] CPU: 1 PID: 289 Comm: systemd-udevd Not tainted 4.19.0extratemps #1 [ 18.371471] Hardware name: Gigabyte Technology Co., Ltd. To be filled by O.E.M./Z77-DS3H, BIOS F8 08/21/2012 [ 18.371488] RIP: 0010:drm_universal_plane_init+0x55b/0x5f0 [drm] [ 18.371490] Code: e8 da e4 ff ff 48 8b b3 b8 04 00 00 31 d2 4c 89 ff e8 c9 e4 ff ff e9 08 fd ff ff 0f 0b c7 44 24 2c ea ff ff ff e9 06 fd ff ff <0f> 0b e9 2c fb ff ff bf 04 00 00 00 48 c7 c6 b0 f6 05 c0 e8 dd 21 [ 18.371492] RSP: 0018:afa001f4f938 EFLAGS: 00010246 [ 18.371494] RAX: c1206780 RBX: 9ffd092a1000 RCX: c00e8ba0 [ 18.371496] RDX: RSI: 9ffd0a84a800 RDI: 9ffd092a1000 [ 18.371497] RBP: afa001f4f9e8 R08: c00e9120 R09: 0002 [ 18.371498] R10: 9ffd1bbc74a0 R11: 0024 R12: 0002 [ 18.371500] R13: R14: 9ffd0a84a800 R15: c00e8ba0 [ 18.371502] FS: 7fe3e78b7480() GS:9ffd1f28() knlGS: [ 18.371504] CS: 0010 DS: ES: CR0: 80050033 [ 18.371505] CR2: 5610c22080b8 CR3: 000458860003 CR4: 001606e0 [ 18.371507] Call Trace: [ 18.371524] ? drm_crtc_init+0x2a/0xb0 [drm_kms_helper] [ 18.371534] drm_crtc_init+0x60/0xb0 [drm_kms_helper] [ 18.371631] dce_v8_0_sw_init+0x17d/0x3b0 [amdgpu] [ 18.371723] amdgpu_device_init.cold.14+0xd5f/0x1253 [amdgpu] [ 18.371730] ? kmalloc_order+0x14/0x40 [ 18.371799] amdgpu_driver_load_kms+0x86/0x2c0 [amdgpu] [ 18.371816] drm_dev_register+0x109/0x140 [drm] [ 18.371885] amdgpu_pci_probe+0x13c/0x1c0 [amdgpu] [ 18.371892] ? _raw_spin_unlock_irqrestore+0x20/0x40 [ 18.371896] local_pci_probe+0x41/0x90 [ 18.371900] pci_device_probe+0x189/0x1a0 [ 18.371906] really_probe+0x235/0x3a0 [ 18.371910] driver_probe_device+0xb3/0xf0 [ 18.371913] __driver_attach+0xdd/0x110 [ 18.371916] ? driver_probe_device+0xf0/0xf0 [ 18.371919] ? driver_probe_device+0xf0/0xf0 [ 18.371921] bus_for_each_dev+0x76/0xc0 [ 18.371924] bus_add_driver+0x152/0x230 [ 18.371927] ? 0xc15ca000 [ 18.371930] driver_register+0x6b/0xb0 [ 18.371932] ? 0xc15ca000 [ 18.371936] do_one_initcall+0x46/0x1f5 [ 18.371942] ? kmem_cache_alloc_trace+0x176/0x1d0 [ 18.371945] ? do_init_module+0x22/0x210 [ 18.371947] do_init_module+0x5a/0x210 [ 18.371950]
Re: [PATCH] drm/syncobj: Avoid kmalloc(GFP_KERNEL) under spinlock
Op 26-10-18 om 08:20 schreef Chunming Zhou:
> drivers/gpu/drm/drm_syncobj.c:202:4-14: ERROR: function
> drm_syncobj_find_signal_pt_for_point called on line 390 inside lock on line
> 389 but uses GFP_KERNEL
>
> Find functions that refer to GFP_KERNEL but are called with locks held.
>
> Generated by: scripts/coccinelle/locks/call_kern.cocci
>
> v2:
> syncobj->timeline still needs protect.
>
> v3:
> use a global signaled fence instead of re-allocation.
>
> v4:
> Don't need moving lock.
> Don't expose func.
>
> v5:
> rename func and directly return.
>
> Tested by: syncobj_wait and ./deqp-vk -n dEQP-VK.*semaphore* with
> lock debug kernel options enabled.
>
> Signed-off-by: Chunming Zhou
> Cc: Maarten Lankhorst
> Cc: intel-...@lists.freedesktop.org
> Cc: Christian König
> Cc: Chris Wilson
> CC: Julia Lawall
> Reviewed-by: Chris Wilson
> ---
> drivers/gpu/drm/drm_syncobj.c | 36 ++-
> 1 file changed, 19 insertions(+), 17 deletions(-)
>
> diff --git a/drivers/gpu/drm/drm_syncobj.c b/drivers/gpu/drm/drm_syncobj.c
> index b7eaa603f368..d1c6f21c72b5 100644
> --- a/drivers/gpu/drm/drm_syncobj.c
> +++ b/drivers/gpu/drm/drm_syncobj.c
> @@ -80,6 +80,23 @@ struct drm_syncobj_signal_pt {
> struct list_head list;
> };
>
> +static DEFINE_SPINLOCK(signaled_fence_lock);
> +static struct dma_fence signaled_fence;
> +
> +static struct dma_fence *drm_syncobj_get_stub_fence(void)
> +{
> + spin_lock(_fence_lock);
> + if (!signaled_fence.ops) {
> + dma_fence_init(_fence,
> +_syncobj_stub_fence_ops,
> +_fence_lock,
> +0, 0);
> + dma_fence_signal_locked(_fence);
> + }
> + spin_unlock(_fence_lock);
Could this be used by drm_syncobj_assign_null_handle too? Maybe as a separate
patch?
Reviewed-by: Maarten Lankhorst
~Maarten
___
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [PATCH v4 3/4] drm: Document variable refresh properties
On Thu, 11 Oct 2018 12:39:41 -0400 Nicholas Kazlauskas wrote: > These include the drm_connector 'vrr_capable' and the drm_crtc > 'vrr_enabled' properties. > > Signed-off-by: Nicholas Kazlauskas > --- > Documentation/gpu/drm-kms.rst | 7 +++ > drivers/gpu/drm/drm_connector.c | 22 ++ > 2 files changed, 29 insertions(+) > > diff --git a/Documentation/gpu/drm-kms.rst b/Documentation/gpu/drm-kms.rst > index 4b1501b4835b..8da2a178cf85 100644 > --- a/Documentation/gpu/drm-kms.rst > +++ b/Documentation/gpu/drm-kms.rst > @@ -575,6 +575,13 @@ Explicit Fencing Properties > .. kernel-doc:: drivers/gpu/drm/drm_atomic_uapi.c > :doc: explicit fencing properties > > + > +Variable Refresh Properties > +--- > + > +.. kernel-doc:: drivers/gpu/drm/drm_connector.c > + :doc: Variable refresh properties > + > Existing KMS Properties > --- > > diff --git a/drivers/gpu/drm/drm_connector.c b/drivers/gpu/drm/drm_connector.c > index f0deeb7298d0..2a12853ca917 100644 > --- a/drivers/gpu/drm/drm_connector.c > +++ b/drivers/gpu/drm/drm_connector.c > @@ -1254,6 +1254,28 @@ int drm_mode_create_scaling_mode_property(struct > drm_device *dev) > } > EXPORT_SYMBOL(drm_mode_create_scaling_mode_property); > > +/** > + * DOC: Variable refresh properties > + * > + * Variable refresh rate control is supported via properties on the > + * _connector and _crtc objects. > + * > + * "vrr_capable": > + * Optional _connector boolean property that drivers should attach > + * with drm_connector_attach_vrr_capable_property() on connectors that > + * could support variable refresh rates. Drivers should update the > + * property value by calling drm_connector_set_vrr_capable_property(). > + * > + * Absence of the property should indicate absence of support. > + * > + * "vrr_enabled": > + * Default _crtc boolean property that notifies the driver that the > + * variable refresh rate adjustment should be enabled for the CRTC. Hi, where is the documentation that explains how drivers must implement "variable refresh rate adjustment"? What should and could userspace expect to get if it flicks this switch? I also think the kernel documentation should include a description of what VRR actually is and how it conceptually works as far as userspace is concerned. That is, the kernel documentation should describe what this thing does, so that we avoid every driver implementing a different thing. For example, one driver could prevent the luminance flickering itself by tuning the timings while another driver might not do that, which means that an application tested on the former driver will look just fine while it is unbearable to watch on the latter. Thanks, pq > + * > + * Support for variable refresh rate will depend on the "vrr_capable" > + * property exposed on the _connector object. > + */ > + > /** > * drm_connector_attach_vrr_capable_property - creates the > * vrr_capable property pgpneGS2oQT_x.pgp Description: OpenPGP digital signature ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [RFC] Allow fd.o to join forces with X.Org
On Fri, Oct 26, 2018 at 1:08 PM Daniel Stone wrote: > > Hi, > > On Fri, 26 Oct 2018 at 11:57, Daniel Vetter wrote: > > On Fri, Oct 26, 2018 at 10:13:51AM +1000, Peter Hutterer wrote: > > > On Wed, Oct 17, 2018 at 02:37:25PM +0200, Daniel Vetter wrote: > > > > On Wed, Oct 17, 2018 at 2:05 PM Daniel Stone > > > > wrote: > > > > > Yeah, I think it makes sense. Some things we do: > > > > > - provide hosted network services for collaborative development, > > > > > testing, and discussion, of open-source projects > > > > > - administer, improve, and extend this suite of services as > > > > > necessary > > > > > - assist open-source projects in their use of these services > > > > > - purchase, lease, or subscribe to, computing and networking > > > > > infrastructure allowing these services to be run > > > > > > > > I fully agree that we should document all this. I don't think the > > > > bylaws are the right place though, much better to put that into > > > > policies that the board approves and which can be adapted as needed. > > > > Imo bylaws should cover the high-level mission and procedural details, > > > > as our "constitution", with the really high acceptance criteria of > > > > 2/3rd of all members approving any changes. Some of the early > > > > discussions tried to spell out a lot of the fd.o policies in bylaw > > > > changes, but then we realized it's all there already. All the details > > > > are much better served in policies enacted by the board, like we do > > > > with everything else. > > > > > > > > As an example, let's look at XDC. Definitely one of the biggest things > > > > the foundation does, with handling finances, travel sponsoring grants, > > > > papers committee, and acquiring lots of sponsors. None of this is > > > > spelled out in the bylaws, it's all in policies that the board > > > > deliberates and approves. I think this same approach will also work > > > > well for fd.o. > > > > > > > > And if members are unhappy with what the board does, they can fix in > > > > the next election by throwing out the unwanted directors. > > > > > > yeah, fair call. though IMO in that case we can just reduce to > > > > > >\item Support free and open source projects through the freedesktop.org > > >infrastructure. > > > > > > because my gripe is less with the fdo bit but more with defining what > > > "project hosting" means, given that we use that term to exclude fdo > > > projects > > > from getting anything else. I think just dropping that bit is sufficient. > > > > Hm yeah, through the lens of "everything not explicitly listed isn't in > > scope as X.org's purpose", leaving this out is probably clearest. And > > under 2.4 (i) the board already has the duty to interpret what exactly > > this means wrt membership eligibility. > > > > Harry, Daniel, what do you think? > > Yeah, that's fine. I didn't specifically want the enumerated list of > what we do in the bylaws, just spelling it out for background as a > handy reference I could point to later. I think maybe we could reduce > it to something like: > Administer, support, and improve the freedesktop.org hosting > infrastructure to support the projects it hosts This feels a bit self-referential, not the best for the purpose of what X.org does. If we do want to be a bit more specific we could do something like with (i) and provide a list that the board can extend: \item Support free and open source projects through the freedesktop.org infrastructure. This includes, but is not limited to: Administering and providing project hosting services. That would make it clear that admins are in scope, and everything else is up to the board. Similar to how drm, mesa, wayland and X are explicitly in scope, and stuff like cros/android gfx stack or libinput is up to the board to decide/clarify. > Gives us enough scope to grow in the future (e.g. we don't need a > bylaws change to move from pure-git to GitLab), avoids the sticky > question of what exactly fd.o hosts in the bylaws (e.g. if > NetworkManager needs a new repo then we don't have to consult > membership to add it), but is still pretty firmly limited in scope. > > Any of the above have my in-principle ack though, I think they're all > reasonable colours for our lovely shed. Well, one more bikeshed from me! Cheers, Daniel > > Cheers, > Daniel -- Daniel Vetter Software Engineer, Intel Corporation +41 (0) 79 365 57 48 - http://blog.ffwll.ch ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [PATCH 1/4] drm/sun4i: tcon: Support an active-low DE signal with RGB interface
On Wed, Oct 24, 2018 at 07:24:32PM +0200, Paul Kocialkowski wrote:
> Hi,
>
> Le mercredi 24 octobre 2018 à 17:47 +0100, Maxime Ripard a écrit :
> > On Tue, Oct 23, 2018 at 11:33:10AM +0200, Paul Kocialkowski wrote:
> > > Hi,
> > >
> > > Le mercredi 10 octobre 2018 à 16:57 +0200, Maxime Ripard a écrit :
> > > > On Wed, Oct 10, 2018 at 01:41:31PM +0200, Paul Kocialkowski wrote:
> > > > > Some panels need an active-low data enable (DE) signal for the RGB
> > > > > interface. This requires flipping a bit in the TCON0 polarity register
> > > > > when setting up the mode for the RGB interface.
> > > > >
> > > > > Add a new helper function to match specific bus flags and use it to
> > > > > set
> > > > > the polarity inversion bit for the DE signal when required.
> > > > >
> > > > > Signed-off-by: Paul Kocialkowski
> > > > > ---
> > > > > drivers/gpu/drm/sun4i/sun4i_tcon.c | 28 ++--
> > > > > drivers/gpu/drm/sun4i/sun4i_tcon.h | 1 +
> > > > > 2 files changed, 27 insertions(+), 2 deletions(-)
> > > > >
> > > > > diff --git a/drivers/gpu/drm/sun4i/sun4i_tcon.c
> > > > > b/drivers/gpu/drm/sun4i/sun4i_tcon.c
> > > > > index 3fb084f802e2..d249a462ec09 100644
> > > > > --- a/drivers/gpu/drm/sun4i/sun4i_tcon.c
> > > > > +++ b/drivers/gpu/drm/sun4i/sun4i_tcon.c
> > > > > @@ -78,6 +78,24 @@ static int sun4i_tcon_get_pixel_depth(const struct
> > > > > drm_encoder *encoder)
> > > > > return -EINVAL;
> > > > > }
> > > > >
> > > > > +static bool sun4i_tcon_match_bus_flags(const struct drm_encoder
> > > > > *encoder,
> > > > > +u32 bus_flags)
> > > > > +{
> > > > > + struct drm_connector *connector;
> > > > > + struct drm_display_info *info;
> > > > > +
> > > > > + connector = sun4i_tcon_get_connector(encoder);
> > > > > + if (!connector)
> > > > > + return false;
> > > > > +
> > > > > + info = >display_info;
> > > > > +
> > > > > + if ((info->bus_flags & bus_flags) == bus_flags)
> > > > > + return true;
> > > > > +
> > > > > + return false;
> > > > > +}
> > > > > +
> > > > > static void sun4i_tcon_channel_set_status(struct sun4i_tcon *tcon,
> > > > > int channel,
> > > > > bool enabled)
> > > > > {
> > > > > @@ -415,6 +433,7 @@ static void sun4i_tcon0_mode_set_lvds(struct
> > > > > sun4i_tcon *tcon,
> > > > > }
> > > > >
> > > > > static void sun4i_tcon0_mode_set_rgb(struct sun4i_tcon *tcon,
> > > > > + const struct drm_encoder *encoder,
> > > > >const struct drm_display_mode
> > > > > *mode)
> > > > > {
> > > > > unsigned int bp, hsync, vsync;
> > > > > @@ -474,8 +493,13 @@ static void sun4i_tcon0_mode_set_rgb(struct
> > > > > sun4i_tcon *tcon,
> > > > > if (mode->flags & DRM_MODE_FLAG_PVSYNC)
> > > > > val |= SUN4I_TCON0_IO_POL_VSYNC_POSITIVE;
> > > > >
> > > > > + if (sun4i_tcon_match_bus_flags(encoder, DRM_BUS_FLAG_DE_LOW))
> > > > > + val |= SUN4I_TCON0_IO_POL_DE_NEGATIVE;
> > > > > +
> > > >
> > > > There's other flags in use in that function, you should also migrate
> > > > them (ideally by splitting that new function into a separate patch).
> > >
> > > Actually, these other flags are not DRM bus flags but DRM mode flags,
> > > which can be accessed directly from the mode pointer passed as
> > > argument. Thus, they don't require a helper.
> >
> > I was talking about DRM_BUS_FLAG_PIXDATA_POSEDGE and
> > DRM_BUS_FLAG_PIXDATA_NEGEDGE, which are in the current tree.
>
> Thanks, I guess I need to rebase these patches atop the current tree!
>
> Looking at how these flags are used in the current tree, I see that the
> connector's display_info structure (that contains the bus flags) is
> retreived from tcon->panel.
>
> Would it make sense to change this and retreive it from the encoder,
> passed as argument to that function (as I've done in this patch)
> instead? This way, this wouldn't only apply to panels, but also to
> external bridges. It would also somewhat streamline the logic.
>
> What do you think?
That looks like a good idea :)
Maxime
--
Maxime Ripard, Bootlin
Embedded Linux and Kernel engineering
https://bootlin.com
___
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel
[PATCH v2] drm/panel: Add simple panel mode for the ARM RTSM
Having failed any attempts at a more generic solution,
I fall back to the very specific solution: define a simple
panel for the ARM RTSM emulated platforms.
I am doing this so we can convert all old users from the
previous fbdev driver to the PL111 DRM driver.
This works fine as far as I can test, provided the
device tree for RTSM AEMv8 is augmented accordingly.
Cc: Sudeep Holla
Cc: Lorenzo Pieralisi
Cc: Mali DP Maintainers
Cc: Robin Murphy
Reviewed-by: Liviu Dudau
Signed-off-by: Linus Walleij
---
ChangeLog v1->v2:
- Change compatible to simply "arm,rtsm-display" as there
are several of these RTSMs
- Collect Liviu's review tag
---
drivers/gpu/drm/panel/panel-simple.c | 30
1 file changed, 30 insertions(+)
diff --git a/drivers/gpu/drm/panel/panel-simple.c
b/drivers/gpu/drm/panel/panel-simple.c
index 97964f7f2ace..cd4e741b94e8 100644
--- a/drivers/gpu/drm/panel/panel-simple.c
+++ b/drivers/gpu/drm/panel/panel-simple.c
@@ -2323,6 +2323,33 @@ static const struct panel_desc winstar_wf35ltiacd = {
.bus_format = MEDIA_BUS_FMT_RGB888_1X24,
};
+static const struct drm_display_mode arm_rtsm_mode[] = {
+ {
+ .clock = 65000,
+ .hdisplay = 1024,
+ .hsync_start = 1024 + 24,
+ .hsync_end = 1024 + 24 + 136,
+ .htotal = 1024 + 24 + 136 + 160,
+ .vdisplay = 768,
+ .vsync_start = 768 + 3,
+ .vsync_end = 768 + 3 + 6,
+ .vtotal = 768 + 3 + 6 + 29,
+ .vrefresh = 60,
+ .flags = DRM_MODE_FLAG_NVSYNC | DRM_MODE_FLAG_NHSYNC,
+ },
+};
+
+static const struct panel_desc arm_rtsm = {
+ .modes = arm_rtsm_mode,
+ .num_modes = 1,
+ .bpc = 8,
+ .size = {
+ .width = 400,
+ .height = 300,
+ },
+ .bus_format = MEDIA_BUS_FMT_RGB888_1X24,
+};
+
static const struct of_device_id platform_of_match[] = {
{
.compatible = "ampire,am-480272h3tmqw-t01h",
@@ -2330,6 +2357,9 @@ static const struct of_device_id platform_of_match[] = {
}, {
.compatible = "ampire,am800480r3tmqwa1h",
.data = _am800480r3tmqwa1h,
+ }, {
+ .compatible = "arm,rtsm-display",
+ .data = _rtsm,
}, {
.compatible = "auo,b101aw03",
.data = _b101aw03,
--
2.17.2
___
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel
