[Bug 105018] Kernel panic when waking up after screen goes blank.
https://bugs.freedesktop.org/show_bug.cgi?id=105018 --- Comment #37 from L.S.S. --- Attempted to lock the screen and leave it blank for a while to see what happens, and it seems for the first time there are still errors related to VBLANK, but they appear minor as the system woke up just fine. I tried letting the screen blank twice this time, and the errors did not show up the second time. Only during first time did the errors appear, though this is probably not enough to prove much. But still, I don't see that kernel bug this time so that must have been fixed. I'm yet to assess whether the bug can no longer be reproduced as I've been avoiding having to leave the screen blank, as back then this bug has caused losses of unsaved work and other problems due to the crash. Jul 05 13:35:42 linuxsys kernel: [drm:dm_logger_write [amdgpu]] *ERROR* Failed to get VBLANK! Jul 05 13:35:55 linuxsys blueman-mechanism[2281]: Exiting Jul 05 13:36:02 linuxsys kernel: [drm:dm_logger_write [amdgpu]] *ERROR* Failed to get VBLANK! Jul 05 13:36:22 linuxsys kernel: [drm:dm_vblank_get_counter [amdgpu]] *ERROR* dc_stream_state is NULL for crtc '1'! Jul 05 13:36:22 linuxsys kernel: [drm:dm_crtc_get_scanoutpos [amdgpu]] *ERROR* dc_stream_state is NULL for crtc '1'! Jul 05 13:36:22 linuxsys kernel: [drm:dm_vblank_get_counter [amdgpu]] *ERROR* dc_stream_state is NULL for crtc '1'! Jul 05 13:36:22 linuxsys kernel: WARNING: CPU: 14 PID: 1998 at drivers/gpu/drm/drm_vblank.c:620 drm_calc_vbltimestamp_from_scanoutpos+0x2af/0x2f0 [drm] Jul 05 13:36:22 linuxsys kernel: Modules linked in: cmac rfcomm fuse bnep vmnet(O) nls_iso8859_1 nls_cp437 vfat fat arc4 amdkfd amd_iommu_v2 amdgpu iwlmvm chash gpu_sched i2c_algo_bit ttm mac80211 drm_kms_helper btusb uvcvideo btrtl iwlwif> Jul 05 13:36:22 linuxsys kernel: glue_helper pcspkr k10temp i2c_piix4 rtc_cmos shpchp tpm_tis_core battery ac tpm wmi rng_core asus_wireless gpio_amdpt i2c_hid pinctrl_amd evdev mac_hid acpi_cpufreq vmmon(O) vmw_vmci vboxnetflt(O) vboxnet> Jul 05 13:36:22 linuxsys kernel: CPU: 14 PID: 1998 Comm: xfwm4 Tainted: G O 4.17.3-1-MANJARO #1 Jul 05 13:36:22 linuxsys kernel: Hardware name: ASUSTeK COMPUTER INC. GL702ZC/GL702ZC, BIOS GL702ZC.303 12/15/2017 Jul 05 13:36:22 linuxsys kernel: RIP: 0010:drm_calc_vbltimestamp_from_scanoutpos+0x2af/0x2f0 [drm] Jul 05 13:36:22 linuxsys kernel: RSP: 0018:9bae0b6d7b38 EFLAGS: 00010082 Jul 05 13:36:22 linuxsys kernel: RAX: c114e400 RBX: 8f4ff7840800 RCX: Jul 05 13:36:22 linuxsys kernel: RDX: 0001 RSI: c0e128a0 RDI: 0001 Jul 05 13:36:22 linuxsys kernel: RBP: 9bae0b6d7b98 R08: R09: c0df1770 Jul 05 13:36:22 linuxsys kernel: R10: R11: c0f814d0 R12: 0001 Jul 05 13:36:22 linuxsys kernel: R13: 8f4fe8eb01d8 R14: 8f4fe8eb R15: 9bae0b6d7bac Jul 05 13:36:22 linuxsys kernel: FS: 7f4c76bcdfc0() GS:8f4ffe98() knlGS: Jul 05 13:36:22 linuxsys kernel: CS: 0010 DS: ES: CR0: 80050033 Jul 05 13:36:22 linuxsys kernel: CR2: 7f8de800bf98 CR3: 0007520fc000 CR4: 003406e0 Jul 05 13:36:22 linuxsys kernel: Call Trace: Jul 05 13:36:22 linuxsys kernel: drm_get_last_vbltimestamp+0x78/0x90 [drm] Jul 05 13:36:22 linuxsys kernel: drm_update_vblank_count+0x79/0x230 [drm] Jul 05 13:36:22 linuxsys kernel: drm_vblank_enable+0x101/0x120 [drm] Jul 05 13:36:22 linuxsys kernel: drm_vblank_get+0x8d/0xb0 [drm] Jul 05 13:36:22 linuxsys kernel: drm_wait_vblank_ioctl+0x138/0x630 [drm] Jul 05 13:36:22 linuxsys kernel: ? import_iovec+0x37/0xd0 Jul 05 13:36:22 linuxsys kernel: ? drm_legacy_modeset_ctl_ioctl+0x100/0x100 [drm] Jul 05 13:36:22 linuxsys kernel: drm_ioctl_kernel+0x5b/0xb0 [drm] Jul 05 13:36:22 linuxsys kernel: drm_ioctl+0x1b7/0x370 [drm] Jul 05 13:36:22 linuxsys kernel: ? drm_legacy_modeset_ctl_ioctl+0x100/0x100 [drm] Jul 05 13:36:22 linuxsys kernel: ? do_iter_write+0xdc/0x190 Jul 05 13:36:22 linuxsys kernel: amdgpu_drm_ioctl+0x49/0x80 [amdgpu] Jul 05 13:36:22 linuxsys kernel: do_vfs_ioctl+0xa4/0x610 Jul 05 13:36:22 linuxsys kernel: ? __sys_recvmsg+0x83/0xa0 Jul 05 13:36:22 linuxsys kernel: ksys_ioctl+0x60/0x90 Jul 05 13:36:22 linuxsys kernel: __x64_sys_ioctl+0x16/0x20 Jul 05 13:36:22 linuxsys kernel: do_syscall_64+0x5b/0x170 Jul 05 13:36:22 linuxsys kernel: entry_SYSCALL_64_after_hwframe+0x44/0xa9 Jul 05 13:36:22 linuxsys kernel: RIP: 0033:0x7f4c731df667 Jul 05 13:36:22 linuxsys kernel: RSP: 002b:7ffcd9a51ae8 EFLAGS: 0246 ORIG_RAX: 0010 Jul 05 13:36:22 linuxsys kernel: RAX: ffda RBX: 7ffcd9a51b10 RCX: 7f4c731df667 Jul 05 13:36:22 linuxsys kernel: RDX: 7ffcd9a51b10 RSI: c018643a RDI: 000c Jul 05 13:36:22 linuxsys kernel: RBP: 017bde80 R08: 01000109 R09: Jul 05 13:36:22 linuxsys kernel: R10: R11: 0246 R12:
[PATCH V2] drm/vkms: Add vblank events simulated by hrtimers
This commit adds regular vblank events simulated through hrtimers, which
is a feature required by VKMS to mimic real hardware. Notice that
keeping the periodicity as close as possible to the one specified in
user space represents a vital constraint. In this sense, the callback
function implements an algorithm based on module operations that avoids
the accumulation of errors.
Assuming we begin operating at timestamp 0, every event should happen at
timestamps that are multiples of the requested period, i.e.,
current_timestamp % period == 0. Since we do *not* generally start at
timestamp 0, we calculate the modulus of this initial timestamp and try
to make all following events happen when current_timestamp % period ==
initial_timestamp % period (variables “current_offset” and
“base_offset”). We do this by measuring the difference between them at
each iteration and adjusting the next waiting period accordingly.
Signed-off-by: Rodrigo Siqueira
---
drivers/gpu/drm/vkms/vkms_crtc.c | 99 +++-
drivers/gpu/drm/vkms/vkms_drv.c | 9 +++
drivers/gpu/drm/vkms/vkms_drv.h | 20 +++
3 files changed, 126 insertions(+), 2 deletions(-)
diff --git a/drivers/gpu/drm/vkms/vkms_crtc.c b/drivers/gpu/drm/vkms/vkms_crtc.c
index 84cc05506b09..35d39d25df34 100644
--- a/drivers/gpu/drm/vkms/vkms_crtc.c
+++ b/drivers/gpu/drm/vkms/vkms_crtc.c
@@ -10,6 +10,83 @@
#include
#include
+static enum hrtimer_restart vkms_vblank_simulate(struct hrtimer *timer)
+{
+ struct vkms_output *output = container_of(timer, struct vkms_output,
+ vblank_hrtimer);
+ struct drm_crtc *crtc = >crtc;
+ ktime_t new_period, current_timestamp, diff_times, current_offset,
+ candidate_expires;
+ unsigned long flags;
+ int ret_overrun;
+ bool ret;
+
+ current_timestamp = ktime_get();
+ current_offset = current_timestamp % output->period_ns;
+ diff_times = ktime_sub(current_offset, output->base_offset);
+ candidate_expires = ktime_sub(current_timestamp, diff_times);
+ if (candidate_expires > output->expires)
+ output->expires = candidate_expires;
+
+ ret = drm_crtc_handle_vblank(crtc);
+ if (!ret)
+ DRM_ERROR("vkms failure on handling vblank");
+
+ if (output->event) {
+ spin_lock_irqsave(>dev->event_lock, flags);
+ drm_crtc_send_vblank_event(crtc, output->event);
+ output->event = NULL;
+ spin_unlock_irqrestore(>dev->event_lock, flags);
+ drm_crtc_vblank_put(crtc);
+ }
+
+ current_timestamp = ktime_get();
+ current_offset = current_timestamp % output->period_ns;
+ diff_times = ktime_sub(output->base_offset, current_offset);
+ new_period = ktime_add(output->period_ns, diff_times);
+ output->expires = ktime_add(current_timestamp, new_period);
+ ret_overrun = hrtimer_forward_now(>vblank_hrtimer, new_period);
+
+ return HRTIMER_RESTART;
+}
+
+static int vkms_enable_vblank(struct drm_crtc *crtc)
+{
+ struct vkms_output *out = drm_crtc_to_vkms_output(crtc);
+ unsigned long period = 1000 * crtc->mode.htotal * crtc->mode.vtotal /
+ crtc->mode.clock;
+ ktime_t current_timestamp;
+
+ hrtimer_init(>vblank_hrtimer, CLOCK_MONOTONIC, HRTIMER_MODE_REL);
+ out->vblank_hrtimer.function = _vblank_simulate;
+ out->period_ns = ktime_set(0, period * US_TO_NS);
+ current_timestamp = ktime_get();
+ out->base_offset = current_timestamp % out->period_ns;
+ out->expires = ktime_add(current_timestamp, out->period_ns);
+ hrtimer_start(>vblank_hrtimer, out->period_ns, HRTIMER_MODE_REL);
+
+ return 0;
+}
+
+static void vkms_disable_vblank(struct drm_crtc *crtc)
+{
+ struct vkms_output *out = drm_crtc_to_vkms_output(crtc);
+
+ hrtimer_cancel(>vblank_hrtimer);
+}
+
+bool vkms_get_vblank_timestamp(struct drm_device *dev, unsigned int pipe,
+ int *max_error, ktime_t *vblank_time,
+ bool in_vblank_irq)
+{
+ struct vkms_device *vkmsdev = drm_device_to_vkms_device(dev);
+ struct vkms_output *output = >output;
+
+ *vblank_time = output->expires;
+
+ return true;
+}
+
static const struct drm_crtc_funcs vkms_crtc_funcs = {
.set_config = drm_atomic_helper_set_config,
.destroy= drm_crtc_cleanup,
@@ -17,6 +94,8 @@ static const struct drm_crtc_funcs vkms_crtc_funcs = {
.reset = drm_atomic_helper_crtc_reset,
.atomic_duplicate_state = drm_atomic_helper_crtc_duplicate_state,
.atomic_destroy_state = drm_atomic_helper_crtc_destroy_state,
+ .enable_vblank = vkms_enable_vblank,
+ .disable_vblank = vkms_disable_vblank,
};
static int vkms_crtc_atomic_check(struct drm_crtc *crtc,
@@
[PULL] drm-misc-next
Hi Dave, Nothing really big just a bunch improvements and a few fixes for Core and Drivers. We have a cross subsystem merge here for fbcon. Regards, Gustavo drm-misc-next-2018-07-04: drm-misc-next for 4.19: UAPI Changes: v3d: add fourcc modicfier for fourcc for the Broadcom UIF format (Eric Anholt) Cross-subsystem Changes: console/fbcon: Add support for deferred console takeover (Hans de Goede) Core Changes: dma-fence clean up, improvements and docs (Daniel Vetter) add mask function for crtc, plane, encoder and connector DRM objects(Ville Syrjälä) Driver Changes: pl111: add Nomadik LCDC variant (Linus Walleij) The following changes since commit eab976693153b9854bfa83d131374748f6ca4280: Merge tag 'drm-misc-next-2018-06-27' of git://anongit.freedesktop.org/drm/drm-misc into drm-next (2018-06-28 13:29:07 +1000) are available in the Git repository at: git://anongit.freedesktop.org/drm/drm-misc tags/drm-misc-next-2018-07-04 for you to fetch changes up to 968d72e6a5105a18fe17c0a8b4ef2951d0eb42dd: drm/savage: off by one in savage_bci_cmdbuf() (2018-07-04 14:27:01 +0200) drm-misc-next for 4.19: UAPI Changes: v3d: add fourcc modicfier for fourcc for the Broadcom UIF format (Eric Anholt) Cross-subsystem Changes: console/fbcon: Add support for deferred console takeover (Hans de Goede) Core Changes: dma-fence clean up, improvements and docs (Daniel Vetter) add mask function for crtc, plane, encoder and connector DRM objects(Ville Syrjälä) Driver Changes: pl111: add Nomadik LCDC variant (Linus Walleij) Dan Carpenter (3): drm/i810: off by one in i810_dma_vertex() drm/vgem: off by one in vgem_gem_fault() drm/savage: off by one in savage_bci_cmdbuf() Daniel Vetter (12): dma-fence: remove fill_driver_data callback dma-fence: Make ->enable_signaling optional dma-fence: Allow wait_any_timeout for all fences drm: Fix hdmi connector content type property docs dma-fence: Make ->wait callback optional drm/amdgpu: Remove unecessary dma_fence_ops drm: Remove unecessary dma_fence_ops drm/etnaviv: Remove unecessary dma_fence_ops drm/qxl: Remove unecessary dma_fence_ops drm/vc4: Remove unecessary dma_fence_ops drm/virtio: Remove unecessary dma_fence_ops dma-fence: Polish kernel-doc for dma-fence.c Dirk Hohndel (VMware) (5): drm: add SPDX idenitifier and clarify license drm: add SPDX identifier and clarify license drm/noveau: add SPDX identifier and clarify license drm/vmwgfx: add SPDX idenitifier and clarify license drm/vmwgfx: add SPDX idenitifier and clarify license Eric Anholt (2): drm/v3d: Define the fourcc modifier for the Broadcom UIF format. drm/vc4: Make DSI call into the bridge after the DSI link is enabled. Gustavo Padovan (2): Merge tag 'ib-fbdev-drm-v4.19-deferred-console-takeover' of https://github.com/bzolnier/linux into drm-misc-next Merge tag 'ib-fbdev-drm-v4.19-deferred-console-takeover-fixup' of https://github.com/bzolnier/linux into drm-misc-next Hans de Goede (4): printk: Export is_console_locked fbcon: Call WARN_CONSOLE_UNLOCKED() where applicable console/fbcon: Add support for deferred console takeover console: dummycon: export dummycon_[un]register_output_notifier Linus Walleij (1): drm/pl111: Support Nomadik LCDC variant Russell King (1): drm: add missing ctx argument to plane transitional helpers Ville Syrjälä (10): drm/atomic-helper: Use old/new state in drm_atomic_helper_commit_planes_on_crtc() drm: Add drm_plane_mask() drm: Use drm_crtc_mask() drm: Add drm_encoder_mask() drm: Add drm_connector_mask() drm/i915: Use drm_plane_mask() & co. drm/imx: Use drm_plane_mask() drm/sun4i: Use drm_crtc_mask() drm/vc4: Use drm_crtc_mask() drm/vmwgfx: Use drm_plane_mask() & co. Documentation/admin-guide/pm/intel_pstate.rst | 2 +- Documentation/core-api/kernel-api.rst | 2 +- Documentation/driver-api/dma-buf.rst | 6 + Documentation/driver-api/infrastructure.rst| 4 +- Documentation/fb/fbcon.txt | 7 + Documentation/filesystems/cifs/AUTHORS | 7 +- Documentation/filesystems/cifs/CHANGES | 3 + Documentation/filesystems/cifs/TODO| 17 +- Documentation/gpu/drm-kms.rst | 2 +- Documentation/trace/histogram.txt | 23 +- Documentation/virtual/kvm/api.txt | 2 +- MAINTAINERS| 24 +- Makefile | 2 +- arch/alpha/Kconfig | 5 - arch/alpha/lib/Makefile| 2 - arch/alpha/lib/dec_and_lock.c
[Bug 102322] System crashes after "[drm] IP block:gmc_v8_0 is hung!" / [drm] IP block:sdma_v3_0 is hung!
https://bugs.freedesktop.org/show_bug.cgi?id=102322 --- Comment #23 from dwagner --- Just for the record: At this point, I can say that with amggpu.vm_update_mode=3 4.17.2-ARCH runs at least for hours, not only the minutes it runs without this option before crashing. I cannot, however, say that above combination reaches the some-days-between-amdgpu-crashes uptimes that 4.13.x reached - in order to be able to test this, I would need S3 resumes to work, which is subject to bug report 107065. Without working S3 resumes, there is no way for me to test longer uptimes because amdgpu consistently crashes (in any version I know of) if I just let the system run but switch off the display, and I do not want to keep the connected 4k TV switched on all day and night. -- 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 99528] Wine game doesn't redraw properly in fullscreen
https://bugs.freedesktop.org/show_bug.cgi?id=99528 Timothy Arceri changed: What|Removed |Added Status|NEW |NEEDINFO --- Comment #8 from Timothy Arceri --- Is this still an issue? -- 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 107065] "BUG: unable to handle kernel paging request at 0000000000002000" in amdgpu_vm_cpu_set_ptes at amdgpu_vm.c:921
https://bugs.freedesktop.org/show_bug.cgi?id=107065 --- Comment #16 from dwagner --- (In reply to Andrey Grodzovsky from comment #15) > We have only minor differences but I can't reproduce it. Maybe the resume > failure is indeed due the eviction failure during suspend. Is S3 failure is > happening only when you switch to CPU update mode ? No, when I boot amd-staging-drm-next with amdgpu.vm_update_mode=0 and suspend to S3 then resuming does also crash, but with different messages - _not_ with "BUG: unable to handle kernel paging request at 2000" like in the vm_update_mode=3 case. In the journal, I can see see after a vm_update_mode=0 S3 resume attempt: Jul 05 00:41:59 ryzen kernel: [TTM] Buffer eviction failed Jul 05 00:41:59 ryzen kernel: ACPI: Preparing to enter system sleep state S3 ... Jul 05 00:42:00 ryzen kernel: [drm:gfx_v8_0_ring_test_ring [amdgpu]] *ERROR* amdgpu: ring 0 test failed (scratch(0xC040)=0xC> Jul 05 00:42:00 ryzen kernel: [drm:amdgpu_device_ip_resume_phase2 [amdgpu]] *ERROR* resume of IP block failed -22 Jul 05 00:42:00 ryzen kernel: [drm:amdgpu_device_resume [amdgpu]] *ERROR* amdgpu_device_ip_resume failed (-22). Jul 05 00:42:00 ryzen kernel: dpm_run_callback(): pci_pm_resume+0x0/0xa0 returns -22 Jul 05 00:42:00 ryzen kernel: PM: Device :0a:00.0 failed to resume async: error -22 ... Jul 05 00:42:00 ryzen kernel: amdgpu :0a:00.0: couldn't schedule ib on ring Jul 05 00:42:00 ryzen kernel: [drm:amdgpu_job_run [amdgpu]] *ERROR* Error scheduling IBs (-22) Jul 05 00:42:00 ryzen kernel: amdgpu :0a:00.0: couldn't schedule ib on ring Jul 05 00:42:00 ryzen kernel: [drm:amdgpu_job_run [amdgpu]] *ERROR* Error scheduling IBs (-22) Jul 05 00:42:00 ryzen kernel: amdgpu :0a:00.0: couldn't schedule ib on ring Jul 05 00:42:00 ryzen kernel: [drm:amdgpu_job_run [amdgpu]] *ERROR* Error scheduling IBs (-22) Jul 05 00:42:00 ryzen kernel: amdgpu :0a:00.0: couldn't schedule ib on ring ... many more of this... but no kernel BUG or Oops. -- 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 107115] Make darktable OpenCL support work on Clover and RadeonSI
https://bugs.freedesktop.org/show_bug.cgi?id=107115 Jan Vesely changed: What|Removed |Added Depends on||87738 --- Comment #2 from Jan Vesely --- This sounds like darktable needs image support. Referenced Bugs: https://bugs.freedesktop.org/show_bug.cgi?id=87738 [Bug 87738] [OpenCL] Please add Image support -- 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 87738] [OpenCL] Please add Image support
https://bugs.freedesktop.org/show_bug.cgi?id=87738 Jan Vesely changed: What|Removed |Added Blocks||107115 Referenced Bugs: https://bugs.freedesktop.org/show_bug.cgi?id=107115 [Bug 107115] Make darktable OpenCL support work on Clover and RadeonSI -- 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
Proposal to merge KFD into amdgpu
Since KFD is only supported by a single GPU driver now (amdgpu), it makes sense to merge the two. This has been raised on the amd-gfx list before and I've been putting it off to avoid more churn while I was working on upstreaming KFD. Now seems a good time to pick this up again. At this stage there are some things that I don't expect to change significantly: * Directory structure * KFD function naming conventions * KFD device and sysfs interfaces This is a rough overview of the changes I have in mind. We should be able to implement these step-by-step and minimize disruption: 1. Change the build system to build KFD into amdgpu.ko This should make KFD similar to DAL or powerplay. It's still a mostly separate code base and Makefile with its own directory, but gets linked into amdgpu.ko. In the kernel configuration HSA_AMD would become a boolean option under DRM_AMDGPU that controls whether KFD functionality gets built into amdgpu. Any code inside #if defined(CONFIG_HSA_AMD_MODULE) can be removed. 2. Simplify the kfd2kgd and kgd2kfg interfaces Function pointers in struct kgd2kfd_calls are no longer needed. These functions can be called directly from amdgpu. Hardware-independent function pointers in kfd2kgd_calls are no longer needed. These function can be called directly from amdkfd. Some of the function pointers in kfd2kgd_calls are used for hardware abstraction with different implementations for each GFX HW generation. These will need to remain function pointers. At some later stage, the HW-specific functions could be moved into gfx_v*.c and the function pointers added to struct amdgpu_gfx. But at this stage I think I'd leave them where they are. 3. Reduce duplicate tracking of device and BO structures Currently KFD and AMDGPU pretend to not know each other's data structures. If both are in the same module, we could allow KFD to access some amdgpu structures directly (e.g. amdgpu_device and amdgpu_bo). This way some of the duplicate tracking of devices and buffer objects could be eliminated. This may present opportunities to simplify some functionality that's currently split across both modules, such as suspend/resume, memory management and evictions. Some interfaces that just query information from amdgpu could be removed if KFD can access that information directly (e.g. firmware versions, CU info, ...). Please let me know if you have any objections, suggestions, ideas ... Regards, Felix -- F e l i x K u e h l i n g PMTS Software Development Engineer | Linux Compute Kernel 1 Commerce Valley Dr. East, Markham, ON L3T 7X6 Canada (O) +1(289)695-1597 _ _ _ _ _ / \ | \ / | | _ \ \ _ | / A \ | \M/ | | |D) ) /|_| | /_/ \_\ |_| |_| |_/ |__/ \| facebook.com/AMD | amd.com ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [PATCH 1/5] drm/i915: Remove unecessary dma_fence_ops
On Wed, Jul 4, 2018 at 7:22 PM, Emil Velikov wrote:
> On 4 July 2018 at 13:34, Daniel Vetter wrote:
>> On Wed, Jul 04, 2018 at 01:03:18PM +0100, Emil Velikov wrote:
>>> Hi Daniel,
>>>
>>> On 4 July 2018 at 10:29, Daniel Vetter wrote:
>>> > dma_fence_default_wait is the default now, same for the trivial
>>> > enable_signaling implementation.
>>> >
>>> > v2: Also remove the relase hook, dma_fence_free is the default.
>>> >
>>> > Signed-off-by: Daniel Vetter
>>> > Cc: Jani Nikula
>>> > Cc: Joonas Lahtinen
>>> > Cc: Rodrigo Vivi
>>> > Cc: Chris Wilson
>>> > Cc: Tvrtko Ursulin
>>> > Cc: Colin Ian King
>>> > Cc: Daniel Vetter
>>> > Cc: Mika Kuoppala
>>> > Cc: intel-...@lists.freedesktop.org
>>> > ---
>>> > drivers/gpu/drm/i915/i915_gem_clflush.c| 7 ---
>>> > drivers/gpu/drm/i915/selftests/i915_sw_fence.c | 8
>>> > 2 files changed, 15 deletions(-)
>>> >
>>> > diff --git a/drivers/gpu/drm/i915/i915_gem_clflush.c
>>> > b/drivers/gpu/drm/i915/i915_gem_clflush.c
>>> > index f5c570d35b2a..8e74c23cbd91 100644
>>> > --- a/drivers/gpu/drm/i915/i915_gem_clflush.c
>>> > +++ b/drivers/gpu/drm/i915/i915_gem_clflush.c
>>> > @@ -45,11 +45,6 @@ static const char
>>> > *i915_clflush_get_timeline_name(struct dma_fence *fence)
>>> > return "clflush";
>>> > }
>>> >
>>> > -static bool i915_clflush_enable_signaling(struct dma_fence *fence)
>>> > -{
>>> > - return true;
>>> > -}
>>> > -
>>> > static void i915_clflush_release(struct dma_fence *fence)
>>> > {
>>> > struct clflush *clflush = container_of(fence, typeof(*clflush),
>>> > dma);
>>> > @@ -63,8 +58,6 @@ static void i915_clflush_release(struct dma_fence
>>> > *fence)
>>> > static const struct dma_fence_ops i915_clflush_ops = {
>>> > .get_driver_name = i915_clflush_get_driver_name,
>>> > .get_timeline_name = i915_clflush_get_timeline_name,
>>> > - .enable_signaling = i915_clflush_enable_signaling,
>>> From a quick look through drm-misc/drm-misc-next removing the
>>> enable_signalling hook may cause functional changes.
>>>
>>> Namely:
>>> A call to trace_dma_fence_enable_signal() (in
>>> dma_fence_enable_sw_signaling(), dma_fence_add_callback() and others)
>>> will be omitted.
>>
>> I'm not sure what this tracepoint is useful for in the absenve of a real
>> signaling mechanism that must be enabled (like interrupts).
>>
>> For all the other bits (begin/end wait, fence signalling itsefl) we have
>> tracepoints already, so I think we're all covered. What do you think will
>> be lost with the tracepoint here? If there's a real need for it I think I
>> can rework the already merged patch to still call the tracpoint, while
>> avoiding everything else. I just don't see the use-case for that.
>
> Nothing obvious comes to mind, yet again my knowledge of the fence API
> is limited.
> Was simply pointing out something that was removed without a small
> note covering it.
>
> A fraction of your explanation would have been great, but obviously
> not a big deal either way.
Yeah would have been good to add to the commit message that made
->enable_signaling optional, but that's now baked into history already
:-/
-Daniel
--
Daniel Vetter
Software Engineer, Intel Corporation
+41 (0) 79 365 57 48 - http://blog.ffwll.ch
___
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: Shared atomic state causing Weston repaint failure
On Wed, Jul 4, 2018 at 5:44 PM, Daniel Stone wrote: > Hi, > The atomic API being super-explicit about how userspace sequences its > calls is great and all, but having shared global state implicitly > dragged in is kind of ruining my day. > > Currently on Intel, Weston sometimes fails on hotplug, because a > commit which only enables CRTC B (not touching CRTC A or any other > CRTC!), causes all commits to CRTC A to fail until CRTC B's modeset > commit has fully retired: > https://gitlab.freedesktop.org/wayland/weston/issues/24 > > The reason is that committing CRTC B resizes the DDB allocation for > CRTC A as well, pulling CRTC A's CRTC state into the commit. This > makes sense, but on the other hand it's totally opaque to userspace, > and impossible for us to reason about when making commits. > > I suggested some options in that GitLab commit, none of which I like: > * if any other CRTCs are pulled into a commit state, always execute > a blocking commit in the kernel > * if we're passing ALLOW_MODESET in userspace, only ever do blocking commits > * whenever we get -EBUSY in userspace, assume we've been screwed by > the kernel and defer until other outputs have completed > * whenever we want to reconfigure any output in userspace, wait > until all outputs are completely quiescent and do a single atomic > commit covering all outputs > > The first one seems completely non-obvious from the kernel, but on the > other hand the current -EBUSY failing behaviour is also non-obvious. > > The second is maybe the most reasonable, but on the other hand just > working around a painful leaky abstraction: we also can't know upfront > from userspace if this is actually going to be required, or if we're > just killing responsiveness blocking for no reason. > > The third is the thing I least want to do, because it might well paper > over legitimate bugs in userspace, and complicates our state tracking > for no reason. > > The fourth is probably the most legitimate, but, well ... someone has > to type up all the code to make our output-configuration API > completely asynchronous. > > I suspect we're the first ones to be hitting this, because Weston has > a truly independent per-CRTC repaint loop, we're one of the few atomic > users, and also because Pekka did some seriously brutal hotplug > testing whilst reworking Weston's output configuration API. Also > because our approach to failed output repaints is to just freeze the > output until it next cycles off and on, which is much more apparent > than just silently dropping a frame here and there. ;) > > Any bright ideas on what could practically be done here? > > Cheers, > Daniel We had an entirely inconclusive chat on irc about this topic. Random notes: - This can fail both on the commit doing a modeset (when it adds some random other CRTC which still has a pending flip), and for normal flips (if they run into a CRTC which has been affected by a modeset). - Just waiting for all nonblocking modesets to complete is not enough, because you don't get an event for these affected CRTC. And they might take longer. So you can still get an EBUSY even when it looks like everything is done. - If you don't combine nonblocking with ALLOW_MODESET there's no issue, because everyone just blocks (and maybe misses a few frames) until the modeset is done. - We want to tell apart an EBUSY due to the redraw loop having lost sync from an EBUSY due to these modesets that affect more than just the passed-in CRTCs. It's a really nice debug feature for compositor developers. -> One way or another we need new/clarified uapi, atomic as implemented doesn't really cut it. A few of the options we've discussed: - Eating all the EBUSY for ALLOW_MODESET. A bit a bigger hammer. Slightly smaller hammer is just eating all the EBUSY for crtc not in the original request. It would still be nice to tell userspace about the additional CRTCs its atomic comit affects. - Some new flags to send out events (what about fence_ptr?) for these affected crtcs, plus reporting them out in a new affected_crtcs_mask in the atomic ioctl struct. - One tricky issue is that drivers might pull in CRTCs which are disabled and stay disabled. Events aren't well-defined on these at all, but they might still provoke an EBUSY! - Compositors would like to know how many frames they've lost. One idea was to split up the retry error codes: EINTR would mean retrying immedetialy due to a signal. EAGAIN would mean the kernel had to insert something to make a modeset possible, retry only after you waited for one vblank. With current userspace this would only result in cpu wasted, but updated userspace could be more intelligent about things. We could also do a special flag or whatever. We didn't zero in on anything specific since it's not really clear what compositors want here at all. Cheers, Daniel -- Daniel Vetter Software Engineer, Intel Corporation +41 (0) 79 365 57 48 - http://blog.ffwll.ch
[Bug 107115] Make darktable OpenCL support work on Clover and RadeonSI
https://bugs.freedesktop.org/show_bug.cgi?id=107115 --- Comment #1 from darkbasic --- Awesome, I really hope that you will succeed! -- 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 99553] Tracker bug for runnning OpenCL applications on Clover
https://bugs.freedesktop.org/show_bug.cgi?id=99553 --- Comment #18 from Greg V --- Added a bug for darktable specifically: bug 107115 -- 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 107115] Make darktable OpenCL support work on Clover and RadeonSI
https://bugs.freedesktop.org/show_bug.cgi?id=107115 Greg V changed: What|Removed |Added Blocks||99553 Referenced Bugs: https://bugs.freedesktop.org/show_bug.cgi?id=99553 [Bug 99553] Tracker bug for runnning OpenCL applications on Clover -- 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 99553] Tracker bug for runnning OpenCL applications on Clover
https://bugs.freedesktop.org/show_bug.cgi?id=99553 Greg V changed: What|Removed |Added Depends on||107115 Referenced Bugs: https://bugs.freedesktop.org/show_bug.cgi?id=107115 [Bug 107115] Make darktable OpenCL support work on Clover and RadeonSI -- 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 107115] Make darktable OpenCL support work on Clover and RadeonSI
https://bugs.freedesktop.org/show_bug.cgi?id=107115 Bug ID: 107115 Summary: Make darktable OpenCL support work on Clover and RadeonSI Product: Mesa Version: git Hardware: x86-64 (AMD64) OS: FreeBSD Status: NEW Severity: normal Priority: medium Component: Drivers/Gallium/radeonsi Assignee: dri-devel@lists.freedesktop.org Reporter: greg@unrelenting.technology QA Contact: dri-devel@lists.freedesktop.org Hi everyone, I've tried to get the darktable photo editor https://www.darktable.org to work, and there's more to it than image support... First, I've picked up funfunctor's patches (mentioned in bug 99553), they applied cleanly to master (except for the little one-line "= 1" since the r600/radeonsi split happened, but that was trivial to fix). My branch with them is https://github.com/myfreeweb/mesa/tree/clover-image Second, there was some weird mismatch between libclc's headers and darktable's code, and I've had to do: %s/write_only image2d_t/image2d_t and %s/__image2d_t/image2d_t on all files in PREFIX/share/darktable/kernels. Third, the liquify.cl kernel triggered the https://bugs.llvm.org/show_bug.cgi?id=36679 crash (which is fixed in llvm 7... but I just removed the function body to proceed without this particular effect) And finally, with all kernels compiled, darktable benchmarked CPU vs GPU and found the CPU to be a bit faster (yay for Ryzen) and disabled OpenCL usage. Enabling OpenCL usage in the settings and then double clicking on an image... uhh... hard locked the GPU :D drmn0: GPU fault detected: 147 0x05004402 drmn0: VM_CONTEXT1_PROTECTION_FAULT_ADDR 0x005000A0 drmn0: VM_CONTEXT1_PROTECTION_FAULT_STATUS 0x08044002 drmn0: VM fault (0x02, vmid 4) at page 5243040, read from 'TC5' (0x54433500) (68) P.S. system info: AMD Radeon RX 480, FreeBSD 12-CURRENT / drm-next-kmod 4.15 (DRM 3.23.0) with amdgpu.dc=1, LLVM 6.0.0, Mesa with patches on top of git rev c2ae9b4052701, libclc git rev c45b9dfe5257, darktable 2.4.4. -- You are receiving this mail because: You are the assignee for the bug.___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
Fwd: [radeon-alex:drm-next-4.19-wip 126/132] drivers/gpu/drm/amd/amdgpu/gmc_v7_0.c:150:1-3: WARNING: possible condition with no effect (if == else)
Please check on lines 150-153. Both branches appear to be the same.
thanks,
julia
Courriel original
Objet: [radeon-alex:drm-next-4.19-wip 126/132]
drivers/gpu/drm/amd/amdgpu/gmc_v7_0.c:150:1-3: WARNING: possible
condition with no effect (if == else)
Date: 04.07.2018 15:10
De: kbuild test robot
À: kbu...@01.org
Cc: Julia Lawall
CC: kbuild-...@01.org
CC: dri-devel@lists.freedesktop.org
TO: Alex Deucher
CC: Chunming Zhou
CC: "Christian König"
tree: git://people.freedesktop.org/~agd5f/linux.git drm-next-4.19-wip
head: 6becad35ec8a249df9f8e8fe196316efbd40faf4
commit: 40b106f7e840f2434c42e8992b3a4871a19ddf1d [126/132] drm/amdgpu:
switch firmware path for CIK parts
:: branch date: 6 hours ago
:: commit date: 6 hours ago
drivers/gpu/drm/amd/amdgpu/gmc_v7_0.c:150:1-3: WARNING: possible
condition with no effect (if == else)
git remote add radeon-alex git://people.freedesktop.org/~agd5f/linux.git
git remote update radeon-alex
git checkout 40b106f7e840f2434c42e8992b3a4871a19ddf1d
vim +150 drivers/gpu/drm/amd/amdgpu/gmc_v7_0.c
a2e73f56 Alex Deucher2015-04-20 115
a2e73f56 Alex Deucher2015-04-20 116 /**
a2e73f56 Alex Deucher2015-04-20 117 * gmc_v7_0_init_microcode -
load ucode images from disk
a2e73f56 Alex Deucher2015-04-20 118 *
a2e73f56 Alex Deucher2015-04-20 119 * @adev: amdgpu_device
pointer
a2e73f56 Alex Deucher2015-04-20 120 *
a2e73f56 Alex Deucher2015-04-20 121 * Use the firmware interface
to load the ucode images into
a2e73f56 Alex Deucher2015-04-20 122 * the driver (not loaded into
hw).
a2e73f56 Alex Deucher2015-04-20 123 * Returns 0 on success, error
on failure.
a2e73f56 Alex Deucher2015-04-20 124 */
a2e73f56 Alex Deucher2015-04-20 125 static int
gmc_v7_0_init_microcode(struct amdgpu_device *adev)
a2e73f56 Alex Deucher2015-04-20 126 {
a2e73f56 Alex Deucher2015-04-20 127const char *chip_name;
a2e73f56 Alex Deucher2015-04-20 128char fw_name[30];
a2e73f56 Alex Deucher2015-04-20 129int err;
a2e73f56 Alex Deucher2015-04-20 130
a2e73f56 Alex Deucher2015-04-20 131DRM_DEBUG("\n");
a2e73f56 Alex Deucher2015-04-20 132
a2e73f56 Alex Deucher2015-04-20 133switch (adev->asic_type) {
a2e73f56 Alex Deucher2015-04-20 134case CHIP_BONAIRE:
a2e73f56 Alex Deucher2015-04-20 135chip_name = "bonaire";
a2e73f56 Alex Deucher2015-04-20 136break;
a2e73f56 Alex Deucher2015-04-20 137case CHIP_HAWAII:
a2e73f56 Alex Deucher2015-04-20 138chip_name = "hawaii";
a2e73f56 Alex Deucher2015-04-20 139break;
429c45de Ken Wang2016-02-03 140case CHIP_TOPAZ:
429c45de Ken Wang2016-02-03 141chip_name = "topaz";
429c45de Ken Wang2016-02-03 142break;
a2e73f56 Alex Deucher2015-04-20 143case CHIP_KAVERI:
a2e73f56 Alex Deucher2015-04-20 144case CHIP_KABINI:
b62774fc Alex Deucher2016-07-29 145case CHIP_MULLINS:
a2e73f56 Alex Deucher2015-04-20 146return 0;
a2e73f56 Alex Deucher2015-04-20 147default: BUG();
a2e73f56 Alex Deucher2015-04-20 148}
a2e73f56 Alex Deucher2015-04-20 149
429c45de Ken Wang2016-02-03 @150 if (adev->asic_type ==
CHIP_TOPAZ)
429c45de Ken Wang2016-02-03 151 snprintf(fw_name,
sizeof(fw_name), "amdgpu/%s_mc.bin", chip_name);
429c45de Ken Wang2016-02-03 152else
40b106f7 Alex Deucher2018-07-02 153 snprintf(fw_name,
sizeof(fw_name), "amdgpu/%s_mc.bin", chip_name);
429c45de Ken Wang2016-02-03 154
770d13b1 Christian König 2018-01-12 155 err =
request_firmware(>gmc.fw, fw_name, adev->dev);
a2e73f56 Alex Deucher2015-04-20 156if (err)
a2e73f56 Alex Deucher2015-04-20 157goto out;
770d13b1 Christian König 2018-01-12 158 err =
amdgpu_ucode_validate(adev->gmc.fw);
a2e73f56 Alex Deucher2015-04-20 159
a2e73f56 Alex Deucher2015-04-20 160 out:
a2e73f56 Alex Deucher2015-04-20 161if (err) {
7ca85295 Joe Perches 2017-02-28 162 pr_err("cik_mc: Failed to
load firmware \"%s\"\n", fw_name);
770d13b1 Christian König 2018-01-12 163
release_firmware(adev->gmc.fw);
770d13b1 Christian König 2018-01-12 164adev->gmc.fw = NULL;
a2e73f56 Alex Deucher2015-04-20 165}
a2e73f56 Alex Deucher2015-04-20 166return err;
a2e73f56 Alex Deucher2015-04-20 167 }
a2e73f56 Alex Deucher2015-04-20 168
:: The code at line 150 was first introduced by commit
:: 429c45deae6e57f1bb91bfb05b671063fb0cef60 drm/amdgpu: iceland use
CI based MC IP
:: TO: Ken Wang
:: CC: Alex Deucher
---
0-DAY kernel test infrastructureOpen Source Technology
Center
https://lists.01.org/pipermail/kbuild-all
Re: [PATCH] drm/amd/display/dc/dce: Fix multiple potential integer overflows
On 2018-07-04 01:54 PM, Gustavo A. R. Silva wrote: > > > On 07/04/2018 12:51 PM, Harry Wentland wrote: > [..] @@ -145,8 +145,8 @@ static bool calculate_fb_and_fractional_fb_divider( * of fractional feedback decimal point and the fractional FB Divider precision * is 2 then the equation becomes (ullfeedbackDivider + 5*100) / (10*100))*/ - feedback_divider += (uint64_t) - (5 * calc_pll_cs->fract_fb_divider_precision_factor); + feedback_divider += 5UL * + calc_pll_cs->fract_fb_divider_precision_factor; >>> >>> This should be 5ULL, as the commit log says, otherwise it's still only >>> 32 bits on 32-bit platforms. >>> >> >> Agreed. >> >> Otherwise this looks good. >> >> With that fixed this patch is >> Reviewed-by: Harry Wentland >> > > Hi Harry, > > I already sent v2: https://patchwork.kernel.org/patch/10506897/ > Thanks. Merged. Harry > Thanks > -- > Gustavo > ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [PATCH] drm/amd/display: Use 2-factor allocator calls
On 2018-07-04 01:27 PM, Kees Cook wrote:
> As already done treewide, switch from open-coded multiplication to
> 2-factor allocation helper.
>
> Signed-off-by: Kees Cook
Reviewed-by: Harry Wentland
Harry
> ---
> drivers/gpu/drm/amd/display/modules/color/color_gamma.c | 8
> 1 file changed, 4 insertions(+), 4 deletions(-)
>
> diff --git a/drivers/gpu/drm/amd/display/modules/color/color_gamma.c
> b/drivers/gpu/drm/amd/display/modules/color/color_gamma.c
> index 98edaefa2b47..ee69c949bfbf 100644
> --- a/drivers/gpu/drm/amd/display/modules/color/color_gamma.c
> +++ b/drivers/gpu/drm/amd/display/modules/color/color_gamma.c
> @@ -1723,8 +1723,8 @@ bool mod_color_calculate_curve(enum
> dc_transfer_func_predefined trans,
> kvfree(rgb_regamma);
> } else if (trans == TRANSFER_FUNCTION_HLG ||
> trans == TRANSFER_FUNCTION_HLG12) {
> - rgb_regamma = kvzalloc(sizeof(*rgb_regamma) *
> -(MAX_HW_POINTS + _EXTRA_POINTS),
> + rgb_regamma = kvcalloc(MAX_HW_POINTS + _EXTRA_POINTS,
> +sizeof(*rgb_regamma),
> GFP_KERNEL);
> if (!rgb_regamma)
> goto rgb_regamma_alloc_fail;
> @@ -1802,8 +1802,8 @@ bool mod_color_calculate_degamma_curve(enum
> dc_transfer_func_predefined trans,
> kvfree(rgb_degamma);
> } else if (trans == TRANSFER_FUNCTION_HLG ||
> trans == TRANSFER_FUNCTION_HLG12) {
> - rgb_degamma = kvzalloc(sizeof(*rgb_degamma) *
> -(MAX_HW_POINTS + _EXTRA_POINTS),
> + rgb_degamma = kvcalloc(MAX_HW_POINTS + _EXTRA_POINTS,
> +sizeof(*rgb_degamma),
> GFP_KERNEL);
> if (!rgb_degamma)
> goto rgb_degamma_alloc_fail;
>
___
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [PATCH] drm/amd/display/dc/dce: Fix multiple potential integer overflows
On 2018-07-04 03:38 AM, Michel Dänzer wrote:
> On 2018-07-04 03:13 AM, Gustavo A. R. Silva wrote:
>> Add suffix ULL to constant 5 and cast variables target_pix_clk_khz and
>> feedback_divider to uint64_t in order to avoid multiple potential integer
>> overflows and give the compiler complete information about the proper
>> arithmetic to use.
>>
>> Notice that such constant and variables are used in contexts that
>> expect expressions of type uint64_t (64 bits, unsigned). The current
>> casts to uint64_t effectively apply to each expression as a whole,
>> but they do not prevent them from being evaluated using 32-bit
>> arithmetic instead of 64-bit arithmetic.
>>
>> Also, once the expressions are properly evaluated using 64-bit
>> arithmentic, there is no need for the parentheses that enclose
>> them.
>>
>> Addresses-Coverity-ID: 1460245 ("Unintentional integer overflow")
>> Addresses-Coverity-ID: 1460286 ("Unintentional integer overflow")
>> Addresses-Coverity-ID: 1460401 ("Unintentional integer overflow")
>> Fixes: 4562236b3bc0 ("drm/amd/dc: Add dc display driver (v2)")
>> Signed-off-by: Gustavo A. R. Silva
>>
>> [...]
>>
>> @@ -145,8 +145,8 @@ static bool calculate_fb_and_fractional_fb_divider(
>> * of fractional feedback decimal point and the fractional FB Divider
>> precision
>> * is 2 then the equation becomes (ullfeedbackDivider + 5*100) / (10*100))*/
>>
>> -feedback_divider += (uint64_t)
>> -(5 * calc_pll_cs->fract_fb_divider_precision_factor);
>> +feedback_divider += 5UL *
>> +calc_pll_cs->fract_fb_divider_precision_factor;
>
> This should be 5ULL, as the commit log says, otherwise it's still only
> 32 bits on 32-bit platforms.
>
Agreed.
Otherwise this looks good.
With that fixed this patch is
Reviewed-by: Harry Wentland
Harry
>
___
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [PATCH 1/5] drm/i915: Remove unecessary dma_fence_ops
On 4 July 2018 at 13:34, Daniel Vetter wrote:
> On Wed, Jul 04, 2018 at 01:03:18PM +0100, Emil Velikov wrote:
>> Hi Daniel,
>>
>> On 4 July 2018 at 10:29, Daniel Vetter wrote:
>> > dma_fence_default_wait is the default now, same for the trivial
>> > enable_signaling implementation.
>> >
>> > v2: Also remove the relase hook, dma_fence_free is the default.
>> >
>> > Signed-off-by: Daniel Vetter
>> > Cc: Jani Nikula
>> > Cc: Joonas Lahtinen
>> > Cc: Rodrigo Vivi
>> > Cc: Chris Wilson
>> > Cc: Tvrtko Ursulin
>> > Cc: Colin Ian King
>> > Cc: Daniel Vetter
>> > Cc: Mika Kuoppala
>> > Cc: intel-...@lists.freedesktop.org
>> > ---
>> > drivers/gpu/drm/i915/i915_gem_clflush.c| 7 ---
>> > drivers/gpu/drm/i915/selftests/i915_sw_fence.c | 8
>> > 2 files changed, 15 deletions(-)
>> >
>> > diff --git a/drivers/gpu/drm/i915/i915_gem_clflush.c
>> > b/drivers/gpu/drm/i915/i915_gem_clflush.c
>> > index f5c570d35b2a..8e74c23cbd91 100644
>> > --- a/drivers/gpu/drm/i915/i915_gem_clflush.c
>> > +++ b/drivers/gpu/drm/i915/i915_gem_clflush.c
>> > @@ -45,11 +45,6 @@ static const char
>> > *i915_clflush_get_timeline_name(struct dma_fence *fence)
>> > return "clflush";
>> > }
>> >
>> > -static bool i915_clflush_enable_signaling(struct dma_fence *fence)
>> > -{
>> > - return true;
>> > -}
>> > -
>> > static void i915_clflush_release(struct dma_fence *fence)
>> > {
>> > struct clflush *clflush = container_of(fence, typeof(*clflush),
>> > dma);
>> > @@ -63,8 +58,6 @@ static void i915_clflush_release(struct dma_fence *fence)
>> > static const struct dma_fence_ops i915_clflush_ops = {
>> > .get_driver_name = i915_clflush_get_driver_name,
>> > .get_timeline_name = i915_clflush_get_timeline_name,
>> > - .enable_signaling = i915_clflush_enable_signaling,
>> From a quick look through drm-misc/drm-misc-next removing the
>> enable_signalling hook may cause functional changes.
>>
>> Namely:
>> A call to trace_dma_fence_enable_signal() (in
>> dma_fence_enable_sw_signaling(), dma_fence_add_callback() and others)
>> will be omitted.
>
> I'm not sure what this tracepoint is useful for in the absenve of a real
> signaling mechanism that must be enabled (like interrupts).
>
> For all the other bits (begin/end wait, fence signalling itsefl) we have
> tracepoints already, so I think we're all covered. What do you think will
> be lost with the tracepoint here? If there's a real need for it I think I
> can rework the already merged patch to still call the tracpoint, while
> avoiding everything else. I just don't see the use-case for that.
Nothing obvious comes to mind, yet again my knowledge of the fence API
is limited.
Was simply pointing out something that was removed without a small
note covering it.
A fraction of your explanation would have been great, but obviously
not a big deal either way.
Thanks
Emil
___
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [v3 03/10] mfd: mtk-mmsys: Add mmsys driver
On Thu, Jul 5, 2018 at 12:17 AM, Matthias Brugger wrote:
>
>
> On 03/07/18 09:11, Lee Jones wrote:
>> On Mon, 25 Jun 2018, Matthias Brugger wrote:
>>> On 30/04/18 12:18, Lee Jones wrote:
On Fri, 27 Apr 2018, matthias@kernel.org wrote:
> From: Matthias Brugger
>
> The MMSYS subsystem includes clocks and drm components.
> This patch adds a MFD device to probe both drivers from the same
> device tree compatible.
>
> Signed-off-by: Matthias Brugger
> ---
> drivers/mfd/Kconfig | 9 ++
> drivers/mfd/Makefile| 2 ++
> drivers/mfd/mtk-mmsys.c | 79
> +
> 3 files changed, 90 insertions(+)
> create mode 100644 drivers/mfd/mtk-mmsys.c
>
> diff --git a/drivers/mfd/Kconfig b/drivers/mfd/Kconfig
> index b860eb5aa194..d23a3b9a2c58 100644
> --- a/drivers/mfd/Kconfig
> +++ b/drivers/mfd/Kconfig
> @@ -378,6 +378,15 @@ config MFD_MC13XXX_I2C
>help
> Select this if your MC13xxx is connected via an I2C bus.
>
> +config MFD_MEDIATEK_MMSYS
> + tristate "Mediatek MMSYS interface"
> + select MFD_CORE
> + select REGMAP_MMIO
> + help
> +Select this if you have a MMSYS subsystem in your SoC. The
> +MMSYS subsystem has at least a clock driver part and some
> +DRM components.
> +
> config MFD_MXS_LRADC
>tristate "Freescale i.MX23/i.MX28 LRADC"
>depends on ARCH_MXS || COMPILE_TEST
> diff --git a/drivers/mfd/Makefile b/drivers/mfd/Makefile
> index d9d2cf0d32ef..b96118bd68d9 100644
> --- a/drivers/mfd/Makefile
> +++ b/drivers/mfd/Makefile
> @@ -98,6 +98,8 @@ obj-$(CONFIG_MFD_MC13XXX)+= mc13xxx-core.o
> obj-$(CONFIG_MFD_MC13XXX_SPI) += mc13xxx-spi.o
> obj-$(CONFIG_MFD_MC13XXX_I2C) += mc13xxx-i2c.o
>
> +obj-$(CONFIG_MFD_MEDIATEK_MMSYS) += mtk-mmsys.o
> +
> obj-$(CONFIG_MFD_CORE)+= mfd-core.o
>
> obj-$(CONFIG_EZX_PCAP)+= ezx-pcap.o
> diff --git a/drivers/mfd/mtk-mmsys.c b/drivers/mfd/mtk-mmsys.c
> new file mode 100644
> index ..c802343fb1c6
> --- /dev/null
> +++ b/drivers/mfd/mtk-mmsys.c
> @@ -0,0 +1,79 @@
> +// SPDX-License-Identifier: GPL-2.0+
> +
> +/*
> + * mtk-mmsys.c -- Mediatek MMSYS multi-function driver
> + *
> + * Copyright (c) 2018 Matthias Brugger
> + *
> + * Author: Matthias Brugger
> + */
> +
> +#include
> +#include
> +#include
> +#include
> +#include
> +#include
> +#include
> +#include
> +
> +enum {
> + MMSYS_MT2701 = 1,
> +};
> +
> +static const struct mfd_cell mmsys_mt2701_devs[] = {
> + { .name = "clk-mt2701-mm", },
> + { .name = "drm-mt2701-mm", },
> +};
> +
> +static int mmsys_probe(struct platform_device *pdev)
> +{
> + const struct mfd_cell *mmsys_cells;
> + int nr_cells;
> + long id;
> + int ret;
> +
> + id = (long) of_device_get_match_data(>dev);
> + if (!id) {
> + dev_err(>dev, "of_device_get match_data() failed\n");
> + return -EINVAL;
> + }
> +
> + switch (id) {
> + case MMSYS_MT2701:
> + mmsys_cells = mmsys_mt2701_devs;
> + nr_cells = ARRAY_SIZE(mmsys_mt2701_devs);
> + break;
> + default:
> + return -ENODEV;
> + }
> +
> + ret = devm_mfd_add_devices(>dev, 0, mmsys_cells, nr_cells,
> + NULL, 0, NULL);
> + if (ret) {
> + dev_err(>dev, "failed to add MFD devices %d\n", ret);
> + return ret;
> + }
> +
> + return 0;
> +};
This driver is pretty pointless. It doesn't actually do anything.
I think you just want to use "simple-mfd" instead.
>>>
>>> I think the problem is, that right now we have two drivers which use the
>>> same
>>> devicetree binding, which are clk and drm driver. With a simple-mfd we would
>>> need two compatibles, and this would break backwards compatibility.
>>
>> So what functionality does this driver provide you with that you do
>> not have currently?
>>
>
> I'm not sure if I get your question. Point is, that the MMSYS implementation
> for
> mt8173 is broken, as it assumes that we can probe two drivers with the
> mediatek,mt8173-mmsys compatible. Somehow it used to work, but from what I
> understand it was a bug. So older devicetrees use just on mt8173-mmsys
> compatible in ther DTB.
This works probably because the clk driver is using CLK_OF_DECLARE, which
does not use the device model (i.e. no attached struct dev). So later on,
when the device model scans and adds this device, it's considered not
probed yet, and the drm driver will probe.
This breaks horribly if you try to move the clk driver to a platform
device driver based
Re: [v3 03/10] mfd: mtk-mmsys: Add mmsys driver
On Wed, 04 Jul 2018, Matthias Brugger wrote:
>
>
> On 03/07/18 09:11, Lee Jones wrote:
> > On Mon, 25 Jun 2018, Matthias Brugger wrote:
> >> On 30/04/18 12:18, Lee Jones wrote:
> >>> On Fri, 27 Apr 2018, matthias@kernel.org wrote:
> >>>
> From: Matthias Brugger
>
> The MMSYS subsystem includes clocks and drm components.
> This patch adds a MFD device to probe both drivers from the same
> device tree compatible.
>
> Signed-off-by: Matthias Brugger
> ---
> drivers/mfd/Kconfig | 9 ++
> drivers/mfd/Makefile| 2 ++
> drivers/mfd/mtk-mmsys.c | 79
> +
> 3 files changed, 90 insertions(+)
> create mode 100644 drivers/mfd/mtk-mmsys.c
>
> diff --git a/drivers/mfd/Kconfig b/drivers/mfd/Kconfig
> index b860eb5aa194..d23a3b9a2c58 100644
> --- a/drivers/mfd/Kconfig
> +++ b/drivers/mfd/Kconfig
> @@ -378,6 +378,15 @@ config MFD_MC13XXX_I2C
> help
> Select this if your MC13xxx is connected via an I2C bus.
>
> +config MFD_MEDIATEK_MMSYS
> +tristate "Mediatek MMSYS interface"
> +select MFD_CORE
> +select REGMAP_MMIO
> +help
> + Select this if you have a MMSYS subsystem in your SoC. The
> + MMSYS subsystem has at least a clock driver part and some
> + DRM components.
> +
> config MFD_MXS_LRADC
> tristate "Freescale i.MX23/i.MX28 LRADC"
> depends on ARCH_MXS || COMPILE_TEST
> diff --git a/drivers/mfd/Makefile b/drivers/mfd/Makefile
> index d9d2cf0d32ef..b96118bd68d9 100644
> --- a/drivers/mfd/Makefile
> +++ b/drivers/mfd/Makefile
> @@ -98,6 +98,8 @@ obj-$(CONFIG_MFD_MC13XXX) += mc13xxx-core.o
> obj-$(CONFIG_MFD_MC13XXX_SPI) += mc13xxx-spi.o
> obj-$(CONFIG_MFD_MC13XXX_I2C) += mc13xxx-i2c.o
>
> +obj-$(CONFIG_MFD_MEDIATEK_MMSYS) += mtk-mmsys.o
> +
> obj-$(CONFIG_MFD_CORE) += mfd-core.o
>
> obj-$(CONFIG_EZX_PCAP) += ezx-pcap.o
> diff --git a/drivers/mfd/mtk-mmsys.c b/drivers/mfd/mtk-mmsys.c
> new file mode 100644
> index ..c802343fb1c6
> --- /dev/null
> +++ b/drivers/mfd/mtk-mmsys.c
> @@ -0,0 +1,79 @@
> +// SPDX-License-Identifier: GPL-2.0+
> +
> +/*
> + * mtk-mmsys.c -- Mediatek MMSYS multi-function driver
> + *
> + * Copyright (c) 2018 Matthias Brugger
> + *
> + * Author: Matthias Brugger
> + */
> +
> +#include
> +#include
> +#include
> +#include
> +#include
> +#include
> +#include
> +#include
> +
> +enum {
> +MMSYS_MT2701 = 1,
> +};
> +
> +static const struct mfd_cell mmsys_mt2701_devs[] = {
> +{ .name = "clk-mt2701-mm", },
> +{ .name = "drm-mt2701-mm", },
> +};
> +
> +static int mmsys_probe(struct platform_device *pdev)
> +{
> +const struct mfd_cell *mmsys_cells;
> +int nr_cells;
> +long id;
> +int ret;
> +
> +id = (long) of_device_get_match_data(>dev);
> +if (!id) {
> +dev_err(>dev, "of_device_get match_data()
> failed\n");
> +return -EINVAL;
> +}
> +
> +switch (id) {
> +case MMSYS_MT2701:
> +mmsys_cells = mmsys_mt2701_devs;
> +nr_cells = ARRAY_SIZE(mmsys_mt2701_devs);
> +break;
> +default:
> +return -ENODEV;
> +}
> +
> +ret = devm_mfd_add_devices(>dev, 0, mmsys_cells, nr_cells,
> +NULL, 0, NULL);
> +if (ret) {
> +dev_err(>dev, "failed to add MFD devices %d\n",
> ret);
> +return ret;
> +}
> +
> +return 0;
> +};
> >>>
> >>> This driver is pretty pointless. It doesn't actually do anything.
> >>>
> >>> I think you just want to use "simple-mfd" instead.
> >>>
> >>
> >> I think the problem is, that right now we have two drivers which use the
> >> same
> >> devicetree binding, which are clk and drm driver. With a simple-mfd we
> >> would
> >> need two compatibles, and this would break backwards compatibility.
> >
> > So what functionality does this driver provide you with that you do
> > not have currently?
> >
>
> I'm not sure if I get your question. Point is, that the MMSYS implementation
> for
> mt8173 is broken, as it assumes that we can probe two drivers with the
> mediatek,mt8173-mmsys compatible. Somehow it used to work, but from what I
> understand it was a bug. So older devicetrees use just on mt8173-mmsys
>
[Bug 99426] gem_mmap_gtt swap tests are too long - perf: interrupt took too long
https://bugs.freedesktop.org/show_bug.cgi?id=99426 Francesco Balestrieri changed: What|Removed |Added Priority|medium |lowest -- 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 89214] intel-gpu-tools does not properly cross-compile when using assembler/intel-gen4asm
https://bugs.freedesktop.org/show_bug.cgi?id=89214 Francesco Balestrieri changed: What|Removed |Added Assignee|marius.c.v...@intel.com |dri-devel@lists.freedesktop ||.org -- You are receiving this mail because: You are the assignee for the bug.___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
[PATCH -next 6/9] drm/vmwgfx: Support for SVGA3dSurfaceAllFlags in vmwgfx
From: Deepak Rawat
Since svga device introduced new 64bit SVGA3dSurfaceAllFlags, vmwgfx
now stores the surface flags internally as SVGA3dSurfaceAllFlags.
For legacy surface define commands, only lower 32-bit is used.
Signed-off-by: Deepak Rawat
Reviewed-by: Sinclair Yeh
Reviewed-by: Brian Paul
Reviewed-by: Thomas Hellstrom
Signed-off-by: Thomas Hellstrom
---
drivers/gpu/drm/vmwgfx/vmwgfx_drv.h | 4 ++--
drivers/gpu/drm/vmwgfx/vmwgfx_surface.c | 21 ++---
2 files changed, 16 insertions(+), 9 deletions(-)
diff --git a/drivers/gpu/drm/vmwgfx/vmwgfx_drv.h
b/drivers/gpu/drm/vmwgfx/vmwgfx_drv.h
index 06cce72b7b9e..7e5c93083036 100644
--- a/drivers/gpu/drm/vmwgfx/vmwgfx_drv.h
+++ b/drivers/gpu/drm/vmwgfx/vmwgfx_drv.h
@@ -166,7 +166,7 @@ struct vmw_surface_offset;
struct vmw_surface {
struct vmw_resource res;
- uint32_t flags;
+ SVGA3dSurfaceAllFlags flags;
uint32_t format;
uint32_t mip_levels[DRM_VMW_MAX_SURFACE_FACES];
struct drm_vmw_size base_size;
@@ -1080,7 +1080,7 @@ extern int vmw_surface_validate(struct vmw_private
*dev_priv,
struct vmw_surface *srf);
int vmw_surface_gb_priv_define(struct drm_device *dev,
uint32_t user_accounting_size,
- uint32_t svga3d_flags,
+ SVGA3dSurfaceAllFlags svga3d_flags,
SVGA3dSurfaceFormat format,
bool for_scanout,
uint32_t num_mip_levels,
diff --git a/drivers/gpu/drm/vmwgfx/vmwgfx_surface.c
b/drivers/gpu/drm/vmwgfx/vmwgfx_surface.c
index 2abf9a895605..a5f93f62c7fa 100644
--- a/drivers/gpu/drm/vmwgfx/vmwgfx_surface.c
+++ b/drivers/gpu/drm/vmwgfx/vmwgfx_surface.c
@@ -224,7 +224,12 @@ static void vmw_surface_define_encode(const struct
vmw_surface *srf,
cmd->header.id = SVGA_3D_CMD_SURFACE_DEFINE;
cmd->header.size = cmd_len;
cmd->body.sid = srf->res.id;
- cmd->body.surfaceFlags = srf->flags;
+ /*
+* Downcast of surfaceFlags, was upcasted when received from user-space,
+* since driver internally stores as 64 bit.
+* For legacy surface define only 32 bit flag is supported.
+*/
+ cmd->body.surfaceFlags = (SVGA3dSurface1Flags)srf->flags;
cmd->body.format = srf->format;
for (i = 0; i < DRM_VMW_MAX_SURFACE_FACES; ++i)
cmd->body.face[i].numMipLevels = srf->mip_levels[i];
@@ -760,7 +765,8 @@ int vmw_surface_define_ioctl(struct drm_device *dev, void
*data,
srf = _srf->srf;
res = >res;
- srf->flags = req->flags;
+ /* Driver internally stores as 64-bit flags */
+ srf->flags = (SVGA3dSurfaceAllFlags)req->flags;
srf->format = req->format;
srf->scanout = req->scanout;
@@ -992,7 +998,8 @@ int vmw_surface_reference_ioctl(struct drm_device *dev,
void *data,
user_srf = container_of(base, struct vmw_user_surface, prime.base);
srf = _srf->srf;
- rep->flags = srf->flags;
+ /* Downcast of flags when sending back to user space */
+ rep->flags = (uint32_t)srf->flags;
rep->format = srf->format;
memcpy(rep->mip_levels, srf->mip_levels, sizeof(srf->mip_levels));
user_sizes = (struct drm_vmw_size __user *)(unsigned long)
@@ -1082,7 +1089,7 @@ static int vmw_gb_surface_create(struct vmw_resource *res)
cmd3->header.id = cmd_id;
cmd3->header.size = cmd_len;
cmd3->body.sid = srf->res.id;
- cmd3->body.surfaceFlags = (SVGA3dSurfaceAllFlags)srf->flags;
+ cmd3->body.surfaceFlags = srf->flags;
cmd3->body.format = srf->format;
cmd3->body.numMipLevels = srf->mip_levels[0];
cmd3->body.multisampleCount = srf->multisample_count;
@@ -1320,7 +1327,7 @@ int vmw_gb_surface_define_ioctl(struct drm_device *dev,
void *data,
/* Define a surface based on the parameters. */
ret = vmw_surface_gb_priv_define(dev,
size,
- req->svga3d_flags,
+ (SVGA3dSurfaceAllFlags)req->svga3d_flags,
req->format,
req->drm_surface_flags & drm_vmw_surface_flag_scanout,
req->mip_levels,
@@ -1451,7 +1458,7 @@ int vmw_gb_surface_reference_ioctl(struct drm_device
*dev, void *data,
goto out_bad_resource;
}
- rep->creq.svga3d_flags = srf->flags;
+ rep->creq.svga3d_flags = (uint32_t)srf->flags;
rep->creq.format = srf->format;
rep->creq.mip_levels = srf->mip_levels[0];
rep->creq.drm_surface_flags = 0;
@@ -1495,7 +1502,7 @@ int vmw_gb_surface_reference_ioctl(struct drm_device
*dev, void *data,
*/
int vmw_surface_gb_priv_define(struct drm_device *dev,
uint32_t user_accounting_size,
[PATCH -next 8/9] drm/vmwgfx: Add support for multisampling
From: Deepak Rawat
Support for SVGA3D_SURFACE_MULTISAMPLE and surface mob size according
to sample count.
Signed-off-by: Deepak Rawat
Reviewed-by: Sinclair Yeh
Reviewed-by: Brian Paul
Reviewed-by: Thomas Hellstrom
Reviewed-by: Charmaine Lee
Signed-off-by: Thomas Hellstrom
---
.../device_include/svga3d_surfacedefs.h | 20 +++
drivers/gpu/drm/vmwgfx/vmwgfx_drv.h | 2 +-
drivers/gpu/drm/vmwgfx/vmwgfx_surface.c | 17
3 files changed, 34 insertions(+), 5 deletions(-)
diff --git a/drivers/gpu/drm/vmwgfx/device_include/svga3d_surfacedefs.h
b/drivers/gpu/drm/vmwgfx/device_include/svga3d_surfacedefs.h
index 6422e3899cdf..809a4ec68e89 100644
--- a/drivers/gpu/drm/vmwgfx/device_include/svga3d_surfacedefs.h
+++ b/drivers/gpu/drm/vmwgfx/device_include/svga3d_surfacedefs.h
@@ -1235,6 +1235,26 @@ svga3dsurface_get_serialized_size(SVGA3dSurfaceFormat
format,
return total_size * num_layers;
}
+/**
+ * svga3dsurface_get_serialized_size_extended - Returns the number of bytes
+ * required for a surface with given parameters. Support for sample count.
+ */
+static inline u32
+svga3dsurface_get_serialized_size_extended(SVGA3dSurfaceFormat format,
+ surf_size_struct base_level_size,
+ u32 num_mip_levels,
+ u32 num_layers,
+ u32 num_samples)
+{
+ uint64_t total_size =
+ svga3dsurface_get_serialized_size(format,
+ base_level_size,
+ num_mip_levels,
+ num_layers);
+ total_size *= max_t(u32, 1, num_samples);
+
+ return min_t(uint64_t, total_size, (uint64_t)U32_MAX);
+}
/**
* svga3dsurface_get_pixel_offset - Compute the offset (in bytes) to a pixel
diff --git a/drivers/gpu/drm/vmwgfx/vmwgfx_drv.h
b/drivers/gpu/drm/vmwgfx/vmwgfx_drv.h
index 59af14714797..a67b54e4fd50 100644
--- a/drivers/gpu/drm/vmwgfx/vmwgfx_drv.h
+++ b/drivers/gpu/drm/vmwgfx/vmwgfx_drv.h
@@ -83,7 +83,7 @@
struct vmw_fpriv {
struct drm_master *locked_master;
struct ttm_object_file *tfile;
- bool gb_aware;
+ bool gb_aware; /* user-space is guest-backed aware */
};
struct vmw_buffer_object {
diff --git a/drivers/gpu/drm/vmwgfx/vmwgfx_surface.c
b/drivers/gpu/drm/vmwgfx/vmwgfx_surface.c
index 1d4c010a0e48..7636bf2db17e 100644
--- a/drivers/gpu/drm/vmwgfx/vmwgfx_surface.c
+++ b/drivers/gpu/drm/vmwgfx/vmwgfx_surface.c
@@ -1399,6 +1399,7 @@ int vmw_surface_gb_priv_define(struct drm_device *dev,
struct vmw_surface *srf;
int ret;
u32 num_layers = 1;
+ u32 sample_count = 1;
*srf_out = NULL;
@@ -1481,11 +1482,15 @@ int vmw_surface_gb_priv_define(struct drm_device *dev,
else if (svga3d_flags & SVGA3D_SURFACE_CUBEMAP)
num_layers = SVGA3D_MAX_SURFACE_FACES;
+ if (srf->flags & SVGA3D_SURFACE_MULTISAMPLE)
+ sample_count = srf->multisample_count;
+
srf->res.backup_size =
- svga3dsurface_get_serialized_size(srf->format,
- srf->base_size,
- srf->mip_levels[0],
- num_layers);
+ svga3dsurface_get_serialized_size_extended(srf->format,
+ srf->base_size,
+ srf->mip_levels[0],
+ num_layers,
+ sample_count);
if (srf->flags & SVGA3D_SURFACE_BIND_STREAM_OUTPUT)
srf->res.backup_size += sizeof(SVGA3dDXSOState);
@@ -1595,6 +1600,10 @@ vmw_gb_surface_define_internal(struct drm_device *dev,
return -EINVAL;
}
+ if ((svga3d_flags_64 & SVGA3D_SURFACE_MULTISAMPLE) &&
+ req->base.multisample_count == 0)
+ return -EINVAL;
+
if (req->base.mip_levels > DRM_VMW_MAX_MIP_LEVELS)
return -EINVAL;
--
2.18.0.rc1
___
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel
[PATCH -next 7/9] drm/vmwgfx: Add new ioctl for GB surface create and reference
From: Deepak Rawat
New ioctls DRM_VMW_GB_SURFACE_CREATE_EXT and DRM_VMW_GB_SURFACE_REF_EXT
are added which support 64-bit wide svga device surface flags, quality
level and multisample pattern.
Signed-off-by: Deepak Rawat
Reviewed-by: Sinclair Yeh
Reviewed-by: Brian Paul
Reviewed-by: Thomas Hellstrom
Reviewed-by: Charmaine Lee
Signed-off-by: Thomas Hellstrom
---
drivers/gpu/drm/vmwgfx/vmwgfx_drv.c | 12 +
drivers/gpu/drm/vmwgfx/vmwgfx_drv.h | 8 +
drivers/gpu/drm/vmwgfx/vmwgfx_kms.c | 20 +-
drivers/gpu/drm/vmwgfx/vmwgfx_stdu.c| 2 +
drivers/gpu/drm/vmwgfx/vmwgfx_surface.c | 468 +++-
include/uapi/drm/vmwgfx_drm.h | 102 ++
6 files changed, 437 insertions(+), 175 deletions(-)
diff --git a/drivers/gpu/drm/vmwgfx/vmwgfx_drv.c
b/drivers/gpu/drm/vmwgfx/vmwgfx_drv.c
index 6cf81e19182f..59229111f303 100644
--- a/drivers/gpu/drm/vmwgfx/vmwgfx_drv.c
+++ b/drivers/gpu/drm/vmwgfx/vmwgfx_drv.c
@@ -137,6 +137,12 @@
#define DRM_IOCTL_VMW_CREATE_EXTENDED_CONTEXT \
DRM_IOWR(DRM_COMMAND_BASE + DRM_VMW_CREATE_EXTENDED_CONTEXT,\
struct drm_vmw_context_arg)
+#define DRM_IOCTL_VMW_GB_SURFACE_CREATE_EXT\
+ DRM_IOWR(DRM_COMMAND_BASE + DRM_VMW_GB_SURFACE_CREATE_EXT, \
+ union drm_vmw_gb_surface_create_ext_arg)
+#define DRM_IOCTL_VMW_GB_SURFACE_REF_EXT \
+ DRM_IOWR(DRM_COMMAND_BASE + DRM_VMW_GB_SURFACE_REF_EXT, \
+ union drm_vmw_gb_surface_reference_ext_arg)
/**
* The core DRM version of this macro doesn't account for
@@ -224,6 +230,12 @@ static const struct drm_ioctl_desc vmw_ioctls[] = {
VMW_IOCTL_DEF(VMW_CREATE_EXTENDED_CONTEXT,
vmw_extended_context_define_ioctl,
DRM_AUTH | DRM_RENDER_ALLOW),
+ VMW_IOCTL_DEF(VMW_GB_SURFACE_CREATE_EXT,
+ vmw_gb_surface_define_ext_ioctl,
+ DRM_AUTH | DRM_RENDER_ALLOW),
+ VMW_IOCTL_DEF(VMW_GB_SURFACE_REF_EXT,
+ vmw_gb_surface_reference_ext_ioctl,
+ DRM_AUTH | DRM_RENDER_ALLOW),
};
static const struct pci_device_id vmw_pci_id_list[] = {
diff --git a/drivers/gpu/drm/vmwgfx/vmwgfx_drv.h
b/drivers/gpu/drm/vmwgfx/vmwgfx_drv.h
index 7e5c93083036..59af14714797 100644
--- a/drivers/gpu/drm/vmwgfx/vmwgfx_drv.h
+++ b/drivers/gpu/drm/vmwgfx/vmwgfx_drv.h
@@ -1087,7 +1087,15 @@ int vmw_surface_gb_priv_define(struct drm_device *dev,
uint32_t multisample_count,
uint32_t array_size,
struct drm_vmw_size size,
+ SVGA3dMSPattern multisample_pattern,
+ SVGA3dMSQualityLevel quality_level,
struct vmw_surface **srf_out);
+extern int vmw_gb_surface_define_ext_ioctl(struct drm_device *dev,
+ void *data,
+ struct drm_file *file_priv);
+extern int vmw_gb_surface_reference_ext_ioctl(struct drm_device *dev,
+ void *data,
+ struct drm_file *file_priv);
/*
* Shader management - vmwgfx_shader.c
diff --git a/drivers/gpu/drm/vmwgfx/vmwgfx_kms.c
b/drivers/gpu/drm/vmwgfx/vmwgfx_kms.c
index 0fb363458ab5..3201b0a51d10 100644
--- a/drivers/gpu/drm/vmwgfx/vmwgfx_kms.c
+++ b/drivers/gpu/drm/vmwgfx/vmwgfx_kms.c
@@ -1238,15 +1238,17 @@ static int vmw_create_bo_proxy(struct drm_device *dev,
content_base_size.depth = 1;
ret = vmw_surface_gb_priv_define(dev,
- 0, /* kernel visible only */
- 0, /* flags */
- format,
- true, /* can be a scanout buffer */
- 1, /* num of mip levels */
- 0,
- 0,
- content_base_size,
- srf_out);
+0, /* kernel visible only */
+0, /* flags */
+format,
+true, /* can be a scanout buffer */
+1, /* num of mip levels */
+0,
+0,
+content_base_size,
+SVGA3D_MS_PATTERN_NONE,
+SVGA3D_MS_QUALITY_NONE,
+srf_out);
if (ret) {
DRM_ERROR("Failed to allocate proxy content buffer\n");
return ret;
diff --git a/drivers/gpu/drm/vmwgfx/vmwgfx_stdu.c
b/drivers/gpu/drm/vmwgfx/vmwgfx_stdu.c
index 6630abf3a95c..f9872c9e60c4 100644
[PATCH -next 4/9] drm/vmwgfx: Add SM4_1 flag
From: Deepak Rawat
A boolean flag in device private structure to specify if the device
support SM4_1.
Signed-off-by: Deepak Rawat
Reviewed-by: Sinclair Yeh
Reviewed-by: Brian Paul
Reviewed-by: Thomas Hellstrom
Signed-off-by: Thomas Hellstrom
---
drivers/gpu/drm/vmwgfx/vmwgfx_drv.c | 18 --
drivers/gpu/drm/vmwgfx/vmwgfx_drv.h | 1 +
2 files changed, 17 insertions(+), 2 deletions(-)
diff --git a/drivers/gpu/drm/vmwgfx/vmwgfx_drv.c
b/drivers/gpu/drm/vmwgfx/vmwgfx_drv.c
index f2fad88e4c54..6cf81e19182f 100644
--- a/drivers/gpu/drm/vmwgfx/vmwgfx_drv.c
+++ b/drivers/gpu/drm/vmwgfx/vmwgfx_drv.c
@@ -916,9 +916,23 @@ static int vmw_driver_load(struct drm_device *dev,
unsigned long chipset)
if (ret)
goto out_no_fifo;
+ if (dev_priv->has_dx) {
+ /*
+* SVGA_CAP2_DX2 (DefineGBSurface_v3) is needed for SM4_1
+* support
+*/
+ if ((dev_priv->capabilities2 & SVGA_CAP2_DX2) != 0) {
+ vmw_write(dev_priv, SVGA_REG_DEV_CAP,
+ SVGA3D_DEVCAP_SM41);
+ dev_priv->has_sm4_1 = vmw_read(dev_priv,
+ SVGA_REG_DEV_CAP);
+ }
+ }
+
DRM_INFO("DX: %s\n", dev_priv->has_dx ? "yes." : "no.");
- DRM_INFO("Atomic: %s\n",
-(dev->driver->driver_features & DRIVER_ATOMIC) ? "yes" : "no");
+ DRM_INFO("Atomic: %s\n", (dev->driver->driver_features & DRIVER_ATOMIC)
+? "yes." : "no.");
+ DRM_INFO("SM4_1: %s\n", dev_priv->has_sm4_1 ? "yes." : "no.");
snprintf(host_log, sizeof(host_log), "vmwgfx: %s-%s",
VMWGFX_REPO, VMWGFX_GIT_VERSION);
diff --git a/drivers/gpu/drm/vmwgfx/vmwgfx_drv.h
b/drivers/gpu/drm/vmwgfx/vmwgfx_drv.h
index 920365c0e9ab..7bb08bac728e 100644
--- a/drivers/gpu/drm/vmwgfx/vmwgfx_drv.h
+++ b/drivers/gpu/drm/vmwgfx/vmwgfx_drv.h
@@ -398,6 +398,7 @@ struct vmw_private {
spinlock_t cap_lock;
bool has_dx;
bool assume_16bpp;
+ bool has_sm4_1;
/*
* VGA registers.
--
2.18.0.rc1
___
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel
[PATCH -next 3/9] drm/vmwgfx: Add support for SVGA3dCmdIntraSurfaceCopy command
From: Neha Bhende
A new command to support Intra-Surface-Copy.
Signed-off-by: Neha Bhende
Reviewed-by: Thomas Hellstrom
Signed-off-by: Thomas Hellstrom
---
drivers/gpu/drm/vmwgfx/vmwgfx_execbuf.c | 28 +
1 file changed, 28 insertions(+)
diff --git a/drivers/gpu/drm/vmwgfx/vmwgfx_execbuf.c
b/drivers/gpu/drm/vmwgfx/vmwgfx_execbuf.c
index 2d6efc36288f..8b8386cb9893 100644
--- a/drivers/gpu/drm/vmwgfx/vmwgfx_execbuf.c
+++ b/drivers/gpu/drm/vmwgfx/vmwgfx_execbuf.c
@@ -3116,6 +3116,32 @@ static int vmw_cmd_dx_transfer_from_buffer(struct
vmw_private *dev_priv,
>body.destSid, NULL);
}
+/**
+ * vmw_cmd_intra_surface_copy -
+ * Validate an SVGA_3D_CMD_INTRA_SURFACE_COPY command
+ *
+ * @dev_priv: Pointer to a device private struct.
+ * @sw_context: The software context being used for this batch.
+ * @header: Pointer to the command header in the command stream.
+ */
+static int vmw_cmd_intra_surface_copy(struct vmw_private *dev_priv,
+ struct vmw_sw_context *sw_context,
+ SVGA3dCmdHeader *header)
+{
+ struct {
+ SVGA3dCmdHeader header;
+ SVGA3dCmdIntraSurfaceCopy body;
+ } *cmd = container_of(header, typeof(*cmd), header);
+
+ if (!(dev_priv->capabilities2 & SVGA_CAP2_INTRA_SURFACE_COPY))
+ return -EINVAL;
+
+ return vmw_cmd_res_check(dev_priv, sw_context, vmw_res_surface,
+ user_surface_converter,
+ >body.surface.sid, NULL);
+}
+
+
static int vmw_cmd_check_not_3d(struct vmw_private *dev_priv,
struct vmw_sw_context *sw_context,
void *buf, uint32_t *size)
@@ -3471,6 +3497,8 @@ static const struct vmw_cmd_entry
vmw_cmd_entries[SVGA_3D_CMD_MAX] = {
VMW_CMD_DEF(SVGA_3D_CMD_DX_TRANSFER_FROM_BUFFER,
_cmd_dx_transfer_from_buffer,
true, false, true),
+ VMW_CMD_DEF(SVGA_3D_CMD_INTRA_SURFACE_COPY, _cmd_intra_surface_copy,
+ true, false, true),
};
bool vmw_cmd_describe(const void *buf, u32 *size, char const **cmd)
--
2.18.0.rc1
___
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel
[PATCH -next 2/9] drm/vmwgfx: Add CAP2 support in vmwgfx
From: Neha Bhende
The device exposes a new capability register. Add upport for it.
Signed-off-by: Neha Bhende
Reviewed-by: Thomas Hellstrom
Signed-off-by: Thomas Hellstrom
---
drivers/gpu/drm/vmwgfx/vmwgfx_drv.c | 17 +
drivers/gpu/drm/vmwgfx/vmwgfx_drv.h | 1 +
drivers/gpu/drm/vmwgfx/vmwgfx_ioctl.c | 3 +++
include/uapi/drm/vmwgfx_drm.h | 1 +
4 files changed, 22 insertions(+)
diff --git a/drivers/gpu/drm/vmwgfx/vmwgfx_drv.c
b/drivers/gpu/drm/vmwgfx/vmwgfx_drv.c
index 1128420de2c0..f2fad88e4c54 100644
--- a/drivers/gpu/drm/vmwgfx/vmwgfx_drv.c
+++ b/drivers/gpu/drm/vmwgfx/vmwgfx_drv.c
@@ -258,6 +258,15 @@ MODULE_PARM_DESC(assume_16bpp, "Assume 16-bpp when
filtering modes");
module_param_named(assume_16bpp, vmw_assume_16bpp, int, 0600);
+static void vmw_print_capabilities2(uint32_t capabilities2)
+{
+ DRM_INFO("Capabilities2:\n");
+ if (capabilities2 & SVGA_CAP2_GROW_OTABLE)
+ DRM_INFO(" Grow oTable.\n");
+ if (capabilities2 & SVGA_CAP2_INTRA_SURFACE_COPY)
+ DRM_INFO(" IntraSurface copy.\n");
+}
+
static void vmw_print_capabilities(uint32_t capabilities)
{
DRM_INFO("Capabilities:\n");
@@ -684,6 +693,12 @@ static int vmw_driver_load(struct drm_device *dev,
unsigned long chipset)
}
dev_priv->capabilities = vmw_read(dev_priv, SVGA_REG_CAPABILITIES);
+
+ if (dev_priv->capabilities & SVGA_CAP_CAP2_REGISTER) {
+ dev_priv->capabilities2 = vmw_read(dev_priv, SVGA_REG_CAP2);
+ }
+
+
ret = vmw_dma_select_mode(dev_priv);
if (unlikely(ret != 0)) {
DRM_INFO("Restricting capabilities due to IOMMU setup.\n");
@@ -752,6 +767,8 @@ static int vmw_driver_load(struct drm_device *dev, unsigned
long chipset)
}
vmw_print_capabilities(dev_priv->capabilities);
+ if (dev_priv->capabilities & SVGA_CAP_CAP2_REGISTER)
+ vmw_print_capabilities2(dev_priv->capabilities2);
ret = vmw_dma_masks(dev_priv);
if (unlikely(ret != 0))
diff --git a/drivers/gpu/drm/vmwgfx/vmwgfx_drv.h
b/drivers/gpu/drm/vmwgfx/vmwgfx_drv.h
index a3a0826958a1..920365c0e9ab 100644
--- a/drivers/gpu/drm/vmwgfx/vmwgfx_drv.h
+++ b/drivers/gpu/drm/vmwgfx/vmwgfx_drv.h
@@ -386,6 +386,7 @@ struct vmw_private {
uint32_t initial_height;
u32 *mmio_virt;
uint32_t capabilities;
+ uint32_t capabilities2;
uint32_t max_gmr_ids;
uint32_t max_gmr_pages;
uint32_t max_mob_pages;
diff --git a/drivers/gpu/drm/vmwgfx/vmwgfx_ioctl.c
b/drivers/gpu/drm/vmwgfx/vmwgfx_ioctl.c
index 6872c7ee8a08..ac6da0da2824 100644
--- a/drivers/gpu/drm/vmwgfx/vmwgfx_ioctl.c
+++ b/drivers/gpu/drm/vmwgfx/vmwgfx_ioctl.c
@@ -56,6 +56,9 @@ int vmw_getparam_ioctl(struct drm_device *dev, void *data,
case DRM_VMW_PARAM_HW_CAPS:
param->value = dev_priv->capabilities;
break;
+ case DRM_VMW_PARAM_HW_CAPS2:
+ param->value = dev_priv->capabilities2;
+ break;
case DRM_VMW_PARAM_FIFO_CAPS:
param->value = dev_priv->fifo.capabilities;
break;
diff --git a/include/uapi/drm/vmwgfx_drm.h b/include/uapi/drm/vmwgfx_drm.h
index 57115a5fe61a..84e81b38ca18 100644
--- a/include/uapi/drm/vmwgfx_drm.h
+++ b/include/uapi/drm/vmwgfx_drm.h
@@ -95,6 +95,7 @@ extern "C" {
#define DRM_VMW_PARAM_MAX_MOB_SIZE 10
#define DRM_VMW_PARAM_SCREEN_TARGET11
#define DRM_VMW_PARAM_DX 12
+#define DRM_VMW_PARAM_HW_CAPS2 13
/**
* enum drm_vmw_handle_type - handle type for ref ioctls
--
2.18.0.rc1
___
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel
[PATCH -next 9/9] drm/vmwgfx: Expose SM4_1 param to user space
From: Deepak Rawat
A new param DRM_VMW_PARAM_SM4_1, is added for user space to determine
availability of SM4.1.
Minor version bump for SM4.1.
Signed-off-by: Deepak Rawat
Reviewed-by: Sinclair Yeh
Reviewed-by: Brian Paul
Reviewed-by: Thomas Hellstrom
Signed-off-by: Thomas Hellstrom
---
drivers/gpu/drm/vmwgfx/vmwgfx_drv.h | 6 +++---
drivers/gpu/drm/vmwgfx/vmwgfx_ioctl.c | 3 +++
include/uapi/drm/vmwgfx_drm.h | 4
3 files changed, 10 insertions(+), 3 deletions(-)
diff --git a/drivers/gpu/drm/vmwgfx/vmwgfx_drv.h
b/drivers/gpu/drm/vmwgfx/vmwgfx_drv.h
index a67b54e4fd50..f1b803d34c59 100644
--- a/drivers/gpu/drm/vmwgfx/vmwgfx_drv.h
+++ b/drivers/gpu/drm/vmwgfx/vmwgfx_drv.h
@@ -43,10 +43,10 @@
#include
#define VMWGFX_DRIVER_NAME "vmwgfx"
-#define VMWGFX_DRIVER_DATE "20180322"
+#define VMWGFX_DRIVER_DATE "20180704"
#define VMWGFX_DRIVER_MAJOR 2
-#define VMWGFX_DRIVER_MINOR 14
-#define VMWGFX_DRIVER_PATCHLEVEL 1
+#define VMWGFX_DRIVER_MINOR 15
+#define VMWGFX_DRIVER_PATCHLEVEL 0
#define VMWGFX_FILE_PAGE_OFFSET 0x0010
#define VMWGFX_FIFO_STATIC_SIZE (1024*1024)
#define VMWGFX_MAX_RELOCATIONS 2048
diff --git a/drivers/gpu/drm/vmwgfx/vmwgfx_ioctl.c
b/drivers/gpu/drm/vmwgfx/vmwgfx_ioctl.c
index ac6da0da2824..e825192ca30e 100644
--- a/drivers/gpu/drm/vmwgfx/vmwgfx_ioctl.c
+++ b/drivers/gpu/drm/vmwgfx/vmwgfx_ioctl.c
@@ -116,6 +116,9 @@ int vmw_getparam_ioctl(struct drm_device *dev, void *data,
case DRM_VMW_PARAM_DX:
param->value = dev_priv->has_dx;
break;
+ case DRM_VMW_PARAM_SM4_1:
+ param->value = dev_priv->has_sm4_1;
+ break;
default:
return -EINVAL;
}
diff --git a/include/uapi/drm/vmwgfx_drm.h b/include/uapi/drm/vmwgfx_drm.h
index 68ff37d4c035..399f58317cff 100644
--- a/include/uapi/drm/vmwgfx_drm.h
+++ b/include/uapi/drm/vmwgfx_drm.h
@@ -82,6 +82,9 @@ extern "C" {
*
* DRM_VMW_PARAM_OVERLAY_IOCTL:
* Does the driver support the overlay ioctl.
+ *
+ * DRM_VMW_PARAM_SM4_1
+ * SM4_1 support is enabled.
*/
#define DRM_VMW_PARAM_NUM_STREAMS 0
@@ -98,6 +101,7 @@ extern "C" {
#define DRM_VMW_PARAM_SCREEN_TARGET11
#define DRM_VMW_PARAM_DX 12
#define DRM_VMW_PARAM_HW_CAPS2 13
+#define DRM_VMW_PARAM_SM4_114
/**
* enum drm_vmw_handle_type - handle type for ref ioctls
--
2.18.0.rc1
___
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel
[PATCH -next 5/9] drm/vmwgfx: Add support for SVGA3dCmdDefineGBSurface_v3
From: Deepak Rawat
SVGA device added new command SVGA3dCmdDefineGBSurface_v3 which allows
64-bit SVGA3dSurfaceAllFlags. This commit adds support for
SVGA3dCmdDefineGBSurface_v3 command in vmwgfx.
Signed-off-by: Deepak Rawat
Reviewed-by: Sinclair Yeh
Reviewed-by: Brian Paul
Reviewed-by: Thomas Hellstrom
Reviewed-by: Charmaine Lee
Signed-off-by: Thomas Hellstrom
---
drivers/gpu/drm/vmwgfx/vmwgfx_drv.h | 2 ++
drivers/gpu/drm/vmwgfx/vmwgfx_stdu.c| 3 +++
drivers/gpu/drm/vmwgfx/vmwgfx_surface.c | 32 +++--
3 files changed, 35 insertions(+), 2 deletions(-)
diff --git a/drivers/gpu/drm/vmwgfx/vmwgfx_drv.h
b/drivers/gpu/drm/vmwgfx/vmwgfx_drv.h
index 7bb08bac728e..06cce72b7b9e 100644
--- a/drivers/gpu/drm/vmwgfx/vmwgfx_drv.h
+++ b/drivers/gpu/drm/vmwgfx/vmwgfx_drv.h
@@ -180,6 +180,8 @@ struct vmw_surface {
SVGA3dTextureFilter autogen_filter;
uint32_t multisample_count;
struct list_head view_list;
+ SVGA3dMSPattern multisample_pattern;
+ SVGA3dMSQualityLevel quality_level;
};
struct vmw_marker_queue {
diff --git a/drivers/gpu/drm/vmwgfx/vmwgfx_stdu.c
b/drivers/gpu/drm/vmwgfx/vmwgfx_stdu.c
index 15f2cb2a151b..6630abf3a95c 100644
--- a/drivers/gpu/drm/vmwgfx/vmwgfx_stdu.c
+++ b/drivers/gpu/drm/vmwgfx/vmwgfx_stdu.c
@@ -1157,6 +1157,9 @@ vmw_stdu_primary_plane_prepare_fb(struct drm_plane *plane,
content_srf.flags = 0;
content_srf.mip_levels[0] = 1;
content_srf.multisample_count = 0;
+ content_srf.multisample_pattern =
+ SVGA3D_MS_PATTERN_NONE;
+ content_srf.quality_level = SVGA3D_MS_QUALITY_NONE;
} else {
content_srf = *new_vfbs->surface;
}
diff --git a/drivers/gpu/drm/vmwgfx/vmwgfx_surface.c
b/drivers/gpu/drm/vmwgfx/vmwgfx_surface.c
index e90f8d39de53..2abf9a895605 100644
--- a/drivers/gpu/drm/vmwgfx/vmwgfx_surface.c
+++ b/drivers/gpu/drm/vmwgfx/vmwgfx_surface.c
@@ -785,6 +785,8 @@ int vmw_surface_define_ioctl(struct drm_device *dev, void
*data,
srf->base_size = *srf->sizes;
srf->autogen_filter = SVGA3D_TEX_FILTER_NONE;
srf->multisample_count = 0;
+ srf->multisample_pattern = SVGA3D_MS_PATTERN_NONE;
+ srf->quality_level = SVGA3D_MS_QUALITY_NONE;
cur_bo_offset = 0;
cur_offset = srf->offsets;
@@ -1031,6 +1033,10 @@ static int vmw_gb_surface_create(struct vmw_resource
*res)
SVGA3dCmdHeader header;
SVGA3dCmdDefineGBSurface_v2 body;
} *cmd2;
+ struct {
+ SVGA3dCmdHeader header;
+ SVGA3dCmdDefineGBSurface_v3 body;
+ } *cmd3;
if (likely(res->id != -1))
return 0;
@@ -1047,7 +1053,11 @@ static int vmw_gb_surface_create(struct vmw_resource
*res)
goto out_no_fifo;
}
- if (srf->array_size > 0) {
+ if (dev_priv->has_sm4_1 && srf->array_size > 0) {
+ cmd_id = SVGA_3D_CMD_DEFINE_GB_SURFACE_V3;
+ cmd_len = sizeof(cmd3->body);
+ submit_len = sizeof(*cmd3);
+ } else if (srf->array_size > 0) {
/* has_dx checked on creation time. */
cmd_id = SVGA_3D_CMD_DEFINE_GB_SURFACE_V2;
cmd_len = sizeof(cmd2->body);
@@ -1060,6 +1070,7 @@ static int vmw_gb_surface_create(struct vmw_resource *res)
cmd = vmw_fifo_reserve(dev_priv, submit_len);
cmd2 = (typeof(cmd2))cmd;
+ cmd3 = (typeof(cmd3))cmd;
if (unlikely(!cmd)) {
DRM_ERROR("Failed reserving FIFO space for surface "
"creation.\n");
@@ -1067,7 +1078,22 @@ static int vmw_gb_surface_create(struct vmw_resource
*res)
goto out_no_fifo;
}
- if (srf->array_size > 0) {
+ if (dev_priv->has_sm4_1 && srf->array_size > 0) {
+ cmd3->header.id = cmd_id;
+ cmd3->header.size = cmd_len;
+ cmd3->body.sid = srf->res.id;
+ cmd3->body.surfaceFlags = (SVGA3dSurfaceAllFlags)srf->flags;
+ cmd3->body.format = srf->format;
+ cmd3->body.numMipLevels = srf->mip_levels[0];
+ cmd3->body.multisampleCount = srf->multisample_count;
+ cmd3->body.multisamplePattern = srf->multisample_pattern;
+ cmd3->body.qualityLevel = srf->quality_level;
+ cmd3->body.autogenFilter = srf->autogen_filter;
+ cmd3->body.size.width = srf->base_size.width;
+ cmd3->body.size.height = srf->base_size.height;
+ cmd3->body.size.depth = srf->base_size.depth;
+ cmd3->body.arraySize = srf->array_size;
+ } else if (srf->array_size > 0) {
cmd2->header.id = cmd_id;
cmd2->header.size = cmd_len;
cmd2->body.sid =
[PATCH -next 0/9] vmwgfx multisample support
This series introduces a header update and support for multisample surfaces. Mesa support will follow in a couple of weeks. ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [PATCH] drm/sun4i: Remove VLA usage
On Fri, Jun 29, 2018 at 11:47:40AM -0700, Kees Cook wrote: > In the quest to remove all stack VLA usage from the kernel[1], this > switches to using a kmalloc allocation and moves all the size calculations > to the start to do an allocation. If an upper bounds on the mode timing > calculations could be determined, a fixed stack size could be used instead. > > [1] > https://lkml.kernel.org/r/CA+55aFzCG-zNmZwX4A2FQpadafLfEzK6CC=qpxydaacu1rq...@mail.gmail.com > > Signed-off-by: Kees Cook Applied, thanks! Maxime -- Maxime Ripard, Bootlin (formerly Free Electrons) Embedded Linux and Kernel engineering https://bootlin.com signature.asc Description: PGP signature ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
Shared atomic state causing Weston repaint failure
Hi, The atomic API being super-explicit about how userspace sequences its calls is great and all, but having shared global state implicitly dragged in is kind of ruining my day. Currently on Intel, Weston sometimes fails on hotplug, because a commit which only enables CRTC B (not touching CRTC A or any other CRTC!), causes all commits to CRTC A to fail until CRTC B's modeset commit has fully retired: https://gitlab.freedesktop.org/wayland/weston/issues/24 The reason is that committing CRTC B resizes the DDB allocation for CRTC A as well, pulling CRTC A's CRTC state into the commit. This makes sense, but on the other hand it's totally opaque to userspace, and impossible for us to reason about when making commits. I suggested some options in that GitLab commit, none of which I like: * if any other CRTCs are pulled into a commit state, always execute a blocking commit in the kernel * if we're passing ALLOW_MODESET in userspace, only ever do blocking commits * whenever we get -EBUSY in userspace, assume we've been screwed by the kernel and defer until other outputs have completed * whenever we want to reconfigure any output in userspace, wait until all outputs are completely quiescent and do a single atomic commit covering all outputs The first one seems completely non-obvious from the kernel, but on the other hand the current -EBUSY failing behaviour is also non-obvious. The second is maybe the most reasonable, but on the other hand just working around a painful leaky abstraction: we also can't know upfront from userspace if this is actually going to be required, or if we're just killing responsiveness blocking for no reason. The third is the thing I least want to do, because it might well paper over legitimate bugs in userspace, and complicates our state tracking for no reason. The fourth is probably the most legitimate, but, well ... someone has to type up all the code to make our output-configuration API completely asynchronous. I suspect we're the first ones to be hitting this, because Weston has a truly independent per-CRTC repaint loop, we're one of the few atomic users, and also because Pekka did some seriously brutal hotplug testing whilst reworking Weston's output configuration API. Also because our approach to failed output repaints is to just freeze the output until it next cycles off and on, which is much more apparent than just silently dropping a frame here and there. ;) Any bright ideas on what could practically be done here? Cheers, Daniel ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [PATCH] fb: fix lost console when the user unplugs a USB adapter
Den 03.07.2018 19.18, skrev Mikulas Patocka:
On Tue, 3 Jul 2018, Bartlomiej Zolnierkiewicz wrote:
Hi,
On Sunday, June 03, 2018 11:46:29 AM Mikulas Patocka wrote:
I have a USB display adapter using the udlfb driver and I use it on an ARM
board that doesn't have any graphics card. When I plug the adapter in, the
console is properly displayed, however when I unplug and re-plug the
adapter, the console is not displayed and I can't access it until I reboot
the board.
The reason is this:
When the adapter is unplugged, dlfb_usb_disconnect calls
unlink_framebuffer, then it waits until the reference count drops to zero
and then it deallocates the framebuffer. However, the console that is
attached to the framebuffer device keeps the reference count non-zero, so
the framebuffer device is never destroyed. When the USB adapter is plugged
again, it creates a new device /dev/fb1 and the console is not attached to
it.
This patch fixes the bug by unbinding the console from unlink_framebuffer.
The code to unbind the console is moved from do_unregister_framebuffer to
a function unbind_console. When the console is unbound, the reference
count drops to zero and the udlfb driver frees the framebuffer. When the
adapter is plugged back, a new framebuffer is created and the console is
attached to it.
Signed-off-by: Mikulas Patocka
Cc: sta...@vger.kernel.org
After this change unbind_console() will be called twice in the standard
framebuffer unregister path:
- first time, directly by do_unregister_framebuffer()
- second time, indirectly by do_unregister_framebuffer()->unlink_framebuffer()
This doesn't look correctly.
unbind_console calls the FB_EVENT_FB_UNBIND notifier, FB_EVENT_FB_UNBIND
goes to the function fbcon_fb_unbind and fbcon_fb_unbind checks if the
console is bound to the framebuffer for which unbind is requested. So a
double call won't cause any trouble.
Also why can't udlfb just use unregister_framebuffer() like all other
drivers (it uses unlink_framebuffer() and it is the only user of this
helper)?
It uses unregister_framebuffer() - but - unregister_framebuffer() may only
be called when the open count of the framebuffer is zero.
AFAIU calling unregister_framebuffer() with open fd's is just fine as
long as fb_info with buffers stay intact. All it does is to remove the
fbX from userspace. Cleanup can be done in fb_ops->fb_destroy.
I have been working on generic fbdev emulation for DRM [1] and I did a
test now to see what would happen if I did unbind the driver from the
device. It worked as expected if I didn't have another fbdev present,
but if there is an fb0 and I remove fb1 with a console on it, I would
sometimes get crashes, often with a call to cursor_timer_handler() in
the traceback.
I think there's index mixup in fbcon_fb_unbind(), at least this change
seems to solve the immediate problem:
diff --git a/drivers/video/fbdev/core/fbcon.c
b/drivers/video/fbdev/core/fbcon.c
index 5fb156bdcf4e..271b9b988b73 100644
--- a/drivers/video/fbdev/core/fbcon.c
+++ b/drivers/video/fbdev/core/fbcon.c
@@ -3066,7 +3072,7 @@ static int fbcon_fb_unbind(int idx)
for (i = first_fb_vc; i <= last_fb_vc; i++) {
if (con2fb_map[i] != idx &&
con2fb_map[i] != -1) {
- new_idx = i;
+ new_idx = con2fb_map[i];
break;
}
}
I haven't got time to follow up on this now, but making sure DRM generic
fbdev emulation device unplug works is on my TODO.
BTW, I believe the udl drm driver should be able to use the generic fbdev
emulation if it had a drm_driver->gem_prime_vmap hook. It uses a shadow
buffer which would also make fbdev mmap work for udl. (shmem buffers and
fbdev deferred I/O doesn't work together since they both use
page->lru/mapping)
Noralf.
[1] https://patchwork.freedesktop.org/series/45847/
So, the udlfb
driver waits until the open count drops to zero and then calls
unregister_framebuffer().
But the console subsystem keeps the framebuffer open - which means that if
user use unplugs the USB adapter, the open count won't drop to zero
(because the console is bound to it) - which means that
unregister_framebuffer() will not be called.
You must unbind the console before calling unregister_framebuffer(). The
PCI framebuffer drivers don't have this problem because the user is not
expected to just unplug the PCI card while it is being used by the
console.
Mikulas
---
drivers/video/fbdev/core/fbmem.c | 21 +
1 file changed, 17 insertions(+), 4 deletions(-)
Index: linux-4.16.12/drivers/video/fbdev/core/fbmem.c
===
--- linux-4.16.12.orig/drivers/video/fbdev/core/fbmem.c 2018-05-26
06:13:20.0 +0200
+++ linux-4.16.12/drivers/video/fbdev/core/fbmem.c 2018-05-26
06:13:20.0 +0200
@@ -1805,12 +1805,12 @@ static int do_register_framebuffer(struc
return 0;
}
-static
[PATCH v2] dma-buf: Move BUG_ON from _add_shared_fence to _add_shared_inplace
From: Michel Dänzer
Fixes the BUG_ON spuriously triggering under the following
circumstances:
* reservation_object_reserve_shared is called with shared_count ==
shared_max - 1, so obj->staged is freed in preparation of an in-place
update.
* reservation_object_add_shared_fence is called with the first fence,
after which shared_count == shared_max.
* reservation_object_add_shared_fence is called with a follow-up fence
from the same context.
In the second reservation_object_add_shared_fence call, the BUG_ON
triggers. However, nothing bad would happen in
reservation_object_add_shared_inplace, since both fences are from the
same context, so they only occupy a single slot.
Prevent this by moving the BUG_ON to where an overflow would actually
happen (e.g. if a buggy caller didn't call
reservation_object_reserve_shared before).
v2:
* Fix description of breaking scenario (Christian König)
* Add bugzilla reference
Cc: sta...@vger.kernel.org
Bugzilla: https://bugs.freedesktop.org/106418
Reviewed-by: Chris Wilson # v1
Reviewed-by: Christian König # v1
Signed-off-by: Michel Dänzer
---
drivers/dma-buf/reservation.c | 6 +++---
1 file changed, 3 insertions(+), 3 deletions(-)
diff --git a/drivers/dma-buf/reservation.c b/drivers/dma-buf/reservation.c
index 314eb1071cce..532545b9488e 100644
--- a/drivers/dma-buf/reservation.c
+++ b/drivers/dma-buf/reservation.c
@@ -141,6 +141,7 @@ reservation_object_add_shared_inplace(struct
reservation_object *obj,
if (signaled) {
RCU_INIT_POINTER(fobj->shared[signaled_idx], fence);
} else {
+ BUG_ON(fobj->shared_count >= fobj->shared_max);
RCU_INIT_POINTER(fobj->shared[fobj->shared_count], fence);
fobj->shared_count++;
}
@@ -230,10 +231,9 @@ void reservation_object_add_shared_fence(struct
reservation_object *obj,
old = reservation_object_get_list(obj);
obj->staged = NULL;
- if (!fobj) {
- BUG_ON(old->shared_count >= old->shared_max);
+ if (!fobj)
reservation_object_add_shared_inplace(obj, old, fence);
- } else
+ else
reservation_object_add_shared_replace(obj, old, fobj, fence);
}
EXPORT_SYMBOL(reservation_object_add_shared_fence);
--
2.18.0
___
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel
[PATCH v8 6/6] media: platform: Add ChromeOS EC CEC driver
The ChromeOS Embedded Controller can expose a CEC bus, this patch add the
driver for such feature of the Embedded Controller.
This driver is part of the cros-ec MFD and will be add as a sub-device when
the feature bit is exposed by the EC.
The controller will only handle a single logical address and handles
all the messages retries and will only expose Success or Error.
The controller will be tied to the HDMI CEC notifier by using the platform
DMI Data and the i915 device name and connector name.
Signed-off-by: Neil Armstrong
Reviewed-by: Enric Balletbo i Serra
Reviewed-by: Hans Verkuil
---
drivers/media/platform/Kconfig | 11 +
drivers/media/platform/Makefile | 2 +
drivers/media/platform/cros-ec-cec/Makefile | 1 +
drivers/media/platform/cros-ec-cec/cros-ec-cec.c | 347 +++
4 files changed, 361 insertions(+)
create mode 100644 drivers/media/platform/cros-ec-cec/Makefile
create mode 100644 drivers/media/platform/cros-ec-cec/cros-ec-cec.c
diff --git a/drivers/media/platform/Kconfig b/drivers/media/platform/Kconfig
index 2728376..e4fc59b 100644
--- a/drivers/media/platform/Kconfig
+++ b/drivers/media/platform/Kconfig
@@ -533,6 +533,17 @@ menuconfig CEC_PLATFORM_DRIVERS
if CEC_PLATFORM_DRIVERS
+config VIDEO_CROS_EC_CEC
+ tristate "ChromeOS EC CEC driver"
+ depends on MFD_CROS_EC || COMPILE_TEST
+ select CEC_CORE
+ select CEC_NOTIFIER
+ ---help---
+ If you say yes here you will get support for the
+ ChromeOS Embedded Controller's CEC.
+ The CEC bus is present in the HDMI connector and enables communication
+ between compatible devices.
+
config VIDEO_MESON_AO_CEC
tristate "Amlogic Meson AO CEC driver"
depends on ARCH_MESON || COMPILE_TEST
diff --git a/drivers/media/platform/Makefile b/drivers/media/platform/Makefile
index 04bc150..890f919 100644
--- a/drivers/media/platform/Makefile
+++ b/drivers/media/platform/Makefile
@@ -93,3 +93,5 @@ obj-$(CONFIG_VIDEO_QCOM_CAMSS)+=
qcom/camss-8x16/
obj-$(CONFIG_VIDEO_QCOM_VENUS) += qcom/venus/
obj-y += meson/
+
+obj-y += cros-ec-cec/
diff --git a/drivers/media/platform/cros-ec-cec/Makefile
b/drivers/media/platform/cros-ec-cec/Makefile
new file mode 100644
index 000..9ce97f9
--- /dev/null
+++ b/drivers/media/platform/cros-ec-cec/Makefile
@@ -0,0 +1 @@
+obj-$(CONFIG_VIDEO_CROS_EC_CEC) += cros-ec-cec.o
diff --git a/drivers/media/platform/cros-ec-cec/cros-ec-cec.c
b/drivers/media/platform/cros-ec-cec/cros-ec-cec.c
new file mode 100644
index 000..7bc4d8a
--- /dev/null
+++ b/drivers/media/platform/cros-ec-cec/cros-ec-cec.c
@@ -0,0 +1,347 @@
+// SPDX-License-Identifier: GPL-2.0+
+/*
+ * CEC driver for ChromeOS Embedded Controller
+ *
+ * Copyright (c) 2018 BayLibre, SAS
+ * Author: Neil Armstrong
+ */
+
+#include
+#include
+#include
+#include
+#include
+#include
+#include
+#include
+#include
+#include
+#include
+#include
+
+#define DRV_NAME "cros-ec-cec"
+
+/**
+ * struct cros_ec_cec - Driver data for EC CEC
+ *
+ * @cros_ec: Pointer to EC device
+ * @notifier: Notifier info for responding to EC events
+ * @adap: CEC adapter
+ * @notify: CEC notifier pointer
+ * @rx_msg: storage for a received message
+ */
+struct cros_ec_cec {
+ struct cros_ec_device *cros_ec;
+ struct notifier_block notifier;
+ struct cec_adapter *adap;
+ struct cec_notifier *notify;
+ struct cec_msg rx_msg;
+};
+
+static void handle_cec_message(struct cros_ec_cec *cros_ec_cec)
+{
+ struct cros_ec_device *cros_ec = cros_ec_cec->cros_ec;
+ uint8_t *cec_message = cros_ec->event_data.data.cec_message;
+ unsigned int len = cros_ec->event_size;
+
+ cros_ec_cec->rx_msg.len = len;
+ memcpy(cros_ec_cec->rx_msg.msg, cec_message, len);
+
+ cec_received_msg(cros_ec_cec->adap, _ec_cec->rx_msg);
+}
+
+static void handle_cec_event(struct cros_ec_cec *cros_ec_cec)
+{
+ struct cros_ec_device *cros_ec = cros_ec_cec->cros_ec;
+ uint32_t events = cros_ec->event_data.data.cec_events;
+
+ if (events & EC_MKBP_CEC_SEND_OK)
+ cec_transmit_attempt_done(cros_ec_cec->adap,
+ CEC_TX_STATUS_OK);
+
+ /* FW takes care of all retries, tell core to avoid more retries */
+ if (events & EC_MKBP_CEC_SEND_FAILED)
+ cec_transmit_attempt_done(cros_ec_cec->adap,
+ CEC_TX_STATUS_MAX_RETRIES |
+ CEC_TX_STATUS_NACK);
+}
+
+static int cros_ec_cec_event(struct notifier_block *nb,
+unsigned long queued_during_suspend,
+void *_notify)
+{
+ struct cros_ec_cec *cros_ec_cec;
+ struct cros_ec_device *cros_ec;
+
+ cros_ec_cec = container_of(nb, struct cros_ec_cec,
[PATCH v8 5/6] mfd: cros_ec_dev: Add CEC sub-device registration
The EC can expose a CEC bus, thus add the cros-ec-cec MFD sub-device
when the CEC feature bit is present.
Signed-off-by: Neil Armstrong
Reviewed-by: Enric Balletbo i Serra
Acked-by: Hans Verkuil
Acked-for-MFD-by: Lee Jones
---
drivers/mfd/cros_ec_dev.c | 16
1 file changed, 16 insertions(+)
diff --git a/drivers/mfd/cros_ec_dev.c b/drivers/mfd/cros_ec_dev.c
index 306e1fd..1e2049f 100644
--- a/drivers/mfd/cros_ec_dev.c
+++ b/drivers/mfd/cros_ec_dev.c
@@ -377,6 +377,10 @@ static void cros_ec_sensors_register(struct cros_ec_dev
*ec)
kfree(msg);
}
+static const struct mfd_cell cros_ec_cec_cells[] = {
+ { .name = "cros-ec-cec" }
+};
+
static const struct mfd_cell cros_ec_rtc_cells[] = {
{ .name = "cros-ec-rtc" }
};
@@ -419,6 +423,18 @@ static int ec_device_probe(struct platform_device *pdev)
if (cros_ec_check_features(ec, EC_FEATURE_MOTION_SENSE))
cros_ec_sensors_register(ec);
+ /* Check whether this EC instance has CEC host command support */
+ if (cros_ec_check_features(ec, EC_FEATURE_CEC)) {
+ retval = mfd_add_devices(ec->dev, PLATFORM_DEVID_AUTO,
+cros_ec_cec_cells,
+ARRAY_SIZE(cros_ec_cec_cells),
+NULL, 0, NULL);
+ if (retval)
+ dev_err(ec->dev,
+ "failed to add cros-ec-cec device: %d\n",
+ retval);
+ }
+
/* Check whether this EC instance has RTC host command support */
if (cros_ec_check_features(ec, EC_FEATURE_RTC)) {
retval = mfd_add_devices(ec->dev, PLATFORM_DEVID_AUTO,
--
2.7.4
___
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel
[PATCH v8 4/6] mfd: cros-ec: Introduce CEC commands and events definitions.
The EC can expose a CEC bus, this patch adds the CEC related definitions
needed by the cros-ec-cec driver.
Signed-off-by: Neil Armstrong
Tested-by: Enric Balletbo i Serra
Reviewed-by: Hans Verkuil
Acked-for-MFD-by: Lee Jones
---
include/linux/mfd/cros_ec_commands.h | 81
1 file changed, 81 insertions(+)
diff --git a/include/linux/mfd/cros_ec_commands.h
b/include/linux/mfd/cros_ec_commands.h
index c4f0caa..584e04e 100644
--- a/include/linux/mfd/cros_ec_commands.h
+++ b/include/linux/mfd/cros_ec_commands.h
@@ -804,6 +804,8 @@ enum ec_feature_code {
EC_FEATURE_MOTION_SENSE_FIFO = 24,
/* EC has RTC feature that can be controlled by host commands */
EC_FEATURE_RTC = 27,
+ /* EC supports CEC commands */
+ EC_FEATURE_CEC = 35,
};
#define EC_FEATURE_MASK_0(event_code) (1UL << (event_code % 32))
@@ -2078,6 +2080,12 @@ enum ec_mkbp_event {
/* EC sent a sysrq command */
EC_MKBP_EVENT_SYSRQ = 6,
+ /* Notify the AP that something happened on CEC */
+ EC_MKBP_EVENT_CEC_EVENT = 8,
+
+ /* Send an incoming CEC message to the AP */
+ EC_MKBP_EVENT_CEC_MESSAGE = 9,
+
/* Number of MKBP events */
EC_MKBP_EVENT_COUNT,
};
@@ -2847,6 +2855,79 @@ struct ec_params_reboot_ec {
/*/
/*
+ * HDMI CEC commands
+ *
+ * These commands are for sending and receiving message via HDMI CEC
+ */
+#define EC_MAX_CEC_MSG_LEN 16
+
+/* CEC message from the AP to be written on the CEC bus */
+#define EC_CMD_CEC_WRITE_MSG 0x00B8
+
+/**
+ * struct ec_params_cec_write - Message to write to the CEC bus
+ * @msg: message content to write to the CEC bus
+ */
+struct ec_params_cec_write {
+ uint8_t msg[EC_MAX_CEC_MSG_LEN];
+} __packed;
+
+/* Set various CEC parameters */
+#define EC_CMD_CEC_SET 0x00BA
+
+/**
+ * struct ec_params_cec_set - CEC parameters set
+ * @cmd: parameter type, can be CEC_CMD_ENABLE or CEC_CMD_LOGICAL_ADDRESS
+ * @val: in case cmd is CEC_CMD_ENABLE, this field can be 0 to disable CEC
+ * or 1 to enable CEC functionality, in case cmd is
CEC_CMD_LOGICAL_ADDRESS,
+ * this field encodes the requested logical address between 0 and 15
+ * or 0xff to unregister
+ */
+struct ec_params_cec_set {
+ uint8_t cmd; /* enum cec_command */
+ uint8_t val;
+} __packed;
+
+/* Read various CEC parameters */
+#define EC_CMD_CEC_GET 0x00BB
+
+/**
+ * struct ec_params_cec_get - CEC parameters get
+ * @cmd: parameter type, can be CEC_CMD_ENABLE or CEC_CMD_LOGICAL_ADDRESS
+ */
+struct ec_params_cec_get {
+ uint8_t cmd; /* enum cec_command */
+} __packed;
+
+/**
+ * struct ec_response_cec_get - CEC parameters get response
+ * @val: in case cmd was CEC_CMD_ENABLE, this field will 0 if CEC is
+ * disabled or 1 if CEC functionality is enabled,
+ * in case cmd was CEC_CMD_LOGICAL_ADDRESS, this will encode the
+ * configured logical address between 0 and 15 or 0xff if unregistered
+ */
+struct ec_response_cec_get {
+ uint8_t val;
+} __packed;
+
+/* CEC parameters command */
+enum ec_cec_command {
+ /* CEC reading, writing and events enable */
+ CEC_CMD_ENABLE,
+ /* CEC logical address */
+ CEC_CMD_LOGICAL_ADDRESS,
+};
+
+/* Events from CEC to AP */
+enum mkbp_cec_event {
+ /* Outgoing message was acknowledged by a follower */
+ EC_MKBP_CEC_SEND_OK = BIT(0),
+ /* Outgoing message was not acknowledged */
+ EC_MKBP_CEC_SEND_FAILED = BIT(1),
+};
+
+/*/
+/*
* Special commands
*
* These do not follow the normal rules for commands. See each command for
--
2.7.4
___
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel
[PATCH v8 3/6] mfd: cros-ec: Increase maximum mkbp event size
Having a 16 byte mkbp event size makes it possible to send CEC
messages from the EC to the AP directly inside the mkbp event
instead of first doing a notification and then a read.
Signed-off-by: Stefan Adolfsson
Signed-off-by: Neil Armstrong
Tested-by: Enric Balletbo i Serra
Acked-by: Hans Verkuil
---
drivers/platform/chrome/cros_ec_proto.c | 40 +
include/linux/mfd/cros_ec.h | 2 +-
include/linux/mfd/cros_ec_commands.h| 16 +
3 files changed, 48 insertions(+), 10 deletions(-)
diff --git a/drivers/platform/chrome/cros_ec_proto.c
b/drivers/platform/chrome/cros_ec_proto.c
index 8350ca2..398393a 100644
--- a/drivers/platform/chrome/cros_ec_proto.c
+++ b/drivers/platform/chrome/cros_ec_proto.c
@@ -506,10 +506,31 @@ int cros_ec_cmd_xfer_status(struct cros_ec_device *ec_dev,
}
EXPORT_SYMBOL(cros_ec_cmd_xfer_status);
+static int get_next_event_xfer(struct cros_ec_device *ec_dev,
+ struct cros_ec_command *msg,
+ int version, uint32_t size)
+{
+ int ret;
+
+ msg->version = version;
+ msg->command = EC_CMD_GET_NEXT_EVENT;
+ msg->insize = size;
+ msg->outsize = 0;
+
+ ret = cros_ec_cmd_xfer(ec_dev, msg);
+ if (ret > 0) {
+ ec_dev->event_size = ret - 1;
+ memcpy(_dev->event_data, msg->data, ec_dev->event_size);
+ }
+
+ return ret;
+}
+
static int get_next_event(struct cros_ec_device *ec_dev)
{
u8 buffer[sizeof(struct cros_ec_command) + sizeof(ec_dev->event_data)];
struct cros_ec_command *msg = (struct cros_ec_command *)
+ static int cmd_version = 1;
int ret;
if (ec_dev->suspended) {
@@ -517,18 +538,19 @@ static int get_next_event(struct cros_ec_device *ec_dev)
return -EHOSTDOWN;
}
- msg->version = 0;
- msg->command = EC_CMD_GET_NEXT_EVENT;
- msg->insize = sizeof(ec_dev->event_data);
- msg->outsize = 0;
+ if (cmd_version == 1) {
+ ret = get_next_event_xfer(ec_dev, msg, cmd_version,
+ sizeof(struct ec_response_get_next_event_v1));
+ if (ret < 0 || msg->result != EC_RES_INVALID_VERSION)
+ return ret;
- ret = cros_ec_cmd_xfer(ec_dev, msg);
- if (ret > 0) {
- ec_dev->event_size = ret - 1;
- memcpy(_dev->event_data, msg->data,
- sizeof(ec_dev->event_data));
+ /* Fallback to version 0 for future send attempts */
+ cmd_version = 0;
}
+ ret = get_next_event_xfer(ec_dev, msg, cmd_version,
+ sizeof(struct ec_response_get_next_event));
+
return ret;
}
diff --git a/include/linux/mfd/cros_ec.h b/include/linux/mfd/cros_ec.h
index 32421df..20949dd 100644
--- a/include/linux/mfd/cros_ec.h
+++ b/include/linux/mfd/cros_ec.h
@@ -147,7 +147,7 @@ struct cros_ec_device {
bool mkbp_event_supported;
struct blocking_notifier_head event_notifier;
- struct ec_response_get_next_event event_data;
+ struct ec_response_get_next_event_v1 event_data;
int event_size;
u32 host_event_wake_mask;
};
diff --git a/include/linux/mfd/cros_ec_commands.h
b/include/linux/mfd/cros_ec_commands.h
index f2edd99..c4f0caa 100644
--- a/include/linux/mfd/cros_ec_commands.h
+++ b/include/linux/mfd/cros_ec_commands.h
@@ -2093,12 +2093,28 @@ union ec_response_get_next_data {
uint32_t sysrq;
} __packed;
+union ec_response_get_next_data_v1 {
+ uint8_t key_matrix[16];
+ uint32_t host_event;
+ uint32_t buttons;
+ uint32_t switches;
+ uint32_t sysrq;
+ uint32_t cec_events;
+ uint8_t cec_message[16];
+} __packed;
+
struct ec_response_get_next_event {
uint8_t event_type;
/* Followed by event data if any */
union ec_response_get_next_data data;
} __packed;
+struct ec_response_get_next_event_v1 {
+ uint8_t event_type;
+ /* Followed by event data if any */
+ union ec_response_get_next_data_v1 data;
+} __packed;
+
/* Bit indices for buttons and switches.*/
/* Buttons */
#define EC_MKBP_POWER_BUTTON 0
--
2.7.4
___
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel
[PATCH v8 2/6] drm/i915: hdmi: add CEC notifier to intel_hdmi
This patchs adds the cec_notifier feature to the intel_hdmi part
of the i915 DRM driver. It uses the HDMI DRM connector name to differentiate
between each HDMI ports.
The changes will allow the i915 HDMI code to notify EDID and HPD changes
to an eventual CEC adapter.
Signed-off-by: Neil Armstrong
Reviewed-by: Hans Verkuil
Reviewed-by: Ville Syrjälä
Acked-by: Rodrigo Vivi
---
drivers/gpu/drm/i915/Kconfig | 1 +
drivers/gpu/drm/i915/intel_display.h | 24
drivers/gpu/drm/i915/intel_drv.h | 2 ++
drivers/gpu/drm/i915/intel_hdmi.c| 13 +
4 files changed, 40 insertions(+)
diff --git a/drivers/gpu/drm/i915/Kconfig b/drivers/gpu/drm/i915/Kconfig
index dfd9588..2d65d56 100644
--- a/drivers/gpu/drm/i915/Kconfig
+++ b/drivers/gpu/drm/i915/Kconfig
@@ -23,6 +23,7 @@ config DRM_I915
select SYNC_FILE
select IOSF_MBI
select CRC32
+ select CEC_CORE if CEC_NOTIFIER
help
Choose this option if you have a system that has "Intel Graphics
Media Accelerator" or "HD Graphics" integrated graphics,
diff --git a/drivers/gpu/drm/i915/intel_display.h
b/drivers/gpu/drm/i915/intel_display.h
index 2ef3161..1f176a71 100644
--- a/drivers/gpu/drm/i915/intel_display.h
+++ b/drivers/gpu/drm/i915/intel_display.h
@@ -126,6 +126,30 @@ enum port {
#define port_name(p) ((p) + 'A')
+/*
+ * Ports identifier referenced from other drivers.
+ * Expected to remain stable over time
+ */
+static inline const char *port_identifier(enum port port)
+{
+ switch (port) {
+ case PORT_A:
+ return "Port A";
+ case PORT_B:
+ return "Port B";
+ case PORT_C:
+ return "Port C";
+ case PORT_D:
+ return "Port D";
+ case PORT_E:
+ return "Port E";
+ case PORT_F:
+ return "Port F";
+ default:
+ return "";
+ }
+}
+
enum dpio_channel {
DPIO_CH0,
DPIO_CH1
diff --git a/drivers/gpu/drm/i915/intel_drv.h b/drivers/gpu/drm/i915/intel_drv.h
index 0361130..cfbeee1 100644
--- a/drivers/gpu/drm/i915/intel_drv.h
+++ b/drivers/gpu/drm/i915/intel_drv.h
@@ -39,6 +39,7 @@
#include
#include
#include
+#include
/**
* __wait_for - magic wait macro
@@ -1017,6 +1018,7 @@ struct intel_hdmi {
bool has_audio;
bool rgb_quant_range_selectable;
struct intel_connector *attached_connector;
+ struct cec_notifier *cec_notifier;
};
struct intel_dp_mst_encoder;
diff --git a/drivers/gpu/drm/i915/intel_hdmi.c
b/drivers/gpu/drm/i915/intel_hdmi.c
index ee929f3..c21b7dd 100644
--- a/drivers/gpu/drm/i915/intel_hdmi.c
+++ b/drivers/gpu/drm/i915/intel_hdmi.c
@@ -1868,6 +1868,8 @@ intel_hdmi_set_edid(struct drm_connector *connector)
connected = true;
}
+ cec_notifier_set_phys_addr_from_edid(intel_hdmi->cec_notifier, edid);
+
return connected;
}
@@ -1876,6 +1878,7 @@ intel_hdmi_detect(struct drm_connector *connector, bool
force)
{
enum drm_connector_status status;
struct drm_i915_private *dev_priv = to_i915(connector->dev);
+ struct intel_hdmi *intel_hdmi = intel_attached_hdmi(connector);
DRM_DEBUG_KMS("[CONNECTOR:%d:%s]\n",
connector->base.id, connector->name);
@@ -1891,6 +1894,9 @@ intel_hdmi_detect(struct drm_connector *connector, bool
force)
intel_display_power_put(dev_priv, POWER_DOMAIN_GMBUS);
+ if (status != connector_status_connected)
+ cec_notifier_phys_addr_invalidate(intel_hdmi->cec_notifier);
+
return status;
}
@@ -2031,6 +2037,8 @@ static void chv_hdmi_pre_enable(struct intel_encoder
*encoder,
static void intel_hdmi_destroy(struct drm_connector *connector)
{
+ if (intel_attached_hdmi(connector)->cec_notifier)
+ cec_notifier_put(intel_attached_hdmi(connector)->cec_notifier);
kfree(to_intel_connector(connector)->detect_edid);
drm_connector_cleanup(connector);
kfree(connector);
@@ -2350,6 +2358,11 @@ void intel_hdmi_init_connector(struct intel_digital_port
*intel_dig_port,
u32 temp = I915_READ(PEG_BAND_GAP_DATA);
I915_WRITE(PEG_BAND_GAP_DATA, (temp & ~0xf) | 0xd);
}
+
+ intel_hdmi->cec_notifier = cec_notifier_get_conn(dev->dev,
+port_identifier(port));
+ if (!intel_hdmi->cec_notifier)
+ DRM_DEBUG_KMS("CEC notifier get failed\n");
}
void intel_hdmi_init(struct drm_i915_private *dev_priv,
--
2.7.4
___
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel
[PATCH v8 1/6] media: cec-notifier: Get notifier by device and connector name
In non device-tree world, we can need to get the notifier by the driver
name directly and eventually defer probe if not yet created.
This patch adds a variant of the get function by using the device name
instead and will not create a notifier if not yet created.
But the i915 driver exposes at least 2 HDMI connectors, this patch also
adds the possibility to add a connector name tied to the notifier device
to form a tuple and associate different CEC controllers for each HDMI
connectors.
Signed-off-by: Neil Armstrong
Reviewed-by: Hans Verkuil
---
drivers/media/cec/cec-notifier.c | 11 ---
include/media/cec-notifier.h | 27 ---
2 files changed, 32 insertions(+), 6 deletions(-)
diff --git a/drivers/media/cec/cec-notifier.c b/drivers/media/cec/cec-notifier.c
index 16dffa0..dd2078b 100644
--- a/drivers/media/cec/cec-notifier.c
+++ b/drivers/media/cec/cec-notifier.c
@@ -21,6 +21,7 @@ struct cec_notifier {
struct list_head head;
struct kref kref;
struct device *dev;
+ const char *conn;
struct cec_adapter *cec_adap;
void (*callback)(struct cec_adapter *adap, u16 pa);
@@ -30,13 +31,14 @@ struct cec_notifier {
static LIST_HEAD(cec_notifiers);
static DEFINE_MUTEX(cec_notifiers_lock);
-struct cec_notifier *cec_notifier_get(struct device *dev)
+struct cec_notifier *cec_notifier_get_conn(struct device *dev, const char
*conn)
{
struct cec_notifier *n;
mutex_lock(_notifiers_lock);
list_for_each_entry(n, _notifiers, head) {
- if (n->dev == dev) {
+ if (n->dev == dev &&
+ (!conn || !strcmp(n->conn, conn))) {
kref_get(>kref);
mutex_unlock(_notifiers_lock);
return n;
@@ -46,6 +48,8 @@ struct cec_notifier *cec_notifier_get(struct device *dev)
if (!n)
goto unlock;
n->dev = dev;
+ if (conn)
+ n->conn = kstrdup(conn, GFP_KERNEL);
n->phys_addr = CEC_PHYS_ADDR_INVALID;
mutex_init(>lock);
kref_init(>kref);
@@ -54,7 +58,7 @@ struct cec_notifier *cec_notifier_get(struct device *dev)
mutex_unlock(_notifiers_lock);
return n;
}
-EXPORT_SYMBOL_GPL(cec_notifier_get);
+EXPORT_SYMBOL_GPL(cec_notifier_get_conn);
static void cec_notifier_release(struct kref *kref)
{
@@ -62,6 +66,7 @@ static void cec_notifier_release(struct kref *kref)
container_of(kref, struct cec_notifier, kref);
list_del(>head);
+ kfree(n->conn);
kfree(n);
}
diff --git a/include/media/cec-notifier.h b/include/media/cec-notifier.h
index cf0add7..814eeef 100644
--- a/include/media/cec-notifier.h
+++ b/include/media/cec-notifier.h
@@ -20,8 +20,10 @@ struct cec_notifier;
#if IS_REACHABLE(CONFIG_CEC_CORE) && IS_ENABLED(CONFIG_CEC_NOTIFIER)
/**
- * cec_notifier_get - find or create a new cec_notifier for the given device.
+ * cec_notifier_get_conn - find or create a new cec_notifier for the given
+ * device and connector tuple.
* @dev: device that sends the events.
+ * @conn: the connector name from which the event occurs
*
* If a notifier for device @dev already exists, then increase the refcount
* and return that notifier.
@@ -31,7 +33,8 @@ struct cec_notifier;
*
* Return NULL if the memory could not be allocated.
*/
-struct cec_notifier *cec_notifier_get(struct device *dev);
+struct cec_notifier *cec_notifier_get_conn(struct device *dev,
+ const char *conn);
/**
* cec_notifier_put - decrease refcount and delete when the refcount reaches 0.
@@ -85,7 +88,8 @@ void cec_register_cec_notifier(struct cec_adapter *adap,
struct cec_notifier *notifier);
#else
-static inline struct cec_notifier *cec_notifier_get(struct device *dev)
+static inline struct cec_notifier *cec_notifier_get_conn(struct device *dev,
+const char *conn)
{
/* A non-NULL pointer is expected on success */
return (struct cec_notifier *)0xdeadfeed;
@@ -121,6 +125,23 @@ static inline void cec_register_cec_notifier(struct
cec_adapter *adap,
#endif
/**
+ * cec_notifier_get - find or create a new cec_notifier for the given device.
+ * @dev: device that sends the events.
+ *
+ * If a notifier for device @dev already exists, then increase the refcount
+ * and return that notifier.
+ *
+ * If it doesn't exist, then allocate a new notifier struct and return a
+ * pointer to that new struct.
+ *
+ * Return NULL if the memory could not be allocated.
+ */
+static inline struct cec_notifier *cec_notifier_get(struct device *dev)
+{
+ return cec_notifier_get_conn(dev, NULL);
+}
+
+/**
* cec_notifier_phys_addr_invalidate() - set the physical address to INVALID
*
* @n: the CEC notifier
--
2.7.4
___
dri-devel mailing list
[PATCH v8 0/6] Add ChromeOS EC CEC Support
Hi All, The new Google "Fizz" Intel-based ChromeOS device is gaining CEC support through it's Embedded Controller, to enable the Linux CEC Core to communicate with it and get the CEC Physical Address from the correct HDMI Connector, the following must be added/changed: - Add the CEC sub-device registration in the ChromeOS EC MFD Driver - Add the CEC related commands and events definitions into the EC MFD driver - Add a way to get a CEC notifier with it's (optional) connector name - Add the CEC notifier to the i915 HDMI driver - Add the proper ChromeOS EC CEC Driver The CEC notifier with the connector name is the tricky point, since even on Device-Tree platforms, there is no way to distinguish between multiple HDMI connectors from the same DRM driver. The solution I implemented is pretty simple and only adds an optional connector name to eventually distinguish an HDMI connector notifier from another if they share the same device. Feel free to comment this patchset ! Changes since v7: - rebased on v4.18-rc1 - Fixed whitespace issues on patch 3 - Added Lee's tags Changes since v6: - Added stable identifier comment in intel_display.h - Renamed to cec_notifier in intel_hdmi.c/intel_drv.h - Added Acked-by/Reviewed-By tags Changes since v5: - Small fixups on include/linux/mfd/cros_ec_commands.h - Fixed on cros-ec-cec driver accordingly - Added Reviewed-By tags Changes since v4: - Split patch 3 to move the mkbp event size change into a separate patch Changes since v3 (incorrectly reported as v2): - Renamed "Chrome OS" to "ChromeOS" - Updated cros_ec_commands.h new structs definitions to kernel doc format - Added Reviewed-By tags Changes since v2: - Add i915 port_identifier() and use this stable name as cec_notifier conn name - Fixed and cleaned up the CEC commands and events handling - Rebased the CEC sub-device registration on top of Enric's serie - Fixed comments typo on cec driver - Protected the DMI match only with PCI and DMI Kconfigs Changes since v1: - Added cec_notifier_put to intel_hdmi - Fixed all small reported issues on the EC CEC driver - Moved the cec_notifier_get out of the #if .. #else .. #endif Changes since RFC: - Moved CEC sub-device registration after CEC commands and events definitions patch - Removed get_notifier_get_byname - Added CEC_CORE select into i915 Kconfig - Removed CEC driver fallback if notifier is not configured on HW, added explicit warn - Fixed CEC core return type on error - Moved to cros-ec-cec media platform directory - Use bus_find_device() to find the pci i915 device instead of get_notifier_get_byname() - Fix Logical Address setup - Added comment about HW support - Removed memset of msg structures Neil Armstrong (6): media: cec-notifier: Get notifier by device and connector name drm/i915: hdmi: add CEC notifier to intel_hdmi mfd: cros-ec: Increase maximum mkbp event size mfd: cros-ec: Introduce CEC commands and events definitions. mfd: cros_ec_dev: Add CEC sub-device registration media: platform: Add ChromeOS EC CEC driver drivers/gpu/drm/i915/Kconfig | 1 + drivers/gpu/drm/i915/intel_display.h | 24 ++ drivers/gpu/drm/i915/intel_drv.h | 2 + drivers/gpu/drm/i915/intel_hdmi.c| 13 + drivers/media/cec/cec-notifier.c | 11 +- drivers/media/platform/Kconfig | 11 + drivers/media/platform/Makefile | 2 + drivers/media/platform/cros-ec-cec/Makefile | 1 + drivers/media/platform/cros-ec-cec/cros-ec-cec.c | 347 +++ drivers/mfd/cros_ec_dev.c| 16 ++ drivers/platform/chrome/cros_ec_proto.c | 40 ++- include/linux/mfd/cros_ec.h | 2 +- include/linux/mfd/cros_ec_commands.h | 97 +++ include/media/cec-notifier.h | 27 +- 14 files changed, 578 insertions(+), 16 deletions(-) create mode 100644 drivers/media/platform/cros-ec-cec/Makefile create mode 100644 drivers/media/platform/cros-ec-cec/cros-ec-cec.c -- 2.7.4 ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [PATCH] fb: fix lost console when the user unplugs a USB adapter
On Tuesday, July 03, 2018 01:18:57 PM Mikulas Patocka wrote: > > On Tue, 3 Jul 2018, Bartlomiej Zolnierkiewicz wrote: > > > > > Hi, > > > > On Sunday, June 03, 2018 11:46:29 AM Mikulas Patocka wrote: > > > I have a USB display adapter using the udlfb driver and I use it on an ARM > > > board that doesn't have any graphics card. When I plug the adapter in, the > > > console is properly displayed, however when I unplug and re-plug the > > > adapter, the console is not displayed and I can't access it until I reboot > > > the board. > > > > > > The reason is this: > > > When the adapter is unplugged, dlfb_usb_disconnect calls > > > unlink_framebuffer, then it waits until the reference count drops to zero > > > and then it deallocates the framebuffer. However, the console that is > > > attached to the framebuffer device keeps the reference count non-zero, so > > > the framebuffer device is never destroyed. When the USB adapter is plugged > > > again, it creates a new device /dev/fb1 and the console is not attached to > > > it. > > > > > > This patch fixes the bug by unbinding the console from unlink_framebuffer. > > > The code to unbind the console is moved from do_unregister_framebuffer to > > > a function unbind_console. When the console is unbound, the reference > > > count drops to zero and the udlfb driver frees the framebuffer. When the > > > adapter is plugged back, a new framebuffer is created and the console is > > > attached to it. > > > > > > Signed-off-by: Mikulas Patocka > > > Cc: sta...@vger.kernel.org > > > > After this change unbind_console() will be called twice in the standard > > framebuffer unregister path: > > > > - first time, directly by do_unregister_framebuffer() > > > > - second time, indirectly by > > do_unregister_framebuffer()->unlink_framebuffer() > > > > This doesn't look correctly. > > unbind_console calls the FB_EVENT_FB_UNBIND notifier, FB_EVENT_FB_UNBIND > goes to the function fbcon_fb_unbind and fbcon_fb_unbind checks if the > console is bound to the framebuffer for which unbind is requested. So a > double call won't cause any trouble. Even if it works okay currently it is not a best design to send duplicate events - especially since this can be easily avoided (for non-udlfb users) by: - renaming "vanilla" unlink_framebuffer() to __unlink_framebuffer() - converting do_unregister_framebuffer() to use __unlink_framebuffer() - adding "new" unlink_framebuffer() that will also call unbind_console() > > Also why can't udlfb just use unregister_framebuffer() like all other > > drivers (it uses unlink_framebuffer() and it is the only user of this > > helper)? > > It uses unregister_framebuffer() - but - unregister_framebuffer() may only > be called when the open count of the framebuffer is zero. So, the udlfb > driver waits until the open count drops to zero and then calls > unregister_framebuffer(). > > But the console subsystem keeps the framebuffer open - which means that if > user use unplugs the USB adapter, the open count won't drop to zero > (because the console is bound to it) - which means that > unregister_framebuffer() will not be called. Is it a really the console subsystem and not the user-space keeping /dev/fb0 (with console binded to fb0) opened after the USB device vanishes? After re-plugging the USB device /dev/fb0 stays and /dev/fb1 appears, right? I also mean that unregister_framebuffer() should be called instead unlink_framebuffer(), not additionally some time later as it is done currently. Moreover the dlfb <-> fb_info locking scheme seems to be reversed (+racy) as it is dlfb that should control lifetime of fb_info, then in dlfb_free() we should just call framebuffer_release() etc. BTW comment in dlfb_ops_release(): /* We can't free fb_info here - fbmem will touch it when we return */ seems to be wrong as fbmem keeps an extra reference on fb_info during ->fb_release(). > You must unbind the console before calling unregister_framebuffer(). The Hmm? The first thing that [do_]unregister_framebuffer) does seems to be unbinding the console. > PCI framebuffer drivers don't have this problem because the user is not > expected to just unplug the PCI card while it is being used by the > console. PCI framebuffer drivers currently don't use .suppress_bind_attrs driver flag so the PCI devices can be unbinded at any time by using sysfs "unbind" functionality (I guess we should be using .suppress_bind_attrs flag if it doesn't work currently). > Mikulas > > > > --- > > > drivers/video/fbdev/core/fbmem.c | 21 + > > > 1 file changed, 17 insertions(+), 4 deletions(-) > > > > > > Index: linux-4.16.12/drivers/video/fbdev/core/fbmem.c > > > === > > > --- linux-4.16.12.orig/drivers/video/fbdev/core/fbmem.c 2018-05-26 > > > 06:13:20.0 +0200 > > > +++ linux-4.16.12/drivers/video/fbdev/core/fbmem.c2018-05-26 > > > 06:13:20.0
[Bug 106306] amdgpu CIK power management issues (overdrive and wattman)
https://bugs.freedesktop.org/show_bug.cgi?id=106306 --- Comment #4 from gr...@sub.red --- > wattman functionality doesn't work at all; > pp_od_clk_voltage prints nothing and doesn't accept anything. Is wattman > even implemented for CI? @Alex Deucher: Currently on 4.17.3 w/ dpm and the situation hasn't changed. Is the wattman functionality supposed to work with CIK or not? Do you need any additional info? -- You are receiving this mail because: You are the assignee for the bug.___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [PATCH] drm/bridge: adv7511: Reset registers on hotplug
On Wed, Jul 4, 2018 at 10:15 AM, Daniel Vetter wrote:
> On Wed, Jul 4, 2018 at 4:06 PM, Rob Clark wrote:
>> On Wed, Jul 4, 2018 at 9:23 AM, Archit Taneja wrote:
>>>
>>>
>>> On Wednesday 04 July 2018 12:28 AM, Rob Clark wrote:
On Tue, Jul 3, 2018 at 12:56 PM, Sean Paul wrote:
>
> The bridge loses its hw state when the cable is unplugged. If we detect
> this case in the hpd handler, reset its state.
>
> Reported-by: Rob Clark
> Signed-off-by: Sean Paul
Tested-by: Rob Clark
> ---
> This is the follow-up to "drm/msm: Disable mdp5 crtc when there are no
> active planes". I incorrectly assumed the modeset was required from the
> msm side, but Rob pointed out that he thought it might be a bridge
> issue.
To elaborate, this is an atomic userspace (weston), when it sees the
display disconnected, it removes the planes from the CRTC but does not
disable the CRTC. So drm/msm sets up the LM to scanout a solid color,
and leaves the encoder and dsi cranking along happily. When
replugging the display, the atomic helpers see the CRTC is still
active and (correctly) doesn't do a full modeset. So the bridge is
not re-configured/re-enabled.
>>>
>>>
>>> The adv7511 connector's detect() op should have re-configured the
>>> registers. I'm assuming it was never called after the cable is
>>> plugged in again in the wetson usecase?
>>>
>>> With this patch, in the case of fb emulation/X, I'm wondering if
>>> we will end up trying to re-enable ADV7511 twice. First, in the new code
>>> in adv7511_hpd_work(), and later in adv7511_detect() (i.e, the
>>> connector's detect op).
>>>
>>
>> jfwiw, fbcon and things using legacy SETCRTC end up triggering a full
>> modeset, which somehow papers over the issue (because we go thru the
>> bridge disable/enable sequence). So now there probably is a double
>> power cycle sequence, although it doesn't seem to be causing any harm
>> (fbcon and x11 still seem happy)
>
> Hm that's strange. Legacy setcrtc on atomic drivers should do the
> exact same opportunistic modeset avoidance as an atomic flip. The only
> difference is around the automagic resetting of the link_status, but
> msm doesn't use that. I suspect the real reason here is that fbcon and
> X aren't broken, and properly shut down the disconnected output before
> trying to re-enable it. This shouldn't have anything to do with atomic
> vs. legacy ioctl paths.
yes, probably it is because they shut down the CRTC on the disconnect,
but userspace using ATOMIC ioctl has to do that explicitly (and weston
does not). So the only connection to ATOMIC vs legacy ioctl paths is
that atomic lets userspace not shut down the CRTC.
Fwiw, I did have a patch to make adv75xx use the link_status (it is
really the bridge, not msm, which should be doing this, since it is
100% about the bridge). But since that relies on userspace support
(which weston does not have yet), and Sean's approach actually seems
to work out ok, I kinda like his approach better.
BR,
-R
> Also, in-kernel fbcon is 100% atomic nowadays, it doesn't use any of
> the legacy entry points anymore.
> -Daniel
>
>>
>> BR,
>> -R
>>
>>> I'm guessing the 'hpd' variable in the check in adv7511_detect() below
>>> should ideally be false, and avoid us trying to configure the registers
>>> again, but I'm not entirely sure.
>>>
>>> if (status == connector_status_connected && hpd && adv7511->powered) {
>>> regcache_mark_dirty(adv7511->regmap);
>>> ...
>>>
>>> In any case:
>>>
>>> Reviewed-by: Archit Taneja
>>>
Arguably this perhaps isn't what weston *wanted* to do, but in the end
the bug is in the bridge.
We could possibly set the connector link_status to BAD as an
alternative. But at least the build of weston I'm using atm doesn't
handle the link_status propery, this approach requires each atomic
userspace to handle this case. Compared to Sean's approach which is
transparent to userspace, which seems attractive.
BR,
-R
>
> drivers/gpu/drm/bridge/adv7511/adv7511_drv.c | 12
> 1 file changed, 12 insertions(+)
>
> diff --git a/drivers/gpu/drm/bridge/adv7511/adv7511_drv.c
> b/drivers/gpu/drm/bridge/adv7511/adv7511_drv.c
> index 73021b388e12..dd3ff2f2cdce 100644
> --- a/drivers/gpu/drm/bridge/adv7511/adv7511_drv.c
> +++ b/drivers/gpu/drm/bridge/adv7511/adv7511_drv.c
> @@ -429,6 +429,18 @@ static void adv7511_hpd_work(struct work_struct
> *work)
> else
> status = connector_status_disconnected;
>
> + /*
> +* The bridge resets its registers on unplug. So when we get a
> plug
> +* event and we're already supposed to be powered, cycle the
> bridge to
> +* restore its state.
> +*/
> + if (status == connector_status_connected &&
>
Re: [PATCH] fb: fix lost console when the user unplugs a USB adapter
On Wed, 4 Jul 2018, Daniel Vetter wrote: > On Sun, Jun 03, 2018 at 11:46:29AM -0400, Mikulas Patocka wrote: > > I have a USB display adapter using the udlfb driver and I use it on an ARM > > board that doesn't have any graphics card. When I plug the adapter in, the > > console is properly displayed, however when I unplug and re-plug the > > adapter, the console is not displayed and I can't access it until I reboot > > the board. > > > > The reason is this: > > When the adapter is unplugged, dlfb_usb_disconnect calls > > unlink_framebuffer, then it waits until the reference count drops to zero > > and then it deallocates the framebuffer. However, the console that is > > attached to the framebuffer device keeps the reference count non-zero, so > > the framebuffer device is never destroyed. When the USB adapter is plugged > > again, it creates a new device /dev/fb1 and the console is not attached to > > it. > > > > This patch fixes the bug by unbinding the console from unlink_framebuffer. > > The code to unbind the console is moved from do_unregister_framebuffer to > > a function unbind_console. When the console is unbound, the reference > > count drops to zero and the udlfb driver frees the framebuffer. When the > > adapter is plugged back, a new framebuffer is created and the console is > > attached to it. > > > > Signed-off-by: Mikulas Patocka > > Cc: sta...@vger.kernel.org > > Does this work correctly with the udl drm driver and the drm fbdev > emulation? If yes I'm not sure what the value is in fixing up the uldfb > driver really ... > > Same for all the uldfb fixes in your other series. If the 2 drivers are > on feature parity I'd just go ahead and remove the uldfb one. > -Daniel The udl drm driver is worse than udlfb with regard to unplug. The udl drm driver destroys the device on USB unplug no matter if someone is using it or not. If you unplug the device while Xserver is running or while some console framebuffer application is running, you get a crash. The udl fb driver correctly waits until all users close the device before destroying it. Mikulas ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
[Bug 106168] kernel NULL pointer dereference
https://bugs.freedesktop.org/show_bug.cgi?id=106168 gr...@sub.red changed: What|Removed |Added Status|NEW |RESOLVED Resolution|--- |FIXED --- Comment #1 from gr...@sub.red --- doesn't happen anymore -- You are receiving this mail because: You are the assignee for the bug.___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [PATCH] drm/amd/display: off by one in find_irq_source_info()
On 2018-07-04 05:46 AM, Dan Carpenter wrote:
> The ->info[] array has DAL_IRQ_SOURCES_NUMBER elements so this condition
> should be >= instead of > or we could read one element beyond the end of
> the array.
>
> Fixes: 4562236b3bc0 ("drm/amd/dc: Add dc display driver (v2)")
> Signed-off-by: Dan Carpenter
Reviewed-by: Harry Wentland
Harry
>
> diff --git a/drivers/gpu/drm/amd/display/dc/irq/irq_service.c
> b/drivers/gpu/drm/amd/display/dc/irq/irq_service.c
> index dcdfa0f01551..604bea01fc13 100644
> --- a/drivers/gpu/drm/amd/display/dc/irq/irq_service.c
> +++ b/drivers/gpu/drm/amd/display/dc/irq/irq_service.c
> @@ -78,7 +78,7 @@ const struct irq_source_info *find_irq_source_info(
> struct irq_service *irq_service,
> enum dc_irq_source source)
> {
> - if (source > DAL_IRQ_SOURCES_NUMBER || source < DC_IRQ_SOURCE_INVALID)
> + if (source >= DAL_IRQ_SOURCES_NUMBER || source < DC_IRQ_SOURCE_INVALID)
> return NULL;
>
> return _service->info[source];
>
___
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [PATCH 3/3] drm/bridge/sii8620: Fix link mode selection
On 28.06.2018 18:44, Maciej Purski wrote:
> Current link mode values do not allow to enable packed pixel modes.
>
> Select packed pixel clock mode, if needed, every time the link mode
> register gets updated.
>
> Signed-off-by: Maciej Purski
Queued all three patches to drm-misc-fixes.
Regards
Andrzej
> ---
> drivers/gpu/drm/bridge/sil-sii8620.c | 30 --
> 1 file changed, 20 insertions(+), 10 deletions(-)
>
> diff --git a/drivers/gpu/drm/bridge/sil-sii8620.c
> b/drivers/gpu/drm/bridge/sil-sii8620.c
> index 16fe7ea..a6e8f45 100644
> --- a/drivers/gpu/drm/bridge/sil-sii8620.c
> +++ b/drivers/gpu/drm/bridge/sil-sii8620.c
> @@ -1165,8 +1165,14 @@ static void sii8620_start_video(struct sii8620 *ctx)
> sii8620_set_format(ctx);
>
> if (!sii8620_is_mhl3(ctx)) {
> - sii8620_mt_write_stat(ctx, MHL_DST_REG(LINK_MODE),
> - MHL_DST_LM_CLK_MODE_NORMAL | MHL_DST_LM_PATH_ENABLED);
> + u8 link_mode = MHL_DST_LM_PATH_ENABLED;
> +
> + if (ctx->use_packed_pixel)
> + link_mode |= MHL_DST_LM_CLK_MODE_PACKED_PIXEL;
> + else
> + link_mode |= MHL_DST_LM_CLK_MODE_NORMAL;
> +
> + sii8620_mt_write_stat(ctx, MHL_DST_REG(LINK_MODE), link_mode);
> sii8620_set_auto_zone(ctx);
> } else {
> static const struct {
> @@ -1677,14 +1683,18 @@ static void sii8620_status_dcap_ready(struct sii8620
> *ctx)
>
> static void sii8620_status_changed_path(struct sii8620 *ctx)
> {
> - if (ctx->stat[MHL_DST_LINK_MODE] & MHL_DST_LM_PATH_ENABLED) {
> - sii8620_mt_write_stat(ctx, MHL_DST_REG(LINK_MODE),
> - MHL_DST_LM_CLK_MODE_NORMAL
> - | MHL_DST_LM_PATH_ENABLED);
> - } else {
> - sii8620_mt_write_stat(ctx, MHL_DST_REG(LINK_MODE),
> - MHL_DST_LM_CLK_MODE_NORMAL);
> - }
> + u8 link_mode;
> +
> + if (ctx->use_packed_pixel)
> + link_mode = MHL_DST_LM_CLK_MODE_PACKED_PIXEL;
> + else
> + link_mode = MHL_DST_LM_CLK_MODE_NORMAL;
> +
> + if (ctx->stat[MHL_DST_LINK_MODE] & MHL_DST_LM_PATH_ENABLED)
> + link_mode |= MHL_DST_LM_PATH_ENABLED;
> +
> + sii8620_mt_write_stat(ctx, MHL_DST_REG(LINK_MODE),
> + link_mode);
> }
>
> static void sii8620_msc_mr_write_stat(struct sii8620 *ctx)
___
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [PATCH] drm/bridge: adv7511: Reset registers on hotplug
On Wed, Jul 4, 2018 at 4:06 PM, Rob Clark wrote:
> On Wed, Jul 4, 2018 at 9:23 AM, Archit Taneja wrote:
>>
>>
>> On Wednesday 04 July 2018 12:28 AM, Rob Clark wrote:
>>>
>>> On Tue, Jul 3, 2018 at 12:56 PM, Sean Paul wrote:
The bridge loses its hw state when the cable is unplugged. If we detect
this case in the hpd handler, reset its state.
Reported-by: Rob Clark
Signed-off-by: Sean Paul
>>>
>>>
>>> Tested-by: Rob Clark
>>>
---
This is the follow-up to "drm/msm: Disable mdp5 crtc when there are no
active planes". I incorrectly assumed the modeset was required from the
msm side, but Rob pointed out that he thought it might be a bridge
issue.
>>>
>>>
>>> To elaborate, this is an atomic userspace (weston), when it sees the
>>> display disconnected, it removes the planes from the CRTC but does not
>>> disable the CRTC. So drm/msm sets up the LM to scanout a solid color,
>>> and leaves the encoder and dsi cranking along happily. When
>>> replugging the display, the atomic helpers see the CRTC is still
>>> active and (correctly) doesn't do a full modeset. So the bridge is
>>> not re-configured/re-enabled.
>>
>>
>> The adv7511 connector's detect() op should have re-configured the
>> registers. I'm assuming it was never called after the cable is
>> plugged in again in the wetson usecase?
>>
>> With this patch, in the case of fb emulation/X, I'm wondering if
>> we will end up trying to re-enable ADV7511 twice. First, in the new code
>> in adv7511_hpd_work(), and later in adv7511_detect() (i.e, the
>> connector's detect op).
>>
>
> jfwiw, fbcon and things using legacy SETCRTC end up triggering a full
> modeset, which somehow papers over the issue (because we go thru the
> bridge disable/enable sequence). So now there probably is a double
> power cycle sequence, although it doesn't seem to be causing any harm
> (fbcon and x11 still seem happy)
Hm that's strange. Legacy setcrtc on atomic drivers should do the
exact same opportunistic modeset avoidance as an atomic flip. The only
difference is around the automagic resetting of the link_status, but
msm doesn't use that. I suspect the real reason here is that fbcon and
X aren't broken, and properly shut down the disconnected output before
trying to re-enable it. This shouldn't have anything to do with atomic
vs. legacy ioctl paths.
Also, in-kernel fbcon is 100% atomic nowadays, it doesn't use any of
the legacy entry points anymore.
-Daniel
>
> BR,
> -R
>
>> I'm guessing the 'hpd' variable in the check in adv7511_detect() below
>> should ideally be false, and avoid us trying to configure the registers
>> again, but I'm not entirely sure.
>>
>> if (status == connector_status_connected && hpd && adv7511->powered) {
>> regcache_mark_dirty(adv7511->regmap);
>> ...
>>
>> In any case:
>>
>> Reviewed-by: Archit Taneja
>>
>>>
>>> Arguably this perhaps isn't what weston *wanted* to do, but in the end
>>> the bug is in the bridge.
>>>
>>> We could possibly set the connector link_status to BAD as an
>>> alternative. But at least the build of weston I'm using atm doesn't
>>> handle the link_status propery, this approach requires each atomic
>>> userspace to handle this case. Compared to Sean's approach which is
>>> transparent to userspace, which seems attractive.
>>>
>>> BR,
>>> -R
>>>
drivers/gpu/drm/bridge/adv7511/adv7511_drv.c | 12
1 file changed, 12 insertions(+)
diff --git a/drivers/gpu/drm/bridge/adv7511/adv7511_drv.c
b/drivers/gpu/drm/bridge/adv7511/adv7511_drv.c
index 73021b388e12..dd3ff2f2cdce 100644
--- a/drivers/gpu/drm/bridge/adv7511/adv7511_drv.c
+++ b/drivers/gpu/drm/bridge/adv7511/adv7511_drv.c
@@ -429,6 +429,18 @@ static void adv7511_hpd_work(struct work_struct
*work)
else
status = connector_status_disconnected;
+ /*
+* The bridge resets its registers on unplug. So when we get a
plug
+* event and we're already supposed to be powered, cycle the
bridge to
+* restore its state.
+*/
+ if (status == connector_status_connected &&
+ adv7511->connector.status == connector_status_disconnected &&
+ adv7511->powered) {
+ regcache_mark_dirty(adv7511->regmap);
+ adv7511_power_on(adv7511);
+ }
+
if (adv7511->connector.status != status) {
adv7511->connector.status = status;
if (status == connector_status_disconnected)
--
Sean Paul, Software Engineer, Google / Chromium OS
>>> --
>>> To unsubscribe from this list: send the line "unsubscribe linux-arm-msm"
>>> in
>>> the body of a message to majord...@vger.kernel.org
>>> More majordomo info at http://vger.kernel.org/majordomo-info.html
>>>
>>
> ___
>
Re: [PATCH] drm/bridge: adv7511: Reset registers on hotplug
On Wed, Jul 4, 2018 at 9:23 AM, Archit Taneja wrote:
>
>
> On Wednesday 04 July 2018 12:28 AM, Rob Clark wrote:
>>
>> On Tue, Jul 3, 2018 at 12:56 PM, Sean Paul wrote:
>>>
>>> The bridge loses its hw state when the cable is unplugged. If we detect
>>> this case in the hpd handler, reset its state.
>>>
>>> Reported-by: Rob Clark
>>> Signed-off-by: Sean Paul
>>
>>
>> Tested-by: Rob Clark
>>
>>> ---
>>> This is the follow-up to "drm/msm: Disable mdp5 crtc when there are no
>>> active planes". I incorrectly assumed the modeset was required from the
>>> msm side, but Rob pointed out that he thought it might be a bridge
>>> issue.
>>
>>
>> To elaborate, this is an atomic userspace (weston), when it sees the
>> display disconnected, it removes the planes from the CRTC but does not
>> disable the CRTC. So drm/msm sets up the LM to scanout a solid color,
>> and leaves the encoder and dsi cranking along happily. When
>> replugging the display, the atomic helpers see the CRTC is still
>> active and (correctly) doesn't do a full modeset. So the bridge is
>> not re-configured/re-enabled.
>
>
> The adv7511 connector's detect() op should have re-configured the
> registers. I'm assuming it was never called after the cable is
> plugged in again in the wetson usecase?
>
> With this patch, in the case of fb emulation/X, I'm wondering if
> we will end up trying to re-enable ADV7511 twice. First, in the new code
> in adv7511_hpd_work(), and later in adv7511_detect() (i.e, the
> connector's detect op).
>
jfwiw, fbcon and things using legacy SETCRTC end up triggering a full
modeset, which somehow papers over the issue (because we go thru the
bridge disable/enable sequence). So now there probably is a double
power cycle sequence, although it doesn't seem to be causing any harm
(fbcon and x11 still seem happy)
BR,
-R
> I'm guessing the 'hpd' variable in the check in adv7511_detect() below
> should ideally be false, and avoid us trying to configure the registers
> again, but I'm not entirely sure.
>
> if (status == connector_status_connected && hpd && adv7511->powered) {
> regcache_mark_dirty(adv7511->regmap);
> ...
>
> In any case:
>
> Reviewed-by: Archit Taneja
>
>>
>> Arguably this perhaps isn't what weston *wanted* to do, but in the end
>> the bug is in the bridge.
>>
>> We could possibly set the connector link_status to BAD as an
>> alternative. But at least the build of weston I'm using atm doesn't
>> handle the link_status propery, this approach requires each atomic
>> userspace to handle this case. Compared to Sean's approach which is
>> transparent to userspace, which seems attractive.
>>
>> BR,
>> -R
>>
>>>
>>> drivers/gpu/drm/bridge/adv7511/adv7511_drv.c | 12
>>> 1 file changed, 12 insertions(+)
>>>
>>> diff --git a/drivers/gpu/drm/bridge/adv7511/adv7511_drv.c
>>> b/drivers/gpu/drm/bridge/adv7511/adv7511_drv.c
>>> index 73021b388e12..dd3ff2f2cdce 100644
>>> --- a/drivers/gpu/drm/bridge/adv7511/adv7511_drv.c
>>> +++ b/drivers/gpu/drm/bridge/adv7511/adv7511_drv.c
>>> @@ -429,6 +429,18 @@ static void adv7511_hpd_work(struct work_struct
>>> *work)
>>> else
>>> status = connector_status_disconnected;
>>>
>>> + /*
>>> +* The bridge resets its registers on unplug. So when we get a
>>> plug
>>> +* event and we're already supposed to be powered, cycle the
>>> bridge to
>>> +* restore its state.
>>> +*/
>>> + if (status == connector_status_connected &&
>>> + adv7511->connector.status == connector_status_disconnected &&
>>> + adv7511->powered) {
>>> + regcache_mark_dirty(adv7511->regmap);
>>> + adv7511_power_on(adv7511);
>>> + }
>>> +
>>> if (adv7511->connector.status != status) {
>>> adv7511->connector.status = status;
>>> if (status == connector_status_disconnected)
>>> --
>>> Sean Paul, Software Engineer, Google / Chromium OS
>>>
>> --
>> To unsubscribe from this list: send the line "unsubscribe linux-arm-msm"
>> in
>> the body of a message to majord...@vger.kernel.org
>> More majordomo info at http://vger.kernel.org/majordomo-info.html
>>
>
___
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [PATCH] drm/bridge: adv7511: Reset registers on hotplug
On Wednesday 04 July 2018 12:28 AM, Rob Clark wrote:
On Tue, Jul 3, 2018 at 12:56 PM, Sean Paul wrote:
The bridge loses its hw state when the cable is unplugged. If we detect
this case in the hpd handler, reset its state.
Reported-by: Rob Clark
Signed-off-by: Sean Paul
Tested-by: Rob Clark
---
This is the follow-up to "drm/msm: Disable mdp5 crtc when there are no
active planes". I incorrectly assumed the modeset was required from the
msm side, but Rob pointed out that he thought it might be a bridge
issue.
To elaborate, this is an atomic userspace (weston), when it sees the
display disconnected, it removes the planes from the CRTC but does not
disable the CRTC. So drm/msm sets up the LM to scanout a solid color,
and leaves the encoder and dsi cranking along happily. When
replugging the display, the atomic helpers see the CRTC is still
active and (correctly) doesn't do a full modeset. So the bridge is
not re-configured/re-enabled.
The adv7511 connector's detect() op should have re-configured the
registers. I'm assuming it was never called after the cable is
plugged in again in the wetson usecase?
With this patch, in the case of fb emulation/X, I'm wondering if
we will end up trying to re-enable ADV7511 twice. First, in the new code
in adv7511_hpd_work(), and later in adv7511_detect() (i.e, the
connector's detect op).
I'm guessing the 'hpd' variable in the check in adv7511_detect() below
should ideally be false, and avoid us trying to configure the registers
again, but I'm not entirely sure.
if (status == connector_status_connected && hpd && adv7511->powered) {
regcache_mark_dirty(adv7511->regmap);
...
In any case:
Reviewed-by: Archit Taneja
Arguably this perhaps isn't what weston *wanted* to do, but in the end
the bug is in the bridge.
We could possibly set the connector link_status to BAD as an
alternative. But at least the build of weston I'm using atm doesn't
handle the link_status propery, this approach requires each atomic
userspace to handle this case. Compared to Sean's approach which is
transparent to userspace, which seems attractive.
BR,
-R
drivers/gpu/drm/bridge/adv7511/adv7511_drv.c | 12
1 file changed, 12 insertions(+)
diff --git a/drivers/gpu/drm/bridge/adv7511/adv7511_drv.c
b/drivers/gpu/drm/bridge/adv7511/adv7511_drv.c
index 73021b388e12..dd3ff2f2cdce 100644
--- a/drivers/gpu/drm/bridge/adv7511/adv7511_drv.c
+++ b/drivers/gpu/drm/bridge/adv7511/adv7511_drv.c
@@ -429,6 +429,18 @@ static void adv7511_hpd_work(struct work_struct *work)
else
status = connector_status_disconnected;
+ /*
+* The bridge resets its registers on unplug. So when we get a plug
+* event and we're already supposed to be powered, cycle the bridge to
+* restore its state.
+*/
+ if (status == connector_status_connected &&
+ adv7511->connector.status == connector_status_disconnected &&
+ adv7511->powered) {
+ regcache_mark_dirty(adv7511->regmap);
+ adv7511_power_on(adv7511);
+ }
+
if (adv7511->connector.status != status) {
adv7511->connector.status = status;
if (status == connector_status_disconnected)
--
Sean Paul, Software Engineer, Google / Chromium OS
--
To unsubscribe from this list: send the line "unsubscribe linux-arm-msm" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
___
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [PATCH v7 5/6] mfd: cros_ec_dev: Add CEC sub-device registration
On Wed, 04 Jul 2018, Neil Armstrong wrote: > Hi Lee, > > On 04/07/2018 09:47, Lee Jones wrote: > > On Fri, 01 Jun 2018, Neil Armstrong wrote: > > > >> The EC can expose a CEC bus, thus add the cros-ec-cec MFD sub-device > >> when the CEC feature bit is present. > >> > >> Signed-off-by: Neil Armstrong > >> Reviewed-by: Enric Balletbo i Serra > >> Acked-by: Hans Verkuil > >> --- > >> drivers/mfd/cros_ec_dev.c | 16 > >> 1 file changed, 16 insertions(+) > > > > For my own reference: > > Acked-for-MFD-by: Lee Jones > > > > Should I keep these Acked-for-MFD-by for the v8 ? Yes please. -- Lee Jones [李琼斯] Linaro Services Technical Lead Linaro.org │ Open source software for ARM SoCs Follow Linaro: Facebook | Twitter | Blog ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [PATCH v7 5/6] mfd: cros_ec_dev: Add CEC sub-device registration
Hi Lee, On 04/07/2018 09:47, Lee Jones wrote: > On Fri, 01 Jun 2018, Neil Armstrong wrote: > >> The EC can expose a CEC bus, thus add the cros-ec-cec MFD sub-device >> when the CEC feature bit is present. >> >> Signed-off-by: Neil Armstrong >> Reviewed-by: Enric Balletbo i Serra >> Acked-by: Hans Verkuil >> --- >> drivers/mfd/cros_ec_dev.c | 16 >> 1 file changed, 16 insertions(+) > > For my own reference: > Acked-for-MFD-by: Lee Jones > Should I keep these Acked-for-MFD-by for the v8 ? Neil ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [PATCH 1/5] drm/i915: Remove unecessary dma_fence_ops
On Wed, Jul 04, 2018 at 01:03:18PM +0100, Emil Velikov wrote:
> Hi Daniel,
>
> On 4 July 2018 at 10:29, Daniel Vetter wrote:
> > dma_fence_default_wait is the default now, same for the trivial
> > enable_signaling implementation.
> >
> > v2: Also remove the relase hook, dma_fence_free is the default.
> >
> > Signed-off-by: Daniel Vetter
> > Cc: Jani Nikula
> > Cc: Joonas Lahtinen
> > Cc: Rodrigo Vivi
> > Cc: Chris Wilson
> > Cc: Tvrtko Ursulin
> > Cc: Colin Ian King
> > Cc: Daniel Vetter
> > Cc: Mika Kuoppala
> > Cc: intel-...@lists.freedesktop.org
> > ---
> > drivers/gpu/drm/i915/i915_gem_clflush.c| 7 ---
> > drivers/gpu/drm/i915/selftests/i915_sw_fence.c | 8
> > 2 files changed, 15 deletions(-)
> >
> > diff --git a/drivers/gpu/drm/i915/i915_gem_clflush.c
> > b/drivers/gpu/drm/i915/i915_gem_clflush.c
> > index f5c570d35b2a..8e74c23cbd91 100644
> > --- a/drivers/gpu/drm/i915/i915_gem_clflush.c
> > +++ b/drivers/gpu/drm/i915/i915_gem_clflush.c
> > @@ -45,11 +45,6 @@ static const char *i915_clflush_get_timeline_name(struct
> > dma_fence *fence)
> > return "clflush";
> > }
> >
> > -static bool i915_clflush_enable_signaling(struct dma_fence *fence)
> > -{
> > - return true;
> > -}
> > -
> > static void i915_clflush_release(struct dma_fence *fence)
> > {
> > struct clflush *clflush = container_of(fence, typeof(*clflush),
> > dma);
> > @@ -63,8 +58,6 @@ static void i915_clflush_release(struct dma_fence *fence)
> > static const struct dma_fence_ops i915_clflush_ops = {
> > .get_driver_name = i915_clflush_get_driver_name,
> > .get_timeline_name = i915_clflush_get_timeline_name,
> > - .enable_signaling = i915_clflush_enable_signaling,
> From a quick look through drm-misc/drm-misc-next removing the
> enable_signalling hook may cause functional changes.
>
> Namely:
> A call to trace_dma_fence_enable_signal() (in
> dma_fence_enable_sw_signaling(), dma_fence_add_callback() and others)
> will be omitted.
I'm not sure what this tracepoint is useful for in the absenve of a real
signaling mechanism that must be enabled (like interrupts).
For all the other bits (begin/end wait, fence signalling itsefl) we have
tracepoints already, so I think we're all covered. What do you think will
be lost with the tracepoint here? If there's a real need for it I think I
can rework the already merged patch to still call the tracpoint, while
avoiding everything else. I just don't see the use-case for that.
-Daniel
>
> Removing the default .wait and .release hooks is fine.
>
> HTH
> Emil
--
Daniel Vetter
Software Engineer, Intel Corporation
http://blog.ffwll.ch
___
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel
[Bug 105819] Window system hang due to GPU Fault
https://bugs.freedesktop.org/show_bug.cgi?id=105819 --- Comment #7 from AmirAli Akbari --- Same problem on RX 460 card when running imagemagick's "mogrify" command. I'm using Arch linux with latest stable version of kernel, mesa, and X. $ mogrify -resize 400x300 piv.jpg [ 3091.155960] amdgpu :00:01.0: GPU fault detected: 147 0x04a82002 [ 3091.158201] amdgpu :00:01.0: VM_CONTEXT1_PROTECTION_FAULT_ADDR 0x0050 3095 [ 3091.160461] amdgpu :00:01.0: VM_CONTEXT1_PROTECTION_FAULT_STATUS 0x0A020002 [ 3091.162695] amdgpu :00:01.0: VM fault (0x02, vmid 5, pasid 32781) at page 5255317, read from 'TC2' (0x54433200) (32) [ 3101.279903] [drm:amdgpu_job_timedout [amdgpu]] *ERROR* ring gfx timeout, last signaled seq=135372, last emitted seq=135373 Strace of the process shows this call: "ioctl(6, DRM_IOCTL_AMDGPU_WAIT_CS". -- You are receiving this mail because: You are the assignee for the bug.___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [PATCH] drm/savage: off by one in savage_bci_cmdbuf()
On Wed, Jul 04, 2018 at 12:48:10PM +0300, Dan Carpenter wrote:
> The > should be >= here so that we don't read beyond the end of the
> dma->buflist[] array.
>
> Signed-off-by: Dan Carpenter
Uh, another root-hole driver ... Applied, thanks for the patch.
-Daniel
>
> diff --git a/drivers/gpu/drm/savage/savage_state.c
> b/drivers/gpu/drm/savage/savage_state.c
> index 2db89bed52e8..7559a820bd43 100644
> --- a/drivers/gpu/drm/savage/savage_state.c
> +++ b/drivers/gpu/drm/savage/savage_state.c
> @@ -971,7 +971,7 @@ int savage_bci_cmdbuf(struct drm_device *dev, void *data,
> struct drm_file *file_
> LOCK_TEST_WITH_RETURN(dev, file_priv);
>
> if (dma && dma->buflist) {
> - if (cmdbuf->dma_idx > dma->buf_count) {
> + if (cmdbuf->dma_idx >= dma->buf_count) {
> DRM_ERROR
> ("vertex buffer index %u out of range (0-%u)\n",
>cmdbuf->dma_idx, dma->buf_count - 1);
> ___
> dri-devel mailing list
> dri-devel@lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/dri-devel
--
Daniel Vetter
Software Engineer, Intel Corporation
http://blog.ffwll.ch
___
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [PATCH v2 2/8] drm/bridge/synopsys: dsi: don't call __dw_mipi_dsi_probe from dw_mipi_dsi_bind
Am Dienstag, 3. Juli 2018, 14:16:28 CEST schrieb Andrzej Hajda:
> On 18.06.2018 12:28, Heiko Stuebner wrote:
> > __dw_mipi_dsi_probe() does all the grabbing of resources and does it using
> > devm-helpers. So this is happening on each try of master bringup possibly
> > slowing down things a lot.
> >
> > Drivers using the component framework may instead want call
> > dw_mipi_dsi_probe
> > separately in their probe function setup resources early. That way the dsi
> > bus also gets created earlier and also not recreated on each bind-try, so
> > that attached panels can load their modules and be probed way before the
> > bridge-attach in the bind call.
> >
> > So drop the call to __dw_mipi_dsi_probe and modify the function to take
> > a struct dw_mipi_dsi instead of the platform-device.
> >
> > Signed-off-by: Heiko Stuebner
> > ---
> > drivers/gpu/drm/bridge/synopsys/dw-mipi-dsi.c | 15 +++
> > include/drm/bridge/dw_mipi_dsi.h | 5 +
> > 2 files changed, 4 insertions(+), 16 deletions(-)
> >
> > diff --git a/drivers/gpu/drm/bridge/synopsys/dw-mipi-dsi.c
> > b/drivers/gpu/drm/bridge/synopsys/dw-mipi-dsi.c
> > index 07cde255cab2..bb4aeca5c0f9 100644
> > --- a/drivers/gpu/drm/bridge/synopsys/dw-mipi-dsi.c
> > +++ b/drivers/gpu/drm/bridge/synopsys/dw-mipi-dsi.c
> > @@ -966,31 +966,22 @@ EXPORT_SYMBOL_GPL(dw_mipi_dsi_remove);
> > /*
> > * Bind/unbind API, used from platforms based on the component framework.
> > */
> > -struct dw_mipi_dsi *
> > -dw_mipi_dsi_bind(struct platform_device *pdev, struct drm_encoder *encoder,
> > -const struct dw_mipi_dsi_plat_data *plat_data)
> > +int dw_mipi_dsi_bind(struct dw_mipi_dsi *dsi, struct drm_encoder *encoder)
> > {
> > - struct dw_mipi_dsi *dsi;
> > int ret;
> >
> > - dsi = __dw_mipi_dsi_probe(pdev, plat_data);
> > - if (IS_ERR(dsi))
> > - return dsi;
> > -
> > ret = drm_bridge_attach(encoder, >bridge, NULL);
> > if (ret) {
> > - dw_mipi_dsi_remove(dsi);
> > DRM_ERROR("Failed to initialize bridge with drm\n");
> > - return ERR_PTR(ret);
> > + return ret;
> > }
> >
> > - return dsi;
> > + return ret;
> > }
> > EXPORT_SYMBOL_GPL(dw_mipi_dsi_bind);
> >
> > void dw_mipi_dsi_unbind(struct dw_mipi_dsi *dsi)
> > {
> > - __dw_mipi_dsi_remove(dsi);
> > }
> > EXPORT_SYMBOL_GPL(dw_mipi_dsi_unbind);
>
> Can't we just remove these two bind/unbind functions and call
> drm_bridge_attach directly?
And I just realized, we can't.
struct dw_mipi_dsi is local to the common bridge driver, which also makes
sense to not expose internal stuff. So drm_bridge attach will need to
be called from the common bridge driver.
Heiko
___
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [PATCH] backlight: remove obsolete comment for ->state
On Wed, 04 Jul 2018, Daniel Vetter wrote:
> Hi Lee,
>
> On Wed, Jul 4, 2018 at 12:34 PM, Lee Jones wrote:
> > On Wed, 04 Jul 2018, Daniel Vetter wrote:
> >> On Wed, Jul 04, 2018 at 10:38:16AM +0100, Lee Jones wrote:
> >> > On Wed, 04 Jul 2018, Lee Jones wrote:
> >> >
> >> > > > Jani spotted this when reviewing my earlier patch to remove the
> >> > > > driver
> >> > > > internal usage of this field in
> >> > > >
> >> > > > commit 3cf91adaa594e8933af1727942ac560e5c7bc70e
> >> > > > Author: Daniel Vetter
> >> > > > Date: Wed Apr 25 19:42:52 2018 +0200
> >> > > >
> >> > > > backlight: Nuke BL_CORE_DRIVER1
> >> > >
> >> > > FYI, sending patches like this is not a good idea.
> >> > >
> >> > > I'll clean it up for you this time, but in future please send patches
> >> > > properly and place any additional comments you may have below the
> >> > > '---' line.
> >> >
> >> > Ah, I see what you've tried to do. This hurt my eyes! :)
> >> >
> >> > It's more conventional to reference commits like:
> >> >
> >> > Commit 3cf91adaa594 ("backlight: Nuke BL_CORE_DRIVER1")
> >> >
> >> > Again, I'll make the amendment to avoid further confusion.
> >>
> >> So the first mail doesn't even bother to explain what's
> >> objectionable
> >
> > In the first instance it looked as though you'd copied and pasted `git
> > log`, which if you'd done so would have been obvious to you and would
> > have required no further explanation.
> >
> > Also bear in mind that I took your standing within the kernel
> > community into consideration, so speaking to you like a n00b or going
> > into unnecessary detail could have been considered superfluous at best
> > and condescending at worst.
>
> Unfortunately minute details like this aren't consistent across the
> kernel at all, and white space and layout issues are the number 1
> reason I get shot at for random patches I'm sending out. So maybe
> there are people who learned all these local expectations (Arnd
> perhaps, or Kees?), it's definitely not me. Not after 10 years for
> sure.
>
> >> the 2nd mail still says "This hurts my eyes!".
> >
> > It certainly did, yes.
> >
> > Usually taken to meaning that it was hard to read in these scenarios.
> >
> >> Over whitespace in the commit message.
> >
> > Nothing to do with white space. It was the format by which you chose
> > to reference a previous commit. Instead it looked like a patch
> > formatting error. I have received patches pasted from `git log`
> > before, and this looked just the same.
> >
> > Once I'd realised what was going on, I followed up to explain and
> > provided some feedback on what to do differently in future.
> >
> >> This kind of stuff is why graphics people really don't enjoy contributing
> >> to the kernel at large. A friendly request to resend with the color choice
> >> adjusted would go a lot further.
> >
> > I apologise if my brevity hurt your feelings. I have 290 emails to
> > get though post-paternity leave before I can start to think about
> > getting some real/paid work done.
>
> This ain't about my feelings, but working together efficiently and in
> a constructive environment.
>
> Also, failing to have adequate maintainer coverage over absence, or
> generally being overloaded, is never a valid excuse for how you deal
> with contributors. It takes some effort and a bit of time, but group
> maintainership in one form or another can take care of this very well.
> Brevity justified as efficient communication tends to torpedo that,
> since at least in my experience it just drive prospective volunteers
> away to more welcoming places.
I'm unsure of the foundations which this scenario builds upon. Maybe
you've had some bad experiences with other Maintainers in the past
which have made you uber-sensitive, but FWIW I think you're
over-reacting to what were perfectly adequate review comments provided
from one Maintainer to another.
There wasn't any malice or harshness in my recommendations to you nor
did I make any unreasonable requests of you. Merely an innocent
misunderstanding in the first instance and some gentle advice in the
second. If constructive feedback isn't something that you deal with
well, perhaps life as a contributor (outside of your own domain at
least) isn't for you.
--
Lee Jones [李琼斯]
Linaro Services Technical Lead
Linaro.org │ Open source software for ARM SoCs
Follow Linaro: Facebook | Twitter | Blog
___
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [PATCH v4 1/2] devicetree/bindings: display: Add document for rockchip RGB output
Hi Rob Herring, Thanks for your review. 在 2018/7/4 2:25, Rob Herring 写道: On Tue, Jun 26, 2018 at 03:15:39PM +0800, Sandy Huang wrote: This path add support rv1108 and px30 rgb output interface driver. Bindings are for h/w, not drivers. I will update at next version as following: This patch add support rv1108 and px30 rgb output interface Signed-off-by: Sandy Huang Link: https://patchwork.freedesktop.org/patch/msgid/1509522765-118759-1-git-send-email-...@rock-chips.com --- Changes in v4: Add support px30 Changes in v3: None Changes in v2: None .../bindings/display/rockchip/rockchip-rgb.txt | 73 ++ 1 file changed, 73 insertions(+) create mode 100644 Documentation/devicetree/bindings/display/rockchip/rockchip-rgb.txt diff --git a/Documentation/devicetree/bindings/display/rockchip/rockchip-rgb.txt b/Documentation/devicetree/bindings/display/rockchip/rockchip-rgb.txt new file mode 100644 index 000..077b9ad --- /dev/null +++ b/Documentation/devicetree/bindings/display/rockchip/rockchip-rgb.txt @@ -0,0 +1,73 @@ +Rockchip RV1108 RGB interface + + +Required properties: +- compatible: matching the soc type: + - "rockchip,px30-rgb"; + - "rockchip,rv1108-rgb"; This doesn't look right? What (and how) is getting programmed here because you don't have any register interface. This is register for DRM encoder and connecter or bridge for some RGB convert chips driver. so far we don't have any register for rgb interface, but it's most probable need to config some register for rgb interface in feauture for rockchip platform. + +Optional properties: +- pinctrl-names: should be a "lcdc" entry or a "default" entry. +- pinctrl-0: pin control group to be used for this interface. + +The rgb has two video ports described by: + Documentation/devicetree/bindings/media/video-interfaces.txt +Their connections are modeled using the OF graph bindings specified in + Documentation/devicetree/bindings/graph.txt. + +- video port 0 for the VOP input, the remote endpoint maybe vopb/vopl/vop So there is a mux for these inputs and they are switched with pinctrl? connetced can connect to vopb/vopl/vop which is same with other connenter like edp/dsi/lvds, there is a sample for edp at:Documentation/devicetree/bindings/display/rockchip/rockchip-vop.txt Just put the pinctrl nodes of the sources and enable whichever one is active. Do you think i need add pinctrl nodes here? Or am I wrong? Rob ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [PATCH 1/5] drm/i915: Remove unecessary dma_fence_ops
Hi Daniel,
On 4 July 2018 at 10:29, Daniel Vetter wrote:
> dma_fence_default_wait is the default now, same for the trivial
> enable_signaling implementation.
>
> v2: Also remove the relase hook, dma_fence_free is the default.
>
> Signed-off-by: Daniel Vetter
> Cc: Jani Nikula
> Cc: Joonas Lahtinen
> Cc: Rodrigo Vivi
> Cc: Chris Wilson
> Cc: Tvrtko Ursulin
> Cc: Colin Ian King
> Cc: Daniel Vetter
> Cc: Mika Kuoppala
> Cc: intel-...@lists.freedesktop.org
> ---
> drivers/gpu/drm/i915/i915_gem_clflush.c| 7 ---
> drivers/gpu/drm/i915/selftests/i915_sw_fence.c | 8
> 2 files changed, 15 deletions(-)
>
> diff --git a/drivers/gpu/drm/i915/i915_gem_clflush.c
> b/drivers/gpu/drm/i915/i915_gem_clflush.c
> index f5c570d35b2a..8e74c23cbd91 100644
> --- a/drivers/gpu/drm/i915/i915_gem_clflush.c
> +++ b/drivers/gpu/drm/i915/i915_gem_clflush.c
> @@ -45,11 +45,6 @@ static const char *i915_clflush_get_timeline_name(struct
> dma_fence *fence)
> return "clflush";
> }
>
> -static bool i915_clflush_enable_signaling(struct dma_fence *fence)
> -{
> - return true;
> -}
> -
> static void i915_clflush_release(struct dma_fence *fence)
> {
> struct clflush *clflush = container_of(fence, typeof(*clflush), dma);
> @@ -63,8 +58,6 @@ static void i915_clflush_release(struct dma_fence *fence)
> static const struct dma_fence_ops i915_clflush_ops = {
> .get_driver_name = i915_clflush_get_driver_name,
> .get_timeline_name = i915_clflush_get_timeline_name,
> - .enable_signaling = i915_clflush_enable_signaling,
From a quick look through drm-misc/drm-misc-next removing the
enable_signalling hook may cause functional changes.
Namely:
A call to trace_dma_fence_enable_signal() (in
dma_fence_enable_sw_signaling(), dma_fence_add_callback() and others)
will be omitted.
Removing the default .wait and .release hooks is fine.
HTH
Emil
___
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [PATCH] backlight: remove obsolete comment for ->state
Hi Lee,
On Wed, Jul 4, 2018 at 12:34 PM, Lee Jones wrote:
> On Wed, 04 Jul 2018, Daniel Vetter wrote:
>> On Wed, Jul 04, 2018 at 10:38:16AM +0100, Lee Jones wrote:
>> > On Wed, 04 Jul 2018, Lee Jones wrote:
>> >
>> > > > Jani spotted this when reviewing my earlier patch to remove the driver
>> > > > internal usage of this field in
>> > > >
>> > > > commit 3cf91adaa594e8933af1727942ac560e5c7bc70e
>> > > > Author: Daniel Vetter
>> > > > Date: Wed Apr 25 19:42:52 2018 +0200
>> > > >
>> > > > backlight: Nuke BL_CORE_DRIVER1
>> > >
>> > > FYI, sending patches like this is not a good idea.
>> > >
>> > > I'll clean it up for you this time, but in future please send patches
>> > > properly and place any additional comments you may have below the
>> > > '---' line.
>> >
>> > Ah, I see what you've tried to do. This hurt my eyes! :)
>> >
>> > It's more conventional to reference commits like:
>> >
>> > Commit 3cf91adaa594 ("backlight: Nuke BL_CORE_DRIVER1")
>> >
>> > Again, I'll make the amendment to avoid further confusion.
>>
>> So the first mail doesn't even bother to explain what's
>> objectionable
>
> In the first instance it looked as though you'd copied and pasted `git
> log`, which if you'd done so would have been obvious to you and would
> have required no further explanation.
>
> Also bear in mind that I took your standing within the kernel
> community into consideration, so speaking to you like a n00b or going
> into unnecessary detail could have been considered superfluous at best
> and condescending at worst.
Unfortunately minute details like this aren't consistent across the
kernel at all, and white space and layout issues are the number 1
reason I get shot at for random patches I'm sending out. So maybe
there are people who learned all these local expectations (Arnd
perhaps, or Kees?), it's definitely not me. Not after 10 years for
sure.
>> the 2nd mail still says "This hurts my eyes!".
>
> It certainly did, yes.
>
> Usually taken to meaning that it was hard to read in these scenarios.
>
>> Over whitespace in the commit message.
>
> Nothing to do with white space. It was the format by which you chose
> to reference a previous commit. Instead it looked like a patch
> formatting error. I have received patches pasted from `git log`
> before, and this looked just the same.
>
> Once I'd realised what was going on, I followed up to explain and
> provided some feedback on what to do differently in future.
>
>> This kind of stuff is why graphics people really don't enjoy contributing
>> to the kernel at large. A friendly request to resend with the color choice
>> adjusted would go a lot further.
>
> I apologise if my brevity hurt your feelings. I have 290 emails to
> get though post-paternity leave before I can start to think about
> getting some real/paid work done.
This ain't about my feelings, but working together efficiently and in
a constructive environment.
Also, failing to have adequate maintainer coverage over absence, or
generally being overloaded, is never a valid excuse for how you deal
with contributors. It takes some effort and a bit of time, but group
maintainership in one form or another can take care of this very well.
Brevity justified as efficient communication tends to torpedo that,
since at least in my experience it just drive prospective volunteers
away to more welcoming places.
Thanks, Daniel
--
Daniel Vetter
Software Engineer, Intel Corporation
+41 (0) 79 365 57 48 - http://blog.ffwll.ch
___
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: DRM COLOR_RANGE property
On 03/07/18 19:18, Russell King - ARM Linux wrote: > Can someone provide a deeper explanation about exactly what this > property represents please? > > Does this represent the range of YCbCr values _into_ a YCbCr-to-RGB > conversion (in other words, the range of values in the framebuffer), > or the expected output range from the conversion? > Yes, it to describe the YCbCr color encoding used in the buffer given to the plane, nothing more. It should then be up to the driver and display HW to decide what to do with the information. But of course since there is currently no way to tell the range of the RGB buffers used, the working assumption is that the RGB buffers are all full range and the blending is done accordingly. It should be easy enough to extend the existing enums, or to create completely new ones, for information about the RGB buffers range. Extending has a downside of adding nasty dependency between the framebuffer format and the encoding properties. The output side is then another matter. Best regards, Jyri > This matters, because a "limited", (iow, eg, BT601 compliant YCbCr) > framebuffer where the Y signal is between 16..235 being displayed > on a full-range RGB output would need different conversion from > needing a limited-range RGB output. > > If it is indeed the output, then why is this a property of the plane? > Is that not a property of: > > (a) whether the plane is being blended or overlaid onto a graphics > plane which uses full-range RGB > (b) the properties of the output(s) to which the plane is being > displayed. > > IOW, it seems that the output of the CSC is more to do with what's > downstream of the plane than with the plane itself. > > For example, take this situation: > > plane 0 - graphics, full range RGB > >-- CRTC --> HDMI sink only supporting > plane 1 - video, limited range YUV limited range RGB > > In order to display the graphics correctly in that scenario, the HDMI > output needs to compress the RGB 0-255 range down to 16..236 to be > compliant. If the video is limited range, and the CSC produces a > limited range RGB output, then plane 1 gets its range further > compressed at the HDMI output, which surely is undesirable. > > It would surely be better, if it's not possible to map the range of > plane 0 to limited range, to instead expand the YUV range and then > recompress it at the HDMI output to match the capabilities of the > attached source. > > It also seems logical that describing the range of the RGB plane would > also be sane - if the application is limiting graphics RGB to 16..235, > then you'd want the CSC output to do the same and there'd be no need > for any range expansion or compression. > > I'd personally like drm_plane_create_color_properties() to allow > creation of COLOR_ENCODING without COLOR_RANGE (iow, supported_ranges > being zero) until COLOR_RANGE is better defined than it is at present. > > Thoughts? > > I'm bringing this up, because the hardware I have has a CSC that > accepts BT601 and BT709 formats, controlled by a single bit. Another > bit controls whether the CSC produces 0..255 output or 16..235 output. > That is then blended/overlaid with the graphics plane (0..255) and > sent to the output. Having a "limited range" YUV plane produce > 16..235 range output makes it look low-contrast compared to the > graphics, which is what would be expected - "16" is not black > compared to the black of the graphics in the same way that "235" is > not white compared to the graphics. > -- Texas Instruments Finland Oy, Porkkalankatu 22, 00180 Helsinki. Y-tunnus/Business ID: 0615521-4. Kotipaikka/Domicile: Helsinki ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: DRM COLOR_RANGE property
On 04/07/18 12:58, Russell King - ARM Linux wrote: >> Hm maybe I misunderstood, but I thought the COLOR_RANGE is on the input >> side. > If that's the case, I should force it to only indicate support for > limited range, while programming the CSC to produce full range RGB > on its output (see below). > That should need no forcing. Just set supported_ranges parameter to BIT(DRM_COLOR_YCBCR_LIMITED_RANGE) in drm_plane_create_color_properties() call to create an enum property with only the allowed value. BR, Jyri -- Texas Instruments Finland Oy, Porkkalankatu 22, 00180 Helsinki. Y-tunnus/Business ID: 0615521-4. Kotipaikka/Domicile: Helsinki ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: DRM COLOR_RANGE property
On Tue, Jul 03, 2018 at 05:18:57PM +0100, Russell King - ARM Linux wrote: > Can someone provide a deeper explanation about exactly what this > property represents please? > > Does this represent the range of YCbCr values _into_ a YCbCr-to-RGB > conversion (in other words, the range of values in the framebuffer), > or the expected output range from the conversion? The color_range/encoding props are about the input. The plane output is generally full range RGB, but presumably some driver could choose to use some other internal format as well. -- Ville Syrjälä Intel ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: DRM COLOR_RANGE property
On Wed, Jul 4, 2018 at 11:58 AM, Russell King - ARM Linux wrote: > On Wed, Jul 04, 2018 at 11:24:10AM +0200, Daniel Vetter wrote: >> On Wed, Jul 04, 2018 at 10:05:41AM +0100, Russell King - ARM Linux wrote: >> > On Wed, Jul 04, 2018 at 10:26:04AM +0200, Daniel Vetter wrote: >> > > On Tue, Jul 03, 2018 at 05:18:57PM +0100, Russell King - ARM Linux wrote: >> > > > Can someone provide a deeper explanation about exactly what this >> > > > property represents please? >> > > > >> > > > Does this represent the range of YCbCr values _into_ a YCbCr-to-RGB >> > > > conversion (in other words, the range of values in the framebuffer), >> > > > or the expected output range from the conversion? >> > > > >> > > > This matters, because a "limited", (iow, eg, BT601 compliant YCbCr) >> > > > framebuffer where the Y signal is between 16..235 being displayed >> > > > on a full-range RGB output would need different conversion from >> > > > needing a limited-range RGB output. >> > > > >> > > > If it is indeed the output, then why is this a property of the plane? >> > > > Is that not a property of: >> > > > >> > > > (a) whether the plane is being blended or overlaid onto a graphics >> > > > plane which uses full-range RGB >> > > > (b) the properties of the output(s) to which the plane is being >> > > > displayed. >> > > > >> > > > IOW, it seems that the output of the CSC is more to do with what's >> > > > downstream of the plane than with the plane itself. >> > > > >> > > > For example, take this situation: >> > > > >> > > > plane 0 - graphics, full range RGB >> > > > >-- CRTC --> HDMI sink only >> > > > supporting >> > > > plane 1 - video, limited range YUV limited range RGB >> > > > >> > > > In order to display the graphics correctly in that scenario, the HDMI >> > > > output needs to compress the RGB 0-255 range down to 16..236 to be >> > > > compliant. If the video is limited range, and the CSC produces a >> > > > limited range RGB output, then plane 1 gets its range further >> > > > compressed at the HDMI output, which surely is undesirable. >> > > > >> > > > It would surely be better, if it's not possible to map the range of >> > > > plane 0 to limited range, to instead expand the YUV range and then >> > > > recompress it at the HDMI output to match the capabilities of the >> > > > attached source. >> > > > >> > > > It also seems logical that describing the range of the RGB plane would >> > > > also be sane - if the application is limiting graphics RGB to 16..235, >> > > > then you'd want the CSC output to do the same and there'd be no need >> > > > for any range expansion or compression. >> > > > >> > > > I'd personally like drm_plane_create_color_properties() to allow >> > > > creation of COLOR_ENCODING without COLOR_RANGE (iow, supported_ranges >> > > > being zero) until COLOR_RANGE is better defined than it is at present. >> > > > >> > > > Thoughts? >> > > > >> > > > I'm bringing this up, because the hardware I have has a CSC that >> > > > accepts BT601 and BT709 formats, controlled by a single bit. Another >> > > > bit controls whether the CSC produces 0..255 output or 16..235 output. >> > > > That is then blended/overlaid with the graphics plane (0..255) and >> > > > sent to the output. Having a "limited range" YUV plane produce >> > > > 16..235 range output makes it look low-contrast compared to the >> > > > graphics, which is what would be expected - "16" is not black >> > > > compared to the black of the graphics in the same way that "235" is >> > > > not white compared to the graphics. >> > > >> > > Drivers are supposed to automatically figure this out by looking at the >> > > edid. In i915 we also allow userspace to override this with the >> > > "Broadcast >> > > RGB" property on the connector. Unfortunately we haven't polished that >> > > property yet (not sure what other drivers are doing tbh), so it's only >> > > listed in the Documentation/gpu/kms-properties.csv graveyard :-/ >> > >> > In which case, I'd like to implement the COLOR_ENCODING property but >> > not the COLOR_RANGE property until COLOR_RANGE is better defined. >> > Unfortunately, at the moment the choices are to have either both >> > properties or no properties - drm_plane_create_color_properties() >> > doesn't support only creating the encoding property. >> > >> > Implementing it without it being well defined is a recipe for having >> > a broken UAPI. So, I propose: >> >> Hm maybe I misunderstood, but I thought the COLOR_RANGE is on the input >> side. > > If that's the case, I should force it to only indicate support for > limited range, while programming the CSC to produce full range RGB > on its output (see below). Agreed (as far as my understanding of all this goes at least). >> And you need to adjust your CSC to make sure the output range is >> whatever makes things Just Work. Probably better to poke Ville on these >> details, and then clarify the docs. > > It's not possible
Re: [PATCH v2 6/8] drm/dsi: add helper function to find the second host in a dual-dsi setup
Am Dienstag, 3. Juli 2018, 17:06:48 CEST schrieb Andrzej Hajda:
> On 18.06.2018 12:28, Heiko Stuebner wrote:
> > >From a specified output port of one dsi controller this function allows to
> > iterate over the list of registered dsi controllers trying to find a second
> > instance connected to the same display, like it is used in dual-dsi setups.
> >
> > Signed-off-by: Heiko Stuebner
>
> The code looks hacky to me - it iterates over external nodes, it assumes
> all graph links are dedicated exclusively to dsi
The loop iterates over all registered dsi controllers and checks the same
port/endpoint combination as on the primary host.
One assumption is that the dsi controllers are identical, but that is
also an argument
As the splitting of the output to two DSI-controllers is a special
feature of the socs CRTC, I'd think the chances are very slim that
there will be two completely different DSI controllers used for
something like this.
> it assumes that there
> is no additional node between dsi host and the panel.
Yep.
Heiko
> > ---
> > drivers/gpu/drm/drm_mipi_dsi.c | 56 ++
> > include/drm/drm_mipi_dsi.h | 2 ++
> > 2 files changed, 58 insertions(+)
> >
> > diff --git a/drivers/gpu/drm/drm_mipi_dsi.c b/drivers/gpu/drm/drm_mipi_dsi.c
> > index bc73b7f5b9fc..0c3c9c7aa3b8 100644
> > --- a/drivers/gpu/drm/drm_mipi_dsi.c
> > +++ b/drivers/gpu/drm/drm_mipi_dsi.c
> > @@ -30,6 +30,7 @@
> > #include
> > #include
> > #include
> > +#include
> > #include
> > #include
> >
> > @@ -282,6 +283,61 @@ struct mipi_dsi_host
> > *of_find_mipi_dsi_host_by_node(struct device_node *node)
> > }
> > EXPORT_SYMBOL(of_find_mipi_dsi_host_by_node);
> >
> > +struct device_node *of_mipi_dsi_find_second_host(struct device_node
> > *first_np,
> > +int port, int endpoint)
> > +{
> > + struct mipi_dsi_host *first, *host;
> > + struct device_node *remote, *np, *second_np = NULL;
> > + int num = 0;
> > +
> > + first = of_find_mipi_dsi_host_by_node(first_np);
> > + if (!first) {
> > + pr_err("no dsi-host for node %s\n", first_np->full_name);
> > + return ERR_PTR(-ENODEV);
> > + }
> > +
> > + /* output-node of the known dsi-host */
> > + remote = of_graph_get_remote_node(first_np, port, endpoint);
> > + if (!remote) {
> > + dev_err(first->dev, "no output node found\n");
> > + return ERR_PTR(-ENODEV);
> > + }
> > +
> > + mutex_lock(_lock);
> > +
> > + list_for_each_entry(host, _list, list) {
> > + np = of_graph_get_remote_node(host->dev->of_node,
> > + port, endpoint);
> > +
> > + /* found a host connected to this panel */
> > + if (np == remote)
> > + num++;
> > +
> > + /* found one second host */
> > + if (host->dev->of_node != first_np)
> > + second_np = host->dev->of_node;
> > +
> > + of_node_put(np);
> > + }
> > +
> > + /* of_node_get the node under host_lock */
> > + if (num == 2)
> > + of_node_get(second_np);
> > +
> > + mutex_unlock(_lock);
> > +
> > + of_node_put(remote);
> > +
> > + if (num > 2) {
> > + dev_err(first->dev,
> > + "too many DSI links for output: %d links\n", num);
> > + return ERR_PTR(-EINVAL);
> > + }
> > +
> > + return second_np;
> > +}
> > +EXPORT_SYMBOL(of_mipi_dsi_find_second_host);
> > +
> > int mipi_dsi_host_register(struct mipi_dsi_host *host)
> > {
> > struct device_node *node;
> > diff --git a/include/drm/drm_mipi_dsi.h b/include/drm/drm_mipi_dsi.h
> > index 4fef19064b0f..89532ae69c91 100644
> > --- a/include/drm/drm_mipi_dsi.h
> > +++ b/include/drm/drm_mipi_dsi.h
> > @@ -107,6 +107,8 @@ struct mipi_dsi_host {
> > int mipi_dsi_host_register(struct mipi_dsi_host *host);
> > void mipi_dsi_host_unregister(struct mipi_dsi_host *host);
> > struct mipi_dsi_host *of_find_mipi_dsi_host_by_node(struct device_node
> > *node);
> > +struct device_node *of_mipi_dsi_find_second_host(struct device_node
> > *first_np,
> > +int port, int endpoint);
> >
> > /* DSI mode flags */
> >
>
>
>
___
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [PATCH v2 3/8] drm/bridge/synopsys: dsi: defer probing if panel not available in bridge-attach
Am Dienstag, 3. Juli 2018, 14:42:49 CEST schrieb Andrzej Hajda: > On 18.06.2018 12:28, Heiko Stuebner wrote: > > When the panel-driver is build as a module it currently fails hard as the > > panel cannot be probed directly: > > > > dw_mipi_dsi_bind() > > __dw_mipi_dsi_probe() > > creates dsi bus > > creates panel device > > triggers panel module load > > panel not probed (module not loaded or panel probe slow) > > drm_bridge_attach > > fails with -EINVAL due to empty panel_bridge > > > > So emit a -EPROBE_DEFER in that case to give the driver another > > chance at getting the display later. > > Thats odd (at least for me), if the resource (panel in this case) is not > present, bridge should not be attached (drm_bridge_attach should not be > called). Ie __dw_mipi_dsi_probe should rather look for the panel and > defer if not found. Resource gathering is the task of probe. > drm_bridge_attach can be called after probe and it would be difficult > for the caller to react properly for -EPROBE_DEFER error. I think one of the problems comes from the fact that the panel only gets probed (and even the panel-device created) after the dsi host bus gets created in __dw_mipi_dsi_probe. In hindsight, I think this is more of a concurrency issue between the panel probing and calling dsi-attach and the dsi-controller looking for the panel vs. panel-bridge. So when I'm using drm_of_find_panel_or_bridge() as indicator of the panel being, it probably makes things racy. I'll try to find a better solution for the issue. Heiko ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [PATCH] backlight: remove obsolete comment for ->state
On Wed, 04 Jul 2018, Daniel Vetter wrote:
> On Wed, Jul 04, 2018 at 10:38:16AM +0100, Lee Jones wrote:
> > On Wed, 04 Jul 2018, Lee Jones wrote:
> >
> > > > Jani spotted this when reviewing my earlier patch to remove the driver
> > > > internal usage of this field in
> > > >
> > > > commit 3cf91adaa594e8933af1727942ac560e5c7bc70e
> > > > Author: Daniel Vetter
> > > > Date: Wed Apr 25 19:42:52 2018 +0200
> > > >
> > > > backlight: Nuke BL_CORE_DRIVER1
> > >
> > > FYI, sending patches like this is not a good idea.
> > >
> > > I'll clean it up for you this time, but in future please send patches
> > > properly and place any additional comments you may have below the
> > > '---' line.
> >
> > Ah, I see what you've tried to do. This hurt my eyes! :)
> >
> > It's more conventional to reference commits like:
> >
> > Commit 3cf91adaa594 ("backlight: Nuke BL_CORE_DRIVER1")
> >
> > Again, I'll make the amendment to avoid further confusion.
>
> So the first mail doesn't even bother to explain what's
> objectionable
In the first instance it looked as though you'd copied and pasted `git
log`, which if you'd done so would have been obvious to you and would
have required no further explanation.
Also bear in mind that I took your standing within the kernel
community into consideration, so speaking to you like a n00b or going
into unnecessary detail could have been considered superfluous at best
and condescending at worst.
> the 2nd mail still says "This hurts my eyes!".
It certainly did, yes.
Usually taken to meaning that it was hard to read in these scenarios.
> Over whitespace in the commit message.
Nothing to do with white space. It was the format by which you chose
to reference a previous commit. Instead it looked like a patch
formatting error. I have received patches pasted from `git log`
before, and this looked just the same.
Once I'd realised what was going on, I followed up to explain and
provided some feedback on what to do differently in future.
> This kind of stuff is why graphics people really don't enjoy contributing
> to the kernel at large. A friendly request to resend with the color choice
> adjusted would go a lot further.
I apologise if my brevity hurt your feelings. I have 290 emails to
get though post-paternity leave before I can start to think about
getting some real/paid work done.
> > > > Cc: Jani Nikula
> > > > Signed-off-by: Daniel Vetter
> > > > Cc: Lee Jones
> > > > Cc: Daniel Thompson
> > > > Cc: Jingoo Han
> > > > ---
> > > > include/linux/backlight.h | 1 -
> > > > 1 file changed, 1 deletion(-)
> > > >
> > > > diff --git a/include/linux/backlight.h b/include/linux/backlight.h
> > > > index 7fbf0539e14a..0b5897446dca 100644
> > > > --- a/include/linux/backlight.h
> > > > +++ b/include/linux/backlight.h
> > > > @@ -79,7 +79,6 @@ struct backlight_properties {
> > > > /* Backlight type */
> > > > enum backlight_type type;
> > > > /* Flags used to signal drivers of state changes */
> > > > - /* Upper 4 bits are reserved for driver internal use */
> > > > unsigned int state;
> > > >
> > > > #define BL_CORE_SUSPENDED (1 << 0)/* backlight is
> > > > suspended */
> > >
> >
>
--
Lee Jones [李琼斯]
Linaro Services Technical Lead
Linaro.org │ Open source software for ARM SoCs
Follow Linaro: Facebook | Twitter | Blog
___
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [PATCH] backlight: remove obsolete comment for ->state
On Wed, Jul 04, 2018 at 10:38:16AM +0100, Lee Jones wrote:
> On Wed, 04 Jul 2018, Lee Jones wrote:
>
> > > Jani spotted this when reviewing my earlier patch to remove the driver
> > > internal usage of this field in
> > >
> > > commit 3cf91adaa594e8933af1727942ac560e5c7bc70e
> > > Author: Daniel Vetter
> > > Date: Wed Apr 25 19:42:52 2018 +0200
> > >
> > > backlight: Nuke BL_CORE_DRIVER1
> >
> > FYI, sending patches like this is not a good idea.
> >
> > I'll clean it up for you this time, but in future please send patches
> > properly and place any additional comments you may have below the
> > '---' line.
>
> Ah, I see what you've tried to do. This hurt my eyes! :)
>
> It's more conventional to reference commits like:
>
> Commit 3cf91adaa594 ("backlight: Nuke BL_CORE_DRIVER1")
>
> Again, I'll make the amendment to avoid further confusion.
So the first mail doesn't even bother to explain what's objectionable and
the 2nd mail still says "This hurts my eyes!".
Over whitespace in the commit message.
This kind of stuff is why graphics people really don't enjoy contributing
to the kernel at large. A friendly request to resend with the color choice
adjusted would go a lot further.
Thanks, Daniel
>
> > > Cc: Jani Nikula
> > > Signed-off-by: Daniel Vetter
> > > Cc: Lee Jones
> > > Cc: Daniel Thompson
> > > Cc: Jingoo Han
> > > ---
> > > include/linux/backlight.h | 1 -
> > > 1 file changed, 1 deletion(-)
> > >
> > > diff --git a/include/linux/backlight.h b/include/linux/backlight.h
> > > index 7fbf0539e14a..0b5897446dca 100644
> > > --- a/include/linux/backlight.h
> > > +++ b/include/linux/backlight.h
> > > @@ -79,7 +79,6 @@ struct backlight_properties {
> > > /* Backlight type */
> > > enum backlight_type type;
> > > /* Flags used to signal drivers of state changes */
> > > - /* Upper 4 bits are reserved for driver internal use */
> > > unsigned int state;
> > >
> > > #define BL_CORE_SUSPENDED(1 << 0)/* backlight is
> > > suspended */
> >
>
> --
> Lee Jones [李琼斯]
> Linaro Services Technical Lead
> Linaro.org │ Open source software for ARM SoCs
> Follow Linaro: Facebook | Twitter | Blog
--
Daniel Vetter
Software Engineer, Intel Corporation
http://blog.ffwll.ch
___
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel
[Bug 200387] amdgpu uses unusually high memory
https://bugzilla.kernel.org/show_bug.cgi?id=200387 --- Comment #28 from phoenix (fe...@feldspaten.org) --- Hi Michel, Hi Christian, that makes sense, I test it on a clean environment. Sorry, that I should have done that in the first place :-/ -- You are receiving this mail because: You are watching the assignee of the bug. ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
[PATCH] drm/savage: off by one in savage_bci_cmdbuf()
The > should be >= here so that we don't read beyond the end of the
dma->buflist[] array.
Signed-off-by: Dan Carpenter
diff --git a/drivers/gpu/drm/savage/savage_state.c
b/drivers/gpu/drm/savage/savage_state.c
index 2db89bed52e8..7559a820bd43 100644
--- a/drivers/gpu/drm/savage/savage_state.c
+++ b/drivers/gpu/drm/savage/savage_state.c
@@ -971,7 +971,7 @@ int savage_bci_cmdbuf(struct drm_device *dev, void *data,
struct drm_file *file_
LOCK_TEST_WITH_RETURN(dev, file_priv);
if (dma && dma->buflist) {
- if (cmdbuf->dma_idx > dma->buf_count) {
+ if (cmdbuf->dma_idx >= dma->buf_count) {
DRM_ERROR
("vertex buffer index %u out of range (0-%u)\n",
cmdbuf->dma_idx, dma->buf_count - 1);
___
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel
[PATCH] drm/amd/display: off by one in find_irq_source_info()
The ->info[] array has DAL_IRQ_SOURCES_NUMBER elements so this condition
should be >= instead of > or we could read one element beyond the end of
the array.
Fixes: 4562236b3bc0 ("drm/amd/dc: Add dc display driver (v2)")
Signed-off-by: Dan Carpenter
diff --git a/drivers/gpu/drm/amd/display/dc/irq/irq_service.c
b/drivers/gpu/drm/amd/display/dc/irq/irq_service.c
index dcdfa0f01551..604bea01fc13 100644
--- a/drivers/gpu/drm/amd/display/dc/irq/irq_service.c
+++ b/drivers/gpu/drm/amd/display/dc/irq/irq_service.c
@@ -78,7 +78,7 @@ const struct irq_source_info *find_irq_source_info(
struct irq_service *irq_service,
enum dc_irq_source source)
{
- if (source > DAL_IRQ_SOURCES_NUMBER || source < DC_IRQ_SOURCE_INVALID)
+ if (source >= DAL_IRQ_SOURCES_NUMBER || source < DC_IRQ_SOURCE_INVALID)
return NULL;
return _service->info[source];
___
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel
[PATCH] drm/virtio: fix bounds check in virtio_gpu_cmd_get_capset()
This doesn't affect runtime because in the current code "idx" is always
valid.
First, we read from "vgdev->capsets[idx].max_size" before checking
whether "idx" is within bounds. And secondly the bounds check is off by
one so we could end up reading one element beyond the end of the
vgdev->capsets[] array.
Fixes: 62fb7a5e1096 ("virtio-gpu: add 3d/virgl support")
Signed-off-by: Dan Carpenter
diff --git a/drivers/gpu/drm/virtio/virtgpu_vq.c
b/drivers/gpu/drm/virtio/virtgpu_vq.c
index 020070d483d3..4735bd1c7321 100644
--- a/drivers/gpu/drm/virtio/virtgpu_vq.c
+++ b/drivers/gpu/drm/virtio/virtgpu_vq.c
@@ -648,11 +648,11 @@ int virtio_gpu_cmd_get_capset(struct virtio_gpu_device
*vgdev,
{
struct virtio_gpu_get_capset *cmd_p;
struct virtio_gpu_vbuffer *vbuf;
- int max_size = vgdev->capsets[idx].max_size;
+ int max_size;
struct virtio_gpu_drv_cap_cache *cache_ent;
void *resp_buf;
- if (idx > vgdev->num_capsets)
+ if (idx >= vgdev->num_capsets)
return -EINVAL;
if (version > vgdev->capsets[idx].max_version)
@@ -662,6 +662,7 @@ int virtio_gpu_cmd_get_capset(struct virtio_gpu_device
*vgdev,
if (!cache_ent)
return -ENOMEM;
+ max_size = vgdev->capsets[idx].max_size;
cache_ent->caps_cache = kmalloc(max_size, GFP_KERNEL);
if (!cache_ent->caps_cache) {
kfree(cache_ent);
___
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel
[PATCH] drm/panel: type promotion bug in s6e8aa0_read_mtp_id()
The ARRAY_SIZE() macro is type size_t. If s6e8aa0_dcs_read() returns a
negative error code, then "ret < ARRAY_SIZE(id)" is false because the
negative error code is type promoted to a high positive value.
Fixes: 02051ca06371 ("drm/panel: add S6E8AA0 driver")
Signed-off-by: Dan Carpenter
diff --git a/drivers/gpu/drm/panel/panel-samsung-s6e8aa0.c
b/drivers/gpu/drm/panel/panel-samsung-s6e8aa0.c
index a188a3959f1a..6ad827b93ae1 100644
--- a/drivers/gpu/drm/panel/panel-samsung-s6e8aa0.c
+++ b/drivers/gpu/drm/panel/panel-samsung-s6e8aa0.c
@@ -823,7 +823,7 @@ static void s6e8aa0_read_mtp_id(struct s6e8aa0 *ctx)
int ret, i;
ret = s6e8aa0_dcs_read(ctx, 0xd1, id, ARRAY_SIZE(id));
- if (ret < ARRAY_SIZE(id) || id[0] == 0x00) {
+ if (ret < 0 || ret < ARRAY_SIZE(id) || id[0] == 0x00) {
dev_err(ctx->dev, "read id failed\n");
ctx->error = -EIO;
return;
___
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [PATCH] backlight: remove obsolete comment for ->state
On Wed, 04 Jul 2018, Lee Jones wrote:
> > Jani spotted this when reviewing my earlier patch to remove the driver
> > internal usage of this field in
> >
> > commit 3cf91adaa594e8933af1727942ac560e5c7bc70e
> > Author: Daniel Vetter
> > Date: Wed Apr 25 19:42:52 2018 +0200
> >
> > backlight: Nuke BL_CORE_DRIVER1
>
> FYI, sending patches like this is not a good idea.
>
> I'll clean it up for you this time, but in future please send patches
> properly and place any additional comments you may have below the
> '---' line.
Ah, I see what you've tried to do. This hurt my eyes! :)
It's more conventional to reference commits like:
Commit 3cf91adaa594 ("backlight: Nuke BL_CORE_DRIVER1")
Again, I'll make the amendment to avoid further confusion.
> > Cc: Jani Nikula
> > Signed-off-by: Daniel Vetter
> > Cc: Lee Jones
> > Cc: Daniel Thompson
> > Cc: Jingoo Han
> > ---
> > include/linux/backlight.h | 1 -
> > 1 file changed, 1 deletion(-)
> >
> > diff --git a/include/linux/backlight.h b/include/linux/backlight.h
> > index 7fbf0539e14a..0b5897446dca 100644
> > --- a/include/linux/backlight.h
> > +++ b/include/linux/backlight.h
> > @@ -79,7 +79,6 @@ struct backlight_properties {
> > /* Backlight type */
> > enum backlight_type type;
> > /* Flags used to signal drivers of state changes */
> > - /* Upper 4 bits are reserved for driver internal use */
> > unsigned int state;
> >
> > #define BL_CORE_SUSPENDED (1 << 0)/* backlight is suspended */
>
--
Lee Jones [李琼斯]
Linaro Services Technical Lead
Linaro.org │ Open source software for ARM SoCs
Follow Linaro: Facebook | Twitter | Blog
___
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [PATCH 5/5] dma-fence: Polish kernel-doc for dma-fence.c
Am 04.07.2018 um 11:29 schrieb Daniel Vetter:
- Intro section that links to how this is exposed to userspace.
- Lots more hyperlinks.
- Minor clarifications and style polish
v2: Add misplaced hunk of kerneldoc from a different patch.
Signed-off-by: Daniel Vetter
Cc: Sumit Semwal
Cc: Gustavo Padovan
Cc: linux-me...@vger.kernel.org
Cc: linaro-mm-...@lists.linaro.org
Reviewed-by: Christian König
---
Documentation/driver-api/dma-buf.rst | 6 ++
drivers/dma-buf/dma-fence.c | 147 +++
2 files changed, 109 insertions(+), 44 deletions(-)
diff --git a/Documentation/driver-api/dma-buf.rst
b/Documentation/driver-api/dma-buf.rst
index dc384f2f7f34..b541e97c7ab1 100644
--- a/Documentation/driver-api/dma-buf.rst
+++ b/Documentation/driver-api/dma-buf.rst
@@ -130,6 +130,12 @@ Reservation Objects
DMA Fences
--
+.. kernel-doc:: drivers/dma-buf/dma-fence.c
+ :doc: DMA fences overview
+
+DMA Fences Functions Reference
+~~
+
.. kernel-doc:: drivers/dma-buf/dma-fence.c
:export:
diff --git a/drivers/dma-buf/dma-fence.c b/drivers/dma-buf/dma-fence.c
index 7a92f85a4cec..1551ca7df394 100644
--- a/drivers/dma-buf/dma-fence.c
+++ b/drivers/dma-buf/dma-fence.c
@@ -38,12 +38,43 @@ EXPORT_TRACEPOINT_SYMBOL(dma_fence_enable_signal);
*/
static atomic64_t dma_fence_context_counter = ATOMIC64_INIT(0);
+/**
+ * DOC: DMA fences overview
+ *
+ * DMA fences, represented by dma_fence, are the kernel internal
+ * synchronization primitive for DMA operations like GPU rendering, video
+ * encoding/decoding, or displaying buffers on a screen.
+ *
+ * A fence is initialized using dma_fence_init() and completed using
+ * dma_fence_signal(). Fences are associated with a context, allocated through
+ * dma_fence_context_alloc(), and all fences on the same context are
+ * fully ordered.
+ *
+ * Since the purposes of fences is to facilitate cross-device and
+ * cross-application synchronization, there's multiple ways to use one:
+ *
+ * - Individual fences can be exposed as a _file, accessed as a file
+ * descriptor from userspace, created by calling sync_file_create(). This is
+ * called explicit fencing, since userspace passes around explicit
+ * synchronization points.
+ *
+ * - Some subsystems also have their own explicit fencing primitives, like
+ * _syncobj. Compared to _file, a _syncobj allows the underlying
+ * fence to be updated.
+ *
+ * - Then there's also implicit fencing, where the synchronization points are
+ * implicitly passed around as part of shared _buf instances. Such
+ * implicit fences are stored in reservation_object through the
+ * _buf.resv pointer.
+ */
+
/**
* dma_fence_context_alloc - allocate an array of fence contexts
- * @num: [in]amount of contexts to allocate
+ * @num: amount of contexts to allocate
*
- * This function will return the first index of the number of fences allocated.
- * The fence context is used for setting fence->context to a unique number.
+ * This function will return the first index of the number of fence contexts
+ * allocated. The fence context is used for setting _fence.context to a
+ * unique number by passing the context to dma_fence_init().
*/
u64 dma_fence_context_alloc(unsigned num)
{
@@ -59,10 +90,14 @@ EXPORT_SYMBOL(dma_fence_context_alloc);
* Signal completion for software callbacks on a fence, this will unblock
* dma_fence_wait() calls and run all the callbacks added with
* dma_fence_add_callback(). Can be called multiple times, but since a fence
- * can only go from unsignaled to signaled state, it will only be effective
- * the first time.
+ * can only go from the unsignaled to the signaled state and not back, it will
+ * only be effective the first time.
+ *
+ * Unlike dma_fence_signal(), this function must be called with _fence.lock
+ * held.
*
- * Unlike dma_fence_signal, this function must be called with fence->lock held.
+ * Returns 0 on success and a negative error value when @fence has been
+ * signalled already.
*/
int dma_fence_signal_locked(struct dma_fence *fence)
{
@@ -102,8 +137,11 @@ EXPORT_SYMBOL(dma_fence_signal_locked);
* Signal completion for software callbacks on a fence, this will unblock
* dma_fence_wait() calls and run all the callbacks added with
* dma_fence_add_callback(). Can be called multiple times, but since a fence
- * can only go from unsignaled to signaled state, it will only be effective
- * the first time.
+ * can only go from the unsignaled to the signaled state and not back, it will
+ * only be effective the first time.
+ *
+ * Returns 0 on success and a negative error value when @fence has been
+ * signalled already.
*/
int dma_fence_signal(struct dma_fence *fence)
{
@@ -136,9 +174,9 @@ EXPORT_SYMBOL(dma_fence_signal);
/**
* dma_fence_wait_timeout - sleep until the fence gets signaled
* or until timeout elapses
- * @fence: [in]the fence to wait
Re: [PATCH -next 05/15] drm/vmwgfx: Fix atomic mode set check
Hi,
On 07/04/2018 10:35 AM, Daniel Vetter wrote:
On Tue, Jul 03, 2018 at 09:14:50PM +0200, Thomas Hellstrom wrote:
From: Sinclair Yeh
vmw_kms_atomic_check_modeset() is currently checking config using the
legacy state, which is updated after a commit has happened.
This means vmw_kms_atomic_check_modeset() will reject an invalid config
on the next update rather than the current one.
Fix this by using the new states for config checking
Signed-off-by: Sinclair Yeh
Reviewed-by: Deepak Rawat
Signed-off-by: Thomas Hellstrom
---
drivers/gpu/drm/vmwgfx/vmwgfx_kms.c | 40 +++--
1 file changed, 26 insertions(+), 14 deletions(-)
diff --git a/drivers/gpu/drm/vmwgfx/vmwgfx_kms.c
b/drivers/gpu/drm/vmwgfx/vmwgfx_kms.c
index e7a7a2e73bbf..6b8541f9215d 100644
--- a/drivers/gpu/drm/vmwgfx/vmwgfx_kms.c
+++ b/drivers/gpu/drm/vmwgfx/vmwgfx_kms.c
@@ -1526,33 +1526,45 @@ static int
vmw_kms_atomic_check_modeset(struct drm_device *dev,
struct drm_atomic_state *state)
{
- struct drm_crtc_state *crtc_state;
+ struct drm_crtc_state *new_crtc_state;
+ struct drm_plane_state *new_plane_state;
+ struct drm_plane *plane;
struct drm_crtc *crtc;
struct vmw_private *dev_priv = vmw_priv(dev);
- int i;
+ int i, ret, cpp = 0;
- for_each_new_crtc_in_state(state, crtc, crtc_state, i) {
- unsigned long requested_bb_mem = 0;
+ ret = drm_atomic_helper_check(dev, state);
- if (dev_priv->active_display_unit == vmw_du_screen_target) {
- struct drm_plane *plane = crtc->primary;
- struct drm_plane_state *plane_state;
+ /* If this is not a STDU, then no more checking is necessary */
+ if (ret || dev_priv->active_display_unit != vmw_du_screen_target)
+ return ret;
- plane_state = drm_atomic_get_new_plane_state(state, plane);
+ for_each_new_plane_in_state(state, plane, new_plane_state, i) {
+ if (new_plane_state->fb) {
+ int current_cpp = new_plane_state->fb->pitches[0] /
+ new_plane_state->fb->width;
- if (plane_state && plane_state->fb) {
- int cpp = plane_state->fb->format->cpp[0];
+ if (cpp == 0)
+ cpp = current_cpp;
+ else if (current_cpp != cpp)
+ return -EINVAL;
+ }
+ }
-requested_bb_mem += crtc->mode.hdisplay * cpp *
- crtc->mode.vdisplay;
- }
+ for_each_new_crtc_in_state(state, crtc, new_crtc_state, i) {
Note that the above too loops only walk over the changed states (by
default), not all planes/crtc. You either need to add them yourself (which
kinda defeats the point of the per-plane/crtc locks for doing pageflips in
parallel). Or you need to track things in a driver private structure
(which you'll only grab on modesets) and just update the delta.
I didn't check the entire code to see whether this is a real problem for
you, just wanted to point this out.
-Daniel
Thanks for pointing this out. This is actually addressed in a later
patch by Deepak which chooses the second alternative. We should perhaps
have squashed these patches, but keeping track internally becomes easier
this way.
/Thomas
___
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [PATCH] backlight: remove obsolete comment for ->state
> Jani spotted this when reviewing my earlier patch to remove the driver
> internal usage of this field in
>
> commit 3cf91adaa594e8933af1727942ac560e5c7bc70e
> Author: Daniel Vetter
> Date: Wed Apr 25 19:42:52 2018 +0200
>
> backlight: Nuke BL_CORE_DRIVER1
FYI, sending patches like this is not a good idea.
I'll clean it up for you this time, but in future please send patches
properly and place any additional comments you may have below the
'---' line.
> Cc: Jani Nikula
> Signed-off-by: Daniel Vetter
> Cc: Lee Jones
> Cc: Daniel Thompson
> Cc: Jingoo Han
> ---
> include/linux/backlight.h | 1 -
> 1 file changed, 1 deletion(-)
>
> diff --git a/include/linux/backlight.h b/include/linux/backlight.h
> index 7fbf0539e14a..0b5897446dca 100644
> --- a/include/linux/backlight.h
> +++ b/include/linux/backlight.h
> @@ -79,7 +79,6 @@ struct backlight_properties {
> /* Backlight type */
> enum backlight_type type;
> /* Flags used to signal drivers of state changes */
> - /* Upper 4 bits are reserved for driver internal use */
> unsigned int state;
>
> #define BL_CORE_SUSPENDED(1 << 0)/* backlight is suspended */
--
Lee Jones [李琼斯]
Linaro Services Technical Lead
Linaro.org │ Open source software for ARM SoCs
Follow Linaro: Facebook | Twitter | Blog
___
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [PATCH] drm: Limite blob creation to drm master
On Mon, Jul 02, 2018 at 01:24:40PM +0300, Ville Syrjälä wrote:
> On Mon, Jul 02, 2018 at 10:12:21AM +0200, Daniel Vetter wrote:
> > This interface allows pretty much unlimited kernel memory allocations,
> > which isn't all that great. But we allow that anyway for any drm
> > master client (through pinning display buffers and stuff, at least for
> > UMA gpus),
>
> At least on i915 memory used by pinned display buffers has some kind
> of upper bound based on the number of planes+max fb size and/or ggtt
> size.
>
> Anyways, patch makes sense so
> Reviewed-by: Ville Syrjälä
CI is unhappy about it because it breaks the testcase. And on second
thought with stuff like drm leases we allow rather unpriviledge things to
be drm master, so this isn't helping all that much really. Also, cgroups
should be able to limit these kernel allocations - we only ever do this in
process context.
Decided to drop this on the floor instead.
-Daniel
>
> > so just limiting it to the active master seems like a
> > reasonable stopgap measure.
> >
> > Fixes: e2f5d2ea479b ("drm/mode: Add user blob-creation ioctl")
> > Cc: Daniel Stone
> > Cc: Ville Syrjälä
> > Cc: Michel Dänzer
> > Signed-off-by: Daniel Vetter
> > ---
> > drivers/gpu/drm/drm_ioctl.c | 4 ++--
> > 1 file changed, 2 insertions(+), 2 deletions(-)
> >
> > diff --git a/drivers/gpu/drm/drm_ioctl.c b/drivers/gpu/drm/drm_ioctl.c
> > index c148eb3be8c2..dc740c381f9e 100644
> > --- a/drivers/gpu/drm/drm_ioctl.c
> > +++ b/drivers/gpu/drm/drm_ioctl.c
> > @@ -656,8 +656,8 @@ static const struct drm_ioctl_desc drm_ioctls[] = {
> > DRM_IOCTL_DEF(DRM_IOCTL_MODE_OBJ_SETPROPERTY,
> > drm_mode_obj_set_property_ioctl, DRM_MASTER|DRM_UNLOCKED),
> > DRM_IOCTL_DEF(DRM_IOCTL_MODE_CURSOR2, drm_mode_cursor2_ioctl,
> > DRM_MASTER|DRM_UNLOCKED),
> > DRM_IOCTL_DEF(DRM_IOCTL_MODE_ATOMIC, drm_mode_atomic_ioctl,
> > DRM_MASTER|DRM_UNLOCKED),
> > - DRM_IOCTL_DEF(DRM_IOCTL_MODE_CREATEPROPBLOB, drm_mode_createblob_ioctl,
> > DRM_UNLOCKED),
> > - DRM_IOCTL_DEF(DRM_IOCTL_MODE_DESTROYPROPBLOB,
> > drm_mode_destroyblob_ioctl, DRM_UNLOCKED),
> > + DRM_IOCTL_DEF(DRM_IOCTL_MODE_CREATEPROPBLOB, drm_mode_createblob_ioctl,
> > DRM_MASTER|DRM_UNLOCKED),
> > + DRM_IOCTL_DEF(DRM_IOCTL_MODE_DESTROYPROPBLOB,
> > drm_mode_destroyblob_ioctl, DRM_MASTER|DRM_UNLOCKED),
> >
> > DRM_IOCTL_DEF(DRM_IOCTL_SYNCOBJ_CREATE, drm_syncobj_create_ioctl,
> > DRM_UNLOCKED|DRM_RENDER_ALLOW),
> > --
> > 2.18.0
>
> --
> Ville Syrjälä
> Intel
--
Daniel Vetter
Software Engineer, Intel Corporation
http://blog.ffwll.ch
___
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [PATCH] dma-buf: Move BUG_ON from _add_shared_fence to _add_shared_inplace
Am 04.07.2018 um 11:09 schrieb Michel Dänzer: On 2018-07-04 10:31 AM, Christian König wrote: Am 26.06.2018 um 16:31 schrieb Michel Dänzer: From: Michel Dänzer Fixes the BUG_ON spuriously triggering under the following circumstances: * ttm_eu_reserve_buffers processes a list containing multiple BOs using the same reservation object, so it calls reservation_object_reserve_shared with that reservation object once for each such BO. * In reservation_object_reserve_shared, old->shared_count == old->shared_max - 1, so obj->staged is freed in preparation of an in-place update. * ttm_eu_fence_buffer_objects calls reservation_object_add_shared_fence once for each of the BOs above, always with the same fence. * The first call adds the fence in the remaining free slot, after which old->shared_count == old->shared_max. Well, the explanation here is not correct. For multiple BOs using the same reservation object we won't call reservation_object_add_shared_fence() multiple times because we move those to the duplicates list in ttm_eu_reserve_buffers(). But this bug can still happen because we call reservation_object_add_shared_fence() manually with fences for the same context in a couple of places. One prominent case which comes to my mind are for the VM BOs during updates. Another possibility are VRAM BOs which need to be cleared. Thanks. How about the following: * ttm_eu_reserve_buffers calls reservation_object_reserve_shared. * In reservation_object_reserve_shared, shared_count == shared_max - 1, so obj->staged is freed in preparation of an in-place update. * ttm_eu_fence_buffer_objects calls reservation_object_add_shared_fence, after which shared_count == shared_max. * The amdgpu driver also calls reservation_object_add_shared_fence for the same reservation object, and the BUG_ON triggers. I would rather completely drop the reference to the ttm_eu_* functions, cause those wrappers are completely unrelated to the problem. Instead let's just note something like the following: * When reservation_object_reserve_shared is called with shared_count == shared_max - 1, so obj->staged is freed in preparation of an in-place update. * Now reservation_object_add_shared_fence is called with the first fence and after that shared_count == shared_max. * After that reservation_object_add_shared_fence can be called with follow up fences from the same context, but since shared_count == shared_max we would run into this BUG_ON. However, nothing bad would happen in reservation_object_add_shared_inplace, since all fences use the same context, so they can only occupy a single slot. Prevent this by moving the BUG_ON to where an overflow would actually happen (e.g. if a buggy caller didn't call reservation_object_reserve_shared before). Also, I'll add a reference to https://bugs.freedesktop.org/106418 in v2, as I suspect this fix is necessary under the circumstances described there as well. The rest sounds good to me. Regards, Christian. ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
[PATCH 4/5] drm/vgem: Remove unecessary dma_fence_ops
dma_fence_default_wait is the default now, same for the trivial
enable_signaling implementation.
Also remove the ->signaled callback, vgem can't peek ahead with a
fastpath, returning false is the default implementation.
Signed-off-by: Daniel Vetter
Cc: Kees Cook
Cc: Cihangir Akturk
Cc: Sean Paul
Cc: Daniel Vetter
---
drivers/gpu/drm/vgem/vgem_fence.c | 14 --
1 file changed, 14 deletions(-)
diff --git a/drivers/gpu/drm/vgem/vgem_fence.c
b/drivers/gpu/drm/vgem/vgem_fence.c
index b28876c222b4..75adedeaa384 100644
--- a/drivers/gpu/drm/vgem/vgem_fence.c
+++ b/drivers/gpu/drm/vgem/vgem_fence.c
@@ -43,16 +43,6 @@ static const char *vgem_fence_get_timeline_name(struct
dma_fence *fence)
return "unbound";
}
-static bool vgem_fence_signaled(struct dma_fence *fence)
-{
- return false;
-}
-
-static bool vgem_fence_enable_signaling(struct dma_fence *fence)
-{
- return true;
-}
-
static void vgem_fence_release(struct dma_fence *base)
{
struct vgem_fence *fence = container_of(base, typeof(*fence), base);
@@ -76,11 +66,7 @@ static void vgem_fence_timeline_value_str(struct dma_fence
*fence, char *str,
static const struct dma_fence_ops vgem_fence_ops = {
.get_driver_name = vgem_fence_get_driver_name,
.get_timeline_name = vgem_fence_get_timeline_name,
- .enable_signaling = vgem_fence_enable_signaling,
- .signaled = vgem_fence_signaled,
- .wait = dma_fence_default_wait,
.release = vgem_fence_release,
-
.fence_value_str = vgem_fence_value_str,
.timeline_value_str = vgem_fence_timeline_value_str,
};
--
2.18.0
___
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel
[PATCH 5/5] dma-fence: Polish kernel-doc for dma-fence.c
- Intro section that links to how this is exposed to userspace.
- Lots more hyperlinks.
- Minor clarifications and style polish
v2: Add misplaced hunk of kerneldoc from a different patch.
Signed-off-by: Daniel Vetter
Cc: Sumit Semwal
Cc: Gustavo Padovan
Cc: linux-me...@vger.kernel.org
Cc: linaro-mm-...@lists.linaro.org
---
Documentation/driver-api/dma-buf.rst | 6 ++
drivers/dma-buf/dma-fence.c | 147 +++
2 files changed, 109 insertions(+), 44 deletions(-)
diff --git a/Documentation/driver-api/dma-buf.rst
b/Documentation/driver-api/dma-buf.rst
index dc384f2f7f34..b541e97c7ab1 100644
--- a/Documentation/driver-api/dma-buf.rst
+++ b/Documentation/driver-api/dma-buf.rst
@@ -130,6 +130,12 @@ Reservation Objects
DMA Fences
--
+.. kernel-doc:: drivers/dma-buf/dma-fence.c
+ :doc: DMA fences overview
+
+DMA Fences Functions Reference
+~~
+
.. kernel-doc:: drivers/dma-buf/dma-fence.c
:export:
diff --git a/drivers/dma-buf/dma-fence.c b/drivers/dma-buf/dma-fence.c
index 7a92f85a4cec..1551ca7df394 100644
--- a/drivers/dma-buf/dma-fence.c
+++ b/drivers/dma-buf/dma-fence.c
@@ -38,12 +38,43 @@ EXPORT_TRACEPOINT_SYMBOL(dma_fence_enable_signal);
*/
static atomic64_t dma_fence_context_counter = ATOMIC64_INIT(0);
+/**
+ * DOC: DMA fences overview
+ *
+ * DMA fences, represented by dma_fence, are the kernel internal
+ * synchronization primitive for DMA operations like GPU rendering, video
+ * encoding/decoding, or displaying buffers on a screen.
+ *
+ * A fence is initialized using dma_fence_init() and completed using
+ * dma_fence_signal(). Fences are associated with a context, allocated through
+ * dma_fence_context_alloc(), and all fences on the same context are
+ * fully ordered.
+ *
+ * Since the purposes of fences is to facilitate cross-device and
+ * cross-application synchronization, there's multiple ways to use one:
+ *
+ * - Individual fences can be exposed as a _file, accessed as a file
+ * descriptor from userspace, created by calling sync_file_create(). This is
+ * called explicit fencing, since userspace passes around explicit
+ * synchronization points.
+ *
+ * - Some subsystems also have their own explicit fencing primitives, like
+ * _syncobj. Compared to _file, a _syncobj allows the underlying
+ * fence to be updated.
+ *
+ * - Then there's also implicit fencing, where the synchronization points are
+ * implicitly passed around as part of shared _buf instances. Such
+ * implicit fences are stored in reservation_object through the
+ * _buf.resv pointer.
+ */
+
/**
* dma_fence_context_alloc - allocate an array of fence contexts
- * @num: [in]amount of contexts to allocate
+ * @num: amount of contexts to allocate
*
- * This function will return the first index of the number of fences allocated.
- * The fence context is used for setting fence->context to a unique number.
+ * This function will return the first index of the number of fence contexts
+ * allocated. The fence context is used for setting _fence.context to a
+ * unique number by passing the context to dma_fence_init().
*/
u64 dma_fence_context_alloc(unsigned num)
{
@@ -59,10 +90,14 @@ EXPORT_SYMBOL(dma_fence_context_alloc);
* Signal completion for software callbacks on a fence, this will unblock
* dma_fence_wait() calls and run all the callbacks added with
* dma_fence_add_callback(). Can be called multiple times, but since a fence
- * can only go from unsignaled to signaled state, it will only be effective
- * the first time.
+ * can only go from the unsignaled to the signaled state and not back, it will
+ * only be effective the first time.
+ *
+ * Unlike dma_fence_signal(), this function must be called with _fence.lock
+ * held.
*
- * Unlike dma_fence_signal, this function must be called with fence->lock held.
+ * Returns 0 on success and a negative error value when @fence has been
+ * signalled already.
*/
int dma_fence_signal_locked(struct dma_fence *fence)
{
@@ -102,8 +137,11 @@ EXPORT_SYMBOL(dma_fence_signal_locked);
* Signal completion for software callbacks on a fence, this will unblock
* dma_fence_wait() calls and run all the callbacks added with
* dma_fence_add_callback(). Can be called multiple times, but since a fence
- * can only go from unsignaled to signaled state, it will only be effective
- * the first time.
+ * can only go from the unsignaled to the signaled state and not back, it will
+ * only be effective the first time.
+ *
+ * Returns 0 on success and a negative error value when @fence has been
+ * signalled already.
*/
int dma_fence_signal(struct dma_fence *fence)
{
@@ -136,9 +174,9 @@ EXPORT_SYMBOL(dma_fence_signal);
/**
* dma_fence_wait_timeout - sleep until the fence gets signaled
* or until timeout elapses
- * @fence: [in]the fence to wait on
- * @intr: [in]if true, do an interruptible wait
- * @timeout: [in]timeout value in jiffies, or
[PATCH 1/5] drm/i915: Remove unecessary dma_fence_ops
dma_fence_default_wait is the default now, same for the trivial
enable_signaling implementation.
v2: Also remove the relase hook, dma_fence_free is the default.
Signed-off-by: Daniel Vetter
Cc: Jani Nikula
Cc: Joonas Lahtinen
Cc: Rodrigo Vivi
Cc: Chris Wilson
Cc: Tvrtko Ursulin
Cc: Colin Ian King
Cc: Daniel Vetter
Cc: Mika Kuoppala
Cc: intel-...@lists.freedesktop.org
---
drivers/gpu/drm/i915/i915_gem_clflush.c| 7 ---
drivers/gpu/drm/i915/selftests/i915_sw_fence.c | 8
2 files changed, 15 deletions(-)
diff --git a/drivers/gpu/drm/i915/i915_gem_clflush.c
b/drivers/gpu/drm/i915/i915_gem_clflush.c
index f5c570d35b2a..8e74c23cbd91 100644
--- a/drivers/gpu/drm/i915/i915_gem_clflush.c
+++ b/drivers/gpu/drm/i915/i915_gem_clflush.c
@@ -45,11 +45,6 @@ static const char *i915_clflush_get_timeline_name(struct
dma_fence *fence)
return "clflush";
}
-static bool i915_clflush_enable_signaling(struct dma_fence *fence)
-{
- return true;
-}
-
static void i915_clflush_release(struct dma_fence *fence)
{
struct clflush *clflush = container_of(fence, typeof(*clflush), dma);
@@ -63,8 +58,6 @@ static void i915_clflush_release(struct dma_fence *fence)
static const struct dma_fence_ops i915_clflush_ops = {
.get_driver_name = i915_clflush_get_driver_name,
.get_timeline_name = i915_clflush_get_timeline_name,
- .enable_signaling = i915_clflush_enable_signaling,
- .wait = dma_fence_default_wait,
.release = i915_clflush_release,
};
diff --git a/drivers/gpu/drm/i915/selftests/i915_sw_fence.c
b/drivers/gpu/drm/i915/selftests/i915_sw_fence.c
index 570e325af93e..cdbc8f134e5e 100644
--- a/drivers/gpu/drm/i915/selftests/i915_sw_fence.c
+++ b/drivers/gpu/drm/i915/selftests/i915_sw_fence.c
@@ -611,17 +611,9 @@ static const char *mock_name(struct dma_fence *fence)
return "mock";
}
-static bool mock_enable_signaling(struct dma_fence *fence)
-{
- return true;
-}
-
static const struct dma_fence_ops mock_fence_ops = {
.get_driver_name = mock_name,
.get_timeline_name = mock_name,
- .enable_signaling = mock_enable_signaling,
- .wait = dma_fence_default_wait,
- .release = dma_fence_free,
};
static DEFINE_SPINLOCK(mock_fence_lock);
--
2.18.0
___
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel
[PATCH 0/5] RESEND: dma-buf cleanup
Hi all, This is a resend of my dma-buf cleanup with the patches that didn't yet get and ack/r-b. Feedback very much welcome. Thanks, Daniel Daniel Vetter (5): drm/i915: Remove unecessary dma_fence_ops drm/msm: Remove unecessary dma_fence_ops drm/nouveau: Remove unecessary dma_fence_ops drm/vgem: Remove unecessary dma_fence_ops dma-fence: Polish kernel-doc for dma-fence.c Documentation/driver-api/dma-buf.rst | 6 + drivers/dma-buf/dma-fence.c | 147 -- drivers/gpu/drm/i915/i915_gem_clflush.c | 7 - .../gpu/drm/i915/selftests/i915_sw_fence.c| 8 - drivers/gpu/drm/msm/msm_fence.c | 8 - drivers/gpu/drm/nouveau/nouveau_fence.c | 1 - drivers/gpu/drm/vgem/vgem_fence.c | 14 -- 7 files changed, 109 insertions(+), 82 deletions(-) -- 2.18.0 ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
[PATCH 2/5] drm/msm: Remove unecessary dma_fence_ops
dma_fence_default_wait is the default now, same for the trivial
enable_signaling implementation.
v2: Also remove the relase hook, dma_fence_free is the default.
Signed-off-by: Daniel Vetter
Cc: Rob Clark
Cc: linux-arm-...@vger.kernel.org
Cc: freedr...@lists.freedesktop.org
---
drivers/gpu/drm/msm/msm_fence.c | 8
1 file changed, 8 deletions(-)
diff --git a/drivers/gpu/drm/msm/msm_fence.c b/drivers/gpu/drm/msm/msm_fence.c
index 349c12f670eb..77263cf97b20 100644
--- a/drivers/gpu/drm/msm/msm_fence.c
+++ b/drivers/gpu/drm/msm/msm_fence.c
@@ -119,11 +119,6 @@ static const char *msm_fence_get_timeline_name(struct
dma_fence *fence)
return f->fctx->name;
}
-static bool msm_fence_enable_signaling(struct dma_fence *fence)
-{
- return true;
-}
-
static bool msm_fence_signaled(struct dma_fence *fence)
{
struct msm_fence *f = to_msm_fence(fence);
@@ -133,10 +128,7 @@ static bool msm_fence_signaled(struct dma_fence *fence)
static const struct dma_fence_ops msm_fence_ops = {
.get_driver_name = msm_fence_get_driver_name,
.get_timeline_name = msm_fence_get_timeline_name,
- .enable_signaling = msm_fence_enable_signaling,
.signaled = msm_fence_signaled,
- .wait = dma_fence_default_wait,
- .release = dma_fence_free,
};
struct dma_fence *
--
2.18.0
___
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel
[PATCH 3/5] drm/nouveau: Remove unecessary dma_fence_ops
dma_fence_default_wait is the default now.
Signed-off-by: Daniel Vetter
Cc: Ben Skeggs
Cc: nouv...@lists.freedesktop.org
---
drivers/gpu/drm/nouveau/nouveau_fence.c | 1 -
1 file changed, 1 deletion(-)
diff --git a/drivers/gpu/drm/nouveau/nouveau_fence.c
b/drivers/gpu/drm/nouveau/nouveau_fence.c
index 412d49bc6e56..99be61ddeb75 100644
--- a/drivers/gpu/drm/nouveau/nouveau_fence.c
+++ b/drivers/gpu/drm/nouveau/nouveau_fence.c
@@ -526,6 +526,5 @@ static const struct dma_fence_ops nouveau_fence_ops_uevent
= {
.get_timeline_name = nouveau_fence_get_timeline_name,
.enable_signaling = nouveau_fence_enable_signaling,
.signaled = nouveau_fence_is_signaled,
- .wait = dma_fence_default_wait,
.release = nouveau_fence_release
};
--
2.18.0
___
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: DRM COLOR_RANGE property
On Wed, Jul 04, 2018 at 10:05:41AM +0100, Russell King - ARM Linux wrote: > On Wed, Jul 04, 2018 at 10:26:04AM +0200, Daniel Vetter wrote: > > On Tue, Jul 03, 2018 at 05:18:57PM +0100, Russell King - ARM Linux wrote: > > > Can someone provide a deeper explanation about exactly what this > > > property represents please? > > > > > > Does this represent the range of YCbCr values _into_ a YCbCr-to-RGB > > > conversion (in other words, the range of values in the framebuffer), > > > or the expected output range from the conversion? > > > > > > This matters, because a "limited", (iow, eg, BT601 compliant YCbCr) > > > framebuffer where the Y signal is between 16..235 being displayed > > > on a full-range RGB output would need different conversion from > > > needing a limited-range RGB output. > > > > > > If it is indeed the output, then why is this a property of the plane? > > > Is that not a property of: > > > > > > (a) whether the plane is being blended or overlaid onto a graphics > > > plane which uses full-range RGB > > > (b) the properties of the output(s) to which the plane is being > > > displayed. > > > > > > IOW, it seems that the output of the CSC is more to do with what's > > > downstream of the plane than with the plane itself. > > > > > > For example, take this situation: > > > > > > plane 0 - graphics, full range RGB > > > >-- CRTC --> HDMI sink only supporting > > > plane 1 - video, limited range YUV limited range RGB > > > > > > In order to display the graphics correctly in that scenario, the HDMI > > > output needs to compress the RGB 0-255 range down to 16..236 to be > > > compliant. If the video is limited range, and the CSC produces a > > > limited range RGB output, then plane 1 gets its range further > > > compressed at the HDMI output, which surely is undesirable. > > > > > > It would surely be better, if it's not possible to map the range of > > > plane 0 to limited range, to instead expand the YUV range and then > > > recompress it at the HDMI output to match the capabilities of the > > > attached source. > > > > > > It also seems logical that describing the range of the RGB plane would > > > also be sane - if the application is limiting graphics RGB to 16..235, > > > then you'd want the CSC output to do the same and there'd be no need > > > for any range expansion or compression. > > > > > > I'd personally like drm_plane_create_color_properties() to allow > > > creation of COLOR_ENCODING without COLOR_RANGE (iow, supported_ranges > > > being zero) until COLOR_RANGE is better defined than it is at present. > > > > > > Thoughts? > > > > > > I'm bringing this up, because the hardware I have has a CSC that > > > accepts BT601 and BT709 formats, controlled by a single bit. Another > > > bit controls whether the CSC produces 0..255 output or 16..235 output. > > > That is then blended/overlaid with the graphics plane (0..255) and > > > sent to the output. Having a "limited range" YUV plane produce > > > 16..235 range output makes it look low-contrast compared to the > > > graphics, which is what would be expected - "16" is not black > > > compared to the black of the graphics in the same way that "235" is > > > not white compared to the graphics. > > > > Drivers are supposed to automatically figure this out by looking at the > > edid. In i915 we also allow userspace to override this with the "Broadcast > > RGB" property on the connector. Unfortunately we haven't polished that > > property yet (not sure what other drivers are doing tbh), so it's only > > listed in the Documentation/gpu/kms-properties.csv graveyard :-/ > > In which case, I'd like to implement the COLOR_ENCODING property but > not the COLOR_RANGE property until COLOR_RANGE is better defined. > Unfortunately, at the moment the choices are to have either both > properties or no properties - drm_plane_create_color_properties() > doesn't support only creating the encoding property. > > Implementing it without it being well defined is a recipe for having > a broken UAPI. So, I propose: Hm maybe I misunderstood, but I thought the COLOR_RANGE is on the input side. And you need to adjust your CSC to make sure the output range is whatever makes things Just Work. Probably better to poke Ville on these details, and then clarify the docs. The commit adding the color stuff also doesn't point at the userspace, and I'm too lazy to dig it out, so no idea how it's supposed to work. Ideally we'll end up with a patch to add a nice graph to the docs. -Daniel > > 8<= > From: Russell King > Subject: drm: allow COLOR_RANGE property to be optional > > The COLOR_RANGE plane property is not well defined - it doesn't > define where in the colour conversion this control is applied. > If it's an attribute of the data in the plane, then it has the > reverse effect from if it's an attribute of the range of RGB > output from the plane colour converter. > > Rather than being
Re: [PATCH v6] Add udmabuf misc device
On Wed, Jul 04, 2018 at 10:58:25AM +0200, Gerd Hoffmann wrote: > Hi, > > > > Hmm, does MAINTAINERS need an update then? Maintainer and mailing lists > > > listed in the "DMA BUFFER SHARING FRAMEWORK" entry are on Cc. > > > > Yeah, maintainers entry with you as maintainer plus dri-devel as mailing > > list plus drm-misc as repo would be good. Just grep for drm-misc.git for > > tons of examples. > > There is an *existing* entry covering drivers/dma-buf/, and I've dropped > udmabuf.c into that directory, so I've assumed get_maintainers.pl picks > up all relevant dma-buf folks ... > > Covering udmabuf.c maintainance is a different issue. I could just add > myself to the existing entry, or create a new one specifically for > udmabuf. That's what I meant, do a more specific entry to add yourself just for udmabuf. > > > Who should be Cc'ed? > > > > dim add-missing-cc ftw :-) > > That just uses get_maintainer.pl according to the docs, so that wouldn't > change things as that is wired up as sendemail.cccmd already. Except > that dim would probably add the list of people to the commit message. Ah right, I totally missed that you have more on the mail's Cc: than on the patch. I never do that, so always miss them :-) -Daniel -- Daniel Vetter Software Engineer, Intel Corporation http://blog.ffwll.ch ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [PATCH] backlight: remove obsolete comment for ->state
On Thu, May 03, 2018 at 03:32:38PM +0100, Daniel Thompson wrote:
> On Thu, May 03, 2018 at 04:15:17PM +0200, Daniel Vetter wrote:
> > Jani spotted this when reviewing my earlier patch to remove the driver
> > internal usage of this field in
> >
> > commit 3cf91adaa594e8933af1727942ac560e5c7bc70e
> > Author: Daniel Vetter
> > Date: Wed Apr 25 19:42:52 2018 +0200
> >
> > backlight: Nuke BL_CORE_DRIVER1
> >
> > Cc: Jani Nikula
> > Signed-off-by: Daniel Vetter
> > Cc: Lee Jones
> > Cc: Daniel Thompson
> > Cc: Jingoo Han
>
> Acked-by: Daniel Thompson
I don't see this yet in linux-next ... should I stuff it into drm-misc or
will it show up?
Thanks, Daniel
>
> > ---
> > include/linux/backlight.h | 1 -
> > 1 file changed, 1 deletion(-)
> >
> > diff --git a/include/linux/backlight.h b/include/linux/backlight.h
> > index 7fbf0539e14a..0b5897446dca 100644
> > --- a/include/linux/backlight.h
> > +++ b/include/linux/backlight.h
> > @@ -79,7 +79,6 @@ struct backlight_properties {
> > /* Backlight type */
> > enum backlight_type type;
> > /* Flags used to signal drivers of state changes */
> > - /* Upper 4 bits are reserved for driver internal use */
> > unsigned int state;
> >
> > #define BL_CORE_SUSPENDED (1 << 0)/* backlight is suspended */
> > --
> > 2.17.0
> >
--
Daniel Vetter
Software Engineer, Intel Corporation
http://blog.ffwll.ch
___
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [PATCH] dma-buf: Move BUG_ON from _add_shared_fence to _add_shared_inplace
On 2018-07-04 10:31 AM, Christian König wrote: > Am 26.06.2018 um 16:31 schrieb Michel Dänzer: >> From: Michel Dänzer >> >> Fixes the BUG_ON spuriously triggering under the following >> circumstances: >> >> * ttm_eu_reserve_buffers processes a list containing multiple BOs using >> the same reservation object, so it calls >> reservation_object_reserve_shared with that reservation object once >> for each such BO. >> * In reservation_object_reserve_shared, old->shared_count == >> old->shared_max - 1, so obj->staged is freed in preparation of an >> in-place update. >> * ttm_eu_fence_buffer_objects calls reservation_object_add_shared_fence >> once for each of the BOs above, always with the same fence. >> * The first call adds the fence in the remaining free slot, after which >> old->shared_count == old->shared_max. > > Well, the explanation here is not correct. For multiple BOs using the > same reservation object we won't call > reservation_object_add_shared_fence() multiple times because we move > those to the duplicates list in ttm_eu_reserve_buffers(). > > But this bug can still happen because we call > reservation_object_add_shared_fence() manually with fences for the same > context in a couple of places. > > One prominent case which comes to my mind are for the VM BOs during > updates. Another possibility are VRAM BOs which need to be cleared. Thanks. How about the following: * ttm_eu_reserve_buffers calls reservation_object_reserve_shared. * In reservation_object_reserve_shared, shared_count == shared_max - 1, so obj->staged is freed in preparation of an in-place update. * ttm_eu_fence_buffer_objects calls reservation_object_add_shared_fence, after which shared_count == shared_max. * The amdgpu driver also calls reservation_object_add_shared_fence for the same reservation object, and the BUG_ON triggers. However, nothing bad would happen in reservation_object_add_shared_inplace, since all fences use the same context, so they can only occupy a single slot. Prevent this by moving the BUG_ON to where an overflow would actually happen (e.g. if a buggy caller didn't call reservation_object_reserve_shared before). Also, I'll add a reference to https://bugs.freedesktop.org/106418 in v2, as I suspect this fix is necessary under the circumstances described there as well. -- Earthling Michel Dänzer | http://www.amd.com Libre software enthusiast | Mesa and X developer ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [PATCH v6] Add udmabuf misc device
Hi, > > Hmm, does MAINTAINERS need an update then? Maintainer and mailing lists > > listed in the "DMA BUFFER SHARING FRAMEWORK" entry are on Cc. > > Yeah, maintainers entry with you as maintainer plus dri-devel as mailing > list plus drm-misc as repo would be good. Just grep for drm-misc.git for > tons of examples. There is an *existing* entry covering drivers/dma-buf/, and I've dropped udmabuf.c into that directory, so I've assumed get_maintainers.pl picks up all relevant dma-buf folks ... Covering udmabuf.c maintainance is a different issue. I could just add myself to the existing entry, or create a new one specifically for udmabuf. > > Who should be Cc'ed? > > dim add-missing-cc ftw :-) That just uses get_maintainer.pl according to the docs, so that wouldn't change things as that is wired up as sendemail.cccmd already. Except that dim would probably add the list of people to the commit message. cheers, Gerd ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [PATCH] drm/amdgpu/acp: Fix slab-out-of-bounds in mfd_add_device in acp_hw_init
On Tuesday 03 July 2018 09:50 PM, Alex Deucher wrote:
On Mon, Jul 2, 2018 at 5:48 PM, Daniel Kurtz wrote:
Hi Alex,
On Sun, Apr 15, 2018 at 9:48 PM Agrawal, Akshu wrote:
On 4/13/2018 9:45 PM, Daniel Kurtz wrote:
Commit 51f7415039d4 ("drm/amd/amdgpu: creating two I2S instances for
stoney/cz") added support for the "BT_I2S" ACP i2s channel. As part of
this change, one additional acp resource was added, but the "num_resource"
count was accidentally incremented by 2.
This incorrect count eventually causes mfd_add_device() to try to access
an invalid memory address (the location of non-existent resource 5.
This fault was detected by running a KASAN enabled kernel, which produced
the following splat at boot:
[6.612987]
==
[6.613509] BUG: KASAN: slab-out-of-bounds in mfd_add_device+0x4bc/0x7a7
[6.613509] Read of size 8 at addr 880107d4dc58 by task swapper/0/1
[6.613509]
[6.613509] CPU: 0 PID: 1 Comm: swapper/0 Not tainted 4.14.33 #349
[6.613509] Hardware name: Google Grunt/Grunt, BIOS
Google_Grunt.10543.0.2018_04_03_1812 04/02/2018
[6.613509] Call Trace:
[6.613509] dump_stack+0x4d/0x63
[6.613509] print_address_description+0x80/0x2d6
[6.613509] ? mfd_add_device+0x4bc/0x7a7
[6.613509] kasan_report+0x255/0x295
[6.613509] mfd_add_device+0x4bc/0x7a7
[6.613509] ? kasan_kmalloc+0x99/0xa8
[6.613509] ? mfd_add_devices+0x58/0xe4
[6.613509] ? __kmalloc+0x154/0x178
[6.613509] mfd_add_devices+0xa5/0xe4
[6.613509] acp_hw_init+0x92e/0xc4a
[6.613509] amdgpu_device_init+0x1dfb/0x22a2
[6.613509] ? kmalloc_order+0x53/0x5d
[6.613509] ? kmalloc_order_trace+0x23/0xb3
[6.613509] amdgpu_driver_load_kms+0xce/0x267
[6.613509] drm_dev_register+0x169/0x2fb
[6.613509] amdgpu_pci_probe+0x217/0x242
[6.613509] pci_device_probe+0x101/0x18e
[6.613509] driver_probe_device+0x1dd/0x419
[6.613509] ? ___might_sleep+0x80/0x1b6
[6.613509] __driver_attach+0x9f/0xc9
[6.613509] ? driver_probe_device+0x419/0x419
[6.613509] bus_for_each_dev+0xbc/0xe1
[6.613509] bus_add_driver+0x189/0x2c0
[6.613509] driver_register+0x108/0x156
[6.613509] ? ttm_init+0x67/0x67
[6.613509] do_one_initcall+0xb2/0x161
[6.613509] kernel_init_freeable+0x25a/0x308
[6.613509] ? rest_init+0xcc/0xcc
[6.613509] kernel_init+0x11/0x10d
[6.613509] ? rest_init+0xcc/0xcc
[6.613509] ret_from_fork+0x22/0x40
[6.613509]
[6.613509] Allocated by task 1:
[6.613509] save_stack+0x46/0xce
[6.613509] kasan_kmalloc+0x99/0xa8
[6.613509] kmem_cache_alloc_trace+0x11a/0x13e
[6.613509] acp_hw_init+0x210/0xc4a
[6.613509] amdgpu_device_init+0x1dfb/0x22a2
[6.613509] amdgpu_driver_load_kms+0xce/0x267
[6.613509] drm_dev_register+0x169/0x2fb
[6.613509] amdgpu_pci_probe+0x217/0x242
[6.613509] pci_device_probe+0x101/0x18e
[6.613509] driver_probe_device+0x1dd/0x419
[6.613509] __driver_attach+0x9f/0xc9
[6.613509] bus_for_each_dev+0xbc/0xe1
[6.613509] bus_add_driver+0x189/0x2c0
[6.613509] driver_register+0x108/0x156
[6.613509] do_one_initcall+0xb2/0x161
[6.613509] kernel_init_freeable+0x25a/0x308
[6.613509] kernel_init+0x11/0x10d
[6.613509] ret_from_fork+0x22/0x40
[6.613509]
[6.613509] Freed by task 0:
[6.613509] (stack is not available)
[6.613509]
[6.613509] The buggy address belongs to the object at 880107d4db08
[6.613509] which belongs to the cache kmalloc-512 of size 512
[6.613509] The buggy address is located 336 bytes inside of
[6.613509] 512-byte region [880107d4db08, 880107d4dd08)
[6.613509] The buggy address belongs to the page:
[6.613509] page:ea00041f5300 count:1 mapcount:0 mapping:
(null) index:0x0 compound_mapcount: 0
[6.613509] flags: 0x80008100(slab|head)
[6.613509] raw: 80008100
000100120012
[6.613509] raw: ea0004208520 88010b001680 88010b002cc0
[6.613509] page dumped because: kasan: bad access detected
[6.613509]
[6.613509] Memory state around the buggy address:
[6.613509] 880107d4db00: fc 00 00 00 00 00 00 00 00 00 00 00 00 00 00
00
[6.613509] 880107d4db80: 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00
00
[6.613509] >880107d4dc00: 00 00 00 00 00 00 00 00 00 fc fc fc fc fc fc
fc
[6.613509] ^
[6.613509] 880107d4dc80: fc fc fc fc fc fc fc fc fc fc fc fc fc fc fc
fc
[6.613509] 880107d4dd00: fc fc fc fc fc fc fc fc fc fc fc fc fc fc fc
fc
[6.613509]
==
Fixes: 51f7415039d4 ("drm/amd/amdgpu: creating two I2S instances for stoney/cz")
Signed-off-by: Daniel Kurtz
Acked-by: Akshu Agrawal
Was this patch
Re: [PATCH] fb: fix lost console when the user unplugs a USB adapter
On Sun, Jun 03, 2018 at 11:46:29AM -0400, Mikulas Patocka wrote:
> I have a USB display adapter using the udlfb driver and I use it on an ARM
> board that doesn't have any graphics card. When I plug the adapter in, the
> console is properly displayed, however when I unplug and re-plug the
> adapter, the console is not displayed and I can't access it until I reboot
> the board.
>
> The reason is this:
> When the adapter is unplugged, dlfb_usb_disconnect calls
> unlink_framebuffer, then it waits until the reference count drops to zero
> and then it deallocates the framebuffer. However, the console that is
> attached to the framebuffer device keeps the reference count non-zero, so
> the framebuffer device is never destroyed. When the USB adapter is plugged
> again, it creates a new device /dev/fb1 and the console is not attached to
> it.
>
> This patch fixes the bug by unbinding the console from unlink_framebuffer.
> The code to unbind the console is moved from do_unregister_framebuffer to
> a function unbind_console. When the console is unbound, the reference
> count drops to zero and the udlfb driver frees the framebuffer. When the
> adapter is plugged back, a new framebuffer is created and the console is
> attached to it.
>
> Signed-off-by: Mikulas Patocka
> Cc: sta...@vger.kernel.org
Does this work correctly with the udl drm driver and the drm fbdev
emulation? If yes I'm not sure what the value is in fixing up the uldfb
driver really ...
Same for all the uldfb fixes in your other series. If the 2 drivers are
on feature parity I'd just go ahead and remove the uldfb one.
-Daniel
>
> ---
> drivers/video/fbdev/core/fbmem.c | 21 +
> 1 file changed, 17 insertions(+), 4 deletions(-)
>
> Index: linux-4.16.12/drivers/video/fbdev/core/fbmem.c
> ===
> --- linux-4.16.12.orig/drivers/video/fbdev/core/fbmem.c 2018-05-26
> 06:13:20.0 +0200
> +++ linux-4.16.12/drivers/video/fbdev/core/fbmem.c2018-05-26
> 06:13:20.0 +0200
> @@ -1805,12 +1805,12 @@ static int do_register_framebuffer(struc
> return 0;
> }
>
> -static int do_unregister_framebuffer(struct fb_info *fb_info)
> +static int unbind_console(struct fb_info *fb_info)
> {
> struct fb_event event;
> - int i, ret = 0;
> + int ret;
> + int i = fb_info->node;
>
> - i = fb_info->node;
> if (i < 0 || i >= FB_MAX || registered_fb[i] != fb_info)
> return -EINVAL;
>
> @@ -1825,6 +1825,16 @@ static int do_unregister_framebuffer(str
> unlock_fb_info(fb_info);
> console_unlock();
>
> + return ret;
> +}
> +
> +static int do_unregister_framebuffer(struct fb_info *fb_info)
> +{
> + struct fb_event event;
> + int ret;
> +
> + ret = unbind_console(fb_info);
> +
> if (ret)
> return -EINVAL;
>
> @@ -1835,7 +1845,7 @@ static int do_unregister_framebuffer(str
> (fb_info->pixmap.flags & FB_PIXMAP_DEFAULT))
> kfree(fb_info->pixmap.addr);
> fb_destroy_modelist(_info->modelist);
> - registered_fb[i] = NULL;
> + registered_fb[fb_info->node] = NULL;
> num_registered_fb--;
> fb_cleanup_device(fb_info);
> event.info = fb_info;
> @@ -1860,6 +1870,9 @@ int unlink_framebuffer(struct fb_info *f
> device_destroy(fb_class, MKDEV(FB_MAJOR, i));
> fb_info->dev = NULL;
> }
> +
> + unbind_console(fb_info);
> +
> return 0;
> }
> EXPORT_SYMBOL(unlink_framebuffer);
> ___
> dri-devel mailing list
> dri-devel@lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/dri-devel
--
Daniel Vetter
Software Engineer, Intel Corporation
http://blog.ffwll.ch
___
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel
[Bug 200387] amdgpu uses unusually high memory
https://bugzilla.kernel.org/show_bug.cgi?id=200387 --- Comment #27 from Christian König (christian.koe...@amd.com) --- (In reply to Michel Dänzer from comment #26) > BTW, ideally you should only test with the kernel's own amdgpu driver, not > with amdgpu-pro, because the later uses its own copies of core DRM and even > some core kernel code, and has other modifications compared to the stock > driver. To be even more precise I'm not sure that this is actually a kernel problem, or just caused by some mix up between the amdgpu-pro driver and the upstream driver. So testing on a clean install could yield some more results. -- 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 0/5] drm: use core pcie functionality for pcie gen/width
Whole series is Acked-by: Christian König . BTW: With patch #2 you created quite some noise in the news: https://www.tomshardware.com/news/amd-vega-20-pcie-4.0,37389.html Cheers, Christian. Am 25.06.2018 um 23:06 schrieb Alex Deucher: This series exports some pcie helper functions for use by drivers and fixes up the amdgpu and radeon drivers to use this core functionality rather than the duplicated functionality in the drm. Finally we remove the drm helpers since the duplicate the pcie functionality of the core. This also adds proper pcie gen4 detection for amdgpu. Alex Deucher (5): pci: export pcie_get_speed_cap and pcie_get_width_cap drm/amdgpu: update amd_pcie.h to include gen4 speeds drm/amdgpu: use pcie functions for link width and speed drm/radeon: use pcie functions for link width drm: drop drm_pcie_get_speed_cap_mask and drm_pcie_get_max_link_width drivers/gpu/drm/amd/amdgpu/amdgpu_device.c | 83 +- drivers/gpu/drm/amd/amdgpu/amdgpu_dpm.c| 7 ++- drivers/gpu/drm/amd/amdgpu/ci_dpm.c| 3 +- drivers/gpu/drm/amd/amdgpu/si_dpm.c| 3 +- drivers/gpu/drm/amd/include/amd_pcie.h | 2 + drivers/gpu/drm/drm_pci.c | 58 - drivers/gpu/drm/radeon/ci_dpm.c| 20 +-- drivers/gpu/drm/radeon/cik.c | 22 drivers/gpu/drm/radeon/r600_dpm.c | 4 +- drivers/gpu/drm/radeon/radeon.h| 4 ++ drivers/gpu/drm/radeon/si.c| 22 drivers/gpu/drm/radeon/si_dpm.c| 20 +-- drivers/pci/pci.c | 2 + include/drm/drm_pci.h | 7 --- include/linux/pci.h| 3 ++ 15 files changed, 132 insertions(+), 128 deletions(-) ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [PATCH -next 05/15] drm/vmwgfx: Fix atomic mode set check
On Tue, Jul 03, 2018 at 09:14:50PM +0200, Thomas Hellstrom wrote:
> From: Sinclair Yeh
>
> vmw_kms_atomic_check_modeset() is currently checking config using the
> legacy state, which is updated after a commit has happened.
>
> This means vmw_kms_atomic_check_modeset() will reject an invalid config
> on the next update rather than the current one.
>
> Fix this by using the new states for config checking
>
> Signed-off-by: Sinclair Yeh
> Reviewed-by: Deepak Rawat
> Signed-off-by: Thomas Hellstrom
> ---
> drivers/gpu/drm/vmwgfx/vmwgfx_kms.c | 40 +++--
> 1 file changed, 26 insertions(+), 14 deletions(-)
>
> diff --git a/drivers/gpu/drm/vmwgfx/vmwgfx_kms.c
> b/drivers/gpu/drm/vmwgfx/vmwgfx_kms.c
> index e7a7a2e73bbf..6b8541f9215d 100644
> --- a/drivers/gpu/drm/vmwgfx/vmwgfx_kms.c
> +++ b/drivers/gpu/drm/vmwgfx/vmwgfx_kms.c
> @@ -1526,33 +1526,45 @@ static int
> vmw_kms_atomic_check_modeset(struct drm_device *dev,
>struct drm_atomic_state *state)
> {
> - struct drm_crtc_state *crtc_state;
> + struct drm_crtc_state *new_crtc_state;
> + struct drm_plane_state *new_plane_state;
> + struct drm_plane *plane;
> struct drm_crtc *crtc;
> struct vmw_private *dev_priv = vmw_priv(dev);
> - int i;
> + int i, ret, cpp = 0;
>
> - for_each_new_crtc_in_state(state, crtc, crtc_state, i) {
> - unsigned long requested_bb_mem = 0;
> + ret = drm_atomic_helper_check(dev, state);
>
> - if (dev_priv->active_display_unit == vmw_du_screen_target) {
> - struct drm_plane *plane = crtc->primary;
> - struct drm_plane_state *plane_state;
> + /* If this is not a STDU, then no more checking is necessary */
> + if (ret || dev_priv->active_display_unit != vmw_du_screen_target)
> + return ret;
>
> - plane_state = drm_atomic_get_new_plane_state(state,
> plane);
> + for_each_new_plane_in_state(state, plane, new_plane_state, i) {
> + if (new_plane_state->fb) {
> + int current_cpp = new_plane_state->fb->pitches[0] /
> + new_plane_state->fb->width;
>
> - if (plane_state && plane_state->fb) {
> - int cpp = plane_state->fb->format->cpp[0];
> + if (cpp == 0)
> + cpp = current_cpp;
> + else if (current_cpp != cpp)
> + return -EINVAL;
> + }
> + }
>
> - requested_bb_mem += crtc->mode.hdisplay * cpp *
> - crtc->mode.vdisplay;
> - }
> + for_each_new_crtc_in_state(state, crtc, new_crtc_state, i) {
Note that the above too loops only walk over the changed states (by
default), not all planes/crtc. You either need to add them yourself (which
kinda defeats the point of the per-plane/crtc locks for doing pageflips in
parallel). Or you need to track things in a driver private structure
(which you'll only grab on modesets) and just update the delta.
I didn't check the entire code to see whether this is a real problem for
you, just wanted to point this out.
-Daniel
> + unsigned long requested_bb_mem = 0;
> +
> + if (drm_atomic_crtc_needs_modeset(new_crtc_state)) {
> + requested_bb_mem += new_crtc_state->mode.hdisplay *
> + new_crtc_state->mode.vdisplay *
> + cpp;
>
> if (requested_bb_mem > dev_priv->prim_bb_mem)
> return -EINVAL;
> }
> }
>
> - return drm_atomic_helper_check(dev, state);
> + return ret;
> }
>
> static const struct drm_mode_config_funcs vmw_kms_funcs = {
> --
> 2.18.0.rc1
>
> ___
> dri-devel mailing list
> dri-devel@lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/dri-devel
--
Daniel Vetter
Software Engineer, Intel Corporation
http://blog.ffwll.ch
___
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel
