Re: [PATCH 0/6] drm/i915: drmP.h include removal w/ drm prep work

2019-01-02 Thread Laurent Pinchart
Hi Jani,

On Wednesday, 2 January 2019 09:47:58 EET Jani Nikula wrote:
> On Fri, 28 Dec 2018, Jani Nikula  wrote:
> > Thanks for all the reviews, pushed patches 1-5 to topic/drmp-cleanup
> > with $(git merge-base drm-misc-next drm-intel-next-queued) as the
> > starting point. It's also included in drm-tip now.
> 
> So I did this *before* I got the review feedback from Laurent, based on
> Daniel's review only. Would you all like me to redo the branch with
> Laurent's comments addressed and r-b added?

If you think my comments are valuable, that may be a good idea :-)

-- 
Regards,

Laurent Pinchart



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


[Bug 109178] Keyboard backlight on Samsung Np900X5N not working due to not loading samsung-laptop.ko module

2019-01-02 Thread bugzilla-daemon
https://bugs.freedesktop.org/show_bug.cgi?id=109178

Martin Peres  changed:

   What|Removed |Added

  Group|spam|

-- 
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 109140] [KBL-G][GL] KHR-GL43.compute_shader.max test failed

2019-01-02 Thread bugzilla-daemon
https://bugs.freedesktop.org/show_bug.cgi?id=109140

--- Comment #1 from Hai  ---
Is there any update for this issue? 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


[Bug 109178] Keyboard backlight on Samsung Np900X5N not working due to not loading samsung-laptop.ko module

2019-01-02 Thread bugzilla-daemon
https://bugs.freedesktop.org/show_bug.cgi?id=109178

--- Comment #4 from Jani Nikula  ---
(In reply to Jan from comment #2)
> The samsung-laptop.ko module is enabling the use of the keyboard backlight
> keys.

This bugzilla is about display and graphics, not about keyboard. Try these
people and lists instead (or bugzilla.kernel.org, but there's a risk nobody
notices:

$ get_maintainer.pl -f ./drivers/platform/x86/samsung-laptop.c
Corentin Chary  (maintainer:SAMSUNG LAPTOP DRIVER)
Darren Hart  (maintainer:X86 PLATFORM DRIVERS)
Andy Shevchenko  (maintainer:X86 PLATFORM DRIVERS)
platform-driver-...@vger.kernel.org (open list:SAMSUNG LAPTOP DRIVER)
linux-ker...@vger.kernel.org (open list)

> The BIOS is put into CSM mode.

For i915 we don't really support the CSM mode. Please use UEFI instead.

-- 
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 109178] Keyboard backlight on Samsung Np900X5N not working due to not loading samsung-laptop.ko module

2019-01-02 Thread bugzilla-daemon
https://bugs.freedesktop.org/show_bug.cgi?id=109178

Jani Nikula  changed:

   What|Removed |Added

 Status|NEEDINFO|RESOLVED
 Resolution|--- |NOTOURBUG
 Whiteboard||

--- Comment #5 from Jani Nikula  ---
NOTOURBUG

-- 
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 109178] Keyboard backlight on Samsung Np900X5N not working due to not loading samsung-laptop.ko module

2019-01-02 Thread bugzilla-daemon
https://bugs.freedesktop.org/show_bug.cgi?id=109178

Jan  changed:

   What|Removed |Added

 Status|RESOLVED|CLOSED

--- Comment #6 from Jan  ---
Ok apologies :-) I understand now. Sorry for bothering you guys in the wrong
group. I am still learning my way to the right people.

I contacted them directly via get_maint

Warm regards!

-- 
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 109001] Freezes when waking up after screen goes blank.

2019-01-02 Thread bugzilla-daemon
https://bugs.freedesktop.org/show_bug.cgi?id=109001

--- Comment #10 from wirch.edu...@gmail.com ---
Disabling stuff does not help. Running kernel 4.20.0-1 and these settings:

Section "Device"
Identifier  "Device0"
Driver  "amdgpu"
BusID   "PCI:14:0:0"
Option "DRI" "2"
Option "TearFree" "off"
  Option "EnablePageFlip" "off"
EndSection

Will still freeze the system after waking up from power save mode.

kernel: [drm:dce110_vblank_set [amdgpu]] *ERROR* Failed to get VBLANK!
kernel: [drm:dce110_vblank_set [amdgpu]] *ERROR* Failed to get VBLANK!
kernel: [drm:dce110_vblank_set [amdgpu]] *ERROR* Failed to get VBLANK!
kernel: [drm:dce110_vblank_set [amdgpu]] *ERROR* Failed to get VBLANK!
kernel: [drm:dce110_vblank_set [amdgpu]] *ERROR* Failed to get VBLANK!
kernel: [drm:dce110_vblank_set [amdgpu]] *ERROR* Failed to get VBLANK!
kernel: [drm:dce110_vblank_set [amdgpu]] *ERROR* Failed to get VBLANK!
kernel: general protection fault:  [#1] PREEMPT SMP NOPTI
kernel: CPU: 12 PID: 1442 Comm: xfwm4 Not tainted 4.20.0-1-MANJARO #1
kernel: Hardware name: To Be Filled By O.E.M. To Be Filled By O.E.M./X370
Taichi, BIOS P3.30 01/15/2018
kernel: RIP: 0010:dce110_vblank_set+0x4a/0xa0 [amdgpu]
kernel: Code: ef e8 ba 7b 03 00 84 db 74 38 83 e8 4e 0f b6 c0 48 69 c0 30
04 00 00 49 03 85 30 01 00 00 48 8b b8 88 02 00 00 48 85 ff 74 32 <48> 8b 07 be
02 00 00 00 48 8b 80 f0 00 00 00 e8 f2 c1 12 e9 84 c0
kernel: RSP: 0018:aa058767bbe0 EFLAGS: 00010002
kernel: RAX: 93ede3b66960 RBX: 0001 RCX: 
kernel: RDX:  RSI: fff9 RDI: 0004000600010001
kernel: RBP: c137d050 R08:  R09: c0888c4b
kernel: R10:  R11: c0875c10 R12: 93ef8c8fa000
kernel: R13: 93ef91e06000 R14: 93f0d4ea01c8 R15: aa058767bd98
kernel: FS:  7fbb6b5e9180() GS:93f0def0()
knlGS:
kernel: CS:  0010 DS:  ES:  CR0: 80050033
kernel: CR2: 10ef9ae91000 CR3: 000e72de2000 CR4: 003406e0
kernel: Call Trace:
kernel:  dm_enable_vblank+0x26/0x30 [amdgpu]
kernel:  drm_vblank_enable+0xd4/0x120 [drm]
kernel:  drm_vblank_get+0x88/0xa0 [drm]
kernel:  drm_wait_vblank_ioctl+0x138/0x630 [drm]
kernel:  ? import_iovec+0x52/0xb0
kernel:  ? drm_legacy_modeset_ctl_ioctl+0x100/0x100 [drm]
kernel:  drm_ioctl_kernel+0xaf/0xf0 [drm]
kernel:  drm_ioctl+0x333/0x3e0 [drm]
kernel:  ? drm_legacy_modeset_ctl_ioctl+0x100/0x100 [drm]
kernel:  ? do_iter_write+0xda/0x190
kernel:  amdgpu_drm_ioctl+0x49/0x80 [amdgpu]
kernel:  do_vfs_ioctl+0xa4/0x630
kernel:  ksys_ioctl+0x60/0x90
kernel:  __x64_sys_ioctl+0x16/0x20
kernel:  do_syscall_64+0x65/0x180
kernel:  entry_SYSCALL_64_after_hwframe+0x44/0xa9
kernel: RIP: 0033:0x7fbb6d8ff80b
kernel: Code: 0f 1e fa 48 8b 05 55 b6 0c 00 64 c7 00 26 00 00 00 48 c7 c0
ff ff ff ff c3 66 0f 1f 44 00 00 f3 0f 1e fa b8 10 00 00 00 0f 05 <48> 3d 01 f0
ff ff 73 01 c3 48 8b 0d 25 b6 0c 00 f7 d8 64 89 01 48
kernel: RSP: 002b:7ffe82b09618 EFLAGS: 0246 ORIG_RAX:
0010
kernel: RAX: ffda RBX: 7ffe82b09640 RCX: 7fbb6d8ff80b
kernel: RDX: 7ffe82b09640 RSI: c018643a RDI: 000c
kernel: RBP: 55843bb359e0 R08: 7ffe82b790b0 R09: 7ffe82b79080
kernel: R10: 0001e54e R11: 0246 R12: c018643a
kernel: R13: 0080e489 R14:  R15: 55843bc31240
kernel: Modules linked in: cmac rfcomm fuse ipt_MASQUERADE
nf_conntrack_netlink nfnetlink xfrm_user xfrm_algo iptable_nat nf_nat_ipv4
xt_addrtype iptable_filter xt_conntrack nf_nat nf_conntrack nf_defrag_ipv6
nf_defrag_ipv4 libcrc32c br_netfilter bridge stp llc overlay aufs bnep arc4
amdgpu iwlmvm edac_mce_amd kvm mac80211 irqbypass nls_iso8859_1 nls_cp437 >
kernel:  crc32c_generic crc16 mbcache jbd2 fscrypto sr_mod cdrom
hid_generic usbhid hid ahci libahci libata xhci_pci crc32c_intel xhci_hcd
scsi_mod
kernel: ---[ end trace 53aef1365621a569 ]---
kernel: RIP: 0010:dce110_vblank_set+0x4a/0xa0 [amdgpu]
kernel: Code: ef e8 ba 7b 03 00 84 db 74 38 83 e8 4e 0f b6 c0 48 69 c0 30
04 00 00 49 03 85 30 01 00 00 48 8b b8 88 02 00 00 48 85 ff 74 32 <48> 8b 07 be
02 00 00 00 48 8b 80 f0 00 00 00 e8 f2 c1 12 e9 84 c0
kernel: RSP: 0018:aa058767bbe0 EFLAGS: 00010002
kernel: RAX: 93ede3b66960 RBX: 0001 RCX: 
kernel: RDX:  RSI: fff9 RDI: 0004000600010001
kernel: RBP: c137d050 R08:  R09: c0888c4b
kernel: R10:  R11: c0875c10 R12: 93ef8c8fa000
kernel: R13: 93ef91e06000 R14: 93f0d4ea01c8 R15: aa058767bd98
kernel: FS:  7fbb6b5e9180() GS:93f0def

Re: iommu_intel or i915 regression in 4.18, 4.19.12 and drm-tip

2019-01-02 Thread Joonas Lahtinen
Quoting Eric Wong (2018-12-27 13:49:48)
> I just got a used Thinkpad X201 (Core i5 M 520, Intel QM57
> chipset) and hit some kernel panics while trying to view
> image/animation-intensive stuff in Firefox (X11) unless I use
> "iommu_intel=igfx_off".
> 
> With Debian stable backport kernels, "linux-image-4.17.0-0.bpo.3-amd64"
> (4.17.17-1~bpo9+1) has no problems.  But "linux-image-4.18.0-0.bpo.3-amd64"
> (4.18.20-2~bpo9+1) gives a blank screen before I can login via agetty
> and run startx.

Could you open a new bug at (and attach relevant information there):

https://01.org/linuxgraphics/documentation/how-report-bugs

Most confusing about this is that 4.17 would have worked to begin with,
without intel_iommu=igfx_off (unless it was the default for older
kernel?)

Did you maybe update other parts of the system while updating the
kernel?

If you could attach full boot dmesg from working and non-working kernel +
have config file of both kernel's in Bugzilla. That'd be a good start!

Regards, Joonas

> Building 4.19.12 myself got me into X11 and able to start
> Firefox to panic the kernel.  I also updated to the latest BIOS
> (1.40), but it's an EOL laptop (but it's still the most powerful
> laptop I use).  I intend to replace the BIOS with Coreboot soon...
> 
> Initially, I thought I was hitting another GPU hang from 4.18+:
> 
> https://bugs.freedesktop.org/show_bug.cgi?id=107945
> 
> But building drm-tip @ commit 28bb1fc015cedadf3b099b8bd0bb27609849f362
> ("drm-tip: 2018y-12m-25d-08h-12m-37s UTC integration manifest")
> I was still able to reproduce the panic unless I use iommu_intel=igfx_off
> "i915.reset=1" did not help matters, either.
> 
> Below is what I got from netconsole while on drm-tip:
> 
> Kernel panic - not syncing: DMAR hardware is malfunctioning
> Shutting down cpus with NMI
> Kernel Offset: disabled
> ---[ end Kernel panic - not syncing: DMAR hardware is malfunctioning ]---
> [ cut here ]
> sched: Unexpected reschedule of offline CPU#3!
> WARNING: CPU: 0 PID: 105 at native_smp_send_reschedule+0x34/0x40
> Modules linked in: netconsole ccm snd_hda_codec_hdmi snd_hda_codec_conexant 
> snd_hda_codec_generic intel_powerclamp coretemp kvm_intel kvm irqbypass 
> crc32_pclmul crc32c_intel ghash_clmulni_intel arc4 iwldvm aesni_intel 
> aes_x86_64 crypto_simd cryptd mac80211 glue_helper intel_cstate iwlwifi 
> intel_uncore i915 intel_gtt i2c_algo_bit iosf_mbi drm_kms_helper cfbfillrect 
> syscopyarea intel_ips cfbimgblt sysfillrect sysimgblt fb_sys_fops cfbcopyarea 
> thinkpad_acpi prime_numbers cfg80211 ledtrig_audio i2c_i801 sg snd_hda_intel 
> led_class snd_hda_codec drm ac drm_panel_orientation_quirks snd_hwdep battery 
> e1000e agpgart snd_hda_core snd_pcm snd_timer ptp snd soundcore pps_core 
> ehci_pci ehci_hcd lpc_ich video mfd_core button acpi_cpufreq ecryptfs 
> ip_tables x_tables ipv6 evdev thermal [last unloaded: netconsole]
> CPU: 0 PID: 105 Comm: kworker/u8:3 Not tainted 4.20.0-rc7b1+ #1
> Hardware name: LENOVO 3680FBU/3680FBU, BIOS 6QET70WW (1.40 ) 10/11/2012
> Workqueue: i915 __i915_gem_free_work [i915]
> RIP: 0010:native_smp_send_reschedule+0x34/0x40
> Code: 05 69 c6 c9 00 73 15 48 8b 05 18 2d b3 00 be fd 00 00 00 48 8b 40 30 e9 
> 9a 58 7d 00 89 fe 48 c7 c7 78 73 af 81 e8 dc c2 01 00 <0f> 0b c3 66 0f 1f 84 
> 00 00 00 00 00 66 66 66 66 90 8b 05 0d 7d df
> RSP: 0018:888075003d98 EFLAGS: 00010092
> RAX: 002e RBX: 8880751a0740 RCX: 0006
> RDX: 0007 RSI: 0082 RDI: 888075015440
> RBP: 88806e823700 R08:  R09: 888072fc07c0
> R10: 888075003d60 R11: fff5c002 R12: 8880751a0740
> R13: 8880751a0740 R14:  R15: 0003
> FS:  () GS:88807500() knlGS:
> CS:  0010 DS:  ES:  CR0: 80050033
> CR2: 7fdb1f53f000 CR3: 01c0a004 CR4: 000206f0
> Call Trace:
>  
>  ? check_preempt_curr+0x4e/0x90
>  ? ttwu_do_wakeup.isra.19+0x14/0xf0
>  ? try_to_wake_up+0x323/0x410
>  ? autoremove_wake_function+0xe/0x30
>  ? __wake_up_common+0x8d/0x140
>  ? __wake_up_common_lock+0x6c/0x90
>  ? irq_work_run_list+0x49/0x80
>  ? tick_sched_handle.isra.6+0x50/0x50
>  ? update_process_times+0x3b/0x50
>  ? tick_sched_handle.isra.6+0x30/0x50
>  ? tick_sched_timer+0x3b/0x80
>  ? __hrtimer_run_queues+0xea/0x270
>  ? hrtimer_interrupt+0x101/0x240
>  ? smp_apic_timer_interrupt+0x6a/0x150
>  ? apic_timer_interrupt+0xf/0x20
>  
>  ? panic+0x1ca/0x212
>  ? panic+0x1c7/0x212
>  ? __iommu_flush_iotlb+0x19e/0x1c0
>  ? iommu_flush_iotlb_psi+0x96/0xf0
>  ? intel_unmap+0xbf/0xf0
>  ? i915_gem_object_put_pages_gtt+0x36/0x220 [i915]
>  ? drm_ht_remove+0x20/0x20 [drm]
>  ? drm_mm_remove_node+0x1ad/0x310 [drm]
>  ? __pm_runtime_resume+0x54/0x70
>  ? __i915_gem_object_unset_pages+0x129/0x170 [i915]
>  ? __i915_gem_object_put_pages+0x70/0xa0 [i915]
>  ? __i915_gem_free_objects+0x245/0x4e0 [i915]
>  ? __switch_to_a

Re: [PATCH 0/6] drm/i915: drmP.h include removal w/ drm prep work

2019-01-02 Thread Jani Nikula
On Wed, 02 Jan 2019, Laurent Pinchart  wrote:
> Hi Jani,
>
> On Wednesday, 2 January 2019 09:47:58 EET Jani Nikula wrote:
>> On Fri, 28 Dec 2018, Jani Nikula  wrote:
>> > Thanks for all the reviews, pushed patches 1-5 to topic/drmp-cleanup
>> > with $(git merge-base drm-misc-next drm-intel-next-queued) as the
>> > starting point. It's also included in drm-tip now.
>> 
>> So I did this *before* I got the review feedback from Laurent, based on
>> Daniel's review only. Would you all like me to redo the branch with
>> Laurent's comments addressed and r-b added?
>
> If you think my comments are valuable, that may be a good idea :-)

Put that way... apologies for even thinking I have an option here! :)

I took the liberty of force pushing the topic branch, with just the
commit messages updated to reflect your review.

Thanks,
Jani.

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


[PULL] topic/drmp-cleanup for drm-misc-next and drm-intel-next-queued

2019-01-02 Thread Jani Nikula

Hi Sean, Maarten & Maxime -

I embarked on removing drmP.h includes from i915, but that requires a
bunch of drm header cleanup to add relevant includes and forward
declarations. Due to the timing, propagating the patches back to i915
would take eons, so Daniel suggested a topic branch to be merged both to
drm-misc-next and drm-intel-next-queued. So here it is, with $(git
merge-base drm-misc-next drm-intel-next-queued) as the starting point.

The topic branch has been part of drm-tip since, uh, last year, but I
did just force push it to update the commit messages to reflect
Laurent's review. No code changes.

I'll merge the same thing to i915 after it's been pulled to
drm-misc-next, and with this, I should be able to get rid of all drmP.h
includes in i915.


BR,
Jani.

The following changes since commit b4bf44d2dcbd6c35d9651bc6286e4940b8b3df95:

  drm/i915: Update DRIVER_DATE to 20181122 (2018-11-22 16:49:47 +0200)

are available in the git repository at:

  git://anongit.freedesktop.org/drm/drm-intel tags/topic/drmp-cleanup-2019-01-02

for you to fetch changes up to dd7ece7f6e220e4d1a2a8ba4c42622d7d73e6376:

  drm: forward declare struct drm_file in drm_syncobj.h (2019-01-02 11:38:08 
+0200)


Make some drm headers self-contained with includes and forward declarations


Jani Nikula (5):
  drm: un-inline drm_legacy_findmap()
  drm: include kernel.h and agp_backend.h from intel-gtt.h
  drm: include idr.h from drm_file.h
  drm: include types.h from drm_hdcp.h
  drm: forward declare struct drm_file in drm_syncobj.h

 drivers/gpu/drm/drm_bufs.c | 11 +++
 include/drm/drm_file.h |  1 +
 include/drm/drm_hdcp.h |  2 ++
 include/drm/drm_legacy.h   | 14 --
 include/drm/drm_syncobj.h  |  4 +++-
 include/drm/intel-gtt.h|  3 +++
 6 files changed, 24 insertions(+), 11 deletions(-)

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


Re: [PATCH] gpu: ipu-v3: Fix i.MX51 CSI control registers offset

2019-01-02 Thread Philipp Zabel
Hi Alexander,

On Thu, 2018-12-20 at 11:06 +0300, Alexander Shiyan wrote:
> The CSI0/CSI1 registers offset is at +0xe03/+0xe038000 relative
> to the control module registers on IPUv3EX.
> This patch fixes wrong values for i.MX51 CSI0/CSI1.
> 
> Signed-off-by: Alexander Shiyan 
> ---
>  drivers/gpu/ipu-v3/ipu-common.c | 4 ++--
>  1 file changed, 2 insertions(+), 2 deletions(-)
> 
> diff --git a/drivers/gpu/ipu-v3/ipu-common.c b/drivers/gpu/ipu-v3/ipu-common.c
> index 474b00e19697..5b7cdbfe062f 100644
> --- a/drivers/gpu/ipu-v3/ipu-common.c
> +++ b/drivers/gpu/ipu-v3/ipu-common.c
> @@ -898,8 +898,8 @@ static struct ipu_devtype ipu_type_imx51 = {
>   .cpmem_ofs = 0x1f00,
>   .srm_ofs = 0x1f04,
>   .tpm_ofs = 0x1f06,
> - .csi0_ofs = 0x1f03,
> - .csi1_ofs = 0x1f038000,
> + .csi0_ofs = 0x1e03,
> + .csi1_ofs = 0x1e038000,

Thank you for the patch. This fix matches the documentation in the
MCIMX51RM, table 42-1 "IPUv3EX internal memory map".
Applied to imx-drm/fixes.

>   .ic_ofs = 0x1e02,
>   .disp0_ofs = 0x1e04,
>   .disp1_ofs = 0x1e048000,

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


Re: [PATCH] drm: Block fb changes for async plane updates

2019-01-02 Thread Kazlauskas, Nicholas
On 12/24/18 7:15 AM, Daniel Vetter wrote:
> On Fri, Dec 21, 2018 at 09:33:24AM -0500, Nicholas Kazlauskas wrote:
>> The behavior of drm_atomic_helper_cleanup_planes differs depending on
>> whether the commit was an asynchronous update or not.
>>
>> For a typical (non-async) atomic commit prepare_fb is called on the
>> new_plane_state and cleanup_fb is called on the old_plane_state.
>>
>> However, async commits are performed in place and don't swap the state
>> for the plane. The call to prepare_fb happens on the new_plane_state
>> and the call to cleanup_fb is also called on the new_plane_state in
>> this case (since the state hasn't swapped).
>>
>> This behavior can lead to use-after-free or unpin of an active fb.
>>
>> Consider the following sequence of events for interleaving fbs:
>>
>> - Async update, fb1 prepare, fb1 cleanup_fb
>> - Async update, fb2 prepare, fb2 cleanup_fb
>> - Non-async update, fb1 prepare, fb2 cleanup_fb
>> - Async update, fb2 cleanup_fb -> double cleanup, use-after-free
> 
> I think I see your bug, but I'm completely lost in your description above.
> 
> I think this is ok as a short-term gap, but imo better if it's a separate
> if condition with a FIXME comment.
> 
> Long-term we want to fix this, and I think simplest way to do that is if
> we expect drivers to store the old fb in the new_plane_state (and check
> that with a WARN_ON like the others). I think that should work.
> 
> We probably also need some locking on top, to prevent races with the
> cleanup_fb calls done by non-blocking commits, to make sure those clean up
> the right fb.
> -Daniel

Hi Daniel,

The description is supposed to be saying that the wrong fb is being 
cleaned up because in state updates to the plane don't swap the state 
pointer - which is required by the cleanup planes helper function.

But putting that aside, I think what you suggest here would work best 
for "fixing" the problem. Storing the old fb pointer in the new state 
looks kind of odd, but as long as it's documented and there's the 
WARN_ON in the helper I think it's fine. I think I would prefer this 
over a solution that blocks fb changes in the helper altogether.

I don't think any additional drm level locking is necessary here. The 
race with cleanup_fb calls is already something you have to worry about 
when dealing with async commits even if the fb hasn't changed. Most 
drivers have their own internal locks or ref-counting that can handle this.

So to summarize, I think I can post a new patch series that addresses 
this problem by fixing existing async commit usage in vc4 and amdgpu for 
framebuffer changes. The last patch in the series could be the one that 
adds a WARN_ON in the drm_atomic_helper_async_commit helper along with a 
comment indicating that the old fb should be in the new plane state.

Does this sound reasonable to you?

Nicholas Kazlauskas

> 
>> This situation has been observed in practice for a double buffered
>> cursor when closing an X client. The non-async update occurs because
>> the new_plane_state->crtc != old_plane_state->crtc which forces the
>> non-async path to occur.
>>
>> The simplest fix for this is to block fb updates in
>> drm_atomic_helper_async_check. This guarantees that the framebuffer
>> will have previously been prepared and any subsequent async updates
>> will always call prepare and cleanup_fb like the non-async atomic
>> commit path would.
>>
>> Cc: Michel Dänzer 
>> Cc: Daniel Vetter 
>> Cc: Andrey Grodzovsky 
>> Cc: Harry Wentland 
>> Signed-off-by: Nicholas Kazlauskas 
>> ---
>>   drivers/gpu/drm/drm_atomic_helper.c | 3 ++-
>>   1 file changed, 2 insertions(+), 1 deletion(-)
>>
>> diff --git a/drivers/gpu/drm/drm_atomic_helper.c 
>> b/drivers/gpu/drm/drm_atomic_helper.c
>> index 54e2ae614dcc..d2f80bf14f86 100644
>> --- a/drivers/gpu/drm/drm_atomic_helper.c
>> +++ b/drivers/gpu/drm/drm_atomic_helper.c
>> @@ -1599,7 +1599,8 @@ int drm_atomic_helper_async_check(struct drm_device 
>> *dev,
>>  return -EINVAL;
>>   
>>  if (!new_plane_state->crtc ||
>> -old_plane_state->crtc != new_plane_state->crtc)
>> +old_plane_state->crtc != new_plane_state->crtc ||
>> +old_plane_state->fb != new_plane_state->fb)
>>  return -EINVAL;
>>   
>>  funcs = plane->helper_private;
>> -- 
>> 2.17.1
>>
> 

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


[Bug 201815] Mouse Pointer Disappears when touching top of the screen

2019-01-02 Thread bugzilla-daemon
https://bugzilla.kernel.org/show_bug.cgi?id=201815

Sam Bazley (sambaz...@protonmail.com) changed:

   What|Removed |Added

 CC||sambaz...@protonmail.com

--- Comment #10 from Sam Bazley (sambaz...@protonmail.com) ---
This issue also affects 2500U and 2700U, but not Vega 64.

-- 
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/virtio: switch to generic fbdev emulation

2019-01-02 Thread Ezequiel Garcia
On Thu, 2018-12-13 at 14:49 +0100, Gerd Hoffmann wrote:

A few words explaining why the generic emulation is OK would be useful.

The commit might be clear for seasoned DRM developers, however
given this driver is a nice target for those learning DRM
(as it can be tested easily), I think having a more verbose
history is useful.

The patch looks good:

Reviewed-by: Ezequiel Garcia 

> Signed-off-by: Gerd Hoffmann 
> ---
>  drivers/gpu/drm/virtio/virtgpu_drv.h |  14 ---
>  drivers/gpu/drm/virtio/virtgpu_display.c |   1 -
>  drivers/gpu/drm/virtio/virtgpu_drv.c |   9 +-
>  drivers/gpu/drm/virtio/virtgpu_fb.c  | 191 
> ---
>  drivers/gpu/drm/virtio/virtgpu_kms.c |   8 --
>  5 files changed, 8 insertions(+), 215 deletions(-)
> 
> diff --git a/drivers/gpu/drm/virtio/virtgpu_drv.h 
> b/drivers/gpu/drm/virtio/virtgpu_drv.h
> index 1deb41d42e..63704915f8 100644
> --- a/drivers/gpu/drm/virtio/virtgpu_drv.h
> +++ b/drivers/gpu/drm/virtio/virtgpu_drv.h
> @@ -137,19 +137,10 @@ struct virtio_gpu_framebuffer {
>  #define to_virtio_gpu_framebuffer(x) \
>   container_of(x, struct virtio_gpu_framebuffer, base)
>  
> -struct virtio_gpu_fbdev {
> - struct drm_fb_helper   helper;
> - struct virtio_gpu_framebuffer  vgfb;
> - struct virtio_gpu_device   *vgdev;
> - struct delayed_workwork;
> -};
> -
>  struct virtio_gpu_mman {
>   struct ttm_bo_devicebdev;
>  };
>  
> -struct virtio_gpu_fbdev;
> -
>  struct virtio_gpu_queue {
>   struct virtqueue *vq;
>   spinlock_t qlock;
> @@ -180,8 +171,6 @@ struct virtio_gpu_device {
>  
>   struct virtio_gpu_mman mman;
>  
> - /* pointer to fbdev info structure */
> - struct virtio_gpu_fbdev *vgfbdev;
>   struct virtio_gpu_output outputs[VIRTIO_GPU_MAX_SCANOUTS];
>   uint32_t num_scanouts;
>  
> @@ -249,9 +238,6 @@ int virtio_gpu_mode_dumb_mmap(struct drm_file *file_priv,
> uint32_t handle, uint64_t *offset_p);
>  
>  /* virtio_fb */
> -#define VIRTIO_GPUFB_CONN_LIMIT 1
> -int virtio_gpu_fbdev_init(struct virtio_gpu_device *vgdev);
> -void virtio_gpu_fbdev_fini(struct virtio_gpu_device *vgdev);
>  int virtio_gpu_surface_dirty(struct virtio_gpu_framebuffer *qfb,
>struct drm_clip_rect *clips,
>unsigned int num_clips);
> diff --git a/drivers/gpu/drm/virtio/virtgpu_display.c 
> b/drivers/gpu/drm/virtio/virtgpu_display.c
> index b5580b11a0..e1c223e18d 100644
> --- a/drivers/gpu/drm/virtio/virtgpu_display.c
> +++ b/drivers/gpu/drm/virtio/virtgpu_display.c
> @@ -390,6 +390,5 @@ void virtio_gpu_modeset_fini(struct virtio_gpu_device 
> *vgdev)
>  
>   for (i = 0 ; i < vgdev->num_scanouts; ++i)
>   kfree(vgdev->outputs[i].edid);
> - virtio_gpu_fbdev_fini(vgdev);
>   drm_mode_config_cleanup(vgdev->ddev);
>  }
> diff --git a/drivers/gpu/drm/virtio/virtgpu_drv.c 
> b/drivers/gpu/drm/virtio/virtgpu_drv.c
> index f7f32a885a..7df50920c1 100644
> --- a/drivers/gpu/drm/virtio/virtgpu_drv.c
> +++ b/drivers/gpu/drm/virtio/virtgpu_drv.c
> @@ -42,13 +42,20 @@ module_param_named(modeset, virtio_gpu_modeset, int, 
> 0400);
>  
>  static int virtio_gpu_probe(struct virtio_device *vdev)
>  {
> + int ret;
> +
>   if (vgacon_text_force() && virtio_gpu_modeset == -1)
>   return -EINVAL;
>  
>   if (virtio_gpu_modeset == 0)
>   return -EINVAL;
>  
> - return drm_virtio_init(&driver, vdev);
> + ret = drm_virtio_init(&driver, vdev);
> + if (ret)
> + return ret;
> +
> + drm_fbdev_generic_setup(vdev->priv, 32);
> + return 0;
>  }
>  
>  static void virtio_gpu_remove(struct virtio_device *vdev)
> diff --git a/drivers/gpu/drm/virtio/virtgpu_fb.c 
> b/drivers/gpu/drm/virtio/virtgpu_fb.c
> index fb1cc8b2f1..b07584b1c2 100644
> --- a/drivers/gpu/drm/virtio/virtgpu_fb.c
> +++ b/drivers/gpu/drm/virtio/virtgpu_fb.c
> @@ -27,8 +27,6 @@
>  #include 
>  #include "virtgpu_drv.h"
>  
> -#define VIRTIO_GPU_FBCON_POLL_PERIOD (HZ / 60)
> -
>  static int virtio_gpu_dirty_update(struct virtio_gpu_framebuffer *fb,
>  bool store, int x, int y,
>  int width, int height)
> @@ -150,192 +148,3 @@ int virtio_gpu_surface_dirty(struct 
> virtio_gpu_framebuffer *vgfb,
> left, top, right - left, bottom - top);
>   return 0;
>  }
> -
> -static void virtio_gpu_fb_dirty_work(struct work_struct *work)
> -{
> - struct delayed_work *delayed_work = to_delayed_work(work);
> - struct virtio_gpu_fbdev *vfbdev =
> - container_of(delayed_work, struct virtio_gpu_fbdev, work);
> - struct virtio_gpu_framebuffer *vgfb = &vfbdev->vgfb;
> -
> - virtio_gpu_dirty_update(&vfbdev->vgfb, false, vgfb->x1, vgfb->y1,
> - vgfb->x2 - vgfb->x1, vgfb->y2 - vgfb->y1);
> -}
> -
> -static void virtio_gpu_3d_fillrect(struct fb_

Re: [PATCH 02/10] drm/virtio: fix pageflip flush

2019-01-02 Thread Ezequiel Garcia
On Wed, 2018-12-19 at 13:27 +0100, Gerd Hoffmann wrote:
> Sending the flush command only makes sense if we actually have
> a framebuffer attached to the scanout (handle != 0).
> 
> Signed-off-by: Gerd Hoffmann 
> ---
>  drivers/gpu/drm/virtio/virtgpu_plane.c | 11 ++-
>  1 file changed, 6 insertions(+), 5 deletions(-)
> 
> diff --git a/drivers/gpu/drm/virtio/virtgpu_plane.c 
> b/drivers/gpu/drm/virtio/virtgpu_plane.c
> index ead5c53d4e..548265b8e8 100644
> --- a/drivers/gpu/drm/virtio/virtgpu_plane.c
> +++ b/drivers/gpu/drm/virtio/virtgpu_plane.c
> @@ -130,11 +130,12 @@ static void virtio_gpu_primary_plane_update(struct 
> drm_plane *plane,
>  plane->state->src_h >> 16,
>  plane->state->src_x >> 16,
>  plane->state->src_y >> 16);
> - virtio_gpu_cmd_resource_flush(vgdev, handle,
> -   plane->state->src_x >> 16,
> -   plane->state->src_y >> 16,
> -   plane->state->src_w >> 16,
> -   plane->state->src_h >> 16);
> + if (handle)
> + virtio_gpu_cmd_resource_flush(vgdev, handle,
> +   plane->state->src_x >> 16,
> +   plane->state->src_y >> 16,
> +   plane->state->src_w >> 16,
> +   plane->state->src_h >> 16);
>  }
>  
>  static int virtio_gpu_cursor_prepare_fb(struct drm_plane *plane,

Reviewed-by: Ezequiel Garcia 

Regards,
Ezequiel

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


Re: [PATCH 03/10] drm/virtio: drop virtio_gpu_fence_cleanup()

2019-01-02 Thread Ezequiel Garcia
On Wed, 2018-12-19 at 13:27 +0100, Gerd Hoffmann wrote:
> Just call drm_fence_put directly instead.
> Also set vgfb->fence to NULL after dropping the reference.
> 
> Signed-off-by: Gerd Hoffmann 
> ---
>  drivers/gpu/drm/virtio/virtgpu_drv.h   | 1 -
>  drivers/gpu/drm/virtio/virtgpu_fence.c | 8 
>  drivers/gpu/drm/virtio/virtgpu_ioctl.c | 2 +-
>  drivers/gpu/drm/virtio/virtgpu_plane.c | 6 --
>  4 files changed, 5 insertions(+), 12 deletions(-)
> 
> diff --git a/drivers/gpu/drm/virtio/virtgpu_drv.h 
> b/drivers/gpu/drm/virtio/virtgpu_drv.h
> index 63704915f8..bfb31fc3d0 100644
> --- a/drivers/gpu/drm/virtio/virtgpu_drv.h
> +++ b/drivers/gpu/drm/virtio/virtgpu_drv.h
> @@ -337,7 +337,6 @@ int virtio_gpu_mmap(struct file *filp, struct 
> vm_area_struct *vma);
>  /* virtio_gpu_fence.c */
>  struct virtio_gpu_fence *virtio_gpu_fence_alloc(
>   struct virtio_gpu_device *vgdev);
> -void virtio_gpu_fence_cleanup(struct virtio_gpu_fence *fence);
>  int virtio_gpu_fence_emit(struct virtio_gpu_device *vgdev,
> struct virtio_gpu_ctrl_hdr *cmd_hdr,
> struct virtio_gpu_fence *fence);
> diff --git a/drivers/gpu/drm/virtio/virtgpu_fence.c 
> b/drivers/gpu/drm/virtio/virtgpu_fence.c
> index 4d6826b278..21bd4c4a32 100644
> --- a/drivers/gpu/drm/virtio/virtgpu_fence.c
> +++ b/drivers/gpu/drm/virtio/virtgpu_fence.c
> @@ -81,14 +81,6 @@ struct virtio_gpu_fence *virtio_gpu_fence_alloc(struct 
> virtio_gpu_device *vgdev)
>   return fence;
>  }
>  
> -void virtio_gpu_fence_cleanup(struct virtio_gpu_fence *fence)
> -{
> - if (!fence)
> - return;
> -
> - dma_fence_put(&fence->f);
> -}
> -
>  int virtio_gpu_fence_emit(struct virtio_gpu_device *vgdev,
> struct virtio_gpu_ctrl_hdr *cmd_hdr,
> struct virtio_gpu_fence *fence)
> diff --git a/drivers/gpu/drm/virtio/virtgpu_ioctl.c 
> b/drivers/gpu/drm/virtio/virtgpu_ioctl.c
> index 161b80fee4..14ce8188c0 100644
> --- a/drivers/gpu/drm/virtio/virtgpu_ioctl.c
> +++ b/drivers/gpu/drm/virtio/virtgpu_ioctl.c
> @@ -351,7 +351,7 @@ static int virtio_gpu_resource_create_ioctl(struct 
> drm_device *dev, void *data,
>   virtio_gpu_cmd_resource_create_3d(vgdev, qobj, &rc_3d);
>   ret = virtio_gpu_object_attach(vgdev, qobj, fence);
>   if (ret) {
> - virtio_gpu_fence_cleanup(fence);
> + dma_fence_put(&fence->f);
>   goto fail_backoff;
>   }
>   ttm_eu_fence_buffer_objects(&ticket, &validate_list, &fence->f);
> diff --git a/drivers/gpu/drm/virtio/virtgpu_plane.c 
> b/drivers/gpu/drm/virtio/virtgpu_plane.c
> index 548265b8e8..024c2aa0c9 100644
> --- a/drivers/gpu/drm/virtio/virtgpu_plane.c
> +++ b/drivers/gpu/drm/virtio/virtgpu_plane.c
> @@ -169,8 +169,10 @@ static void virtio_gpu_cursor_cleanup_fb(struct 
> drm_plane *plane,
>   return;
>  
>   vgfb = to_virtio_gpu_framebuffer(plane->state->fb);
> - if (vgfb->fence)
> - virtio_gpu_fence_cleanup(vgfb->fence);
> + if (vgfb->fence) {
> + dma_fence_put(&vgfb->fence->f);
> + vgfb->fence = NULL;
> + }
>  }
>  
>  static void virtio_gpu_cursor_plane_update(struct drm_plane *plane,

Reviewed-by: Ezequiel Garcia 

Regards,
Ezequiel

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


Re: [PATCH 05/10] drm/virtio: use struct to pass params to virtio_gpu_object_create()

2019-01-02 Thread Ezequiel Garcia
On Wed, 2018-12-19 at 13:27 +0100, Gerd Hoffmann wrote:
> Create virtio_gpu_object_params, use that to pass object parameters to
> virtio_gpu_object_create.  Also drop unused "kernel" parameter (unused,
> always false).
> 
> Signed-off-by: Gerd Hoffmann 
> ---
>  drivers/gpu/drm/virtio/virtgpu_drv.h| 15 ++-
>  drivers/gpu/drm/virtio/virtgpu_gem.c| 18 +++---
>  drivers/gpu/drm/virtio/virtgpu_ioctl.c  | 12 +++-
>  drivers/gpu/drm/virtio/virtgpu_object.c | 22 +-
>  4 files changed, 37 insertions(+), 30 deletions(-)
> 
> diff --git a/drivers/gpu/drm/virtio/virtgpu_drv.h 
> b/drivers/gpu/drm/virtio/virtgpu_drv.h
> index bfb31fc3d0..8cff8a3f7c 100644
> --- a/drivers/gpu/drm/virtio/virtgpu_drv.h
> +++ b/drivers/gpu/drm/virtio/virtgpu_drv.h
> @@ -53,6 +53,11 @@
>  /* virtgpu_drm_bus.c */
>  int drm_virtio_init(struct drm_driver *driver, struct virtio_device *vdev);
>  
> +struct virtio_gpu_object_params {
> + unsigned long size;
> + bool pinned;
> +};
> +
>  struct virtio_gpu_object {
>   struct drm_gem_object gem_base;
>   uint32_t hw_res_handle;
> @@ -220,16 +225,16 @@ int virtio_gpu_gem_init(struct virtio_gpu_device 
> *vgdev);
>  void virtio_gpu_gem_fini(struct virtio_gpu_device *vgdev);
>  int virtio_gpu_gem_create(struct drm_file *file,
> struct drm_device *dev,
> -   uint64_t size,
> +   struct virtio_gpu_object_params *params,
> struct drm_gem_object **obj_p,
> uint32_t *handle_p);
>  int virtio_gpu_gem_object_open(struct drm_gem_object *obj,
>  struct drm_file *file);
>  void virtio_gpu_gem_object_close(struct drm_gem_object *obj,
>struct drm_file *file);
> -struct virtio_gpu_object *virtio_gpu_alloc_object(struct drm_device *dev,
> -   size_t size, bool kernel,
> -   bool pinned);
> +struct virtio_gpu_object*
> +virtio_gpu_alloc_object(struct drm_device *dev,
> + struct virtio_gpu_object_params *params);
>  int virtio_gpu_mode_dumb_create(struct drm_file *file_priv,
>   struct drm_device *dev,
>   struct drm_mode_create_dumb *args);
> @@ -345,7 +350,7 @@ void virtio_gpu_fence_event_process(struct 
> virtio_gpu_device *vdev,
>  
>  /* virtio_gpu_object */
>  int virtio_gpu_object_create(struct virtio_gpu_device *vgdev,
> -  unsigned long size, bool kernel, bool pinned,
> +  struct virtio_gpu_object_params *params,
>struct virtio_gpu_object **bo_ptr);
>  void virtio_gpu_object_kunmap(struct virtio_gpu_object *bo);
>  int virtio_gpu_object_kmap(struct virtio_gpu_object *bo);
> diff --git a/drivers/gpu/drm/virtio/virtgpu_gem.c 
> b/drivers/gpu/drm/virtio/virtgpu_gem.c
> index f065863939..384cd80bf3 100644
> --- a/drivers/gpu/drm/virtio/virtgpu_gem.c
> +++ b/drivers/gpu/drm/virtio/virtgpu_gem.c
> @@ -34,15 +34,15 @@ void virtio_gpu_gem_free_object(struct drm_gem_object 
> *gem_obj)
>   virtio_gpu_object_unref(&obj);
>  }
>  
> -struct virtio_gpu_object *virtio_gpu_alloc_object(struct drm_device *dev,
> -   size_t size, bool kernel,
> -   bool pinned)
> +struct virtio_gpu_object*
> +virtio_gpu_alloc_object(struct drm_device *dev,
> + struct virtio_gpu_object_params *params)
>  {
>   struct virtio_gpu_device *vgdev = dev->dev_private;
>   struct virtio_gpu_object *obj;
>   int ret;
>  
> - ret = virtio_gpu_object_create(vgdev, size, kernel, pinned, &obj);
> + ret = virtio_gpu_object_create(vgdev, params, &obj);
>   if (ret)
>   return ERR_PTR(ret);
>  
> @@ -51,7 +51,7 @@ struct virtio_gpu_object *virtio_gpu_alloc_object(struct 
> drm_device *dev,
>  
>  int virtio_gpu_gem_create(struct drm_file *file,
> struct drm_device *dev,
> -   uint64_t size,
> +   struct virtio_gpu_object_params *params,
> struct drm_gem_object **obj_p,
> uint32_t *handle_p)
>  {
> @@ -59,7 +59,7 @@ int virtio_gpu_gem_create(struct drm_file *file,
>   int ret;
>   u32 handle;
>  
> - obj = virtio_gpu_alloc_object(dev, size, false, false);
> + obj = virtio_gpu_alloc_object(dev, params);
>   if (IS_ERR(obj))
>   return PTR_ERR(obj);
>  
> @@ -85,6 +85,9 @@ int virtio_gpu_mode_dumb_create(struct drm_file *file_priv,
>   struct virtio_gpu_device *vgdev = dev->dev_private;
>   struct drm_gem_object *gobj;
>   struct virtio_gpu_object *obj;
> + struct virtio_gpu_object_params params = {
> + .pinned = false,
> + };

Nit: I think it's more r

[PULL] drm-misc-next-fixes

2019-01-02 Thread Maarten Lankhorst
drm-misc-next-fixes-2019-01-02:
Fixes for v4.21:
- Fix null pointer dereference on null state pointer.
- Fix leaking damage clip when destroying plane state.
The following changes since commit 2e6e902d185027f8e3cb8b7305238f7e35d6a436:

  Linux 4.20-rc4 (2018-11-25 14:19:31 -0800)

are available in the Git repository at:

  git://anongit.freedesktop.org/drm/drm-misc tags/drm-misc-next-fixes-2019-01-02

for you to fetch changes up to c75ff001f4fe21a8c9f15aad799a8949aea109f7:

  drm: Put damage blob when destroy plane state (2018-12-24 11:53:50 +0100)


Fixes for v4.21:
- Fix null pointer dereference on null state pointer.
- Fix leaking damage clip when destroying plane state.


Colin Ian King (1):
  drm: fix null pointer dereference on null state pointer

Deepak Rawat (1):
  drm: Put damage blob when destroy plane state

 drivers/gpu/drm/drm_atomic_state_helper.c | 3 +++
 drivers/gpu/drm/drm_damage_helper.c   | 3 ++-
 2 files changed, 5 insertions(+), 1 deletion(-)
___
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel


Re: [PATCH] drm/nouveau: fix incorrect FB_BACKLIGHT usage in Kconfig

2019-01-02 Thread Bartlomiej Zolnierkiewicz

On 12/28/2018 07:03 PM, Randy Dunlap wrote:
> On 12/28/18 7:15 AM, Bartlomiej Zolnierkiewicz wrote:
>> Making FB_BACKLIGHT tristate by commit b4a1ed0cd18b ("fbdev:
>> make FB_BACKLIGHT a tristate") caused unmet dependencies in
>> some configurations:
>>
>> WARNING: unmet direct dependencies detected for FB_BACKLIGHT
>>   Depends on [m]: HAS_IOMEM [=y] && FB [=m]
>>   Selected by [y]:
>>   - DRM_NOUVEAU [=y] && HAS_IOMEM [=y] && DRM [=y] && PCI [=y] && MMU [=y] 
>> && DRM_NOUVEAU_BACKLIGHT [=y]
>>   Selected by [m]:
>>   - FB_NVIDIA [=m] && HAS_IOMEM [=y] && FB [=m] && PCI [=y] && 
>> FB_NVIDIA_BACKLIGHT [=y]
>>
>> Fix it by making DRM_NOUVEAU select BACKLIGHT_CLASS_DEVICE and
>> BACKLIGHT_LCD_SUPPORT instead of FB_BACKLIGHT.
>>
>> Fixes: b4a1ed0cd18b ("fbdev: make FB_BACKLIGHT a tristate")
>> Reported-by: Randy Dunlap 
>> Cc: Rob Clark 
>> Cc: Arnd Bergmann 
>> Cc: Ben Skeggs 
>> Cc: David Airlie 
>> Cc: Daniel Vetter 
>> Cc: Stephen Rothwell 
>> Signed-off-by: Bartlomiej Zolnierkiewicz 
> 
> Works for me.  Thanks.
> 
> Acked-by: Randy Dunlap  # build-tested

Thanks, I queued the patch for 4.21.

>> ---
>> I would like to merge this patch through fbdev tree for v4.21
>> (as it is needed to fix commit currently in fbdev tree).
>>
>>  drivers/gpu/drm/nouveau/Kconfig |3 ++-
>>  1 file changed, 2 insertions(+), 1 deletion(-)
>>
>> Index: b/drivers/gpu/drm/nouveau/Kconfig
>> ===
>> --- a/drivers/gpu/drm/nouveau/Kconfig2018-11-30 11:46:55.716464505 
>> +0100
>> +++ b/drivers/gpu/drm/nouveau/Kconfig2018-12-28 14:54:51.655965845 
>> +0100
>> @@ -4,7 +4,8 @@ config DRM_NOUVEAU
>>  select FW_LOADER
>>  select DRM_KMS_HELPER
>>  select DRM_TTM
>> -select FB_BACKLIGHT if DRM_NOUVEAU_BACKLIGHT
>> +select BACKLIGHT_CLASS_DEVICE if DRM_NOUVEAU_BACKLIGHT
>> +select BACKLIGHT_LCD_SUPPORT if DRM_NOUVEAU_BACKLIGHT
>>  select ACPI_VIDEO if ACPI && X86 && BACKLIGHT_CLASS_DEVICE && INPUT
>>  select X86_PLATFORM_DEVICES if ACPI && X86
>>  select ACPI_WMI if ACPI && X86

Best regards,
--
Bartlomiej Zolnierkiewicz
Samsung R&D Institute Poland
Samsung Electronics
___
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel


[PATCH] drm/virtio: Add missing virtqueue reset

2019-01-02 Thread Ezequiel Garcia
As per the VirtIO spec, the virtqueues must be reset during cleanup
(see "3.3.1 Driver Requirements: Device Cleanup").

Signed-off-by: Ezequiel Garcia 
---
 drivers/gpu/drm/virtio/virtgpu_kms.c | 1 +
 1 file changed, 1 insertion(+)

diff --git a/drivers/gpu/drm/virtio/virtgpu_kms.c 
b/drivers/gpu/drm/virtio/virtgpu_kms.c
index 1072064a0db2..c340be252fce 100644
--- a/drivers/gpu/drm/virtio/virtgpu_kms.c
+++ b/drivers/gpu/drm/virtio/virtgpu_kms.c
@@ -239,6 +239,7 @@ void virtio_gpu_driver_unload(struct drm_device *dev)
flush_work(&vgdev->ctrlq.dequeue_work);
flush_work(&vgdev->cursorq.dequeue_work);
flush_work(&vgdev->config_changed_work);
+   vgdev->vdev->config->reset(vgdev->vdev);
vgdev->vdev->config->del_vqs(vgdev->vdev);
 
virtio_gpu_modeset_fini(vgdev);
-- 
2.20.0

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


[PATCH] drm/virtio: Remove incorrect kfree()

2019-01-02 Thread Ezequiel Garcia
The virtio_gpu_output is a member of struct virtio_gpu_device
and is not a dynamically-allocated chunk, so it's wrong to kfree() it.
Removing it fixes a memory corruption BUG() that can be triggered
when the virtio-gpu driver is removed.

Signed-off-by: Ezequiel Garcia 
---
 drivers/gpu/drm/virtio/virtgpu_display.c | 4 
 1 file changed, 4 deletions(-)

diff --git a/drivers/gpu/drm/virtio/virtgpu_display.c 
b/drivers/gpu/drm/virtio/virtgpu_display.c
index e1c223e18d86..d539bc28dc97 100644
--- a/drivers/gpu/drm/virtio/virtgpu_display.c
+++ b/drivers/gpu/drm/virtio/virtgpu_display.c
@@ -243,12 +243,8 @@ static enum drm_connector_status virtio_gpu_conn_detect(
 
 static void virtio_gpu_conn_destroy(struct drm_connector *connector)
 {
-   struct virtio_gpu_output *virtio_gpu_output =
-   drm_connector_to_virtio_gpu_output(connector);
-
drm_connector_unregister(connector);
drm_connector_cleanup(connector);
-   kfree(virtio_gpu_output);
 }
 
 static const struct drm_connector_funcs virtio_gpu_connector_funcs = {
-- 
2.20.0

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


[PATCH v5 03/11] vga-switcheroo: make PCI dependency explicit

2019-01-02 Thread Sinan Kaya
This driver depends on the PCI infrastructure but the dependency has not
been explicitly called out.

Fixes: 5d32a66541c46 ("PCI/ACPI: Allow ACPI to be built without CONFIG_PCI set")
Signed-off-by: Sinan Kaya 
Reviewed-by: Lukas Wunner 
Acked-by: Daniel Vetter 
---
 drivers/gpu/vga/Kconfig | 1 +
 1 file changed, 1 insertion(+)

diff --git a/drivers/gpu/vga/Kconfig b/drivers/gpu/vga/Kconfig
index b677e5d524e6..d5f1d8e1c6f8 100644
--- a/drivers/gpu/vga/Kconfig
+++ b/drivers/gpu/vga/Kconfig
@@ -21,6 +21,7 @@ config VGA_SWITCHEROO
bool "Laptop Hybrid Graphics - GPU switching support"
depends on X86
depends on ACPI
+   depends on PCI
select VGA_ARB
help
  Many laptops released in 2008/9/10 have two GPUs with a multiplexer
-- 
2.19.0

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


[Bug 201067] [bisected] [4.19-rc2 regression] Display corruption with Vega 64 in 4.19-rc2

2019-01-02 Thread bugzilla-daemon
https://bugzilla.kernel.org/show_bug.cgi?id=201067

--- Comment #15 from Axel (at...@t-online.de) ---
It is fixed for me with kernel 4.20.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


[Bug 109178] Keyboard backlight on Samsung Np900X5N not working due to not loading samsung-laptop.ko module

2019-01-02 Thread bugzilla-daemon
https://bugs.freedesktop.org/show_bug.cgi?id=109178

Jan  changed:

   What|Removed |Added

 Resolution|NOTOURBUG   |---
 Status|CLOSED  |REOPENED

--- Comment #7 from Jan  ---
Indeed nobody notices :-) Is there another way to reach the maintainers?

ps: only reopened it for notification purpuses. As soon there is an answer I
will close it again.

-- 
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 109177] Blender 2.8 triggers GPU lockup when entering Edit Mode

2019-01-02 Thread bugzilla-daemon
https://bugs.freedesktop.org/show_bug.cgi?id=109177

Alex Deucher  changed:

   What|Removed |Added

 QA Contact|mesa-dev@lists.freedesktop. |dri-devel@lists.freedesktop
   |org |.org
   Assignee|mesa-dev@lists.freedesktop. |dri-devel@lists.freedesktop
   |org |.org
  Component|Mesa core   |Drivers/Gallium/radeonsi

--- Comment #3 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 0/3] drm/mxsfb: support swapped RGB lanes

2019-01-02 Thread Stefan Agner
On 02.01.2019 18:02, Ahmad Fatoum wrote:
> Hello,
> 
> I got a board with the RED[0:7]/BLUE[0:7] lanes originating from the
> LCDIF swapped and would like to describe this in the device tree:
> 
> This first patch extends the mxsfb driver to support
> following bus formats:
>   MEDIA_BUS_FMT_BGR888_1X24
>   MEDIA_BUS_FMT_RBG888_1X24
>   MEDIA_BUS_FMT_GBR888_1X24
> 
> The latter two patches add a new interface-pix-fmt property
> (named so because fsl,imx-parallel-display has one),
> which allows a device tree to override the bus format to account
> for swapped signal lanes.
> 
> Thoughts?

On a quick glance patch 1 looks good.

However, patch 2/3 are probably unnecessary when using of graph/panel
support. E.g. panel-simple.c supports bus formats.

Is the display you are using regular RGB and only the board/connectors
happen to swap colors?

--
Stefan

> 
> Cheers
> Ahmad
> 
> --
> 
> Ahmad Fatoum (3):
>   drm/mxsfb: use bus_format to determine pixel RGB component order
>   drm/mxsfb: implement interface-pix-fmt of_property to override bus
> format
>   dt-bindings: mxsfb: document new interface-pix-fmt property
> 
>  .../devicetree/bindings/display/mxsfb.txt |  5 +++
>  drivers/gpu/drm/mxsfb/mxsfb_crtc.c| 45 +++
>  drivers/gpu/drm/mxsfb/mxsfb_drv.c | 13 ++
>  drivers/gpu/drm/mxsfb/mxsfb_drv.h |  7 +++
>  drivers/gpu/drm/mxsfb/mxsfb_regs.h| 17 +++
>  5 files changed, 79 insertions(+), 8 deletions(-)
___
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel


Re: [PATCH 0/3] drm/mxsfb: support swapped RGB lanes

2019-01-02 Thread Sam Ravnborg
Hi Ahmad.

On Wed, Jan 02, 2019 at 10:05:31PM +0100, Stefan Agner wrote:
> On 02.01.2019 18:02, Ahmad Fatoum wrote:
> > Hello,
> > 
> > I got a board with the RED[0:7]/BLUE[0:7] lanes originating from the
> > LCDIF swapped and would like to describe this in the device tree:
> > 
> > This first patch extends the mxsfb driver to support
> > following bus formats:
> > MEDIA_BUS_FMT_BGR888_1X24
> > MEDIA_BUS_FMT_RBG888_1X24
> > MEDIA_BUS_FMT_GBR888_1X24
> > 
> > The latter two patches add a new interface-pix-fmt property
> > (named so because fsl,imx-parallel-display has one),
> > which allows a device tree to override the bus format to account
> > for swapped signal lanes.
> > 
> > Thoughts?
I have not seen the original mail, so a reply to this mail.
The problem with the RED/BLUE lines swapped is something I
have encountered while working with DRM support for Atmel at91sam9263 too.

The solution selected is to extend the endpoint with
a new optional property:

- wiring: Wiring of data lines to display.
  "straight" - normal wiring.
  "red-blue-reversed" - red and blue lines reversed.

(media/video-interfaces.txt)


The DT node looks like this:

   port@0 {
reg = <0>;
#address-cells = <1>;
#size-cells = <0>;
lcdc_panel_output: endpoint@0 {
reg = <0>;
wiring = "red-blue-reversed";
remote-endpoint = <&panel_input>;
};
};

This allows us to specify the swapping in the endpoint and
not in the panel.
So we can use the same panel, with the same bus_format, in several
designs some with red-blue swapped (reversed), and some not.

This above is inspired by some earlier thread on dri-devel.
I recall Peter Rosin was one of the main source of inspiration.

Patches I refer to are not yet posted, this is work-in-progess.
They need more polishing and testing before they are dri-devel ready.

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


Re: [PATCH] staging: android: ion: add buffer flag update ioctl

2019-01-02 Thread Laura Abbott

On 12/23/18 6:47 PM, Zengtao (B) wrote:

Hi laura:


-Original Message-
From: Laura Abbott [mailto:labb...@redhat.com]
Sent: Friday, December 21, 2018 4:50 AM
To: Zengtao (B) ; sumit.sem...@linaro.org
Cc: Greg Kroah-Hartman ; Arve Hjønnevåg
; Todd Kjos ; Martijn Coenen
; Joel Fernandes ;
de...@driverdev.osuosl.org; dri-devel@lists.freedesktop.org;
linaro-mm-...@lists.linaro.org; linux-ker...@vger.kernel.org
Subject: Re: [PATCH] staging: android: ion: add buffer flag update ioctl

On 12/19/18 5:39 PM, Zengtao (B) wrote:

Hi laura:


-Original Message-
From: Laura Abbott [mailto:labb...@redhat.com]
Sent: Thursday, December 20, 2018 2:10 AM
To: Zengtao (B) ;

sumit.sem...@linaro.org

Cc: Greg Kroah-Hartman ; Arve

Hjønnevåg

; Todd Kjos ; Martijn

Coenen

; Joel Fernandes ;
de...@driverdev.osuosl.org; dri-devel@lists.freedesktop.org;
linaro-mm-...@lists.linaro.org; linux-ker...@vger.kernel.org
Subject: Re: [PATCH] staging: android: ion: add buffer flag update
ioctl

On 12/19/18 9:19 AM, Zeng Tao wrote:

In some usecases, the buffer cached attribute is not determined at
allocation time, it's determined just before the real cpu mapping.
And from the memory view of point, a buffer should not have the

cached

attribute util is really mapped by the cpu. So in this patch, we
introduced the new ioctl command to target the requirement.



This is racy and error prone. Can you explain more what problem you
are trying to solve?


My use case is like this:
1.  There are two process A and B, A takes case of ion buffer allocation,

and

   pass the buffer fd to B, then B maps and uses it.
2.  Process B need to map the buffer with different cached attribute
for different use case, for example, if the buffer is used for pure
software algorithm, then we need to map it as cached, otherwise
non-cached, and B needs to deal with both cases.
And unfortunately the mmap syscall takes no cached flags and we can't
decide the cache attribute when we are doing the mmap, so I introduce
new the ioctl even though I think the solution is not as good.




Thanks for the explanation, this was about the use case I expected.
I'm pretty sure I had this exact problem once upon a time and we didn't
come up with a solution. I'd still like to get rid of uncached buffers in
general and just use cached buffers (see
http://driverdev.linuxdriverproject.org/pipermail/driverdev-devel/2018-N
ovember/128842.html)
What's your usecase for uncached buffers?


Some buffers are only used by HW, in this case, we tend to use uncached buffers.
But sometimes the SW need to read few buffer contents for debug purpose and
no synchronization is needed.



If this is primarily for debug, that's not really a compelling reason to
support uncached buffers. It's incredibly difficult to do uncached
buffers correctly I'd rather spend effort on making the cached
use cases work better.

Thanks,
Laura






Signed-off-by: Zeng Tao 
---
drivers/staging/android/ion/ion-ioctl.c |  4 
drivers/staging/android/ion/ion.c   | 17

+

drivers/staging/android/ion/ion.h   |  1 +
drivers/staging/android/uapi/ion.h  | 22

++

4 files changed, 44 insertions(+)

diff --git a/drivers/staging/android/ion/ion-ioctl.c
b/drivers/staging/android/ion/ion-ioctl.c
index a8d3cc4..60bb702 100644
--- a/drivers/staging/android/ion/ion-ioctl.c
+++ b/drivers/staging/android/ion/ion-ioctl.c
@@ -12,6 +12,7 @@

union ion_ioctl_arg {
struct ion_allocation_data allocation;
+   struct ion_buffer_flag_data update;
struct ion_heap_query query;
};

@@ -83,6 +84,9 @@ long ion_ioctl(struct file *filp, unsigned int
cmd, unsigned long arg)

break;
}
+   case ION_IOC_BUFFER_UPDATE:
+   ret = ion_buffer_update(data.update.fd, data.update.flags);
+   break;
case ION_IOC_HEAP_QUERY:
ret = ion_query_heaps(&data.query);
break;
diff --git a/drivers/staging/android/ion/ion.c
b/drivers/staging/android/ion/ion.c
index 9907332..f1404dc 100644
--- a/drivers/staging/android/ion/ion.c
+++ b/drivers/staging/android/ion/ion.c
@@ -436,6 +436,23 @@ int ion_alloc(size_t len, unsigned int

heap_id_mask, unsigned int flags)

return fd;
}

+int ion_buffer_update(unsigned int fd, unsigned int flags) {
+   struct dma_buf *dmabuf;
+   struct ion_buffer *buffer;
+
+   dmabuf = dma_buf_get(fd);
+
+   if (!dmabuf)
+   return -EINVAL;
+
+   buffer = dmabuf->priv;
+   buffer->flags = flags;
+   dma_buf_put(dmabuf);
+
+   return 0;
+}
+
int ion_query_heaps(struct ion_heap_query *query)
{
struct ion_device *dev = internal_dev; diff --git
a/drivers/staging/android/ion/ion.h
b/drivers/staging/android/ion/ion.h
index c006fc1..99bf9ab 100644
--- a/drivers/staging/android/ion/ion.h
+++ b/drivers/staging/android/ion/ion.h
@@ -199,6 +199,7 @@ int ion_heap_pages_zero(struct page *page

Re: [PATCH] staging: android: ion: move map_kernel to ion_dma_buf_kmap

2019-01-02 Thread Laura Abbott

On 12/24/18 12:19 AM, Qing Xia wrote:

Now, as Google's user guide, if userspace need clean ION buffer's
cache, they should call ioctl(fd, DMA_BUF_IOCTL_SYNC, sync). Then
we found that ion_dma_buf_begin_cpu_access/ion_dma_buf_end_cpu_access
will do ION buffer's map_kernel, it's not necessary. And if usersapce
only need clean cache, they will call ion_dma_buf_end_cpu_access by
dmabuf's ioctl, ION buffer's kmap_cnt will be wrong value -1, then
driver could not get right kernel vaddr by dma_buf_kmap.



The problem is this subtly violates how dma_buf is supposed
to work. All setup is supposed to happen in begin_cpu_access.
I agree calling map kernel each time isn't great but I think
this needs to be worked out with dma_buf.

Thanks,
Laura


Signed-off-by: Qing Xia 
---
  drivers/staging/android/ion/ion.c | 46 ++-
  1 file changed, 21 insertions(+), 25 deletions(-)

diff --git a/drivers/staging/android/ion/ion.c 
b/drivers/staging/android/ion/ion.c
index 9907332..f7e2812 100644
--- a/drivers/staging/android/ion/ion.c
+++ b/drivers/staging/android/ion/ion.c
@@ -303,45 +303,47 @@ static void ion_dma_buf_release(struct dma_buf *dmabuf)
  static void *ion_dma_buf_kmap(struct dma_buf *dmabuf, unsigned long offset)
  {
struct ion_buffer *buffer = dmabuf->priv;
+   void *vaddr;
  
-	return buffer->vaddr + offset * PAGE_SIZE;

+   if (buffer->heap->ops->map_kernel) {
+   mutex_lock(&buffer->lock);
+   vaddr = ion_buffer_kmap_get(buffer);
+   mutex_unlock(&buffer->lock);
+   if (IS_ERR(vaddr))
+   return vaddr;
+
+   return vaddr + offset * PAGE_SIZE;
+   }
+
+   return NULL;
  }
  
  static void ion_dma_buf_kunmap(struct dma_buf *dmabuf, unsigned long offset,

   void *ptr)
  {
+   struct ion_buffer *buffer = dmabuf->priv;
+
+   if (buffer->heap->ops->map_kernel) {
+   mutex_lock(&buffer->lock);
+   ion_buffer_kmap_put(buffer);
+   mutex_unlock(&buffer->lock);
+   }
  }
  
  static int ion_dma_buf_begin_cpu_access(struct dma_buf *dmabuf,

enum dma_data_direction direction)
  {
struct ion_buffer *buffer = dmabuf->priv;
-   void *vaddr;
struct ion_dma_buf_attachment *a;
-   int ret = 0;
-
-   /*
-* TODO: Move this elsewhere because we don't always need a vaddr
-*/
-   if (buffer->heap->ops->map_kernel) {
-   mutex_lock(&buffer->lock);
-   vaddr = ion_buffer_kmap_get(buffer);
-   if (IS_ERR(vaddr)) {
-   ret = PTR_ERR(vaddr);
-   goto unlock;
-   }
-   mutex_unlock(&buffer->lock);
-   }
  
  	mutex_lock(&buffer->lock);

list_for_each_entry(a, &buffer->attachments, list) {
dma_sync_sg_for_cpu(a->dev, a->table->sgl, a->table->nents,
direction);
}
-
-unlock:
mutex_unlock(&buffer->lock);
-   return ret;
+
+   return 0;
  }
  
  static int ion_dma_buf_end_cpu_access(struct dma_buf *dmabuf,

@@ -350,12 +352,6 @@ static int ion_dma_buf_end_cpu_access(struct dma_buf 
*dmabuf,
struct ion_buffer *buffer = dmabuf->priv;
struct ion_dma_buf_attachment *a;
  
-	if (buffer->heap->ops->map_kernel) {

-   mutex_lock(&buffer->lock);
-   ion_buffer_kmap_put(buffer);
-   mutex_unlock(&buffer->lock);
-   }
-
mutex_lock(&buffer->lock);
list_for_each_entry(a, &buffer->attachments, list) {
dma_sync_sg_for_device(a->dev, a->table->sgl, a->table->nents,



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


[PATCH v3 04/16] drm/dp_mst: Stop releasing VCPI when removing ports from topology

2019-01-02 Thread Lyude Paul
This has never actually worked, and isn't needed anyway: the driver's
always going to try to deallocate VCPI when it tears down the display
that the VCPI belongs to.

Signed-off-by: Lyude Paul 
Cc: Daniel Vetter 
Cc: David Airlie 
Cc: Jerry Zuo 
Cc: Harry Wentland 
Cc: Juston Li 
---
 drivers/gpu/drm/drm_dp_mst_topology.c | 8 
 1 file changed, 8 deletions(-)

diff --git a/drivers/gpu/drm/drm_dp_mst_topology.c 
b/drivers/gpu/drm/drm_dp_mst_topology.c
index b8a47c795fa9..f10a7edb401e 100644
--- a/drivers/gpu/drm/drm_dp_mst_topology.c
+++ b/drivers/gpu/drm/drm_dp_mst_topology.c
@@ -1176,8 +1176,6 @@ static void drm_dp_destroy_port(struct kref *kref)
struct drm_dp_mst_topology_mgr *mgr = port->mgr;
 
if (!port->input) {
-   port->vcpi.num_slots = 0;
-
kfree(port->cached_edid);
 
/*
@@ -3493,12 +3491,6 @@ static void drm_dp_destroy_connector_work(struct 
work_struct *work)
drm_dp_port_teardown_pdt(port, port->pdt);
port->pdt = DP_PEER_DEVICE_NONE;
 
-   if (!port->input && port->vcpi.vcpi > 0) {
-   drm_dp_mst_reset_vcpi_slots(mgr, port);
-   drm_dp_update_payload_part1(mgr);
-   drm_dp_mst_put_payload_id(mgr, port->vcpi.vcpi);
-   }
-
drm_dp_mst_put_port_malloc(port);
send_hotplug = true;
}
-- 
2.20.1

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


[PATCH v3 03/16] drm/dp_mst: Restart last_connected_port_and_mstb() if topology ref fails

2019-01-02 Thread Lyude Paul
While this isn't a complete fix, this will improve the reliability of
drm_dp_get_last_connected_port_and_mstb() pretty significantly during
hotplug events, since there's a chance that the in-memory topology tree
may not be fully updated when drm_dp_get_last_connected_port_and_mstb()
is called and thus might end up causing our search to fail on an mstb
whose topology refcount has reached 0, but has not yet been removed from
it's parent.

Ideally, we should further fix this problem by ensuring that we deal
with the potential for racing with a hotplug event, which would look
like this:

* drm_dp_payload_send_msg() retrieves the last living relative of mstb
  with drm_dp_get_last_connected_port_and_mstb()
* drm_dp_payload_send_msg() starts building payload message
  At the same time, mstb gets unplugged from the topology and is no
  longer the actual last living relative of the original mstb
* drm_dp_payload_send_msg() tries sending the payload message, hub times
  out
* Hub timed out, we give up and run away-resulting in the payload being
  leaked

This could be fixed by restarting the
drm_dp_get_last_connected_port_and_mstb() search whenever we get a
timeout, sending the payload to the new mstb, then repeating until
either the entire topology is removed from the system or
drm_dp_get_last_connected_port_and_mstb() fails. But since the above
race condition is not terribly likely, we'll address that in a later
patch series once we've improved the recovery handling for VCPI
allocations in the rest of the DP MST helpers.

Signed-off-by: Lyude Paul 
Cc: Daniel Vetter 
Cc: David Airlie 
Cc: Jerry Zuo 
Cc: Harry Wentland 
Cc: Juston Li 
---
 drivers/gpu/drm/drm_dp_mst_topology.c | 54 ++-
 1 file changed, 44 insertions(+), 10 deletions(-)

diff --git a/drivers/gpu/drm/drm_dp_mst_topology.c 
b/drivers/gpu/drm/drm_dp_mst_topology.c
index c0dc20fbd55a..b8a47c795fa9 100644
--- a/drivers/gpu/drm/drm_dp_mst_topology.c
+++ b/drivers/gpu/drm/drm_dp_mst_topology.c
@@ -2045,24 +2045,50 @@ static struct drm_dp_mst_port 
*drm_dp_get_last_connected_port_to_mstb(struct drm
return 
drm_dp_get_last_connected_port_to_mstb(mstb->port_parent->parent);
 }
 
-static struct drm_dp_mst_branch 
*drm_dp_get_last_connected_port_and_mstb(struct drm_dp_mst_topology_mgr *mgr,
-struct 
drm_dp_mst_branch *mstb,
-int 
*port_num)
+/**
+ * drm_dp_get_last_connected_port_and_mstb() - Find the last living relatives
+ * in a topology of a given branch device
+ * @mgr: The topology manager to use
+ * @mstb: The disconnected branch device
+ * @port_num: Where to store the number of the last connected port
+ *
+ * Searches upwards in the topology starting from @mstb to try to find the
+ * closest available parent of @mstb that's still connected to the rest of the
+ * topology. This can be used in order to perform operations like releasing
+ * payloads, where the branch device which owned the payload may no longer be
+ * around and thus would require that the payload on the last living relative
+ * be freed instead.
+ *
+ * Returns:
+ * The last connected &drm_dp_mst_branch in the topology that was a parent of
+ * @mstb, if there is one.
+ */
+static struct drm_dp_mst_branch *
+drm_dp_get_last_connected_port_and_mstb(struct drm_dp_mst_topology_mgr *mgr,
+   struct drm_dp_mst_branch *mstb,
+   int *port_num)
 {
struct drm_dp_mst_branch *rmstb = NULL;
struct drm_dp_mst_port *found_port;
+
mutex_lock(&mgr->lock);
-   if (mgr->mst_primary) {
+   if (!mgr->mst_primary)
+   goto out;
+
+   do {
found_port = drm_dp_get_last_connected_port_to_mstb(mstb);
+   if (!found_port)
+   break;
 
-   if (found_port) {
+   if (drm_dp_mst_topology_try_get_mstb(found_port->parent)) {
rmstb = found_port->parent;
-   if (drm_dp_mst_topology_try_get_mstb(rmstb))
-   *port_num = found_port->port_num;
-   else
-   rmstb = NULL;
+   *port_num = found_port->port_num;
+   } else {
+   /* Search again, starting from this parent */
+   mstb = found_port->parent;
}
-   }
+   } while (!rmstb);
+out:
mutex_unlock(&mgr->lock);
return rmstb;
 }
@@ -2111,6 +2137,14 @@ static int drm_dp_payload_send_msg(struct 
drm_dp_mst_topology_mgr *mgr,
 
drm_dp_queue_down_tx(mgr, txmsg);
 
+   /*
+* FIXME: there is a small chance that between getting the last
+* connected mstb and sending the payload message, the last connected
+* mstb could also be removed from the topology. In th

[PATCH v3 06/16] drm/i915: Keep malloc references to MST ports

2019-01-02 Thread Lyude Paul
So that the ports stay around until we've destroyed the connectors, in
order to ensure that we don't pass an invalid pointer to any MST helpers
once we introduce the new MST VCPI helpers.

Changes since v1:
* Move drm_dp_mst_get_port_malloc() to where we assign
  intel_connector->port - danvet

Signed-off-by: Lyude Paul 
Reviewed-by: Daniel Vetter 
Cc: David Airlie 
Cc: Jerry Zuo 
Cc: Harry Wentland 
Cc: Juston Li 
---
 drivers/gpu/drm/i915/intel_connector.c | 4 
 drivers/gpu/drm/i915/intel_dp_mst.c| 1 +
 2 files changed, 5 insertions(+)

diff --git a/drivers/gpu/drm/i915/intel_connector.c 
b/drivers/gpu/drm/i915/intel_connector.c
index 18e370f607bc..37d2c644f4b8 100644
--- a/drivers/gpu/drm/i915/intel_connector.c
+++ b/drivers/gpu/drm/i915/intel_connector.c
@@ -95,6 +95,10 @@ void intel_connector_destroy(struct drm_connector *connector)
intel_panel_fini(&intel_connector->panel);
 
drm_connector_cleanup(connector);
+
+   if (intel_connector->port)
+   drm_dp_mst_put_port_malloc(intel_connector->port);
+
kfree(connector);
 }
 
diff --git a/drivers/gpu/drm/i915/intel_dp_mst.c 
b/drivers/gpu/drm/i915/intel_dp_mst.c
index f05427b74e34..631fd1537252 100644
--- a/drivers/gpu/drm/i915/intel_dp_mst.c
+++ b/drivers/gpu/drm/i915/intel_dp_mst.c
@@ -457,6 +457,7 @@ static struct drm_connector 
*intel_dp_add_mst_connector(struct drm_dp_mst_topolo
intel_connector->get_hw_state = intel_dp_mst_get_hw_state;
intel_connector->mst_port = intel_dp;
intel_connector->port = port;
+   drm_dp_mst_get_port_malloc(port);
 
connector = &intel_connector->base;
ret = drm_connector_init(dev, connector, &intel_dp_mst_connector_funcs,
-- 
2.20.1

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


[PATCH v3 00/16] MST refcounting/atomic helpers cleanup

2019-01-02 Thread Lyude Paul
This is the series I've been working on for a while now to get all of
the atomic DRM drivers in the tree to use the atomic MST helpers, and to
make the atomic MST helpers actually idempotent. Turns out it's a lot
more difficult to do that without also fixing how port and branch device
refcounting works so that it actually makes sense, since the current
upstream implementation requires a ton of magic in the atomic helpers to
work around properly and in many situations just plain doesn't work as
intended.

There's still more cleanup that can be done, but I think this is a good
place to start off for now :).

Lyude Paul (16):
  drm/dp_mst: Rename drm_dp_mst_get_validated_(port|mstb)_ref and
friends
  drm/dp_mst: Introduce new refcounting scheme for mstbs and ports
  drm/dp_mst: Restart last_connected_port_and_mstb() if topology ref
fails
  drm/dp_mst: Stop releasing VCPI when removing ports from topology
  drm/dp_mst: Fix payload deallocation on hotplugs using malloc refs
  drm/i915: Keep malloc references to MST ports
  drm/amdgpu/display: Keep malloc ref to MST port
  drm/nouveau: Remove bogus cleanup in nv50_mstm_add_connector()
  drm/nouveau: Remove unnecessary VCPI checks in nv50_msto_cleanup()
  drm/nouveau: Keep malloc references to MST ports
  drm/nouveau: Stop unsetting mstc->port, use malloc refs
  drm/nouveau: Grab payload lock in nv50_msto_payload()
  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

 .../gpu/dp-mst/topology-figure-1.dot  |  52 +
 .../gpu/dp-mst/topology-figure-2.dot  |  56 ++
 .../gpu/dp-mst/topology-figure-3.dot  |  59 ++
 Documentation/gpu/drm-kms-helpers.rst |  26 +-
 .../display/amdgpu_dm/amdgpu_dm_mst_types.c   |  11 +-
 drivers/gpu/drm/drm_dp_mst_topology.c | 938 ++
 drivers/gpu/drm/i915/intel_connector.c|   4 +
 drivers/gpu/drm/i915/intel_display.c  |   4 +
 drivers/gpu/drm/i915/intel_dp_mst.c   |  65 +-
 drivers/gpu/drm/nouveau/dispnv50/disp.c   |  94 +-
 include/drm/drm_dp_mst_helper.h   | 151 ++-
 11 files changed, 1208 insertions(+), 252 deletions(-)
 create mode 100644 Documentation/gpu/dp-mst/topology-figure-1.dot
 create mode 100644 Documentation/gpu/dp-mst/topology-figure-2.dot
 create mode 100644 Documentation/gpu/dp-mst/topology-figure-3.dot

-- 
2.20.1

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


[PATCH v3 02/16] drm/dp_mst: Introduce new refcounting scheme for mstbs and ports

2019-01-02 Thread Lyude Paul
The current way of handling refcounting in the DP MST helpers is really
confusing and probably just plain wrong because it's been hacked up many
times over the years without anyone actually going over the code and
seeing if things could be simplified.

To the best of my understanding, the current scheme works like this:
drm_dp_mst_port and drm_dp_mst_branch both have a single refcount. When
this refcount hits 0 for either of the two, they're removed from the
topology state, but not immediately freed. Both ports and branch devices
will reinitialize their kref once it's hit 0 before actually destroying
themselves. The intended purpose behind this is so that we can avoid
problems like not being able to free a remote payload that might still
be active, due to us having removed all of the port/branch device
structures in memory, as per:

commit 91a25e463130 ("drm/dp/mst: deallocate payload on port destruction")

Which may have worked, but then it caused use-after-free errors. Being
new to MST at the time, I tried fixing it;

commit 263efde31f97 ("drm/dp/mst: Get validated port ref in 
drm_dp_update_payload_part1()")

But, that was broken: both drm_dp_mst_port and drm_dp_mst_branch structs
are validated in almost every DP MST helper function. Simply put, this
means we go through the topology and try to see if the given
drm_dp_mst_branch or drm_dp_mst_port is still attached to something
before trying to use it in order to avoid dereferencing freed memory
(something that has happened a LOT in the past with this library).
Because of this it doesn't actually matter whether or not we keep keep
the ports and branches around in memory as that's not enough, because
any function that validates the branches and ports passed to it will
still reject them anyway since they're no longer in the topology
structure. So, use-after-free errors were fixed but payload deallocation
was completely broken.

Two years later, AMD informed me about this issue and I attempted to
come up with a temporary fix, pending a long-overdue cleanup of this
library:

commit c54c7374ff44 ("drm/dp_mst: Skip validating ports during destruction, 
just ref")

But then that introduced use-after-free errors, so I quickly reverted
it:

commit 9765635b3075 ("Revert "drm/dp_mst: Skip validating ports during 
destruction, just ref"")

And in the process, learned that there is just no simple fix for this:
the design is just broken. Unfortuntely, the usage of these helpers are
quite broken as well. Some drivers like i915 have been smart enough to
avoid accessing any kind of information from MST port structures, but
others like nouveau have assumed, understandably so, that
drm_dp_mst_port structures are normal and can just be accessed at any
time without worrying about use-after-free errors.

After a lot of discussion, me and Daniel Vetter came up with a better
idea to replace all of this.

To summarize, since this is documented far more indepth in the
documentation this patch introduces, we make it so that drm_dp_mst_port
and drm_dp_mst_branch structures have two different classes of
refcounts: topology_kref, and malloc_kref. topology_kref corresponds to
the lifetime of the given drm_dp_mst_port or drm_dp_mst_branch in it's
given topology. Once it hits zero, any associated connectors are removed
and the branch or port can no longer be validated. malloc_kref
corresponds to the lifetime of the memory allocation for the actual
structure, and will always be non-zero so long as the topology_kref is
non-zero. This gives us a way to allow callers to hold onto port and
branch device structures past their topology lifetime, and dramatically
simplifies the lifetimes of both structures. This also finally fixes the
port deallocation problem, properly.

Additionally: since this now means that we can keep ports and branch
devices allocated in memory for however long we need, we no longer need
a significant amount of the port validation that we currently do.

Additionally, there is one last scenario that this fixes, which couldn't
have been fixed properly beforehand:

- CPU1 unrefs port from topology (refcount 1->0)
- CPU2 refs port in topology(refcount 0->1)

Since we now can guarantee memory safety for ports and branches
as-needed, we also can make our main reference counting functions fix
this problem by using kref_get_unless_zero() internally so that topology
refcounts can only ever reach 0 once.

Changes since v2:
* Fix commit message - checkpatch
Changes since v1:
* Remove forward declarations - danvet
* Move "Branch device and port refcounting" section from documentation
  into kernel-doc comments - danvet
* Export internal topology lifetime functions into their own section in
  the kernel-docs - danvet
* s/@/&/g for struct references in kernel-docs - danvet
* Drop the "when they are no longer being used" bits from the kernel
  docs - danvet
* Modify diagrams to show how the DRM driver interacts with the topology
  and payloads - danvet
* Make suggested documentation cha

[PATCH v3 12/16] drm/nouveau: Grab payload lock in nv50_msto_payload()

2019-01-02 Thread Lyude Paul
Going through the currently programmed payloads isn't safe without
holding mgr->payload_lock, so actually do that and warn if anyone tries
calling nv50_msto_payload() in the future without grabbing the right
locks.

Signed-off-by: Lyude Paul 
Cc: Daniel Vetter 
Cc: David Airlie 
Cc: Jerry Zuo 
Cc: Harry Wentland 
Cc: Juston Li 
---
 drivers/gpu/drm/nouveau/dispnv50/disp.c | 6 ++
 1 file changed, 6 insertions(+)

diff --git a/drivers/gpu/drm/nouveau/dispnv50/disp.c 
b/drivers/gpu/drm/nouveau/dispnv50/disp.c
index 157d208d37b5..67f7bf97e5d9 100644
--- a/drivers/gpu/drm/nouveau/dispnv50/disp.c
+++ b/drivers/gpu/drm/nouveau/dispnv50/disp.c
@@ -680,6 +680,8 @@ nv50_msto_payload(struct nv50_msto *msto)
struct nv50_mstm *mstm = mstc->mstm;
int vcpi = mstc->port->vcpi.vcpi, i;
 
+   WARN_ON(!mutex_is_locked(&mstm->mgr.payload_lock));
+
NV_ATOMIC(drm, "%s: vcpi %d\n", msto->encoder.name, vcpi);
for (i = 0; i < mstm->mgr.max_payloads; i++) {
struct drm_dp_payload *payload = &mstm->mgr.payloads[i];
@@ -733,6 +735,8 @@ nv50_msto_prepare(struct nv50_msto *msto)
   (0x0100 << msto->head->base.index),
};
 
+   mutex_lock(&mstm->mgr.payload_lock);
+
NV_ATOMIC(drm, "%s: msto prepare\n", msto->encoder.name);
if (mstc->port->vcpi.vcpi > 0) {
struct drm_dp_payload *payload = nv50_msto_payload(msto);
@@ -748,7 +752,9 @@ nv50_msto_prepare(struct nv50_msto *msto)
  msto->encoder.name, msto->head->base.base.name,
  args.vcpi.start_slot, args.vcpi.num_slots,
  args.vcpi.pbn, args.vcpi.aligned_pbn);
+
nvif_mthd(&drm->display->disp.object, 0, &args, sizeof(args));
+   mutex_unlock(&mstm->mgr.payload_lock);
 }
 
 static int
-- 
2.20.1

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


[PATCH v3 01/16] drm/dp_mst: Rename drm_dp_mst_get_validated_(port|mstb)_ref and friends

2019-01-02 Thread Lyude Paul
s/drm_dp_get_validated_port_ref/drm_dp_mst_topology_get_port_validated/
s/drm_dp_put_port/drm_dp_mst_topology_put_port/
s/drm_dp_get_validated_mstb_ref/drm_dp_mst_topology_get_mstb_validated/
s/drm_dp_put_mst_branch_device/drm_dp_mst_topology_put_mstb/

This is a much more consistent naming scheme, and will make even more
sense once we redesign how the current refcounting scheme here works.

Signed-off-by: Lyude Paul 
Reviewed-by: Daniel Vetter 
Cc: David Airlie 
Cc: Jerry Zuo 
Cc: Harry Wentland 
Cc: Juston Li 
---
 drivers/gpu/drm/drm_dp_mst_topology.c | 114 ++
 1 file changed, 62 insertions(+), 52 deletions(-)

diff --git a/drivers/gpu/drm/drm_dp_mst_topology.c 
b/drivers/gpu/drm/drm_dp_mst_topology.c
index 2ab16c9e6243..6f9b211069a7 100644
--- a/drivers/gpu/drm/drm_dp_mst_topology.c
+++ b/drivers/gpu/drm/drm_dp_mst_topology.c
@@ -46,7 +46,7 @@ static bool dump_dp_payload_table(struct 
drm_dp_mst_topology_mgr *mgr,
  char *buf);
 static int test_calc_pbn_mode(void);
 
-static void drm_dp_put_port(struct drm_dp_mst_port *port);
+static void drm_dp_mst_topology_put_port(struct drm_dp_mst_port *port);
 
 static int drm_dp_dpcd_write_payload(struct drm_dp_mst_topology_mgr *mgr,
 int id,
@@ -888,7 +888,7 @@ static void drm_dp_destroy_mst_branch_device(struct kref 
*kref)
 */
list_for_each_entry_safe(port, tmp, &mstb->ports, next) {
list_del(&port->next);
-   drm_dp_put_port(port);
+   drm_dp_mst_topology_put_port(port);
}
 
/* drop any tx slots msg */
@@ -911,7 +911,7 @@ static void drm_dp_destroy_mst_branch_device(struct kref 
*kref)
kref_put(kref, drm_dp_free_mst_branch_device);
 }
 
-static void drm_dp_put_mst_branch_device(struct drm_dp_mst_branch *mstb)
+static void drm_dp_mst_topology_put_mstb(struct drm_dp_mst_branch *mstb)
 {
kref_put(&mstb->kref, drm_dp_destroy_mst_branch_device);
 }
@@ -930,7 +930,7 @@ static void drm_dp_port_teardown_pdt(struct drm_dp_mst_port 
*port, int old_pdt)
case DP_PEER_DEVICE_MST_BRANCHING:
mstb = port->mstb;
port->mstb = NULL;
-   drm_dp_put_mst_branch_device(mstb);
+   drm_dp_mst_topology_put_mstb(mstb);
break;
}
 }
@@ -970,12 +970,14 @@ static void drm_dp_destroy_port(struct kref *kref)
kfree(port);
 }
 
-static void drm_dp_put_port(struct drm_dp_mst_port *port)
+static void drm_dp_mst_topology_put_port(struct drm_dp_mst_port *port)
 {
kref_put(&port->kref, drm_dp_destroy_port);
 }
 
-static struct drm_dp_mst_branch 
*drm_dp_mst_get_validated_mstb_ref_locked(struct drm_dp_mst_branch *mstb, 
struct drm_dp_mst_branch *to_find)
+static struct drm_dp_mst_branch *
+drm_dp_mst_topology_get_mstb_validated_locked(struct drm_dp_mst_branch *mstb,
+ struct drm_dp_mst_branch *to_find)
 {
struct drm_dp_mst_port *port;
struct drm_dp_mst_branch *rmstb;
@@ -985,7 +987,8 @@ static struct drm_dp_mst_branch 
*drm_dp_mst_get_validated_mstb_ref_locked(struct
}
list_for_each_entry(port, &mstb->ports, next) {
if (port->mstb) {
-   rmstb = 
drm_dp_mst_get_validated_mstb_ref_locked(port->mstb, to_find);
+   rmstb = drm_dp_mst_topology_get_mstb_validated_locked(
+   port->mstb, to_find);
if (rmstb)
return rmstb;
}
@@ -993,12 +996,15 @@ static struct drm_dp_mst_branch 
*drm_dp_mst_get_validated_mstb_ref_locked(struct
return NULL;
 }
 
-static struct drm_dp_mst_branch *drm_dp_get_validated_mstb_ref(struct 
drm_dp_mst_topology_mgr *mgr, struct drm_dp_mst_branch *mstb)
+static struct drm_dp_mst_branch *
+drm_dp_mst_topology_get_mstb_validated(struct drm_dp_mst_topology_mgr *mgr,
+  struct drm_dp_mst_branch *mstb)
 {
struct drm_dp_mst_branch *rmstb = NULL;
mutex_lock(&mgr->lock);
if (mgr->mst_primary)
-   rmstb = 
drm_dp_mst_get_validated_mstb_ref_locked(mgr->mst_primary, mstb);
+   rmstb = drm_dp_mst_topology_get_mstb_validated_locked(
+   mgr->mst_primary, mstb);
mutex_unlock(&mgr->lock);
return rmstb;
 }
@@ -1021,7 +1027,9 @@ static struct drm_dp_mst_port 
*drm_dp_mst_get_port_ref_locked(struct drm_dp_mst_
return NULL;
 }
 
-static struct drm_dp_mst_port *drm_dp_get_validated_port_ref(struct 
drm_dp_mst_topology_mgr *mgr, struct drm_dp_mst_port *port)
+static struct drm_dp_mst_port *
+drm_dp_mst_topology_get_port_validated(struct drm_dp_mst_topology_mgr *mgr,
+  struct drm_dp_mst_port *port)
 {
struct drm_dp_mst_port *rport = NULL;
mutex_lock(&mgr->lock);
@@ -1210,7 +1218,7 @@ static void drm_dp_add_port(struct drm

[PATCH v3 09/16] drm/nouveau: Remove unnecessary VCPI checks in nv50_msto_cleanup()

2019-01-02 Thread Lyude Paul
There is no need to look at the port's VCPI allocation before calling
drm_dp_mst_deallocate_vcpi(), as we already have msto->disabled to let
us avoid cleaning up an msto more then once. The DP MST core will never
call drm_dp_mst_deallocate_vcpi() on it's own, which is presumably what
these checks are meant to protect against.

More importantly though, we're about to stop clearing mstc->port in the
next commit, which means if we could potentially hit a use-after-free
error if we tried to check mstc->port->vcpi here. So to make life easier
for anyone who bisects this code in the future, use msto->disabled
instead to check whether or not we need to deallocate VCPI instead.

Signed-off-by: Lyude Paul 
Cc: Daniel Vetter 
Cc: David Airlie 
Cc: Jerry Zuo 
Cc: Harry Wentland 
Cc: Juston Li 
---
 drivers/gpu/drm/nouveau/dispnv50/disp.c | 15 +--
 1 file changed, 9 insertions(+), 6 deletions(-)

diff --git a/drivers/gpu/drm/nouveau/dispnv50/disp.c 
b/drivers/gpu/drm/nouveau/dispnv50/disp.c
index 641252208e67..0f7d72518604 100644
--- a/drivers/gpu/drm/nouveau/dispnv50/disp.c
+++ b/drivers/gpu/drm/nouveau/dispnv50/disp.c
@@ -704,14 +704,17 @@ nv50_msto_cleanup(struct nv50_msto *msto)
struct nv50_mstc *mstc = msto->mstc;
struct nv50_mstm *mstm = mstc->mstm;
 
+   if (!msto->disabled)
+   return;
+
NV_ATOMIC(drm, "%s: msto cleanup\n", msto->encoder.name);
-   if (mstc->port && mstc->port->vcpi.vcpi > 0 && !nv50_msto_payload(msto))
+
+   if (mstc->port)
drm_dp_mst_deallocate_vcpi(&mstm->mgr, mstc->port);
-   if (msto->disabled) {
-   msto->mstc = NULL;
-   msto->head = NULL;
-   msto->disabled = false;
-   }
+
+   msto->mstc = NULL;
+   msto->head = NULL;
+   msto->disabled = false;
 }
 
 static void
-- 
2.20.1

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


[PATCH v3 11/16] drm/nouveau: Stop unsetting mstc->port, use malloc refs

2019-01-02 Thread Lyude Paul
Same as we did for i915, but for nouveau this time. Additionally, we
grab a malloc reference to the port that lasts for the entire lifetime
of nv50_mstc, which gives us the guarantee that mstc->port will always
point to valid memory for as long as the mstc stays around.

Signed-off-by: Lyude Paul 
Cc: Daniel Vetter 
Cc: David Airlie 
Cc: Jerry Zuo 
Cc: Harry Wentland 
Cc: Juston Li 
---
 drivers/gpu/drm/nouveau/dispnv50/disp.c | 18 +-
 1 file changed, 5 insertions(+), 13 deletions(-)

diff --git a/drivers/gpu/drm/nouveau/dispnv50/disp.c 
b/drivers/gpu/drm/nouveau/dispnv50/disp.c
index 982054bbcc8b..157d208d37b5 100644
--- a/drivers/gpu/drm/nouveau/dispnv50/disp.c
+++ b/drivers/gpu/drm/nouveau/dispnv50/disp.c
@@ -709,8 +709,7 @@ nv50_msto_cleanup(struct nv50_msto *msto)
 
NV_ATOMIC(drm, "%s: msto cleanup\n", msto->encoder.name);
 
-   if (mstc->port)
-   drm_dp_mst_deallocate_vcpi(&mstm->mgr, mstc->port);
+   drm_dp_mst_deallocate_vcpi(&mstm->mgr, mstc->port);
 
msto->mstc = NULL;
msto->head = NULL;
@@ -735,7 +734,7 @@ nv50_msto_prepare(struct nv50_msto *msto)
};
 
NV_ATOMIC(drm, "%s: msto prepare\n", msto->encoder.name);
-   if (mstc->port && mstc->port->vcpi.vcpi > 0) {
+   if (mstc->port->vcpi.vcpi > 0) {
struct drm_dp_payload *payload = nv50_msto_payload(msto);
if (payload) {
args.vcpi.start_slot = payload->start_slot;
@@ -832,8 +831,7 @@ nv50_msto_disable(struct drm_encoder *encoder)
struct nv50_mstc *mstc = msto->mstc;
struct nv50_mstm *mstm = mstc->mstm;
 
-   if (mstc->port)
-   drm_dp_mst_reset_vcpi_slots(&mstm->mgr, mstc->port);
+   drm_dp_mst_reset_vcpi_slots(&mstm->mgr, mstc->port);
 
mstm->outp->update(mstm->outp, msto->head->base.index, NULL, 0, 0);
mstm->modified = true;
@@ -945,7 +943,7 @@ nv50_mstc_detect(struct drm_connector *connector, bool 
force)
enum drm_connector_status conn_status;
int ret;
 
-   if (!mstc->port)
+   if (drm_connector_is_unregistered(connector))
return connector_status_disconnected;
 
ret = pm_runtime_get_sync(connector->dev->dev);
@@ -966,8 +964,7 @@ nv50_mstc_destroy(struct drm_connector *connector)
struct nv50_mstc *mstc = nv50_mstc(connector);
 
drm_connector_cleanup(&mstc->connector);
-   if (mstc->port)
-   drm_dp_mst_put_port_malloc(mstc->port);
+   drm_dp_mst_put_port_malloc(mstc->port);
 
kfree(mstc);
 }
@@ -1081,11 +1078,6 @@ nv50_mstm_destroy_connector(struct 
drm_dp_mst_topology_mgr *mgr,
 
drm_fb_helper_remove_one_connector(&drm->fbcon->helper, 
&mstc->connector);
 
-   drm_modeset_lock(&drm->dev->mode_config.connection_mutex, NULL);
-   drm_dp_mst_put_port_malloc(mstc->port);
-   mstc->port = NULL;
-   drm_modeset_unlock(&drm->dev->mode_config.connection_mutex);
-
drm_connector_put(&mstc->connector);
 }
 
-- 
2.20.1

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


[PATCH v3 13/16] drm/dp_mst: Add some atomic state iterator macros

2019-01-02 Thread Lyude Paul
Changes since v6:
 - Move EXPORT_SYMBOL() for drm_dp_mst_topology_state_funcs to this
   commit
 - Document __drm_dp_mst_state_iter_get() and note that it shouldn't be
   called directly

Signed-off-by: Lyude Paul 
Reviewed-by: Daniel Vetter 
Cc: David Airlie 
Cc: Jerry Zuo 
Cc: Harry Wentland 
Cc: Juston Li 
---
 drivers/gpu/drm/drm_dp_mst_topology.c |  5 +-
 include/drm/drm_dp_mst_helper.h   | 96 +++
 2 files changed, 99 insertions(+), 2 deletions(-)

diff --git a/drivers/gpu/drm/drm_dp_mst_topology.c 
b/drivers/gpu/drm/drm_dp_mst_topology.c
index 769e2c0419c2..f79962759bc4 100644
--- a/drivers/gpu/drm/drm_dp_mst_topology.c
+++ b/drivers/gpu/drm/drm_dp_mst_topology.c
@@ -3527,10 +3527,11 @@ static void drm_dp_mst_destroy_state(struct 
drm_private_obj *obj,
kfree(mst_state);
 }
 
-static const struct drm_private_state_funcs mst_state_funcs = {
+const struct drm_private_state_funcs drm_dp_mst_topology_state_funcs = {
.atomic_duplicate_state = drm_dp_mst_duplicate_state,
.atomic_destroy_state = drm_dp_mst_destroy_state,
 };
+EXPORT_SYMBOL(drm_dp_mst_topology_state_funcs);
 
 /**
  * drm_atomic_get_mst_topology_state: get MST topology state
@@ -3614,7 +3615,7 @@ int drm_dp_mst_topology_mgr_init(struct 
drm_dp_mst_topology_mgr *mgr,
 
drm_atomic_private_obj_init(dev, &mgr->base,
&mst_state->base,
-   &mst_state_funcs);
+   &drm_dp_mst_topology_state_funcs);
 
return 0;
 }
diff --git a/include/drm/drm_dp_mst_helper.h b/include/drm/drm_dp_mst_helper.h
index 8eca5f29242c..581163c8d7d7 100644
--- a/include/drm/drm_dp_mst_helper.h
+++ b/include/drm/drm_dp_mst_helper.h
@@ -650,4 +650,100 @@ int drm_dp_send_power_updown_phy(struct 
drm_dp_mst_topology_mgr *mgr,
 void drm_dp_mst_get_port_malloc(struct drm_dp_mst_port *port);
 void drm_dp_mst_put_port_malloc(struct drm_dp_mst_port *port);
 
+extern const struct drm_private_state_funcs drm_dp_mst_topology_state_funcs;
+
+/**
+ * __drm_dp_mst_state_iter_get - private atomic state iterator function for
+ * macro-internal use
+ * @state: &struct drm_atomic_state pointer
+ * @mgr: pointer to the &struct drm_dp_mst_topology_mgr iteration cursor
+ * @old_state: optional pointer to the old &struct drm_dp_mst_topology_state
+ * iteration cursor
+ * @new_state: optional pointer to the new &struct drm_dp_mst_topology_state
+ * iteration cursor
+ * @i: int iteration cursor, for macro-internal use
+ *
+ * Used by for_each_oldnew_mst_mgr_in_state(),
+ * for_each_old_mst_mgr_in_state(), and for_each_new_mst_mgr_in_state(). Don't
+ * call this directly.
+ *
+ * Returns:
+ * True if the current &struct drm_private_obj is a &struct
+ * drm_dp_mst_topology_mgr, false otherwise.
+ */
+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 = &state->private_objs[i];
+
+   if (objs_state->ptr->funcs != &drm_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: &struct drm_atomic_state pointer
+ * @mgr: &struct drm_dp_mst_topology_mgr iteration cursor
+ * @old_state: &struct drm_dp_mst_topology_state iteration cursor for the old
+ * state
+ * @new_state: &struct 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: &struct drm_atomic_state pointer
+ * @mgr: &struct drm_dp_mst_topology_mgr iteration cursor
+ * @old_state: &struct 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

[PATCH v3 05/16] drm/dp_mst: Fix payload deallocation on hotplugs using malloc refs

2019-01-02 Thread Lyude Paul
Up until now, freeing payloads on remote MST hubs that just had ports
removed has almost never worked because we've been relying on port
validation in order to stop us from accessing ports that have already
been freed from memory, but ports which need their payloads released due
to being removed will never be a valid part of the topology after
they've been removed.

Since we've introduced malloc refs, we can replace all of the validation
logic in payload helpers which are used for deallocation with some
well-placed malloc krefs. This ensures that regardless of whether or not
the ports are still valid and in the topology, any port which has an
allocated payload will remain allocated in memory until it's payloads
have been removed - finally allowing us to actually release said
payloads correctly.

Signed-off-by: Lyude Paul 
Reviewed-by: Daniel Vetter 
Cc: David Airlie 
Cc: Jerry Zuo 
Cc: Harry Wentland 
Cc: Juston Li 
---
 drivers/gpu/drm/drm_dp_mst_topology.c | 54 +++
 1 file changed, 30 insertions(+), 24 deletions(-)

diff --git a/drivers/gpu/drm/drm_dp_mst_topology.c 
b/drivers/gpu/drm/drm_dp_mst_topology.c
index f10a7edb401e..769e2c0419c2 100644
--- a/drivers/gpu/drm/drm_dp_mst_topology.c
+++ b/drivers/gpu/drm/drm_dp_mst_topology.c
@@ -2102,10 +2102,6 @@ static int drm_dp_payload_send_msg(struct 
drm_dp_mst_topology_mgr *mgr,
u8 sinks[DRM_DP_MAX_SDP_STREAMS];
int i;
 
-   port = drm_dp_mst_topology_get_port_validated(mgr, port);
-   if (!port)
-   return -EINVAL;
-
port_num = port->port_num;
mstb = drm_dp_mst_topology_get_mstb_validated(mgr, port->parent);
if (!mstb) {
@@ -2113,10 +2109,8 @@ static int drm_dp_payload_send_msg(struct 
drm_dp_mst_topology_mgr *mgr,
   port->parent,
   &port_num);
 
-   if (!mstb) {
-   drm_dp_mst_topology_put_port(port);
+   if (!mstb)
return -EINVAL;
-   }
}
 
txmsg = kzalloc(sizeof(*txmsg), GFP_KERNEL);
@@ -2153,7 +2147,6 @@ static int drm_dp_payload_send_msg(struct 
drm_dp_mst_topology_mgr *mgr,
kfree(txmsg);
 fail_put:
drm_dp_mst_topology_put_mstb(mstb);
-   drm_dp_mst_topology_put_port(port);
return ret;
 }
 
@@ -2258,15 +2251,16 @@ static int drm_dp_destroy_payload_step2(struct 
drm_dp_mst_topology_mgr *mgr,
  */
 int drm_dp_update_payload_part1(struct drm_dp_mst_topology_mgr *mgr)
 {
-   int i, j;
-   int cur_slots = 1;
struct drm_dp_payload req_payload;
struct drm_dp_mst_port *port;
+   int i, j;
+   int cur_slots = 1;
 
mutex_lock(&mgr->payload_lock);
for (i = 0; i < mgr->max_payloads; i++) {
struct drm_dp_vcpi *vcpi = mgr->proposed_vcpis[i];
struct drm_dp_payload *payload = &mgr->payloads[i];
+   bool put_port = false;
 
/* solve the current payloads - compare to the hw ones
   - update the hw view */
@@ -2274,12 +2268,20 @@ int drm_dp_update_payload_part1(struct 
drm_dp_mst_topology_mgr *mgr)
if (vcpi) {
port = container_of(vcpi, struct drm_dp_mst_port,
vcpi);
-   port = drm_dp_mst_topology_get_port_validated(mgr,
- port);
-   if (!port) {
-   mutex_unlock(&mgr->payload_lock);
-   return -EINVAL;
+
+   /* Validated ports don't matter if we're releasing
+* VCPI
+*/
+   if (vcpi->num_slots) {
+   port = drm_dp_mst_topology_get_port_validated(
+   mgr, port);
+   if (!port) {
+   mutex_unlock(&mgr->payload_lock);
+   return -EINVAL;
+   }
+   put_port = true;
}
+
req_payload.num_slots = vcpi->num_slots;
req_payload.vcpi = vcpi->vcpi;
} else {
@@ -2311,7 +2313,7 @@ int drm_dp_update_payload_part1(struct 
drm_dp_mst_topology_mgr *mgr)
}
cur_slots += req_payload.num_slots;
 
-   if (port)
+   if (put_port)
drm_dp_mst_topology_put_port(port);
}
 
@@ -3126,6 +3128,8 @@ bool drm_dp_mst_allocate_vcpi(struct 
drm_dp_mst_topology_mgr *mgr,
DRM_DEBUG_KMS("initing vcpi for pbn=%d slots=%d\n",
  pbn, port->vcpi.num_slots);
 
+   /* Keep port allocated until it's payload has been removed *

[PATCH v3 14/16] drm/dp_mst: Start tracking per-port VCPI allocations

2019-01-02 Thread Lyude Paul
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 v8:
 * Fix compile errors, whoops!

Changes since v7:
 - Don't check for mixed stale/valid VCPI allocations, just rely on
 connector registration to stop such erroneous modesets

Changes since v6:
 - Keep a kref to all of the ports we have allocations on. This required
   a good bit of changing to when we call drm_dp_find_vcpi_slots(),
   mainly that we need to ensure that we only redo VCPI allocations on
   actual mode or CRTC changes, not crtc_state->active changes.
   Additionally, we no longer take the registration of the DRM connector
   for each port into account because so long as we have a kref to the
   port in the new or previous atomic state, the connector will stay
   registered.
 - Use the small changes to drm_dp_put_port() to add even more error
   checking to make misusage of the helpers more obvious. I added this
   after having to chase down various use-after-free conditions that
   started popping up from the new helpers so no one else has to
   troubleshoot that.
 - Move some accidental DRM_DEBUG_KMS() calls to DRM_DEBUG_ATOMIC()
 - Update documentation again, note that find/release() should both not be
   called on the same port in a single atomic check phase (but multiple
   calls to one or the other is OK)

Changes since v4:
 - Don't skip the atomic checks for VCPI allocations if no new VCPI
   allocations happen in a state. This makes the next change I'm about
   to list here a lot easier to implement.
 - Don't ignore VCPI allocations on destroyed ports, instead ensure that
   when ports are destroyed and still have VCPI allocations in the
   topology state, the only state changes allowed are releasing said
   ports' VCPI. This prevents a state with a mix of VCPI allocations
   from destroyed ports, and allocations from valid ports.

Changes since v3:
 - Don't release VCPI allocations in the topology state immediately in
   drm_dp_atomic_release_vcpi_slots(), instead mark them as 0 and skip
   over them in drm_dp_mst_duplicate_state(). This makes it so
   drm_dp_atomic_release_vcpi_slots() is still idempotent while also
   throwing warnings if the driver messes up it's book keeping and tries
   to release VCPI slots on a port that doesn't have any pre-existing
   VCPI allocation - danvet
 - Change mst_state/state in some debugging messages to "mst state"

Changes since v2:
 - Use kmemdup() for duplicating MST state - danvet
 - Move port validation out of duplicate state callback - danvet
 - Handle looping through MST topology states in
   drm_dp_mst_atomic_check() so the driver doesn't have to do it
 - Fix documentation in drm_dp_atomic_find_vcpi_slots()
 - Move the atomic check for each individual topology state into it's
   own function, reduces indenting
 - Don't consider "stale" MST ports when calculating the bandwidth
   requirements. This is needed because originally we relied on the
   state duplication functions to prune any stale ports from the new
   state, which would prevent us from incorrectly considering their
   bandwidth requirements alongside legitimate new payloads.
 - Add function references in drm_dp_atomic_release_vcpi_slots() - danvet
 - Annotate atomic VCPI and atomic check functions with __must_check
   - danvet

Changes since 

[PATCH v3 16/16] drm/nouveau: Use atomic VCPI helpers for MST

2019-01-02 Thread Lyude Paul
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.

Changes since v5:
 - Update nv50_msto_atomic_check() and nv50_mstc_atomic_check() to the
   new requirements for drm_dp_atomic_find_vcpi_slots() and
   drm_dp_atomic_release_vcpi_slots()

Signed-off-by: Lyude Paul 
Cc: Daniel Vetter 
Cc: David Airlie 
Cc: Jerry Zuo 
Cc: Harry Wentland 
Cc: Juston Li 
---
 drivers/gpu/drm/nouveau/dispnv50/disp.c | 52 ++---
 1 file changed, 46 insertions(+), 6 deletions(-)

diff --git a/drivers/gpu/drm/nouveau/dispnv50/disp.c 
b/drivers/gpu/drm/nouveau/dispnv50/disp.c
index 67f7bf97e5d9..df696008d205 100644
--- a/drivers/gpu/drm/nouveau/dispnv50/disp.c
+++ b/drivers/gpu/drm/nouveau/dispnv50/disp.c
@@ -762,16 +762,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);
+   mstc->pbn = drm_dp_calc_pbn_mode(crtc_state->adjusted_mode.clock,
+bpp);
 
-   slots = drm_dp_find_vcpi_slots(&mstm->mgr, mstc->pbn);
-   if (slots < 0)
-   return slots;
+   if (crtc_state->connectors_changed || crtc_state->mode_changed) {
+   slots = drm_dp_atomic_find_vcpi_slots(state, &mstm->mgr,
+ mstc->port, mstc->pbn);
+   if (slots < 0)
+   return slots;
+   }
 
return nv50_outp_atomic_check_view(encoder, crtc_state, conn_state,
   mstc->native);
@@ -934,12 +940,42 @@ 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 = &mstc->mstm->mgr;
+   struct drm_connector_state *old_conn_state =
+   drm_atomic_get_old_connector_state(state, connector);
+   struct drm_crtc_state *old_crtc_state;
+   struct drm_crtc *new_crtc = new_conn_state->crtc,
+   *old_crtc = old_conn_state->crtc;
+
+   if (!old_crtc)
+   return 0;
+
+   old_crtc_state = drm_atomic_get_old_crtc_state(state, old_crtc);
+   if (!old_crtc_state || !old_crtc_state->enable)
+   return 0;
+
+   if (new_crtc)
+   return 0;
+
+   /* This connector will be left without an enabled CRTC, so its VCPI
+* must be released here
+*/
+   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
@@ -2121,6 +2157,10 @@ nv50_disp_atomic_check(struct drm_device *dev, struct 
drm_atomic_state *state)
return ret;
}
 
+   ret = drm_dp_mst_atomic_check(state);
+   if (ret)
+   return ret;
+
return 0;
 }
 
-- 
2.20.1

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


[PATCH v3 10/16] drm/nouveau: Keep malloc references to MST ports

2019-01-02 Thread Lyude Paul
Now that we finally have a sane way to keep port allocations around, use
it to fix the potential unchecked ->port accesses that nouveau makes by
making sure we keep the mst port allocated for as long as it's
drm_connector is accessible.

Additionally, now that we've guaranteed that mstc->port is allocated for
as long as we keep mstc around we can remove the connector registration
checks for codepaths which release payloads, allowing us to release
payloads on active topologies properly. These registration checks were
only required before in order to avoid situations where mstc->port could
technically be pointing at freed memory.

Signed-off-by: Lyude Paul 
Cc: Daniel Vetter 
Cc: David Airlie 
Cc: Jerry Zuo 
Cc: Harry Wentland 
Cc: Juston Li 
---
 drivers/gpu/drm/nouveau/dispnv50/disp.c | 6 ++
 1 file changed, 6 insertions(+)

diff --git a/drivers/gpu/drm/nouveau/dispnv50/disp.c 
b/drivers/gpu/drm/nouveau/dispnv50/disp.c
index 0f7d72518604..982054bbcc8b 100644
--- a/drivers/gpu/drm/nouveau/dispnv50/disp.c
+++ b/drivers/gpu/drm/nouveau/dispnv50/disp.c
@@ -964,7 +964,11 @@ static void
 nv50_mstc_destroy(struct drm_connector *connector)
 {
struct nv50_mstc *mstc = nv50_mstc(connector);
+
drm_connector_cleanup(&mstc->connector);
+   if (mstc->port)
+   drm_dp_mst_put_port_malloc(mstc->port);
+
kfree(mstc);
 }
 
@@ -1012,6 +1016,7 @@ nv50_mstc_new(struct nv50_mstm *mstm, struct 
drm_dp_mst_port *port,
drm_object_attach_property(&mstc->connector.base, 
dev->mode_config.path_property, 0);
drm_object_attach_property(&mstc->connector.base, 
dev->mode_config.tile_property, 0);
drm_connector_set_path_property(&mstc->connector, path);
+   drm_dp_mst_get_port_malloc(port);
return 0;
 }
 
@@ -1077,6 +1082,7 @@ nv50_mstm_destroy_connector(struct 
drm_dp_mst_topology_mgr *mgr,
drm_fb_helper_remove_one_connector(&drm->fbcon->helper, 
&mstc->connector);
 
drm_modeset_lock(&drm->dev->mode_config.connection_mutex, NULL);
+   drm_dp_mst_put_port_malloc(mstc->port);
mstc->port = NULL;
drm_modeset_unlock(&drm->dev->mode_config.connection_mutex);
 
-- 
2.20.1

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


[PATCH v3 08/16] drm/nouveau: Remove bogus cleanup in nv50_mstm_add_connector()

2019-01-02 Thread Lyude Paul
Trying to destroy the connector using mstc->connector.funcs->destroy()
if connector initialization fails is wrong: there is no possible
codepath in nv50_mstc_new where nv50_mstm_add_connector() would return
<0 and mstc would be non-NULL.

Signed-off-by: Lyude Paul 
Cc: Daniel Vetter 
Cc: David Airlie 
Cc: Jerry Zuo 
Cc: Harry Wentland 
Cc: Juston Li 
---
 drivers/gpu/drm/nouveau/dispnv50/disp.c | 5 +
 1 file changed, 1 insertion(+), 4 deletions(-)

diff --git a/drivers/gpu/drm/nouveau/dispnv50/disp.c 
b/drivers/gpu/drm/nouveau/dispnv50/disp.c
index 26af45785939..641252208e67 100644
--- a/drivers/gpu/drm/nouveau/dispnv50/disp.c
+++ b/drivers/gpu/drm/nouveau/dispnv50/disp.c
@@ -1099,11 +1099,8 @@ nv50_mstm_add_connector(struct drm_dp_mst_topology_mgr 
*mgr,
int ret;
 
ret = nv50_mstc_new(mstm, port, path, &mstc);
-   if (ret) {
-   if (mstc)
-   mstc->connector.funcs->destroy(&mstc->connector);
+   if (ret)
return NULL;
-   }
 
return &mstc->connector;
 }
-- 
2.20.1

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


[PATCH v3 15/16] drm/dp_mst: Check payload count in drm_dp_mst_atomic_check()

2019-01-02 Thread Lyude Paul
It occurred to me that we never actually check this! So let's start
doing that.

Signed-off-by: Lyude Paul 
Reviewed-by: Daniel Vetter 
Cc: David Airlie 
Cc: Jerry Zuo 
Cc: Harry Wentland 
Cc: Juston Li 
---
 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 c33c4a3aec34..fc9bcbb55417 100644
--- a/drivers/gpu/drm/drm_dp_mst_topology.c
+++ b/drivers/gpu/drm/drm_dp_mst_topology.c
@@ -3647,7 +3647,7 @@ drm_dp_mst_atomic_check_topology_state(struct 
drm_dp_mst_topology_mgr *mgr,
   struct drm_dp_mst_topology_state 
*mst_state)
 {
struct drm_dp_vcpi_allocation *vcpi;
-   int avail_slots = 63;
+   int avail_slots = 63, payload_count = 0;
 
list_for_each_entry(vcpi, &mst_state->vcpis, next) {
/* Releasing VCPI is always OK-even if the port is gone */
@@ -3667,6 +3667,12 @@ drm_dp_mst_atomic_check_topology_state(struct 
drm_dp_mst_topology_mgr *mgr,
 avail_slots + vcpi->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, mst_state, mgr->max_payloads);
+   return -EINVAL;
+   }
}
DRM_DEBUG_ATOMIC("[MST MGR:%p] mst state %p VCPI avail=%d used=%d\n",
 mgr, mst_state, avail_slots,
-- 
2.20.1

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


[PATCH v3 07/16] drm/amdgpu/display: Keep malloc ref to MST port

2019-01-02 Thread Lyude Paul
Just like i915 and nouveau, it's a good idea for us to hold a malloc
reference to the port here so that we never pass a freed pointer to any
of the DP MST helper functions.

Also, we stop unsetting aconnector->port in
dm_dp_destroy_mst_connector(). There's literally no point to that
assignment that I can see anyway.

Signed-off-by: Lyude Paul 
Cc: Daniel Vetter 
Cc: David Airlie 
Cc: Jerry Zuo 
Cc: Harry Wentland 
Cc: Juston Li 
---
 .../drm/amd/display/amdgpu_dm/amdgpu_dm_mst_types.c   | 11 +++
 1 file changed, 7 insertions(+), 4 deletions(-)

diff --git a/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm_mst_types.c 
b/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm_mst_types.c
index 5e7ca1f3a8d1..24632727e127 100644
--- a/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm_mst_types.c
+++ b/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm_mst_types.c
@@ -191,6 +191,7 @@ dm_dp_mst_connector_destroy(struct drm_connector *connector)
drm_encoder_cleanup(&amdgpu_encoder->base);
kfree(amdgpu_encoder);
drm_connector_cleanup(connector);
+   drm_dp_mst_put_port_malloc(amdgpu_dm_connector->port);
kfree(amdgpu_dm_connector);
 }
 
@@ -363,7 +364,9 @@ dm_dp_add_mst_connector(struct drm_dp_mst_topology_mgr *mgr,
amdgpu_dm_connector_funcs_reset(connector);
 
DRM_INFO("DM_MST: added connector: %p [id: %d] [master: %p]\n",
-   aconnector, connector->base.id, aconnector->mst_port);
+aconnector, connector->base.id, aconnector->mst_port);
+
+   drm_dp_mst_get_port_malloc(port);
 
DRM_DEBUG_KMS(":%d\n", connector->base.id);
 
@@ -379,12 +382,12 @@ static void dm_dp_destroy_mst_connector(struct 
drm_dp_mst_topology_mgr *mgr,
struct amdgpu_dm_connector *aconnector = 
to_amdgpu_dm_connector(connector);
 
DRM_INFO("DM_MST: Disabling connector: %p [id: %d] [master: %p]\n",
-   aconnector, connector->base.id, 
aconnector->mst_port);
+aconnector, connector->base.id, aconnector->mst_port);
 
-   aconnector->port = NULL;
if (aconnector->dc_sink) {
amdgpu_dm_update_freesync_caps(connector, NULL);
-   dc_link_remove_remote_sink(aconnector->dc_link, 
aconnector->dc_sink);
+   dc_link_remove_remote_sink(aconnector->dc_link,
+  aconnector->dc_sink);
dc_sink_release(aconnector->dc_sink);
aconnector->dc_sink = NULL;
}
-- 
2.20.1

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


[Bug 108992] Regression: Lenovo e585 (ryzen 2500u) freezes during boot with 4.20-rc5/rc6, amdgpu error

2019-01-02 Thread bugzilla-daemon
https://bugs.freedesktop.org/show_bug.cgi?id=108992

--- Comment #21 from Zheng Luo  ---
With iommu=soft I still occasionally experience frozen screen with following
logs:

Jan 02 16:11:18 lzThinkpad gnome-shell[1647]: Failed to flip: Cannot allocate
memory
Jan 02 16:11:18 lzThinkpad kernel: amdgpu :05:00.0: a2e0b642 pin
failed
Jan 02 16:11:18 lzThinkpad kernel: [drm:dm_plane_helper_prepare_fb [amdgpu]]
*ERROR* Failed to pin framebuffer with error -12

-- 
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 109177] Blender 2.8 triggers GPU lockup when entering Edit Mode

2019-01-02 Thread bugzilla-daemon
https://bugs.freedesktop.org/show_bug.cgi?id=109177

--- Comment #4 from MirceaKitsune  ---
Created attachment 142947
  --> https://bugs.freedesktop.org/attachment.cgi?id=142947&action=edit
dmesg.txt

-- 
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 109177] Blender 2.8 triggers GPU lockup when entering Edit Mode

2019-01-02 Thread bugzilla-daemon
https://bugs.freedesktop.org/show_bug.cgi?id=109177

--- Comment #5 from MirceaKitsune  ---
Created attachment 142948
  --> https://bugs.freedesktop.org/attachment.cgi?id=142948&action=edit
Xorg.0.log

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


[Bug 109177] Blender 2.8 triggers GPU lockup when entering Edit Mode

2019-01-02 Thread bugzilla-daemon
https://bugs.freedesktop.org/show_bug.cgi?id=109177

--- Comment #6 from MirceaKitsune  ---
Created attachment 142949
  --> https://bugs.freedesktop.org/attachment.cgi?id=142949&action=edit
Xorg.0.log.old

-- 
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 109171] AMD's GPU is Slower than Intel's

2019-01-02 Thread bugzilla-daemon
https://bugs.freedesktop.org/show_bug.cgi?id=109171

--- Comment #5 from Sherif  ---
Created attachment 142951
  --> https://bugs.freedesktop.org/attachment.cgi?id=142951&action=edit
I used something like dmesg | vim.

-- 
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 109171] AMD's GPU is Slower than Intel's

2019-01-02 Thread bugzilla-daemon
https://bugs.freedesktop.org/show_bug.cgi?id=109171

--- Comment #6 from Sherif  ---
Sorry, I didn't pay attention to the dmesg part.

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