- 27 Jul, 2020 12 commits
-
-
Sam Ravnborg authored
Prepare for chained bridge with the addition of get_edid support. v2: - Fixed handling of edid storage (Laurent) Signed-off-by: Sam Ravnborg <sam@ravnborg.org> Reviewed-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com> Cc: Andrzej Hajda <a.hajda@samsung.com> Cc: Neil Armstrong <narmstrong@baylibre.com> Cc: Laurent Pinchart <Laurent.pinchart@ideasonboard.com> Cc: Jonas Karlman <jonas@kwiboo.se> Cc: Jernej Skrabec <jernej.skrabec@siol.net> Link: https://patchwork.freedesktop.org/patch/msgid/20200727170320.959777-3-sam@ravnborg.org
-
Sam Ravnborg authored
Prepare the bridge driver for chained operation by adding support for the detect operation. v3: - Fix code to make it readable (Laurent) v2: - Do not announce detect operation if there is no hpd pin (Laurent) Signed-off-by: Sam Ravnborg <sam@ravnborg.org> Reviewed-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com> Cc: Andrzej Hajda <a.hajda@samsung.com> Cc: Neil Armstrong <narmstrong@baylibre.com> Cc: Laurent Pinchart <Laurent.pinchart@ideasonboard.com> Cc: Jonas Karlman <jonas@kwiboo.se> Cc: Jernej Skrabec <jernej.skrabec@siol.net> Link: https://patchwork.freedesktop.org/patch/msgid/20200727170320.959777-2-sam@ravnborg.org
-
Sam Ravnborg authored
Make the connector creation optional to enable usage of the megachips-stdpxxxx-ge-b850v3-fw bridge with the DRM bridge connector helper. v2: - Set bridge.type to DRM_MODE_CONNECTOR_DisplayPort Signed-off-by: Sam Ravnborg <sam@ravnborg.org> Reviewed-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com> Cc: Peter Senna Tschudin <peter.senna@gmail.com> Cc: Martin Donnelly <martin.donnelly@ge.com> Cc: Martyn Welch <martyn.welch@collabora.co.uk> Cc: Andrzej Hajda <a.hajda@samsung.com> Cc: Neil Armstrong <narmstrong@baylibre.com> Cc: Laurent Pinchart <Laurent.pinchart@ideasonboard.com> Cc: Jonas Karlman <jonas@kwiboo.se> Cc: Jernej Skrabec <jernej.skrabec@siol.net> Link: https://patchwork.freedesktop.org/patch/msgid/20200726203324.3722593-14-sam@ravnborg.org
-
Sam Ravnborg authored
To prepare for a chained bridge setup add support for the get_edid bridge operation. There is no need for a copy of the edid - so drop the pointer to the copy. v2: - Fix so we do not leak memory (Laurent) Signed-off-by: Sam Ravnborg <sam@ravnborg.org> Reviewed-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com> Cc: Peter Senna Tschudin <peter.senna@gmail.com> Cc: Martin Donnelly <martin.donnelly@ge.com> Cc: Martyn Welch <martyn.welch@collabora.co.uk> Cc: Andrzej Hajda <a.hajda@samsung.com> Cc: Neil Armstrong <narmstrong@baylibre.com> Cc: Laurent Pinchart <Laurent.pinchart@ideasonboard.com> Cc: Jonas Karlman <jonas@kwiboo.se> Cc: Jernej Skrabec <jernej.skrabec@siol.net> Link: https://patchwork.freedesktop.org/patch/msgid/20200726203324.3722593-13-sam@ravnborg.org
-
Sam Ravnborg authored
To prepare for use in a chained bridge setup enable the detect operation. Signed-off-by: Sam Ravnborg <sam@ravnborg.org> Reviewed-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com> Cc: Peter Senna Tschudin <peter.senna@gmail.com> Cc: Martin Donnelly <martin.donnelly@ge.com> Cc: Martyn Welch <martyn.welch@collabora.co.uk> Cc: Andrzej Hajda <a.hajda@samsung.com> Cc: Neil Armstrong <narmstrong@baylibre.com> Cc: Laurent Pinchart <Laurent.pinchart@ideasonboard.com> Cc: Jonas Karlman <jonas@kwiboo.se> Cc: Jernej Skrabec <jernej.skrabec@siol.net> Link: https://patchwork.freedesktop.org/patch/msgid/20200726203324.3722593-12-sam@ravnborg.org
-
Sam Ravnborg authored
Fix so drm_device is read from the bridge. This is a preparation for the connector being optional. Signed-off-by: Sam Ravnborg <sam@ravnborg.org> Reviewed-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com> Cc: Peter Senna Tschudin <peter.senna@gmail.com> Cc: Martin Donnelly <martin.donnelly@ge.com> Cc: Martyn Welch <martyn.welch@collabora.co.uk> Cc: Andrzej Hajda <a.hajda@samsung.com> Cc: Neil Armstrong <narmstrong@baylibre.com> Cc: Laurent Pinchart <Laurent.pinchart@ideasonboard.com> Cc: Jonas Karlman <jonas@kwiboo.se> Cc: Jernej Skrabec <jernej.skrabec@siol.net> Link: https://patchwork.freedesktop.org/patch/msgid/20200726203324.3722593-11-sam@ravnborg.org
-
Sam Ravnborg authored
Factor out connector creation to a small helper function. Signed-off-by: Sam Ravnborg <sam@ravnborg.org> Reviewed-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com> Cc: Peter Senna Tschudin <peter.senna@gmail.com> Cc: Martin Donnelly <martin.donnelly@ge.com> Cc: Martyn Welch <martyn.welch@collabora.co.uk> Cc: Andrzej Hajda <a.hajda@samsung.com> Cc: Neil Armstrong <narmstrong@baylibre.com> Cc: Laurent Pinchart <Laurent.pinchart@ideasonboard.com> Cc: Jonas Karlman <jonas@kwiboo.se> Cc: Jernej Skrabec <jernej.skrabec@siol.net> Link: https://patchwork.freedesktop.org/patch/msgid/20200726203324.3722593-10-sam@ravnborg.org
-
Sam Ravnborg authored
Prepare the bridge driver for use in a chained setup by replacing direct use of drm_panel with drm_panel_bridge support. The connecter is now either created by the panel bridge or the display driver. So all code for connector creation in this driver is no longer relevant and thus dropped. The connector code had some special polling handling: connector.polled = DRM_CONNECTOR_POLL_HPD; drm_helper_hpd_irq_event(ps8622->bridge.dev); This code was most likely added to speed up detection of the connector. If really needed then this functionality belongs somewhere else. Note: the bridge panel will use the connector type from the panel. v2: - Fix to avoid creating connector twice (Laurent) - Drop all connector code - defer to bridge panel - Use panel_bridge for local variable to align with other drivers - Set bridge.type to DRM_MODE_CONNECTOR_LVDS; Signed-off-by: Sam Ravnborg <sam@ravnborg.org> Reviewed-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com> Cc: Andrzej Hajda <a.hajda@samsung.com> Cc: Neil Armstrong <narmstrong@baylibre.com> Cc: Laurent Pinchart <Laurent.pinchart@ideasonboard.com> Cc: Jonas Karlman <jonas@kwiboo.se> Cc: Jernej Skrabec <jernej.skrabec@siol.net> Link: https://patchwork.freedesktop.org/patch/msgid/20200726203324.3722593-9-sam@ravnborg.org
-
Sam Ravnborg authored
Prepare the tc358764 bridge driver for use in a chained setup by replacing direct use of drm_panel with drm_panel_bridge support. The bridge panel will use the connector type reported by the panel, where the connector for this driver hardcodes DRM_MODE_CONNECTOR_LVDS. The tc358764 did not add any additional info the the connector so the connector creation is passed to the bridge panel driver. v3: - Merge with patch to make connector creation optional to avoid creating two connectors (Laurent) - Pass connector creation to bridge panel, as this bridge driver did not add any extra info to the connector. - Set bridge.type to DRM_MODE_CONNECTOR_LVDS. v2: - Use PTR_ERR_OR_ZERO() (kbuild test robot) Signed-off-by: Sam Ravnborg <sam@ravnborg.org> Reviewed-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com> Cc: Laurent Pinchart <laurent.pinchart@ideasonboard.com> Cc: kbuild test robot <lkp@intel.com> Cc: Andrzej Hajda <a.hajda@samsung.com> Cc: Neil Armstrong <narmstrong@baylibre.com> Cc: Jonas Karlman <jonas@kwiboo.se> Cc: Jernej Skrabec <jernej.skrabec@siol.net> Link: https://patchwork.freedesktop.org/patch/msgid/20200726203324.3722593-5-sam@ravnborg.org
-
Sam Ravnborg authored
Drop drm_connector handling that is not needed: - drm_dev_register() in the display controller driver takes care of registering connectors. So the call to drm_connector_register() call is not needed in the bridge driver. - Use of drm_connector_unregister() is only required for drivers that explicit have called drm_dev_register. - The reference counting using drm_connector_put() is likewise not needed. Signed-off-by: Sam Ravnborg <sam@ravnborg.org> Reviewed-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com> Cc: Andrzej Hajda <a.hajda@samsung.com> Cc: Neil Armstrong <narmstrong@baylibre.com> Cc: Laurent Pinchart <Laurent.pinchart@ideasonboard.com> Cc: Jonas Karlman <jonas@kwiboo.se> Cc: Jernej Skrabec <jernej.skrabec@siol.net> Link: https://patchwork.freedesktop.org/patch/msgid/20200726203324.3722593-4-sam@ravnborg.org
-
Sam Ravnborg authored
All panels shall report a connector type. panel-simple has a lot of panels with no connector_type, and for these fall back to DPI as the default. v2: - Rebased on top of validation of panel description Signed-off-by: Sam Ravnborg <sam@ravnborg.org> Reviewed-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com> Cc: Laurent Pinchart <laurent.pinchart@ideasonboard.com> Cc: Thierry Reding <thierry.reding@gmail.com> Cc: Sam Ravnborg <sam@ravnborg.org> Link: https://patchwork.freedesktop.org/patch/msgid/20200726203324.3722593-3-sam@ravnborg.org
-
Sam Ravnborg authored
Warn if we detect a panel with incomplete/wrong description. This is inspired by a similar patch by Laurent that introduced checks for LVDS panels - this extends the checks to the remaining type of connectors. This is known to warn for some of the existing panels but added despite this as we need help from people using the panels to add the missing info. The checks are not complete but will catch the most common mistakes. The checks at the same time serve as documentation for the minimum required description for a panel. The checks uses dev_warn() as we know this will hit. WARN() was too noisy at the moment for anything else than LVDS. v3: - %d => %u for bpc (Laurent) v2: - Use dev_warn (Laurent) - Check for empty bus_flags Signed-off-by: Sam Ravnborg <sam@ravnborg.org> Reviewed-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com> Cc: Laurent Pinchart <laurent.pinchart@ideasonboard.com> Cc: Thierry Reding <thierry.reding@gmail.com> Cc: Sam Ravnborg <sam@ravnborg.org> Link: https://patchwork.freedesktop.org/patch/msgid/20200726203324.3722593-2-sam@ravnborg.org
-
- 26 Jul, 2020 4 commits
-
-
Sam Ravnborg authored
The boe,hv070wsa-100 panel is a LVDS panel. Fix connector type to reflect this. With this change users of this panel do not have to specify the connector type. v4: - Add .bpc = 4 (Laurent) v3: - Drop PIXDATA bus_flag, not relevant v2: - Add .bus_format (Laurent) - Add .bus_flags Signed-off-by: Sam Ravnborg <sam@ravnborg.org> Reviewed-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com> Cc: Laurent Pinchart <laurent.pinchart@ideasonboard.com> Cc: Thierry Reding <thierry.reding@gmail.com> Cc: Sam Ravnborg <sam@ravnborg.org> Link: https://patchwork.freedesktop.org/patch/msgid/20200703192417.372164-2-sam@ravnborg.org
-
Vinod Koul authored
Lontium Lt9611 is a DSI to HDMI bridge which supports two DSI ports and I2S port as an input and HDMI port as output Co-developed-by: Bjorn Andersson <bjorn.andersson@linaro.org> Signed-off-by: Bjorn Andersson <bjorn.andersson@linaro.org> Co-developed-by: Srinivas Kandagatla <srinivas.kandagatla@linaro.org> Signed-off-by: Srinivas Kandagatla <srinivas.kandagatla@linaro.org> Tested-by: John Stultz <john.stultz@linaro.org> Signed-off-by: Vinod Koul <vkoul@kernel.org> Signed-off-by: Sam Ravnborg <sam@ravnborg.org> # fix lt9611_bridge_mode_valid Link: https://patchwork.freedesktop.org/patch/msgid/20200723163442.1280414-4-vkoul@kernel.org
-
Vinod Koul authored
Lontium LT9611 is a DSI to HDMI bridge which supports 2 DSI ports and I2S port as input and one HDMI port as output Reviewed-by: Rob Herring <robh@kernel.org> Tested-by: John Stultz <john.stultz@linaro.org> Signed-off-by: Vinod Koul <vkoul@kernel.org> Signed-off-by: Sam Ravnborg <sam@ravnborg.org> Link: https://patchwork.freedesktop.org/patch/msgid/20200723163442.1280414-3-vkoul@kernel.org
-
Vinod Koul authored
Add prefix for Lontium Semiconductor Corporation Acked-by: Rob Herring <robh@kernel.org> Tested-by: John Stultz <john.stultz@linaro.org> Signed-off-by: Vinod Koul <vkoul@kernel.org> Signed-off-by: Sam Ravnborg <sam@ravnborg.org> Link: https://patchwork.freedesktop.org/patch/msgid/20200723163442.1280414-2-vkoul@kernel.org
-
- 24 Jul, 2020 1 commit
-
-
Ville Syrjälä authored
Add a TODO for plumbing drm_atomic_state all over to ease the hurdles of accessing additional object states. Reviewed-by: Daniel Vetter <daniel.vetter@ffwll.ch> #irc Signed-off-by: Ville Syrjälä <ville.syrjala@linux.intel.com> Link: https://patchwork.freedesktop.org/patch/msgid/20200717135439.5996-1-ville.syrjala@linux.intel.com
-
- 23 Jul, 2020 1 commit
-
-
Linus Walleij authored
Remove notes about migrating from the old driver which is retired as all users are now migrated. Update the text to reflect that we support PL110 and PL111 alike. Drop the bullet on memory bandwidth scaling: this has been implemented. Signed-off-by: Linus Walleij <linus.walleij@linaro.org> Acked-by: Daniel Vetter <daniel.vetter@ffwll.ch> Cc: Russell King <linux@armlinux.org.uk> Link: https://patchwork.freedesktop.org/patch/msgid/20200720130327.92364-1-linus.walleij@linaro.org
-
- 22 Jul, 2020 3 commits
-
-
Christian König authored
Not used any more. And it is bad design to use a TTM flag to do a check inside a driver. Signed-off-by: Christian König <christian.koenig@amd.com> Reviewed-by: Daniel Vetter <daniel.vetter@ffwll.ch> Link: https://patchwork.freedesktop.org/patch/378245/
-
Christian König authored
The driver does support some not-mapable resources, but those are already handled correctly in the switch/case statement in the code. Signed-off-by: Christian König <christian.koenig@amd.com> Reviewed-by: Daniel Vetter <daniel.vetter@ffwll.ch> Link: https://patchwork.freedesktop.org/patch/378243/
-
Christian König authored
The driver doesn't expose any not-mapable memory resources. Signed-off-by: Christian König <christian.koenig@amd.com> Reviewed-by: Daniel Vetter <daniel.vetter@ffwll.ch> Link: https://patchwork.freedesktop.org/patch/378239/
-
- 21 Jul, 2020 19 commits
-
-
Ondrej Jirman authored
The display has one port. Allow it in the binding. Reviewed-by: Rob Herring <robh@kernel.org> Reviewed-by: Guido Günther <agx@sigxcpu.org> Signed-off-by: Ondrej Jirman <megous@megous.com> Signed-off-by: Rob Herring <robh@kernel.org> Link: https://patchwork.freedesktop.org/patch/msgid/20200703114717.2140832-3-megous@megous.com
-
Ondrej Jirman authored
The example is now validated against rocktech,jh057n00900 schema that was ported to yaml, and didn't validate with: - '#address-cells', '#size-cells', 'port@0' do not match any of the regexes: 'pinctrl-[0-9]+' - 'vcc-supply' is a required property - 'iovcc-supply' is a required property - 'reset-gpios' is a required property Fix it. Reviewed-by: Rob Herring <robh@kernel.org> Reviewed-by: Guido Günther <agx@sigxcpu.org> Signed-off-by: Ondrej Jirman <megous@megous.com> Signed-off-by: Rob Herring <robh@kernel.org> Link: https://patchwork.freedesktop.org/patch/msgid/20200703114717.2140832-2-megous@megous.com
-
Lyude Paul authored
This is an ioctl callback, so we're guaranteed to have IRQs enabled when calling this function. Use the plain _irq() variants of spin_(un)lock() to make this more obvious. Signed-off-by: Lyude Paul <lyude@redhat.com> Cc: Daniel Vetter <daniel@ffwll.ch> Link: https://patchwork.freedesktop.org/patch/msgid/20200720190736.180297-6-lyude@redhat.comReviewed-by: Daniel Vetter <daniel.vetter@ffwll.ch>
-
Lyude Paul authored
This one's easy - we're already calling kzalloc(GFP_KERNEL) in this function, so we must already be guaranteed to have IRQs enabled when calling this. So, use the plain _irq() variants of spin_(un)lock() to make this more obvious. Signed-off-by: Lyude Paul <lyude@redhat.com> Cc: Daniel Vetter <daniel@ffwll.ch> Reviewed-by: Daniel Vetter <daniel.vetter@ffwll.ch> Link: https://patchwork.freedesktop.org/patch/msgid/20200720190736.180297-5-lyude@redhat.com
-
Lyude Paul authored
This function is only ever called from ioctl context, so we're guaranteed to have interrupts enabled. Stop using the irqsave/irqrestore variants of spin_(un)lock_irq() to make this more obvious. Signed-off-by: Lyude Paul <lyude@redhat.com> Cc: Daniel Vetter <daniel@ffwll.ch> Link: https://patchwork.freedesktop.org/patch/msgid/20200720190736.180297-4-lyude@redhat.comReviewed-by: Daniel Vetter <daniel.vetter@ffwll.ch>
-
Lyude Paul authored
This is only called from: * Atomic modesetting hooks * Module probing routines * Legacy modesetting hooks All of which have IRQs enabled, so we can also get rid of irqsave/restore here to make the IRQ context of this function more obvious. Signed-off-by: Lyude Paul <lyude@redhat.com> Cc: Daniel Vetter <daniel@ffwll.ch> Link: https://patchwork.freedesktop.org/patch/msgid/20200720190736.180297-3-lyude@redhat.comReviewed-by: Daniel Vetter <daniel.vetter@ffwll.ch>
-
Lyude Paul authored
All of the drivers in the kernel tree only call this from one of the following contexts: * drm_crtc_funcs->reset * During initial module load Since both of these contexts are guaranteed to have interrupts enabled beforehand, there's no need to use the irqsave/irqrestore variants of spin_(un)lock(). So, fix this to make the irq context of this function more obvious. Signed-off-by: Lyude Paul <lyude@redhat.com> Cc: Daniel Vetter <daniel@ffwll.ch> Link: https://patchwork.freedesktop.org/patch/msgid/20200720190736.180297-2-lyude@redhat.comReviewed-by: Daniel Vetter <daniel.vetter@ffwll.ch>
-
Christian König authored
The driver doesn't expose any not-mapable memory resources. v2: remove unused man variable as well Signed-off-by: Christian König <christian.koenig@amd.com> Reviewed-by: Daniel Vetter <daniel.vetter@ffwll.ch> Link: https://patchwork.freedesktop.org/patch/378246/
-
Christian König authored
The driver doesn't expose any not-mapable memory resources. Signed-off-by: Christian König <christian.koenig@amd.com> Reviewed-by: Daniel Vetter <daniel.vetter@ffwll.ch> Link: https://patchwork.freedesktop.org/patch/378244/
-
Christian König authored
The driver doesn't expose any not-mapable memory resources. Signed-off-by: Christian König <christian.koenig@amd.com> Reviewed-by: Thomas Zimmermann <tzimmermann@suse.de> Reviewed-by: Daniel Vetter <daniel.vetter@ffwll.ch> Link: https://patchwork.freedesktop.org/patch/378241/
-
Christian König authored
The original intention was to avoid CPU page table unmaps when BOs move between the GTT and SYSTEM domain. The problem is that this never correctly handled changes in the caching attributes or backing pages. Just drop this for now and simply unmap the CPU page tables in all cases. Signed-off-by: Christian König <christian.koenig@amd.com> Reviewed-by: Daniel Vetter <daniel.vetter@ffwll.ch> Link: https://patchwork.freedesktop.org/patch/378240/
-
Christian König authored
Only functional change is to always keep io_reserved_count up to date for debugging even when it is not used otherwise. Signed-off-by: Christian König <christian.koenig@amd.com> Reviewed-by: Daniel Vetter <daniel.vetter@ffwll.ch> Link: https://patchwork.freedesktop.org/patch/378242/
-
Christian König authored
Just use the use_io_reserve_lru flag. It doesn't make much sense to have two flags. Signed-off-by: Christian König <christian.koenig@amd.com> Reviewed-by: Daniel Vetter <daniel.vetter@ffwll.ch> Link: https://patchwork.freedesktop.org/patch/378238/
-
Christian König authored
Nouveau is the only user of this functionality and evicting io space on -EAGAIN is really a misuse of the return code. Instead switch to using -ENOSPC here which makes much more sense and simplifies the code. This could unbreak something as we now cleanly return EAGAIN, but the chance for this are rather low. Signed-off-by: Christian König <christian.koenig@amd.com> Reviewed-by: Daniel Vetter <daniel.vetter@ffwll.ch> Link: https://patchwork.freedesktop.org/patch/378237/
-
Christian König authored
Implementing those is completely unnecessary. Signed-off-by: Christian König <christian.koenig@amd.com> Reviewed-by: Daniel Vetter <daniel.vetter@ffwll.ch> Reviewed-by: Madhav Chauhan <madhav.chauhan@amd.com> Link: https://patchwork.freedesktop.org/patch/378236/
-
Daniel Vetter authored
Comes up every few years, gets somewhat tedious to discuss, let's write this down once and for all. What I'm not sure about is whether the text should be more explicit in flat out mandating the amdkfd eviction fences for long running compute workloads or workloads where userspace fencing is allowed. v2: Now with dot graph! v3: Typo (Dave Airlie) Reviewed-by: Thomas Hellstrom <thomas.hellstrom@intel.com> Acked-by: Jason Ekstrand <jason@jlekstrand.net> Acked-by: Christian König <christian.koenig@amd.com> Acked-by: Daniel Stone <daniels@collabora.com> Acked-by: Dave Airlie <airlied@redhat.com> Reviewed-by: Maarten Lankhorst <maarten.lankhorst@linux.intel.com> Cc: Jesse Natalie <jenatali@microsoft.com> Cc: Steve Pronovost <spronovo@microsoft.com> Cc: Jason Ekstrand <jason@jlekstrand.net> Cc: Felix Kuehling <Felix.Kuehling@amd.com> Cc: Mika Kuoppala <mika.kuoppala@intel.com> Cc: Thomas Hellstrom <thomas.hellstrom@intel.com> Cc: linux-media@vger.kernel.org Cc: linaro-mm-sig@lists.linaro.org Cc: linux-rdma@vger.kernel.org Cc: amd-gfx@lists.freedesktop.org Cc: intel-gfx@lists.freedesktop.org Cc: Chris Wilson <chris@chris-wilson.co.uk> Cc: Maarten Lankhorst <maarten.lankhorst@linux.intel.com> Cc: Christian König <christian.koenig@amd.com> Signed-off-by: Daniel Vetter <daniel.vetter@intel.com> Link: https://patchwork.freedesktop.org/patch/msgid/20200709123339.547390-1-daniel.vetter@ffwll.ch
-
Daniel Vetter authored
Two in one go: - it is allowed to call dma_fence_wait() while holding a dma_resv_lock(). This is fundamental to how eviction works with ttm, so required. - it is allowed to call dma_fence_wait() from memory reclaim contexts, specifically from shrinker callbacks (which i915 does), and from mmu notifier callbacks (which amdgpu does, and which i915 sometimes also does, and probably always should, but that's kinda a debate). Also for stuff like HMM we really need to be able to do this, or things get real dicey. Consequence is that any critical path necessary to get to a dma_fence_signal for a fence must never a) call dma_resv_lock nor b) allocate memory with GFP_KERNEL. Also by implication of dma_resv_lock(), no userspace faulting allowed. That's some supremely obnoxious limitations, which is why we need to sprinkle the right annotations to all relevant paths. The one big locking context we're leaving out here is mmu notifiers, added in commit 23b68395 Author: Daniel Vetter <daniel.vetter@ffwll.ch> Date: Mon Aug 26 22:14:21 2019 +0200 mm/mmu_notifiers: add a lockdep map for invalidate_range_start/end that one covers a lot of other callsites, and it's also allowed to wait on dma-fences from mmu notifiers. But there's no ready-made functions exposed to prime this, so I've left it out for now. v2: Also track against mmu notifier context. v3: kerneldoc to spec the cross-driver contract. Note that currently i915 throws in a hard-coded 10s timeout on foreign fences (not sure why that was done, but it's there), which is why that rule is worded with SHOULD instead of MUST. Also some of the mmu_notifier/shrinker rules might surprise SoC drivers, I haven't fully audited them all. Which is infeasible anyway, we'll need to run them with lockdep and dma-fence annotations and see what goes boom. v4: A spelling fix from Mika v5: #ifdef for CONFIG_MMU_NOTIFIER. Reported by 0day. Unfortunately this means lockdep enforcement is slightly inconsistent, it won't spot GFP_NOIO and GFP_NOFS allocations in the wrong spot if CONFIG_MMU_NOTIFIER is disabled in the kernel config. Oh well. v5: Note that only drivers/gpu has a reasonable (or at least historical) excuse to use dma_fence_wait() from shrinker and mmu notifier callbacks. Everyone else should either have a better memory manager model, or better hardware. This reflects discussions with Jason Gunthorpe. Cc: Jason Gunthorpe <jgg@mellanox.com> Cc: Felix Kuehling <Felix.Kuehling@amd.com> Cc: kernel test robot <lkp@intel.com> Acked-by: Christian König <christian.koenig@amd.com> Acked-by: Dave Airlie <airlied@redhat.com> Reviewed-by: Maarten Lankhorst <maarten.lankhorst@linux.intel.com> Reviewed-by: Thomas Hellström <thomas.hellstrom@intel.com> (v4) Cc: Mika Kuoppala <mika.kuoppala@intel.com> Cc: Thomas Hellstrom <thomas.hellstrom@intel.com> Cc: linux-media@vger.kernel.org Cc: linaro-mm-sig@lists.linaro.org Cc: linux-rdma@vger.kernel.org Cc: amd-gfx@lists.freedesktop.org Cc: intel-gfx@lists.freedesktop.org Cc: Chris Wilson <chris@chris-wilson.co.uk> Cc: Maarten Lankhorst <maarten.lankhorst@linux.intel.com> Cc: Christian König <christian.koenig@amd.com> Signed-off-by: Daniel Vetter <daniel.vetter@intel.com> Link: https://patchwork.freedesktop.org/patch/msgid/20200707201229.472834-3-daniel.vetter@ffwll.ch
-
Daniel Vetter authored
Design is similar to the lockdep annotations for workers, but with some twists: - We use a read-lock for the execution/worker/completion side, so that this explicit annotation can be more liberally sprinkled around. With read locks lockdep isn't going to complain if the read-side isn't nested the same way under all circumstances, so ABBA deadlocks are ok. Which they are, since this is an annotation only. - We're using non-recursive lockdep read lock mode, since in recursive read lock mode lockdep does not catch read side hazards. And we _very_ much want read side hazards to be caught. For full details of this limitation see commit e9149858 Author: Peter Zijlstra <peterz@infradead.org> Date: Wed Aug 23 13:13:11 2017 +0200 locking/lockdep/selftests: Add mixed read-write ABBA tests - To allow nesting of the read-side explicit annotations we explicitly keep track of the nesting. lock_is_held() allows us to do that. - The wait-side annotation is a write lock, and entirely done within dma_fence_wait() for everyone by default. - To be able to freely annotate helper functions I want to make it ok to call dma_fence_begin/end_signalling from soft/hardirq context. First attempt was using the hardirq locking context for the write side in lockdep, but this forces all normal spinlocks nested within dma_fence_begin/end_signalling to be spinlocks. That bollocks. The approach now is to simple check in_atomic(), and for these cases entirely rely on the might_sleep() check in dma_fence_wait(). That will catch any wrong nesting against spinlocks from soft/hardirq contexts. The idea here is that every code path that's critical for eventually signalling a dma_fence should be annotated with dma_fence_begin/end_signalling. The annotation ideally starts right after a dma_fence is published (added to a dma_resv, exposed as a sync_file fd, attached to a drm_syncobj fd, or anything else that makes the dma_fence visible to other kernel threads), up to and including the dma_fence_wait(). Examples are irq handlers, the scheduler rt threads, the tail of execbuf (after the corresponding fences are visible), any workers that end up signalling dma_fences and really anything else. Not annotated should be code paths that only complete fences opportunistically as the gpu progresses, like e.g. shrinker/eviction code. The main class of deadlocks this is supposed to catch are: Thread A: mutex_lock(A); mutex_unlock(A); dma_fence_signal(); Thread B: mutex_lock(A); dma_fence_wait(); mutex_unlock(A); Thread B is blocked on A signalling the fence, but A never gets around to that because it cannot acquire the lock A. Note that dma_fence_wait() is allowed to be nested within dma_fence_begin/end_signalling sections. To allow this to happen the read lock needs to be upgraded to a write lock, which means that any other lock is acquired between the dma_fence_begin_signalling() call and the call to dma_fence_wait(), and still held, this will result in an immediate lockdep complaint. The only other option would be to not annotate such calls, defeating the point. Therefore these annotations cannot be sprinkled over the code entirely mindless to avoid false positives. Originally I hope that the cross-release lockdep extensions would alleviate the need for explicit annotations: https://lwn.net/Articles/709849/ But there's a few reasons why that's not an option: - It's not happening in upstream, since it got reverted due to too many false positives: commit e966eaee Author: Ingo Molnar <mingo@kernel.org> Date: Tue Dec 12 12:31:16 2017 +0100 locking/lockdep: Remove the cross-release locking checks This code (CONFIG_LOCKDEP_CROSSRELEASE=y and CONFIG_LOCKDEP_COMPLETIONS=y), while it found a number of old bugs initially, was also causing too many false positives that caused people to disable lockdep - which is arguably a worse overall outcome. - cross-release uses the complete() call to annotate the end of critical sections, for dma_fence that would be dma_fence_signal(). But we do not want all dma_fence_signal() calls to be treated as critical, since many are opportunistic cleanup of gpu requests. If these get stuck there's still the main completion interrupt and workers who can unblock everyone. Automatically annotating all dma_fence_signal() calls would hence cause false positives. - cross-release had some educated guesses for when a critical section starts, like fresh syscall or fresh work callback. This would again cause false positives without explicit annotations, since for dma_fence the critical sections only starts when we publish a fence. - Furthermore there can be cases where a thread never does a dma_fence_signal, but is still critical for reaching completion of fences. One example would be a scheduler kthread which picks up jobs and pushes them into hardware, where the interrupt handler or another completion thread calls dma_fence_signal(). But if the scheduler thread hangs, then all the fences hang, hence we need to manually annotate it. cross-release aimed to solve this by chaining cross-release dependencies, but the dependency from scheduler thread to the completion interrupt handler goes through hw where cross-release code can't observe it. In short, without manual annotations and careful review of the start and end of critical sections, cross-relese dependency tracking doesn't work. We need explicit annotations. v2: handle soft/hardirq ctx better against write side and dont forget EXPORT_SYMBOL, drivers can't use this otherwise. v3: Kerneldoc. v4: Some spelling fixes from Mika v5: Amend commit message to explain in detail why cross-release isn't the solution. v6: Pull out misplaced .rst hunk. Acked-by: Christian König <christian.koenig@amd.com> Acked-by: Dave Airlie <airlied@redhat.com> Cc: Felix Kuehling <Felix.Kuehling@amd.com> Reviewed-by: Thomas Hellström <thomas.hellstrom@intel.com> Reviewed-by: Maarten Lankhorst <maarten.lankhorst@linux.intel.com> Cc: Mika Kuoppala <mika.kuoppala@intel.com> Cc: Thomas Hellstrom <thomas.hellstrom@intel.com> Cc: linux-media@vger.kernel.org Cc: linaro-mm-sig@lists.linaro.org Cc: linux-rdma@vger.kernel.org Cc: amd-gfx@lists.freedesktop.org Cc: intel-gfx@lists.freedesktop.org Cc: Chris Wilson <chris@chris-wilson.co.uk> Cc: Maarten Lankhorst <maarten.lankhorst@linux.intel.com> Cc: Christian König <christian.koenig@amd.com> Signed-off-by: Daniel Vetter <daniel.vetter@intel.com> Link: https://patchwork.freedesktop.org/patch/msgid/20200707201229.472834-2-daniel.vetter@ffwll.ch
-
Christian König authored
The helper doesn't expose any not-mapable memory resources. Signed-off-by: Christian König <christian.koenig@amd.com> Reviewed-by: Thomas Zimmermann <tzimmermann@suse.de> Link: https://patchwork.freedesktop.org/patch/377649/
-